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
philiptaron committed Jul 26, 2024
1 parent a5856f0 commit bb1b9e1
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 37 deletions.
23 changes: 23 additions & 0 deletions Cargo.lock
View file

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
View file
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"
148 changes: 111 additions & 37 deletions src/nix_file.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,12 @@ impl NixFile {
};

if ast::Attr::can_cast(node.kind()) {
// Something like `foo`, `"foo"` or `${"foo"}`
// Something like `foo`, `"foo"` or `${"foo"}`.
// For Nix versions >= 2.21, this is also `foo` in `inherit (bar) 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.
// For Nix versions <= 2.20, this is something like `inherit <attr>` or
// `inherit (<source>) <attr>`. Since we only look for expressions like
// `<attr-path> = <value>`, ignore this.
return Ok(Left(node.to_string()));
} else {
// However, anything else is not expected and smells like a bug.
Expand All @@ -210,6 +210,13 @@ impl NixFile {
)
};

// For Nix versions >= 2.21, an `ast::Attr` can have a parent of `ast::Inherit`.
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 +456,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 +486,101 @@ 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.
//
// ```
// for v in Versions.{stable,minimum,latest} lixVersions.{stable,latest}; do
// nix-instantiate --strict --eval --json driver.nix | jq -r .[]
// done | sort -u
// ```
//
// 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;")),
// generated with all Nix versions
(2, 3, Right("testA = 1;")),
// generated with all Nix versions
(3, 3, Right("\"testB\" = 2;")),
// generated with all Nix versions
(4, 3, Right("${\"testC\"} = 3;")),
// generated with all Nix versions
(5, 3, Right("\"${\"testD\"}\" = 4;")),
// generated with all Nix versions
(8, 3, Right("testE\n # B\n =\n # C\n 5\n # D\n ;")),
// generated on all Nix versions
(17, 7, Right("testF/**/=/**/5/**/;")),
// generated with Nix <= 2.20
(19, 10, Left("inherit testG;")),
// generated with Nix >= 2.21
(19, 11, Left("testG")),
// generated with Nix <= 2.20
(20, 15, Left("inherit ({}) testH;")),
// generated with Nix >= 2.21
(20, 16, Left("testH")),
// generated with Nix <= 2.20
(
22,
15,
Left("inherit ({})\n testI testJ\n testK testL;"),
),
// generated with Nix >= 2.21
(23, 5, Left("testI")),
// generated with Nix >= 2.21
(23, 11, Left("testJ")),
// generated with Nix >= 2.21
(24, 5, Left("testK")),
// generated with Nix >= 2.21
(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 bb1b9e1

Please sign in to comment.