diff --git a/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs b/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs index 3e4f2668f3ea..e550f5c459f5 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs @@ -1,11 +1,11 @@ //! This is a utility module for interacting with the syntax of Nix files -use rnix::SyntaxKind; 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; @@ -66,12 +66,14 @@ impl NixFile { .with_context(|| format!("Could not read file {}", path.as_ref().display()))?; let line_index = LineIndex::new(&contents); - Ok(rnix::Root::parse(&contents).ok().map(|syntax_root| NixFile { - parent_dir: parent_dir.to_path_buf(), - path: path.as_ref().to_owned(), - syntax_root, - line_index, - })) + Ok(rnix::Root::parse(&contents) + .ok() + .map(|syntax_root| NixFile { + parent_dir: parent_dir.to_path_buf(), + path: path.as_ref().to_owned(), + syntax_root, + line_index, + })) } } @@ -110,18 +112,30 @@ impl NixFile { /// /// 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> { + pub fn call_package_argument_info_at( + &self, + line: usize, + column: usize, + relative_to: &Path, + ) -> anyhow::Result> { let Some(attrpath_value) = self.attrpath_value_at(line, column)? else { - return Ok(None) + 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> { + fn attrpath_value_at( + &self, + line: usize, + column: usize, + ) -> anyhow::Result> { let index = self.line_index.fromlinecolumn(line, column); - let token_at_offset = self.syntax_root.syntax().token_at_offset(TextSize::from(index as u32)); + 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: @@ -136,24 +150,36 @@ impl NixFile { // 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()) + 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()) + anyhow::bail!( + "Node in {} does not have a parent node: {node:?}", + self.path.display() + ) }; if attrpath_node.kind() != SyntaxKind::NODE_ATTRPATH { - return Ok(None) + 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()) + 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()) + 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;" @@ -247,8 +273,10 @@ impl NixFile { // Check that 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 ` - ident, + // This means it's something like `foo = callPackage ` + { + ident + } Expr::Select(select) => { // This means it's something like `foo = self.callPackage `. // We also end up here for e.g. `pythonPackages.callPackage`, but the @@ -257,7 +285,7 @@ impl NixFile { 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) + return Ok(None); } let Some(attrpath) = select.attrpath() else { anyhow::bail!("select node doesn't have an attrpath: {select:?}") @@ -272,9 +300,9 @@ impl NixFile { } else { // Here it's something like `foo = self."callPackage" /test.nix {}` // which we're not gonna bother with - return Ok(None) + return Ok(None); } - }, + } // Any other expression we're not gonna treat as callPackage _ => return Ok(None), }; @@ -284,7 +312,10 @@ impl NixFile { }; if token.text() == "callPackage" { - Ok(Some(CallPackageArgumentInfo { relative_path: path, empty_arg })) + Ok(Some(CallPackageArgumentInfo { + relative_path: path, + empty_arg, + })) } else { Ok(None) } @@ -334,19 +365,21 @@ impl NixFile { 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 + // 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 crate::tests; use super::*; + use crate::tests; use indoc::indoc; #[test] @@ -392,7 +425,9 @@ mod tests { ]; for (line, column, expected_result) in cases { - let actual_result = nix_file.attrpath_value_at(line, column)?.map(|node| node.to_string()); + let actual_result = nix_file + .attrpath_value_at(line, column)? + .map(|node| node.to_string()); assert_eq!(actual_result.as_deref(), expected_result); } @@ -426,11 +461,41 @@ mod tests { (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 })), + ( + 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 { diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs index 5da5097952ac..5ff8cf516882 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/references.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs @@ -3,8 +3,8 @@ use crate::utils; use crate::validation::{self, ResultIteratorExt, Validation::Success}; use crate::NixFileCache; -use rowan::ast::AstNode; use anyhow::Context; +use rowan::ast::AstNode; use std::ffi::OsStr; use std::path::Path; @@ -17,7 +17,13 @@ pub fn check_references( ) -> 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(nix_file_cache, 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() @@ -64,10 +70,13 @@ fn check_path( .into_iter() .map(|entry| { let entry_subpath = subpath.join(entry.file_name()); - check_path(nix_file_cache, relative_package_dir, absolute_package_dir, &entry_subpath) - .with_context(|| { - format!("Error while recursing into {}", subpath.display()) - }) + check_path( + nix_file_cache, + relative_package_dir, + absolute_package_dir, + &entry_subpath, + ) + .with_context(|| format!("Error while recursing into {}", subpath.display())) }) .collect_vec()?, ) @@ -75,9 +84,13 @@ fn check_path( // Only check Nix files if let Some(ext) = path.extension() { if ext == OsStr::new("nix") { - check_nix_file(nix_file_cache, relative_package_dir, absolute_package_dir, subpath).with_context( - || format!("Error while checking Nix file {}", subpath.display()), - )? + check_nix_file( + nix_file_cache, + relative_package_dir, + absolute_package_dir, + subpath, + ) + .with_context(|| format!("Error while checking Nix file {}", subpath.display()))? } else { Success(()) } @@ -107,66 +120,74 @@ fn check_nix_file( // 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(), + { + return Ok(NixpkgsProblem::CouldNotParseNix { + relative_package_dir: relative_package_dir.to_path_buf(), + subpath: subpath.to_path_buf(), + error: error.clone(), + } + .into()) } - .into()) }; - Ok(validation::sequence_(nix_file.syntax_root.syntax().descendants().map( - |node| { + 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()); - // We're only interested in Path expressions - let Some(path) = rnix::ast::Path::cast(node) else { - return Success(()) - }; + // We're only interested in Path expressions + let Some(path) = rnix::ast::Path::cast(node) else { + return Success(()); + }; - use crate::nix_file::ResolvedPath::*; + use crate::nix_file::ResolvedPath::*; - match nix_file.static_resolve_path(path, absolute_package_dir) { + match nix_file.static_resolve_path(path, absolute_package_dir) { Interpolated => // Filters out ./foo/${bar}/baz // TODO: We can just check ./foo - NixpkgsProblem::PathInterpolation { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into(), - SearchPath => - // Filters out search paths like - NixpkgsProblem::SearchPath { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into(), - Outside => NixpkgsProblem::OutsidePathReference { - relative_package_dir: relative_package_dir.to_path_buf(), - subpath: subpath.to_path_buf(), - line, - text, - } - .into(), - Unresolvable(e) => NixpkgsProblem::UnresolvablePathReference { + { + NixpkgsProblem::PathInterpolation { 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 + .into() + } + SearchPath => + // Filters out search paths like + { + NixpkgsProblem::SearchPath { + relative_package_dir: relative_package_dir.to_path_buf(), + subpath: subpath.to_path_buf(), + line, + text, + } + .into() + } + Outside => NixpkgsProblem::OutsidePathReference { + relative_package_dir: relative_package_dir.to_path_buf(), + subpath: subpath.to_path_buf(), + line, + text, + } + .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(()) } + } }), )) } diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs index 084b0185563c..7f9e7d4f717f 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs @@ -35,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, nix_file_cache: &mut NixFileCache) -> validation::Result> { +pub fn check_structure( + path: &Path, + nix_file_cache: &mut NixFileCache, +) -> validation::Result> { let base_dir = path.join(BASE_SUBPATH); let shard_results = utils::read_dir_sorted(&base_dir)? @@ -89,7 +92,13 @@ pub fn check_structure(path: &Path, nix_file_cache: &mut NixFileCache) -> valida let package_results = entries .into_iter() .map(|package_entry| { - check_package(nix_file_cache, path, &shard_name, shard_name_valid, package_entry) + check_package( + nix_file_cache, + path, + &shard_name, + shard_name_valid, + package_entry, + ) }) .collect_vec()?;