Skip to content

Commit

Permalink
nix_file: skip inherit parent nodes also
Browse files Browse the repository at this point in the history
This causes the following tests to pass:

- `nix-build -A nixpkgsCheck.nixVersions.nix_2_21`
- `nix-build -A nixpkgsCheck.nixVersions.nix_2_22`
- `nix-build -A nixpkgsCheck.nixVersions.nix_2_23`
- `nix-build -A nixpkgsCheck.nixVersions.git`
- `nix-build -A nixpkgsCheck.lixVersions.stable`

Add a regression test for this multiline inherit situation.

In that test, use `pretty_assertions` to get a nice diff expression for
the assertion, and make it do one assertion instead of asserting in a
loop.
  • Loading branch information
philiptaron committed Jul 15, 2024
1 parent 4cbaee9 commit d128f5e
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 35 deletions.
23 changes: 23 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ relative-path = "1.9.3"
textwrap = "0.16.1"

[dev-dependencies]
pretty_assertions = "1.4.0"
temp-env = "0.3.6"
117 changes: 82 additions & 35 deletions src/nix_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ impl NixFile {
// Something like `foo`, `"foo"` or `${"foo"}`
} else if ast::Inherit::can_cast(node.kind()) {
// Something like `inherit <attr>` or `inherit (<source>) <attr>`.
// This is the only other way how `builtins.unsafeGetAttrPos` can return
// attribute positions, but we only look for ones like `<attr-path> = <value>`,
// so ignore this.
// We only look for ones like `<attr-path> = <value>`, so ignore this.
return Ok(Left(node.to_string()));
} else {
// However, anything else is not expected and smells like a bug.
Expand All @@ -210,6 +208,12 @@ impl NixFile {
)
};

if ast::Inherit::can_cast(attrpath_node.kind()) {
// Something like `inherit <attr>` or `inherit (<source>) <attr>`.
// We only look for ones like `<attr-path> = <value>`, so ignore this.
return Ok(Left(node.to_string()));
};

if !ast::Attrpath::can_cast(attrpath_node.kind()) {
// We know that `node` is an attribute, so its parent should be an attribute path.
anyhow::bail!(
Expand Down Expand Up @@ -449,20 +453,28 @@ mod tests {
use super::*;
use crate::tests;
use indoc::indoc;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
struct Position {
line: usize,
column: usize,
}

#[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;
testG: {
testA = 1;
"testB" = 2;
${"testC"} = 3;
"${"testD"}" = 4;
# A
quux
testE
# B
=
# C
Expand All @@ -471,42 +483,77 @@ mod tests {
;
# E
/**/quuux/**/=/**/5/**/;/*E*/
/**/testF/**/=/**/5/**/;/*E*/
inherit toInherit;
inherit (toInherit) toInherit;
inherit testG;
inherit ({}) testH;
inherit ({})
testI testJ
testK testL;
}
"#};

std::fs::write(&file, contents)?;

let nix_file = NixFile::new(&file)?;

// These are `builtins.unsafeGetAttrPos` locations for the attributes
// These are `builtins.unsafeGetAttrPos` locations for the attributes, generated by saving
// the Nix file above as `test.nix`, then making this `driver.nix` file.
//
// ```nix
// let
// value = import ./test.nix null;
// info =
// name:
// let
// inherit (builtins.unsafeGetAttrPos name value) line column;
// in
// _: ''(${toString line}, ${toString column}, Left("fill me in")),'';
// in
// builtins.mapAttrs info value
// ```
//
// Then, run this command to get the raw test data. Use any compatible Nix, i.e. Lix.
//
// ```
// nix run nixpkgs#nixVersions.latest -- eval --impure --json --file driver.nix | jq -r .[]
// ```
//
// Fill in the expected data until the test passes.
let cases = [
(2, 3, Right("foo = 1;")),
(3, 3, Right(r#""bar" = 2;"#)),
(4, 3, Right(r#"${"baz"} = 3;"#)),
(5, 3, Right(r#""${"qux"}" = 4;"#)),
(8, 3, Right("quux\n # B\n =\n # C\n 5\n # D\n ;")),
(17, 7, Right("quuux/**/=/**/5/**/;")),
(19, 10, Left("inherit toInherit;")),
(20, 22, Left("inherit (toInherit) toInherit;")),
(2, 3, Right("testA = 1;")),
(3, 3, Right("\"testB\" = 2;")),
(4, 3, Right("${\"testC\"} = 3;")),
(5, 3, Right("\"${\"testD\"}\" = 4;")),
(8, 3, Right("testE\n # B\n =\n # C\n 5\n # D\n ;")),
(17, 7, Right("testF/**/=/**/5/**/;")),
(19, 11, Left("testG")),
(20, 16, Left("testH")),
(23, 5, Left("testI")),
(23, 11, Left("testJ")),
(24, 5, Left("testK")),
(24, 11, Left("testL")),
];

for (line, column, expected_result) in cases {
let actual_result = nix_file
.attrpath_value_at(line, column)
.context(format!("line {line}, column {column}"))?
.map_right(|node| node.to_string());
let owned_expected_result = expected_result
.map(|x| x.to_string())
.map_left(|x| x.to_string());
assert_eq!(
actual_result, owned_expected_result,
"line {line}, column {column}"
);
}
let expected = BTreeMap::from_iter(cases.map(|(line, column, result)| {
(
Position { line, column },
result
.map(|node| node.to_string())
.map_right(|node| node.to_string()),
)
}));
let actual = BTreeMap::from_iter(cases.map(|(line, column, _)| {
(
Position { line, column },
nix_file
.attrpath_value_at(line, column)
.context(format!("line {line}, column {column}"))
.expect("value")
.map_right(|node| node.to_string()),
)
}));
assert_eq!(expected, actual);

Ok(())
}
Expand Down

0 comments on commit d128f5e

Please sign in to comment.