tests.nixpkgs-check-by-name: Re-use nixFileCache more

Makes the reference check use the nixFileCache instead of separately
parsing and resolving paths

pkgs/test/nixpkgs-check-by-name/src/main.rs

@@ -136,7 +136,7 @@ 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_cache)?.result_map(|package_names|
                 // Only if we could successfully parse the structure, we do the evaluation checks
                 eval::check_values(&nixpkgs_path, &mut nix_file_cache, package_names, keep_nix_path))?
         }

pkgs/test/nixpkgs-check-by-name/src/references.rs

@@ -1,23 +1,23 @@
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::utils;
-use crate::utils::LineIndex;
 use crate::validation::{self, ResultIteratorExt, Validation::Success};
+use crate::NixFileCache;
 
+use rowan::ast::AstNode;
 use anyhow::Context;
-use rnix::{Root, SyntaxKind::NODE_PATH};
 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_cache: &mut NixFileCache,
     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(|| {
+    check_path(nix_file_cache, relative_package_dir, absolute_package_dir, Path::new("")).with_context(|| {
         format!(
             "While checking the references in package directory {}",
             relative_package_dir.display()
@@ -27,6 +27,7 @@ pub fn check_references(
 
 /// Checks for a specific path to not have references outside
 fn check_path(
+    nix_file_cache: &mut NixFileCache,
     relative_package_dir: &Path,
     absolute_package_dir: &Path,
     subpath: &Path,
@@ -63,7 +64,7 @@ fn check_path(
                 .into_iter()
                 .map(|entry| {
                     let entry_subpath = subpath.join(entry.file_name());
-                    check_path(relative_package_dir, absolute_package_dir, &entry_subpath)
+                    check_path(nix_file_cache, relative_package_dir, absolute_package_dir, &entry_subpath)
                         .with_context(|| {
                             format!("Error while recursing into {}", subpath.display())
                         })
@@ -74,7 +75,7 @@ fn check_path(
         // 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(
+                check_nix_file(nix_file_cache, relative_package_dir, absolute_package_dir, subpath).with_context(
                     || format!("Error while checking Nix file {}", subpath.display()),
                 )?
             } else {
@@ -92,20 +93,16 @@ fn check_path(
 /// Check whether a nix file contains path expression references pointing outside the package
 /// directory
 fn check_nix_file(
+    nix_file_cache: &mut NixFileCache,
     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 root = Root::parse(&contents);
-    if let Some(error) = root.errors().first() {
+    let nix_file = match nix_file_cache.get(&path)? {
+        Ok(nix_file) => nix_file,
+        Err(error) =>
         // 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
@@ -115,20 +112,23 @@ fn check_nix_file(
             subpath: subpath.to_path_buf(),
             error: error.clone(),
         }
-        .into());
-    }
-
-    let line_index = LineIndex::new(&contents);
+        .into())
+    };
 
-    Ok(validation::sequence_(root.syntax().descendants().map(
+    Ok(validation::sequence_(nix_file.syntax_root.syntax().descendants().map(
         |node| {
             let text = node.text().to_string();
-            let line = line_index.line(node.text_range().start().into());
+            let line = nix_file.line_index.line(node.text_range().start().into());
 
-            if node.kind() != NODE_PATH {
                 // We're only interested in Path expressions
-                Success(())
-            } else if node.children().count() != 0 {
+                let Some(path) = rnix::ast::Path::cast(node) else {
+                    return Success(())
+                };
+
+                use crate::nix_file::ResolvedPath::*;
+
+                match nix_file.static_resolve_path(path, absolute_package_dir) {
+                Interpolated =>
                 // Filters out ./foo/${bar}/baz
                 // TODO: We can just check ./foo
                 NixpkgsProblem::PathInterpolation {
@@ -137,46 +137,36 @@ fn check_nix_file(
                     line,
                     text,
                 }
-                .into()
-            } else if text.starts_with('<') {
+                .into(),
+                SearchPath =>
                 // Filters out search paths like <nixpkgs>
                 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 {
+                .into(),
+                            Outside => 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 {
+                            .into(),
+                    Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference {
                         relative_package_dir: relative_package_dir.to_path_buf(),
                         subpath: subpath.to_path_buf(),
                         line,
                         text,
                         io_error: e,
                     }
                     .into(),
+                    Within(_p) =>
+                    // No need to handle the case of it being inside the directory, since we scan through the
+                    // entire directory recursively anyways
+                    Success(())
                 }
-            }
-        },
-    )))
+        }),
+    ))
 }

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::NixFileCache;
 use itertools::concat;
 use lazy_static::lazy_static;
 use regex::Regex;
@@ -34,7 +35,7 @@ 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_cache: &mut NixFileCache) -> validation::Result<Vec<String>> {
     let base_dir = path.join(BASE_SUBPATH);
 
     let shard_results = utils::read_dir_sorted(&base_dir)?
@@ -88,7 +89,7 @@ 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_cache, path, &shard_name, shard_name_valid, package_entry)
                     })
                     .collect_vec()?;
 
@@ -102,6 +103,7 @@ pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
 }
 
 fn check_package(
+    nix_file_cache: &mut NixFileCache,
     path: &Path,
     shard_name: &str,
     shard_name_valid: bool,
@@ -161,6 +163,7 @@ fn check_package(
         });
 
         let result = result.and(references::check_references(
+            nix_file_cache,
             &relative_package_dir,
             &path.join(&relative_package_dir),
         )?);