gador/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tests.nixpkgs-check-by-name: Format

Browse Source 
With `cargo fmt`

Explicitly didn't do that for previous commits in order to keep the diff
more easily reviewable
This commit is contained in:
Silvan Mosberger
2024-01-30 21:54:04 +01:00
parent db435ae516
commit 46da6c5c1f
3 changed files with 179 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
+96 -31
View File
@@ -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<Option<CallPackageArgumentInfo>> {
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)
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>> {
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));
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 <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,
// 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
@@ -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 {
pkgs/test/nixpkgs-check-by-name/src/references.rs
+72 -51
View File
@@ -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 <nixpkgs>
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 <nixpkgs>
{
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(())
}
}
}),
))
}
pkgs/test/nixpkgs-check-by-name/src/structure.rs
+11 -2
View File
@@ -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<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)?
@@ -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()?;