diff --git a/Cargo.lock b/Cargo.lock index aeb3908..6231d75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -146,6 +146,12 @@ version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7704b5fdd17b18ae31c4c1da5a2e0305a2bf17b5249300a9ee9ed7b72114c636" +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "either" version = "1.13.0" @@ -260,6 +266,7 @@ dependencies = [ "indoc", "itertools", "lazy_static", + "pretty_assertions", "regex", "relative-path", "rnix", @@ -294,6 +301,16 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "pretty_assertions" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af7cee1a6c8a5b9208b3cb1061f10c0cb689087b3d8ce85fb9d2dd7a29b6ba66" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "proc-macro2" version = "1.0.86" @@ -669,3 +686,9 @@ name = "windows_x86_64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + +[[package]] +name = "yansi" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" diff --git a/Cargo.toml b/Cargo.toml index 95754be..44e7960 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/src/nix_file.rs b/src/nix_file.rs index 3b1695f..6a5c665 100644 --- a/src/nix_file.rs +++ b/src/nix_file.rs @@ -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 ` or `inherit () `. - // This is the only other way how `builtins.unsafeGetAttrPos` can return - // attribute positions, but we only look for ones like ` = `, - // so ignore this. + // For Nix versions <= 2.20, this is something like `inherit ` or + // `inherit () `. Since we only look for expressions like + // ` = `, ignore this. return Ok(Left(node.to_string())); } else { // However, anything else is not expected and smells like a bug. @@ -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 ` or `inherit () `. + // We only look for ones like ` = `, 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!( @@ -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 @@ -471,10 +486,14 @@ mod tests { ; # E - /**/quuux/**/=/**/5/**/;/*E*/ + /**/testF/**/=/**/5/**/;/*E*/ - inherit toInherit; - inherit (toInherit) toInherit; + inherit testG; + inherit ({}) testH; + + inherit ({}) + testI testJ + testK testL; } "#}; @@ -482,31 +501,86 @@ mod tests { 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(()) }