Merge master into haskell-updates

21 files changed, 901 insertions, 243 deletions

diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.lock b/pkgs/test/nixpkgs-check-by-name/Cargo.lock
index fc3aeb9fd79b4..904a9cff0e789 100644
--- a/pkgs/test/nixpkgs-check-by-name/Cargo.lock
+++ b/pkgs/test/nixpkgs-check-by-name/Cargo.lock
@@ -214,6 +214,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "443144c8cdadd93ebf52ddb4056d257f5b52c04d3c804e657d19eb73fc33668b"
 
 [[package]]
+name = "indoc"
+version = "2.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1e186cfbae8084e513daff4240b4797e342f988cecda4fb6c939150f96315fd8"
+
+[[package]]
 name = "is-terminal"
 version = "0.4.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -289,10 +295,12 @@ dependencies = [
  "anyhow",
  "clap",
  "colored",
+ "indoc",
  "itertools",
  "lazy_static",
  "regex",
  "rnix",
+ "rowan",
  "serde",
  "serde_json",
  "temp-env",
diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.toml b/pkgs/test/nixpkgs-check-by-name/Cargo.toml
index 1e6eaa1106d54..5240cd69f996e 100644
--- a/pkgs/test/nixpkgs-check-by-name/Cargo.toml
+++ b/pkgs/test/nixpkgs-check-by-name/Cargo.toml
@@ -14,6 +14,8 @@ anyhow = "1.0"
 lazy_static = "1.4.0"
 colored = "2.0.4"
 itertools = "0.11.0"
+rowan = "0.15.11"
 
 [dev-dependencies]
 temp-env = "0.3.5"
+indoc = "2.0.4"
diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.nix b/pkgs/test/nixpkgs-check-by-name/src/eval.nix
index 87c54b6444ee3..7179951d41cf0 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.nix
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.nix
@@ -76,6 +76,7 @@ let
         CallPackage = {
           call_package_variant = value._callPackageVariant;
           is_derivation = pkgs.lib.isDerivation value;
+          location = builtins.unsafeGetAttrPos name pkgs;
         };
       };
 
diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
index dd30cb9045e53..cb49c3acbef30 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -1,7 +1,10 @@
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::ratchet;
 use crate::structure;
+use crate::utils;
+use crate::validation::ResultIteratorExt as _;
 use crate::validation::{self, Validation::Success};
+use crate::NixFileStore;
 use std::path::Path;
 
 use anyhow::Context;
@@ -48,6 +51,15 @@ struct CallPackageInfo {
     call_package_variant: CallPackageVariant,
     /// Whether the attribute is a derivation (`lib.isDerivation`)
     is_derivation: bool,
+    location: Option<Location>,
+}
+
+/// The structure returned by `builtins.unsafeGetAttrPos`
+#[derive(Deserialize, Clone, Debug)]
+struct Location {
+    pub file: PathBuf,
+    pub line: usize,
+    pub column: usize,
 }
 
 #[derive(Deserialize)]
@@ -70,6 +82,7 @@ enum CallPackageVariant {
 /// See the `eval.nix` file for how this is achieved on the Nix side
 pub fn check_values(
     nixpkgs_path: &Path,
+    nix_file_store: &mut NixFileStore,
     package_names: Vec<String>,
     keep_nix_path: bool,
 ) -> validation::Result<ratchet::Nixpkgs> {
@@ -142,150 +155,223 @@ pub fn check_values(
             )
         })?;
 
