From 7258d472bcd8886be8b1463bea6030d1afe93bbf Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 22 Feb 2024 23:43:02 +0100 Subject: [PATCH] tests.nixpkgs-check-by-name: Improve non-syntactic callPackage error more --- pkgs/test/nixpkgs-check-by-name/Cargo.lock | 30 +++++++++++++++ pkgs/test/nixpkgs-check-by-name/Cargo.toml | 1 + .../src/nixpkgs_problem.rs | 38 +++++++++++++++---- .../expected | 10 ++++- .../tests/override-different-file/expected | 4 +- .../tests/override-no-call-package/expected | 10 ++++- 6 files changed, 79 insertions(+), 14 deletions(-) diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.lock b/pkgs/test/nixpkgs-check-by-name/Cargo.lock index b02f7075ab61..19435c2ab76e 100644 --- a/pkgs/test/nixpkgs-check-by-name/Cargo.lock +++ b/pkgs/test/nixpkgs-check-by-name/Cargo.lock @@ -306,6 +306,7 @@ dependencies = [ "serde_json", "temp-env", "tempfile", + "textwrap", ] [[package]] @@ -489,6 +490,12 @@ version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "942b4a808e05215192e39f4ab80813e599068285906cc91aa64f923db842bd5a" +[[package]] +name = "smawk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" + [[package]] name = "strsim" version = "0.10.0" @@ -534,12 +541,35 @@ version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f18aa187839b2bdb1ad2fa35ead8c4c2976b64e4363c386d45ac0f7ee85c9233" +[[package]] +name = "textwrap" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" +dependencies = [ + "smawk", + "unicode-linebreak", + "unicode-width", +] + [[package]] name = "unicode-ident" version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" +[[package]] +name = "unicode-linebreak" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" + +[[package]] +name = "unicode-width" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" + [[package]] name = "utf8parse" version = "0.2.1" diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.toml b/pkgs/test/nixpkgs-check-by-name/Cargo.toml index 1b8f8e9085d4..50cdabb7e2dd 100644 --- a/pkgs/test/nixpkgs-check-by-name/Cargo.toml +++ b/pkgs/test/nixpkgs-check-by-name/Cargo.toml @@ -17,6 +17,7 @@ itertools = "0.11.0" rowan = "0.15.11" indoc = "2.0.4" relative-path = "1.9.2" +textwrap = "0.16.1" [dev-dependencies] temp-env = "0.3.5" diff --git a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs index dda89895697f..d8ae2db4dc22 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs @@ -188,28 +188,50 @@ impl fmt::Display for NixpkgsProblem { " - Because {} exists, the attribute `pkgs.{package_name}` must be defined like - {package_name} = callPackage {} {{ /* ... */ }} + {package_name} = callPackage {} {{ /* ... */ }}; This is however not the case: The first `callPackage` argument is the wrong path. It is defined in {}:{}:{} as - {package_name} = callPackage {} {{ /* ... */ }}", + {package_name} = callPackage {} {{ /* ... */ }};", structure::relative_dir_for_package(package_name).display(), create_path_expr(file, expected_path), file.display(), line, column, create_path_expr(file, actual_path), }, - NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => - write!( + NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => { + // This is needed such multi-line definitions don't look odd + // A bit round-about, but it works and we might not need anything more complicated + let definition_indented = + // The entire code should be indented 4 spaces + textwrap::indent( + // But first we want to strip the code's natural indentation + &textwrap::dedent( + // The definition _doesn't_ include the leading spaces, but we can + // recover those from the column + &format!("{}{definition}", + " ".repeat(column - 1)), + ), + " ", + ); + writedoc!( f, - "Because {} exists, the attribute `pkgs.{package_name}` must be defined as `callPackage {} {{ ... }}`. This is however not the case: The attribute is defined in {}:{}:{} as\n\t{}", + " + - Because {} exists, the attribute `pkgs.{package_name}` must be defined like + + {package_name} = callPackage {} {{ /* ... */ }}; + + This is however not the case. + It is defined in {}:{} as + + {}", structure::relative_dir_for_package(package_name).display(), structure::relative_file_for_package(package_name).display(), file.display(), line, - column, - definition, - ), + definition_indented, + ) + } NixpkgsProblem::NonDerivation { relative_package_file, package_name } => write!( f, diff --git a/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected b/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected index f590ef4ff9fa..e62009633885 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/expected @@ -1,2 +1,8 @@ -Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined as `callPackage pkgs/by-name/fo/foo/package.nix { ... }`. This is however not the case: The attribute is defined in all-packages.nix:4:3 as - foo = self.bar; +- Because pkgs/by-name/fo/foo exists, the attribute `pkgs.foo` must be defined like + + foo = callPackage pkgs/by-name/fo/foo/package.nix { /* ... */ }; + + This is however not the case. + It is defined in all-packages.nix:4 as + + foo = self.bar; diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected index 3566b15bdbc4..18bf137364fe 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-different-file/expected @@ -1,8 +1,8 @@ - Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like - nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ } + nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; This is however not the case: The first `callPackage` argument is the wrong path. It is defined in all-packages.nix:2:3 as - nonDerivation = callPackage ./someDrv.nix { /* ... */ } + nonDerivation = callPackage ./someDrv.nix { /* ... */ }; diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected index 25fc867b04fe..e31f9eb6d539 100644 --- a/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected +++ b/pkgs/test/nixpkgs-check-by-name/tests/override-no-call-package/expected @@ -1,2 +1,8 @@ -Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined as `callPackage pkgs/by-name/no/nonDerivation/package.nix { ... }`. This is however not the case: The attribute is defined in all-packages.nix:2:3 as - nonDerivation = self.someDrv; +- Because pkgs/by-name/no/nonDerivation exists, the attribute `pkgs.nonDerivation` must be defined like + + nonDerivation = callPackage pkgs/by-name/no/nonDerivation/package.nix { /* ... */ }; + + This is however not the case. + It is defined in all-packages.nix:2 as + + nonDerivation = self.someDrv;