-    let check_result = validation::sequence(attributes.into_iter().map(
-        |(attribute_name, attribute_value)| {
-            let relative_package_file = structure::relative_file_for_package(&attribute_name);
+    let check_result = validation::sequence(
+        attributes
+            .into_iter()
+            .map(|(attribute_name, attribute_value)| {
+                let check_result = match attribute_value {
+                    Attribute::NonByName(non_by_name_attribute) => handle_non_by_name_attribute(
+                        nixpkgs_path,
+                        nix_file_store,
+                        non_by_name_attribute,
+                    )?,
+                    Attribute::ByName(by_name_attribute) => {
+                        by_name(&attribute_name, by_name_attribute)
+                    }
+                };
+                Ok::<_, anyhow::Error>(check_result.map(|value| (attribute_name.clone(), value)))
+            })
+            .collect_vec()?,
+    );
 
-            use ratchet::RatchetState::*;
-            use Attribute::*;
-            use AttributeInfo::*;
-            use ByNameAttribute::*;
-            use CallPackageVariant::*;
-            use NonByNameAttribute::*;
+    Ok(check_result.map(|elems| ratchet::Nixpkgs {
+        package_names: elems.iter().map(|(name, _)| name.to_owned()).collect(),
+        package_map: elems.into_iter().collect(),
+    }))
+}
 
-            let check_result = match attribute_value {
-                // The attribute succeeds evaluation and is NOT defined in pkgs/by-name
-                NonByName(EvalSuccess(attribute_info)) => {
-                    let uses_by_name = match attribute_info {
-                        // In these cases the package doesn't qualify for being in pkgs/by-name,
-                        // so the UsesByName ratchet is already as tight as it can be
-                        NonAttributeSet => Success(NonApplicable),
-                        NonCallPackage => Success(NonApplicable),
-                        // This is the case when the `pkgs/by-name`-internal _internalCallByNamePackageFile
-                        // is used for a package outside `pkgs/by-name`
-                        CallPackage(CallPackageInfo {
-                            call_package_variant: Auto,
-                            ..
-                        }) => {
-                            // With the current detection mechanism, this also triggers for aliases
-                            // to pkgs/by-name packages, and there's no good method of
-                            // distinguishing alias vs non-alias.
-                            // Using `config.allowAliases = false` at least currently doesn't work
-                            // because there's nothing preventing people from defining aliases that
-                            // are present even with that disabled.
-                            // In the future we could kind of abuse this behavior to have better
-                            // enforcement of conditional aliases, but for now we just need to not
-                            // give an error.
-                            Success(NonApplicable)
-                        }
-                        // Only derivations can be in pkgs/by-name,
-                        // so this attribute doesn't qualify
-                        CallPackage(CallPackageInfo {
-                            is_derivation: false,
-                            ..
-                        }) => Success(NonApplicable),
+/// Handles the evaluation result for an attribute in `pkgs/by-name`,
+/// turning it into a validation result.
+fn by_name(
+    attribute_name: &str,
+    by_name_attribute: ByNameAttribute,
+) -> validation::Validation<ratchet::Package> {
+    use ratchet::RatchetState::*;
+    use AttributeInfo::*;
+    use ByNameAttribute::*;
+    use CallPackageVariant::*;
 
-                        // The case of an attribute that qualifies:
-                        // - Uses callPackage
-                        // - Is a derivation
-                        CallPackage(CallPackageInfo {
-                            is_derivation: true,
-                            call_package_variant: Manual { path, empty_arg },
-                        }) => Success(Loose(ratchet::CouldUseByName {
-                            call_package_path: path,
-                            empty_arg,
-                        })),
-                    };
-                    uses_by_name.map(|x| ratchet::Package {
-                        manual_definition: Tight,
-                        uses_by_name: x,
-                    })
-                }
-                NonByName(EvalFailure) => {
-                    // We don't know anything about this attribute really
-                    Success(ratchet::Package {
-                        // We'll assume that we can't remove any manual definitions, which has the
-                        // minimal drawback that if there was a manual definition that could've
-                        // been removed, fixing the package requires removing the definition, no
-                        // big deal, that's a minor edit.
-                        manual_definition: Tight,
+    let relative_package_file = structure::relative_file_for_package(attribute_name);
 
-                        // Regarding whether this attribute could `pkgs/by-name`, we don't really
-                        // know, so return NonApplicable, which has the effect that if a
-                        // package evaluation gets broken temporarily, the fix can remove it from
-                        // pkgs/by-name again. For now this isn't our problem, but in the future we
-                        // might have another check to enforce that evaluation must not be broken.
-                        // The alternative of assuming that it's using `pkgs/by-name` already
-                        // has the problem that if a package evaluation gets broken temporarily,
-                        // fixing it requires a move to pkgs/by-name, which could happen more
-                        // often and isn't really justified.
-                        uses_by_name: NonApplicable,
-                    })
-                }
-                ByName(Missing) => NixpkgsProblem::UndefinedAttr {
-                    relative_package_file: relative_package_file.clone(),
-                    package_name: attribute_name.clone(),
-                }
-                .into(),
-                ByName(Existing(NonAttributeSet)) => NixpkgsProblem::NonDerivation {
-                    relative_package_file: relative_package_file.clone(),
-                    package_name: attribute_name.clone(),
-                }
-                .into(),
-                ByName(Existing(NonCallPackage)) => NixpkgsProblem::WrongCallPackage {
-                    relative_package_file: relative_package_file.clone(),
-                    package_name: attribute_name.clone(),
+    match by_name_attribute {
+        Missing => NixpkgsProblem::UndefinedAttr {
+            relative_package_file: relative_package_file.to_owned(),
+            package_name: attribute_name.to_owned(),
+        }
+        .into(),
+        Existing(NonAttributeSet) => NixpkgsProblem::NonDerivation {
+            relative_package_file: relative_package_file.to_owned(),
+            package_name: attribute_name.to_owned(),
+        }
+        .into(),
+        Existing(NonCallPackage) => NixpkgsProblem::WrongCallPackage {
+            relative_package_file: relative_package_file.to_owned(),
+            package_name: attribute_name.to_owned(),
+        }
+        .into(),
+        Existing(CallPackage(CallPackageInfo {
+            is_derivation,
+            call_package_variant,
+            ..
+        })) => {
+            let check_result = if !is_derivation {
+                NixpkgsProblem::NonDerivation {
+                    relative_package_file: relative_package_file.to_owned(),
+                    package_name: attribute_name.to_owned(),
                 }
-                .into(),
-                ByName(Existing(CallPackage(CallPackageInfo {
-                    is_derivation,
-                    call_package_variant,
-                }))) => {
-                    let check_result = if !is_derivation {
-                        NixpkgsProblem::NonDerivation {
-                            relative_package_file: relative_package_file.clone(),
-                            package_name: attribute_name.clone(),
-                        }
-                        .into()
+                .into()
+            } else {
+                Success(())
+            };
+
+            check_result.and(match &call_package_variant {
+                Auto => Success(ratchet::Package {
+                    manual_definition: Tight,
+                    uses_by_name: Tight,
+                }),
+                // TODO: Use the call_package_argument_info_at instead/additionally and
+                // simplify the eval.nix code
+                Manual { path, empty_arg } => {
+                    let correct_file = if let Some(call_package_path) = path {
+                        relative_package_file == *call_package_path
                     } else {
-                        Success(())
+                        false
                     };
 
-                    check_result.and(match &call_package_variant {
-                        Auto => Success(ratchet::Package {
-                            manual_definition: Tight,
+                    if correct_file {
+                        Success(ratchet::Package {
+                            // Empty arguments for non-auto-called packages are not allowed anymore.
+                            manual_definition: if *empty_arg { Loose(()) } else { Tight },
                             uses_by_name: Tight,
-                        }),
-                        Manual { path, empty_arg } => {
-                            let correct_file = if let Some(call_package_path) = path {
-                                relative_package_file == *call_package_path
-                            } else {
-                                false
-                            };
+                        })
+                    } else {
+                        NixpkgsProblem::WrongCallPackage {
+                            relative_package_file: relative_package_file.to_owned(),
+                            package_name: attribute_name.to_owned(),
+                        }
+                        .into()
+                    }
+                }
+            })
+        }
+    }
+}
 
-                            if correct_file {
-                                Success(ratchet::Package {
-                                    // Empty arguments for non-auto-called packages are not allowed anymore.
-                                    manual_definition: if *empty_arg { Loose(()) } else { Tight },
-                                    uses_by_name: Tight,
-                                })
-                            } else {
-                                NixpkgsProblem::WrongCallPackage {
-                                    relative_package_file: relative_package_file.clone(),
-                                    package_name: attribute_name.clone(),
+/// Handles the evaluation result for an attribute _not_ in `pkgs/by-name`,
+/// turning it into a validation result.
+fn handle_non_by_name_attribute(
+    nixpkgs_path: &Path,
+    nix_file_store: &mut NixFileStore,
+    non_by_name_attribute: NonByNameAttribute,
+) -> validation::Result<ratchet::Package> {
+    use ratchet::RatchetState::*;
+    use AttributeInfo::*;
+    use CallPackageVariant::*;
+    use NonByNameAttribute::*;
+
+    Ok(match non_by_name_attribute {
+        // The attribute succeeds evaluation and is NOT defined in pkgs/by-name
+        EvalSuccess(attribute_info) => {
+            let uses_by_name = match attribute_info {
+                // In these cases the package doesn't qualify for being in pkgs/by-name,
+                // so the UsesByName ratchet is already as tight as it can be
+                NonAttributeSet => Success(NonApplicable),
+                NonCallPackage => Success(NonApplicable),
+                // This is the case when the `pkgs/by-name`-internal _internalCallByNamePackageFile
+                // is used for a package outside `pkgs/by-name`
+                CallPackage(CallPackageInfo {
+                    call_package_variant: Auto,
+                    ..
+                }) => {
+                    // With the current detection mechanism, this also triggers for aliases
+                    // to pkgs/by-name packages, and there's no good method of
+                    // distinguishing alias vs non-alias.
+                    // Using `config.allowAliases = false` at least currently doesn't work
+                    // because there's nothing preventing people from defining aliases that
+                    // are present even with that disabled.
+                    // In the future we could kind of abuse this behavior to have better
+                    // enforcement of conditional aliases, but for now we just need to not
+                    // give an error.
+                    Success(NonApplicable)
+                }
+                // Only derivations can be in pkgs/by-name,
+                // so this attribute doesn't qualify
+                CallPackage(CallPackageInfo {
+                    is_derivation: false,
+                    ..
+                }) => Success(NonApplicable),
+                // A location of None indicates something weird, we can't really know where
+                // this attribute is defined, probably an alias
+                CallPackage(CallPackageInfo { location: None, .. }) => Success(Tight),
+                // The case of an attribute that qualifies:
+                // - Uses callPackage
+                // - Is a derivation
+                CallPackage(CallPackageInfo {
+                    is_derivation: true,
+                    call_package_variant: Manual { .. },
+                    location: Some(location),
+                }) =>
+                // We'll use the attribute's location to parse the file that defines it
+                {
+                    match nix_file_store
+                        .get(&location.file)?
+                        .call_package_argument_info_at(
+                            location.line,
+                            location.column,
+                            nixpkgs_path,
+                        )? {
+                        // If the definition is not of the form `<attr> = callPackage <arg1> <arg2>;`,
+                        // it's generally not possible to migrate to `pkgs/by-name`
+                        None => Success(NonApplicable),
+                        Some(call_package_argument_info) => {
+                            if let Some(ref rel_path) = call_package_argument_info.relative_path {
+                                if rel_path.starts_with(utils::BASE_SUBPATH) {
+                                    // Package variants of by-name packages are explicitly allowed according to RFC 140
+                                    // https://github.com/NixOS/rfcs/blob/master/rfcs/0140-simple-package-paths.md#package-variants:
+                                    //
+                                    // foo-variant = callPackage ../by-name/fo/foo/package.nix {
+                                    //   someFlag = true;
+                                    // }
+                                    //
+                                    // While such definitions could be moved to `pkgs/by-name` by using
+                                    // `.override { someFlag = true; }` instead, this changes the semantics in
+                                    // relation with overlays.
+                                    Success(NonApplicable)
+                                } else {
+                                    Success(Loose(call_package_argument_info))
                                 }
-                                .into()
+                            } else {
+                                Success(Loose(call_package_argument_info))
                             }
                         }
-                    })
+                    }
                 }
             };
-            check_result.map(|value| (attribute_name.clone(), value))
-        },
-    ));
+            uses_by_name.map(|x| ratchet::Package {
+                manual_definition: Tight,
+                uses_by_name: x,
+            })
+        }
+        EvalFailure => {
+            // We don't know anything about this attribute really
+            Success(ratchet::Package {
+                // We'll assume that we can't remove any manual definitions, which has the
+                // minimal drawback that if there was a manual definition that could've
+                // been removed, fixing the package requires removing the definition, no
+                // big deal, that's a minor edit.
+                manual_definition: Tight,
 
-    Ok(check_result.map(|elems| ratchet::Nixpkgs {
-        package_names: elems.iter().map(|(name, _)| name.to_owned()).collect(),
-        package_map: elems.into_iter().collect(),
-    }))
+                // Regarding whether this attribute could `pkgs/by-name`, we don't really
+                // know, so return NonApplicable, which has the effect that if a
+                // package evaluation gets broken temporarily, the fix can remove it from
+                // pkgs/by-name again. For now this isn't our problem, but in the future we
+                // might have another check to enforce that evaluation must not be broken.
+                // The alternative of assuming that it's using `pkgs/by-name` already
+                // has the problem that if a package evaluation gets broken temporarily,
+                // fixing it requires a move to pkgs/by-name, which could happen more
+                // often and isn't really justified.
+                uses_by_name: NonApplicable,
+            })
+        }
+    })
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
index 8179ec8ded74b..0d0ddcd7e6327 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -1,4 +1,6 @@
+use crate::nix_file::NixFileStore;
 mod eval;
+mod nix_file;
 mod nixpkgs_problem;
 mod ratchet;
 mod references;
@@ -116,6 +118,8 @@ pub fn check_nixpkgs<W: io::Write>(
     keep_nix_path: bool,
     error_writer: &mut W,
 ) -> validation::Result<ratchet::Nixpkgs> {
+    let mut nix_file_store = NixFileStore::default();
+
     Ok({
         let nixpkgs_path = nixpkgs_path.canonicalize().with_context(|| {
             format!(
@@ -132,9 +136,9 @@ pub fn check_nixpkgs<W: io::Write>(
             )?;
             Success(ratchet::Nixpkgs::default())
         } else {
-            check_structure(&nixpkgs_path)?.result_map(|package_names|
+            check_structure(&nixpkgs_path, &mut nix_file_store)?.result_map(|package_names|
                 // Only if we could successfully parse the structure, we do the evaluation checks
-                eval::check_values(&nixpkgs_path, package_names, keep_nix_path))?
+                eval::check_values(&nixpkgs_path, &mut nix_file_store, package_names, keep_nix_path))?
         }
     })
 }
@@ -169,7 +173,7 @@ mod tests {
 
     // tempfile::tempdir needs to be wrapped in temp_env lock
     // because it accesses TMPDIR environment variable.
-    fn tempdir() -> anyhow::Result<TempDir> {
+    pub fn tempdir() -> anyhow::Result<TempDir> {
         let empty_list: [(&str, Option<&str>); 0] = [];
         Ok(temp_env::with_vars(empty_list, tempfile::tempdir)?)
     }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs b/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
new file mode 100644
index 0000000000000..836c5e2dcdda5
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
@@ -0,0 +1,510 @@
+//! This is a utility module for interacting with the syntax of Nix files
+
+use crate::utils::LineIndex;
+use anyhow::Context;
+use rnix::ast;
+use rnix::ast::Expr;
+use rnix::ast::HasEntry;
+use rnix::SyntaxKind;
+use rowan::ast::AstNode;
+use rowan::TextSize;
+use rowan::TokenAtOffset;
+use std::collections::hash_map::Entry;
+use std::collections::HashMap;
+use std::fs::read_to_string;
+use std::path::Path;
+use std::path::PathBuf;
+
+/// A structure to store parse results of Nix files in memory,
+/// making sure that the same file never has to be parsed twice
+#[derive(Default)]
+pub struct NixFileStore {
+    entries: HashMap<PathBuf, NixFile>,
+}
+
+impl NixFileStore {
+    /// Get the store entry for a Nix file if it exists, otherwise parse the file, insert it into
+    /// the store, and return the value
+    ///
+    /// Note that this function only gives an anyhow::Result::Err for I/O errors.
+    /// A parse error is anyhow::Result::Ok(Result::Err(error))
+    pub fn get(&mut self, path: &Path) -> anyhow::Result<&NixFile> {
+        match self.entries.entry(path.to_owned()) {
+            Entry::Occupied(entry) => Ok(entry.into_mut()),
+            Entry::Vacant(entry) => Ok(entry.insert(NixFile::new(path)?)),
+        }
+    }
+}
+
+/// A structure for storing a successfully parsed Nix file
+pub struct NixFile {
+    /// The parent directory of the Nix file, for more convenient error handling
+    pub parent_dir: PathBuf,
+    /// The path to the file itself, for errors
+    pub path: PathBuf,
+    pub syntax_root: rnix::Root,
+    pub line_index: LineIndex,
+}
+
+impl NixFile {
+    /// Creates a new NixFile, failing for I/O or parse errors
+    fn new(path: impl AsRef<Path>) -> anyhow::Result<NixFile> {
+        let Some(parent_dir) = path.as_ref().parent() else {
+            anyhow::bail!("Could not get parent of path {}", path.as_ref().display())
+        };
+
+        let contents = read_to_string(&path)
+            .with_context(|| format!("Could not read file {}", path.as_ref().display()))?;
+        let line_index = LineIndex::new(&contents);
+
+        // NOTE: There's now another Nixpkgs CI check to make sure all changed Nix files parse
+        // correctly, though that uses mainline Nix instead of rnix, so it doesn't give the same
+        // errors. In the future we should unify these two checks, ideally moving the other CI
+        // check into this tool as well and checking for both mainline Nix and rnix.
+        rnix::Root::parse(&contents)
+            // rnix's ::ok returns Result<_, _> , so no error is thrown away like it would be with
+            // std::result's ::ok
+            .ok()
+            .map(|syntax_root| NixFile {
+                parent_dir: parent_dir.to_path_buf(),
+                path: path.as_ref().to_owned(),
+                syntax_root,
+                line_index,
+            })
+            .with_context(|| format!("Could not parse file {} with rnix", path.as_ref().display()))
+    }
+}
+
+/// Information about callPackage arguments
+#[derive(Debug, PartialEq)]
+pub struct CallPackageArgumentInfo {
+    /// The relative path of the first argument, or `None` if it's not a path.
+    pub relative_path: Option<PathBuf>,
+    /// Whether the second argument is an empty attribute set
+    pub empty_arg: bool,
+}
+
+impl NixFile {
+    /// Returns information about callPackage arguments for an attribute at a specific line/column
+    /// index.
+    /// If the location is not of the form `<attr> = callPackage <arg1> <arg2>;`, `None` is
+    /// returned.
+    /// This function only returns `Err` for problems that can't be caused by the Nix contents,
+    /// but rather problems in this programs code itself.
+    ///
+    /// This is meant to be used with the location returned from `builtins.unsafeGetAttrPos`, e.g.:
+    /// - Create file `default.nix` with contents
+    ///   ```nix
+    ///   self: {
+    ///     foo = self.callPackage ./default.nix { };
+    ///   }
+    ///   ```
+    /// - Evaluate
+    ///   ```nix
+    ///   builtins.unsafeGetAttrPos "foo" (import ./default.nix { })
+    ///   ```
+    ///   results in `{ file = ./default.nix; line = 2; column = 3; }`
+    /// - Get the NixFile for `.file` from a `NixFileStore`
+    /// - Call this function with `.line`, `.column` and `relative_to` as the (absolute) current directory
+    ///
+    /// You'll get back
+    /// ```rust
+    /// Some(CallPackageArgumentInfo { path = Some("default.nix"), empty_arg: true })
+    /// ```
+    ///
+    /// Note that this also returns the same for `pythonPackages.callPackage`. It doesn't make an
+    /// attempt at distinguishing this.
+    pub fn call_package_argument_info_at(
+        &self,
+        line: usize,
+        column: usize,
+        relative_to: &Path,
+    ) -> anyhow::Result<Option<CallPackageArgumentInfo>> {
+        let Some(attrpath_value) = self.attrpath_value_at(line, column)? else {
+            return Ok(None);
+        };
+        self.attrpath_value_call_package_argument_info(attrpath_value, relative_to)
+    }
+
+    // Internal function mainly to make it independently testable
+    fn attrpath_value_at(
+        &self,
+        line: usize,
+        column: usize,
+    ) -> anyhow::Result<Option<ast::AttrpathValue>> {
+        let index = self.line_index.fromlinecolumn(line, column);
+
+        let token_at_offset = self
+            .syntax_root
+            .syntax()
+            .token_at_offset(TextSize::from(index as u32));
+
+        // The token_at_offset function takes indices to mean a location _between_ characters,
+        // which in this case is some spacing followed by the attribute name:
+        //
+        //   foo = 10;
+        //  /\
+        //  This is the token offset, we get both the (newline + indentation) on the left side,
+        //  and the attribute name on the right side.
+        let TokenAtOffset::Between(_space, token) = token_at_offset else {
+            anyhow::bail!("Line {line} column {column} in {} is not the start of a token, but rather {token_at_offset:?}", self.path.display())
+        };
+
+        // token looks like "foo"
+        let Some(node) = token.parent() else {
+            anyhow::bail!(
+                "Token on line {line} column {column} in {} does not have a parent node: {token:?}",
+                self.path.display()
+            )
+        };
+
+        // node looks like "foo"
+        let Some(attrpath_node) = node.parent() else {
+            anyhow::bail!(
+                "Node in {} does not have a parent node: {node:?}",
+                self.path.display()
+            )
+        };
+
+        if attrpath_node.kind() != SyntaxKind::NODE_ATTRPATH {
+            // This can happen for e.g. `inherit foo`, so definitely not a syntactic `callPackage`
+            return Ok(None);
+        }
+        // attrpath_node looks like "foo.bar"
+        let Some(attrpath_value_node) = attrpath_node.parent() else {
+            anyhow::bail!(
+                "Attribute path node in {} does not have a parent node: {attrpath_node:?}",
+                self.path.display()
+            )
+        };
+
+        if !ast::AttrpathValue::can_cast(attrpath_value_node.kind()) {
+            anyhow::bail!(
+                "Node in {} is not an attribute path value node: {attrpath_value_node:?}",
+                self.path.display()
+            )
+        }
+        // attrpath_value_node looks like "foo.bar = 10;"
+
+        // unwrap is fine because we confirmed that we can cast with the above check.
+        // We could avoid this `unwrap` for a `clone`, since `cast` consumes the argument,
+        // but we still need it for the error message when the cast fails.
+        Ok(Some(ast::AttrpathValue::cast(attrpath_value_node).unwrap()))
+    }
+
+    // Internal function mainly to make attrpath_value_at independently testable
+    fn attrpath_value_call_package_argument_info(
+        &self,
+        attrpath_value: ast::AttrpathValue,
+        relative_to: &Path,
+    ) -> anyhow::Result<Option<CallPackageArgumentInfo>> {
+        let Some(attrpath) = attrpath_value.attrpath() else {
+            anyhow::bail!("attrpath value node doesn't have an attrpath: {attrpath_value:?}")
+        };
+
+        // At this point we know it's something like `foo...bar = ...`
+
+        if attrpath.attrs().count() > 1 {
+            // If the attribute path has multiple entries, the left-most entry is an attribute and
+            // can't be a `callPackage`.
+            //
+            // FIXME: `builtins.unsafeGetAttrPos` will return the same position for all attribute
+            // paths and we can't really know which one it is. We could have a case like
+            // `foo.bar = callPackage ... { }` and trying to determine if `bar` is a `callPackage`,
+            // where this is not correct.
+            // However, this case typically doesn't occur anyways,
+            // because top-level packages wouldn't be nested under an attribute set.
+            return Ok(None);
+        }
+        let Some(value) = attrpath_value.value() else {
+            anyhow::bail!("attrpath value node doesn't have a value: {attrpath_value:?}")
+        };
+
+        // At this point we know it's something like `foo = ...`
+
+        let Expr::Apply(apply1) = value else {
+            // Not even a function call, instead something like `foo = null`
+            return Ok(None);
+        };
+        let Some(function1) = apply1.lambda() else {
+            anyhow::bail!("apply node doesn't have a lambda: {apply1:?}")
+        };
+        let Some(arg1) = apply1.argument() else {
+            anyhow::bail!("apply node doesn't have an argument: {apply1:?}")
+        };
+
+        // At this point we know it's something like `foo = <fun> <arg>`.
+        // For a callPackage, `<fun>` would be `callPackage ./file` and `<arg>` would be `{ }`
+
+        let empty_arg = if let Expr::AttrSet(attrset) = arg1 {
+            // We can only statically determine whether the argument is empty if it's an attribute
+            // set _expression_, even though other kind of expressions could evaluate to an attribute
+            // set _value_. But this is what we want anyways
+            attrset.entries().next().is_none()
+        } else {
+            false
+        };
+
+        // Because callPackage takes two curried arguments, the first function needs to be a
+        // function call itself
+        let Expr::Apply(apply2) = function1 else {
+            // Not a callPackage, instead something like `foo = import ./foo`
+            return Ok(None);
+        };
+        let Some(function2) = apply2.lambda() else {
+            anyhow::bail!("apply node doesn't have a lambda: {apply2:?}")
+        };
+        let Some(arg2) = apply2.argument() else {
+            anyhow::bail!("apply node doesn't have an argument: {apply2:?}")
+        };
+
+        // At this point we know it's something like `foo = <fun2> <arg2> <arg1>`.
+        // For a callPackage, `<fun2>` would be `callPackage`, `<arg2>` would be `./file`
+
+        // Check that <arg2> is a path expression
+        let path = if let Expr::Path(actual_path) = arg2 {
+            // Try to statically resolve the path and turn it into a nixpkgs-relative path
+            if let ResolvedPath::Within(p) = self.static_resolve_path(actual_path, relative_to) {
+                Some(p)
+            } else {
+                // We can't statically know an existing path inside Nixpkgs used as <arg2>
+                None
+            }
+        } else {
+            // <arg2> is not a path, but rather e.g. an inline expression
+            None
+        };
+
+        // Check that <fun2> is an identifier, or an attribute path with an identifier at the end
+        let ident = match function2 {
+            Expr::Ident(ident) => {
+                // This means it's something like `foo = callPackage <arg2> <arg1>`
+                ident
+            }
+            Expr::Select(select) => {
+                // This means it's something like `foo = self.callPackage <arg2> <arg1>`.
+                // We also end up here for e.g. `pythonPackages.callPackage`, but the
+                // callPackage-mocking method will take care of not triggering for this case.
+
+                if select.default_expr().is_some() {
+                    // Very odd case, but this would be `foo = self.callPackage or true ./test.nix {}
+                    // (yes this is valid Nix code)
+                    return Ok(None);
+                }
+                let Some(attrpath) = select.attrpath() else {
+                    anyhow::bail!("select node doesn't have an attrpath: {select:?}")
+                };
+                let Some(last) = attrpath.attrs().last() else {
+                    // This case shouldn't be possible, it would be `foo = self. ./test.nix {}`,
+                    // which shouldn't parse
+                    anyhow::bail!("select node has an empty attrpath: {select:?}")
+                };
+                if let ast::Attr::Ident(ident) = last {
+                    ident
+                } else {
+                    // Here it's something like `foo = self."callPackage" /test.nix {}`
+                    // which we're not gonna bother with
+                    return Ok(None);
+                }
+            }
+            // Any other expression we're not gonna treat as callPackage
+            _ => return Ok(None),
+        };
+
+        let Some(token) = ident.ident_token() else {
+            anyhow::bail!("ident node doesn't have a token: {ident:?}")
+        };
+
+        if token.text() == "callPackage" {
+            Ok(Some(CallPackageArgumentInfo {
+                relative_path: path,
+                empty_arg,
+            }))
+        } else {
+            Ok(None)
+        }
+    }
+}
+
+/// The result of trying to statically resolve a Nix path expression
+pub enum ResolvedPath {
+    /// Something like `./foo/${bar}/baz`, can't be known statically
+    Interpolated,
+    /// Something like `<nixpkgs>`, can't be known statically
+    SearchPath,
+    /// Path couldn't be resolved due to an IO error,
+    /// e.g. if the path doesn't exist or you don't have the right permissions
+    Unresolvable(std::io::Error),
+    /// The path is outside the given absolute path
+    Outside,
+    /// The path is within the given absolute path.
+    /// The `PathBuf` is the relative path under the given absolute path.
+    Within(PathBuf),
+}
+
+impl NixFile {
+    /// Statically resolves a Nix path expression and checks that it's within an absolute path
+    ///
+    /// E.g. for the path expression `./bar.nix` in `./foo.nix` and an absolute path of the
+    /// current directory, the function returns `ResolvedPath::Within(./bar.nix)`
+    pub fn static_resolve_path(&self, node: ast::Path, relative_to: &Path) -> ResolvedPath {
+        if node.parts().count() != 1 {
+            // If there's more than 1 interpolated part, it's of the form `./foo/${bar}/baz`.
+            return ResolvedPath::Interpolated;
+        }
+
+        let text = node.to_string();
+
+        if text.starts_with('<') {
+            // A search path like `<nixpkgs>`. There doesn't appear to be better way to detect
+            // these in rnix
+            return ResolvedPath::SearchPath;
+        }
+
+        // Join the file's parent directory and the path expression, then resolve it
+        // FIXME: Expressions like `../../../../foo/bar/baz/qux` or absolute paths
+        // may resolve close to the original file, but may have left the relative_to.
+        // That should be checked more strictly
+        match self.parent_dir.join(Path::new(&text)).canonicalize() {
+            Err(resolution_error) => ResolvedPath::Unresolvable(resolution_error),
+            Ok(resolved) => {
+                // Check if it's within relative_to
+                match resolved.strip_prefix(relative_to) {
+                    Err(_prefix_error) => ResolvedPath::Outside,
+                    Ok(suffix) => ResolvedPath::Within(suffix.to_path_buf()),
+                }
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::tests;
+    use indoc::indoc;
+
+    #[test]
+    fn detects_attributes() -> anyhow::Result<()> {
+        let temp_dir = tests::tempdir()?;
+        let file = temp_dir.path().join("file.nix");
+        let contents = indoc! {r#"
+            toInherit: {
+              foo = 1;
+              "bar" = 2;
+              ${"baz"} = 3;
+              "${"qux"}" = 4;
+
+              # A
+              quux
+              # B
+              =
+              # C
+              5
+              # D
+              ;
+              # E
+
+              /**/quuux/**/=/**/5/**/;/*E*/
+
+              inherit toInherit;
+            }
+        "#};
+
+        std::fs::write(&file, contents)?;
+
+        let nix_file = NixFile::new(&file)?;
+
+        // These are builtins.unsafeGetAttrPos locations for the attributes
+        let cases = [
+            (2, 3, Some("foo = 1;")),
+            (3, 3, Some(r#""bar" = 2;"#)),
+            (4, 3, Some(r#"${"baz"} = 3;"#)),
+            (5, 3, Some(r#""${"qux"}" = 4;"#)),
+            (8, 3, Some("quux\n  # B\n  =\n  # C\n  5\n  # D\n  ;")),
+            (17, 7, Some("quuux/**/=/**/5/**/;")),
+            (19, 10, None),
+        ];
+
+        for (line, column, expected_result) in cases {
+            let actual_result = nix_file
+                .attrpath_value_at(line, column)?
+                .map(|node| node.to_string());
+            assert_eq!(actual_result.as_deref(), expected_result);
+        }
+
+        Ok(())
+    }
+
+    #[test]
+    fn detects_call_package() -> anyhow::Result<()> {
+        let temp_dir = tests::tempdir()?;
+        let file = temp_dir.path().join("file.nix");
+        let contents = indoc! {r#"
+            self: with self; {
+              a.sub = null;
+              b = null;
+              c = import ./file.nix;
+              d = import ./file.nix { };
+              e = pythonPackages.callPackage ./file.nix { };
+              f = callPackage ./file.nix { };
+              g = callPackage ({ }: { }) { };
+              h = callPackage ./file.nix { x = 0; };
+              i = callPackage ({ }: { }) (let in { });
+            }
+        "#};
+
+        std::fs::write(&file, contents)?;
+
+        let nix_file = NixFile::new(&file)?;
+
+        let cases = [
+            (2, None),
+            (3, None),
+            (4, None),
+            (5, None),
+            (
+                6,
+                Some(CallPackageArgumentInfo {
+                    relative_path: Some(PathBuf::from("file.nix")),
+                    empty_arg: true,
+                }),
+            ),
+            (
+                7,
+                Some(CallPackageArgumentInfo {
+                    relative_path: Some(PathBuf::from("file.nix")),
+                    empty_arg: true,
+                }),
+            ),
+            (
+                8,
+                Some(CallPackageArgumentInfo {
+                    relative_path: None,
+                    empty_arg: true,
+                }),
+            ),
+            (
+                9,
+                Some(CallPackageArgumentInfo {
+                    relative_path: Some(PathBuf::from("file.nix")),
+                    empty_arg: false,
+                }),
+            ),
+            (
+                10,
+                Some(CallPackageArgumentInfo {
+                    relative_path: None,
+                    empty_arg: false,
+                }),
+            ),
+        ];
+
+        for (line, expected_result) in cases {
+            let actual_result = nix_file.call_package_argument_info_at(line, 3, temp_dir.path())?;
+            assert_eq!(actual_result, expected_result);
+        }
+
+        Ok(())
+    }
+}
diff --git a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
index 16ea65deebfce..25e3ef4863e41 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
@@ -1,6 +1,5 @@
 use crate::structure;
 use crate::utils::PACKAGE_NIX_FILENAME;
-use rnix::parser::ParseError;
 use std::ffi::OsString;
 use std::fmt;
 use std::io;
@@ -58,11 +57,6 @@ pub enum NixpkgsProblem {
         subpath: PathBuf,
         io_error: io::Error,
     },
-    CouldNotParseNix {
-        relative_package_dir: PathBuf,
-        subpath: PathBuf,
-        error: ParseError,
-    },
     PathInterpolation {
         relative_package_dir: PathBuf,
         subpath: PathBuf,
@@ -184,14 +178,6 @@ impl fmt::Display for NixpkgsProblem {
                     relative_package_dir.display(),
                     subpath.display(),
                 ),
-            NixpkgsProblem::CouldNotParseNix { relative_package_dir, subpath, error } =>
-                write!(
-                    f,
-                    "{}: File {} could not be parsed by rnix: {}",
-                    relative_package_dir.display(),
-                    subpath.display(),
-                    error,
-                ),
             NixpkgsProblem::PathInterpolation { relative_package_dir, subpath, line, text } =>
                 write!(
                     f,
diff --git a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
index 10ecc01d3580c..200bf92c516a1 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
@@ -2,11 +2,11 @@
 //!
 //! Each type has a `compare` method that validates the ratchet checks for that item.
 
+use crate::nix_file::CallPackageArgumentInfo;
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::structure;
 use crate::validation::{self, Validation, Validation::Success};
 use std::collections::HashMap;
-use std::path::PathBuf;
 
 /// The ratchet value for the entirety of Nixpkgs.
 #[derive(Default)]
@@ -148,16 +148,8 @@ impl ToNixpkgsProblem for ManualDefinition {
 /// It also checks that once a package uses pkgs/by-name, it can't switch back to all-packages.nix
 pub enum UsesByName {}
 
-#[derive(Clone)]
-pub struct CouldUseByName {
-    /// The first callPackage argument, used for better errors
-    pub call_package_path: Option<PathBuf>,
-    /// Whether the second callPackage argument is empty, used for better errors
-    pub empty_arg: bool,
-}
-
 impl ToNixpkgsProblem for UsesByName {
-    type ToContext = CouldUseByName;
+    type ToContext = CallPackageArgumentInfo;
 
     fn to_nixpkgs_problem(
         name: &str,
@@ -167,13 +159,13 @@ impl ToNixpkgsProblem for UsesByName {
         if let Some(()) = optional_from {
             NixpkgsProblem::MovedOutOfByName {
                 package_name: name.to_owned(),
-                call_package_path: to.call_package_path.clone(),
+                call_package_path: to.relative_path.clone(),
                 empty_arg: to.empty_arg,
             }
         } else {
             NixpkgsProblem::NewPackageNotUsingByName {
                 package_name: name.to_owned(),
-                call_package_path: to.call_package_path.clone(),
+                call_package_path: to.relative_path.clone(),
                 empty_arg: to.empty_arg,
             }
         }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs
index ce7403afb32d6..169e996300baa 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/references.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs
@@ -1,23 +1,32 @@
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::utils;
-use crate::utils::LineIndex;
 use crate::validation::{self, ResultIteratorExt, Validation::Success};
+use crate::NixFileStore;
 
 use anyhow::Context;
-use rnix::{Root, SyntaxKind::NODE_PATH};
+use rowan::ast::AstNode;
 use std::ffi::OsStr;
-use std::fs::read_to_string;
 use std::path::Path;
 
 /// Check that every package directory in pkgs/by-name doesn't link to outside that directory.
 /// Both symlinks and Nix path expressions are checked.
 pub fn check_references(
+    nix_file_store: &mut NixFileStore,
     relative_package_dir: &Path,
     absolute_package_dir: &Path,
 ) -> validation::Result<()> {
-    // The empty argument here is the subpath under the package directory to check
-    // An empty one means the package directory itself
-    check_path(relative_package_dir, absolute_package_dir, Path::new("")).with_context(|| {
+    // The first subpath to check is the package directory itself, which we can represent as an
+    // empty path, since the absolute package directory gets prepended to this.
+    // We don't use `./.` to keep the error messages cleaner
+    // (there's no canonicalisation going on underneath)
+    let subpath = Path::new("");
+    check_path(
+        nix_file_store,
+        relative_package_dir,
+        absolute_package_dir,
+        subpath,
+    )
+    .with_context(|| {
         format!(
             "While checking the references in package directory {}",
             relative_package_dir.display()
@@ -26,7 +35,12 @@ pub fn check_references(
 }
 
 /// Checks for a specific path to not have references outside
+///
+/// The subpath is the relative path within the package directory we're currently checking.
+/// A relative path so that the error messages don't get absolute paths (which are messy in CI).
+/// The absolute package directory gets prepended before doing anything with it though.
 fn check_path(
+    nix_file_store: &mut NixFileStore,
     relative_package_dir: &Path,
     absolute_package_dir: &Path,
     subpath: &Path,
@@ -62,21 +76,27 @@ fn check_path(
             utils::read_dir_sorted(&path)?
                 .into_iter()
                 .map(|entry| {
-                    let entry_subpath = subpath.join(entry.file_name());
-                    check_path(relative_package_dir, absolute_package_dir, &entry_subpath)
-                        .with_context(|| {
-                            format!("Error while recursing into {}", subpath.display())
-                        })
+                    check_path(
+                        nix_file_store,
+                        relative_package_dir,
+                        absolute_package_dir,
+                        &subpath.join(entry.file_name()),
+                    )
                 })
-                .collect_vec()?,
+                .collect_vec()
+                .with_context(|| format!("Error while recursing into {}", subpath.display()))?,
         )
     } else if path.is_file() {
         // Only check Nix files
         if let Some(ext) = path.extension() {
             if ext == OsStr::new("nix") {
-                check_nix_file(relative_package_dir, absolute_package_dir, subpath).with_context(
-                    || format!("Error while checking Nix file {}", subpath.display()),
-                )?
+                check_nix_file(
+                    nix_file_store,
+                    relative_package_dir,
+                    absolute_package_dir,
+                    subpath,
+                )
+                .with_context(|| format!("Error while checking Nix file {}", subpath.display()))?
             } else {
                 Success(())
             }
@@ -92,91 +112,63 @@ fn check_path(
 /// Check whether a nix file contains path expression references pointing outside the package
 /// directory
 fn check_nix_file(
+    nix_file_store: &mut NixFileStore,
     relative_package_dir: &Path,
     absolute_package_dir: &Path,
     subpath: &Path,
 ) -> validation::Result<()> {
     let path = absolute_package_dir.join(subpath);
-    let parent_dir = path
-        .parent()
-        .with_context(|| format!("Could not get parent of path {}", subpath.display()))?;
 
-    let contents = read_to_string(&path)
-        .with_context(|| format!("Could not read file {}", subpath.display()))?;
+    let nix_file = nix_file_store.get(&path)?;
 
-    let root = Root::parse(&contents);
-    if let Some(error) = root.errors().first() {
-        // NOTE: There's now another Nixpkgs CI check to make sure all changed Nix files parse
-        // correctly, though that uses mainline Nix instead of rnix, so it doesn't give the same
-        // errors. In the future we should unify these two checks, ideally moving the other CI
-        // check into this tool as well and checking for both mainline Nix and rnix.
-        return Ok(NixpkgsProblem::CouldNotParseNix {
-            relative_package_dir: relative_package_dir.to_path_buf(),
-            subpath: subpath.to_path_buf(),
-            error: error.clone(),
-        }
-        .into());
-    }
+    Ok(validation::sequence_(
+        nix_file.syntax_root.syntax().descendants().map(|node| {
+            let text = node.text().to_string();
+            let line = nix_file.line_index.line(node.text_range().start().into());
 
-    let line_index = LineIndex::new(&contents);
+            // We're only interested in Path expressions
+            let Some(path) = rnix::ast::Path::cast(node) else {
+                return Success(());
+            };
 
-    Ok(validation::sequence_(root.syntax().descendants().map(
-        |node| {
-            let text = node.text().to_string();
-            let line = line_index.line(node.text_range().start().into());
+            use crate::nix_file::ResolvedPath;
 
-            if node.kind() != NODE_PATH {
-                // We're only interested in Path expressions
-                Success(())
-            } else if node.children().count() != 0 {
-                // Filters out ./foo/${bar}/baz
-                // TODO: We can just check ./foo
-                NixpkgsProblem::PathInterpolation {
+            match nix_file.static_resolve_path(path, absolute_package_dir) {
+                ResolvedPath::Interpolated => NixpkgsProblem::PathInterpolation {
                     relative_package_dir: relative_package_dir.to_path_buf(),
                     subpath: subpath.to_path_buf(),
                     line,
                     text,
                 }
-                .into()
-            } else if text.starts_with('<') {
-                // Filters out search paths like <nixpkgs>
-                NixpkgsProblem::SearchPath {
+                .into(),
+                ResolvedPath::SearchPath => NixpkgsProblem::SearchPath {
                     relative_package_dir: relative_package_dir.to_path_buf(),
                     subpath: subpath.to_path_buf(),
                     line,
                     text,
                 }
-                .into()
-            } else {
-                // Resolves the reference of the Nix path
-                // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz`
-                match parent_dir.join(Path::new(&text)).canonicalize() {
-                    Ok(target) => {
-                        // Then checking if it's still in the package directory
-                        // No need to handle the case of it being inside the directory, since we scan through the
-                        // entire directory recursively anyways
-                        if let Err(_prefix_error) = target.strip_prefix(absolute_package_dir) {
-                            NixpkgsProblem::OutsidePathReference {
-                                relative_package_dir: relative_package_dir.to_path_buf(),
-                                subpath: subpath.to_path_buf(),
-                                line,
-                                text,
-                            }
-                            .into()
-                        } else {
-                            Success(())
-                        }
-                    }
-                    Err(e) => NixpkgsProblem::UnresolvablePathReference {
-                        relative_package_dir: relative_package_dir.to_path_buf(),
-                        subpath: subpath.to_path_buf(),
-                        line,
-                        text,
-                        io_error: e,
-                    }
-                    .into(),
+                .into(),
+                ResolvedPath::Outside => NixpkgsProblem::OutsidePathReference {
+                    relative_package_dir: relative_package_dir.to_path_buf(),
+                    subpath: subpath.to_path_buf(),
+                    line,
+                    text,
+                }
+                .into(),
+                ResolvedPath::Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference {
+                    relative_package_dir: relative_package_dir.to_path_buf(),
+                    subpath: subpath.to_path_buf(),
+                    line,
+                    text,
+                    io_error: e,
+                }
+                .into(),
+                ResolvedPath::Within(..) => {
+                    // No need to handle the case of it being inside the directory, since we scan through the
+                    // entire directory recursively anyways
+                    Success(())
                 }
             }
-        },
-    )))
+        }),
+    ))
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
index 4051ca037c9a3..9b615dd9969ac 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
@@ -3,6 +3,7 @@ use crate::references;
 use crate::utils;
 use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME};
 use crate::validation::{self, ResultIteratorExt, Validation::Success};
+use crate::NixFileStore;
 use itertools::concat;
 use lazy_static::lazy_static;
 use regex::Regex;
@@ -34,7 +35,10 @@ pub fn relative_file_for_package(package_name: &str) -> PathBuf {
 
 /// Check the structure of Nixpkgs, returning the attribute names that are defined in
 /// `pkgs/by-name`
-pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
+pub fn check_structure(
+    path: &Path,
+    nix_file_store: &mut NixFileStore,
+) -> validation::Result<Vec<String>> {
     let base_dir = path.join(BASE_SUBPATH);
 
     let shard_results = utils::read_dir_sorted(&base_dir)?
@@ -88,7 +92,13 @@ pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
                 let package_results = entries
                     .into_iter()
                     .map(|package_entry| {
-                        check_package(path, &shard_name, shard_name_valid, package_entry)
+                        check_package(
+                            nix_file_store,
+                            path,
+                            &shard_name,
+                            shard_name_valid,
+                            package_entry,
+                        )
                     })
                     .collect_vec()?;
 
@@ -102,6 +112,7 @@ pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
 }
 
 fn check_package(
+    nix_file_store: &mut NixFileStore,
     path: &Path,
     shard_name: &str,
     shard_name_valid: bool,
@@ -161,6 +172,7 @@ fn check_package(
         });
 
         let result = result.and(references::check_references(
+            nix_file_store,
             &relative_package_dir,
             &path.join(&relative_package_dir),
         )?);
diff --git a/pkgs/test/nixpkgs-check-by-name/src/utils.rs b/pkgs/test/nixpkgs-check-by-name/src/utils.rs
index 7e0198dede424..9a5d12748918a 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/utils.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/utils.rs
@@ -35,12 +35,13 @@ impl LineIndex {
         // the vec
         for split in s.split_inclusive('\n') {
             index += split.len();
-            newlines.push(index);
+            newlines.push(index - 1);
         }
         LineIndex { newlines }
     }
 
-    /// Returns the line number for a string index
+    /// Returns the line number for a string index.
+    /// If the index points to a newline, returns the line number before the newline
     pub fn line(&self, index: usize) -> usize {
         match self.newlines.binary_search(&index) {
             // +1 because lines are 1-indexed
@@ -48,4 +49,47 @@ impl LineIndex {
             Err(x) => x + 1,
         }
     }
+
+    /// Returns the string index for a line and column.
+    pub fn fromlinecolumn(&self, line: usize, column: usize) -> usize {
+        // If it's the 1th line, the column is the index
+        if line == 1 {
+            // But columns are 1-indexed
+            column - 1
+        } else {
+            // For the nth line, we add the index of the (n-1)st newline to the column,
+            // and remove one more from the index since arrays are 0-indexed.
+            // Then add the 1-indexed column to get not the newline index itself,
+            // but rather the index of the position on the next line
+            self.newlines[line - 2] + column
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn line_index() {
+        let line_index = LineIndex::new("a\nbc\n\ndef\n");
+
+        let pairs = [
+            (0, 1, 1),
+            (1, 1, 2),
+            (2, 2, 1),
+            (3, 2, 2),
+            (4, 2, 3),
+            (5, 3, 1),
+            (6, 4, 1),
+            (7, 4, 2),
+            (8, 4, 3),
+            (9, 4, 4),
+        ];
+
+        for (index, line, column) in pairs {
+            assert_eq!(line_index.line(index), line);
+            assert_eq!(line_index.fromlinecolumn(line, column), index);
+        }
+    }
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/all-packages.nix b/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/all-packages.nix
new file mode 100644
index 0000000000000..306d719c9e9d6
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/all-packages.nix
@@ -0,0 +1,7 @@
+self: super: {
+  set = self.callPackages ({ callPackage }: {
+    foo = callPackage ({ someDrv }: someDrv) { };
+  }) { };
+
+  inherit (self.set) foo;
+}
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/default.nix
new file mode 100644
index 0000000000000..861260cdca4b2
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/default.nix
@@ -0,0 +1 @@
+import <test-nixpkgs> { root = ./.; }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/pkgs/by-name/README.md b/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/pkgs/by-name/README.md
new file mode 100644
index 0000000000000..e69de29bb2d1d
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/callPackage-syntax/pkgs/by-name/README.md
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/all-packages.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/all-packages.nix
new file mode 100644
index 0000000000000..85f8c6138c5c2
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/all-packages.nix
@@ -0,0 +1,5 @@
+self: super: {
+  foo-variant-unvarianted = self.callPackage ./package.nix { };
+
+  foo-variant-new = self.callPackage ./pkgs/by-name/fo/foo/package.nix { };
+}
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/all-packages.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/all-packages.nix
new file mode 100644
index 0000000000000..734604360073b
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/all-packages.nix
@@ -0,0 +1,3 @@
+self: super: {
+  foo-variant-unvarianted = self.callPackage ./pkgs/by-name/fo/foo/package.nix { };
+}
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/default.nix
new file mode 100644
index 0000000000000..861260cdca4b2
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/default.nix
@@ -0,0 +1 @@
+import <test-nixpkgs> { root = ./.; }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/pkgs/by-name/fo/foo/package.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/pkgs/by-name/fo/foo/package.nix
new file mode 100644
index 0000000000000..a1b92efbbadb9
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/base/pkgs/by-name/fo/foo/package.nix
@@ -0,0 +1 @@
+{ someDrv }: someDrv
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/default.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/default.nix
new file mode 100644
index 0000000000000..861260cdca4b2
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/default.nix
@@ -0,0 +1 @@
+import <test-nixpkgs> { root = ./.; }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/package.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/package.nix
new file mode 100644
index 0000000000000..a1b92efbbadb9
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/package.nix
@@ -0,0 +1 @@
+{ someDrv }: someDrv
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-variants/pkgs/by-name/fo/foo/package.nix b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/pkgs/by-name/fo/foo/package.nix
new file mode 100644
index 0000000000000..a1b92efbbadb9
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-variants/pkgs/by-name/fo/foo/package.nix
@@ -0,0 +1 @@
+{ someDrv }: someDrv