tests.nixpkgs-check-by-name: Improve errors for new packages

Or rather, make it more consistent

6 files changed, 75 insertions, 23 deletions

diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
index 4748e907db72..a8427a27ee43 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -504,7 +504,7 @@ fn handle_non_by_name_attribute(
                     _ => {
                         // Otherwise, the path is outside `pkgs/by-name`, which means it can be
                         // migrated
-                        Loose(syntactic_call_package)
+                        Loose((syntactic_call_package, location.file))
                     }
                 }
             }
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 762d0e9fb0e0..fac789e3e03f 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
@@ -121,11 +121,13 @@ pub enum NixpkgsProblem {
         package_name: String,
         call_package_path: Option<PathBuf>,
         empty_arg: bool,
+        file: PathBuf,
     },
     NewPackageNotUsingByName {
         package_name: String,
         call_package_path: Option<PathBuf>,
         empty_arg: bool,
+        file: PathBuf,
     },
     InternalCallPackageUsed {
         attr_name: String,
@@ -340,7 +342,7 @@ impl fmt::Display for NixpkgsProblem {
                     subpath.display(),
                     text,
                 ),
-            NixpkgsProblem::MovedOutOfByName { package_name, call_package_path, empty_arg } => {
+            NixpkgsProblem::MovedOutOfByName { package_name, call_package_path, empty_arg, file } => {
                 let call_package_arg =
                     if let Some(path) = &call_package_path {
                         format!("./{}", path.display())
@@ -348,22 +350,30 @@ impl fmt::Display for NixpkgsProblem {
                         "...".into()
                     };
                 if *empty_arg {
-                    write!(
+                    writedoc!(
                         f,
-                        "pkgs.{package_name}: This top-level package was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ }}` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.",
+                        "
+                        - Attribute `pkgs.{package_name} was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ /* ... */ }}` in {}.
+                          Please move the package back and remove the manual `callPackage`.
+                        ",
                         structure::relative_file_for_package(package_name).display(),
+                        file.display(),
                         )
                 } else {
                     // This can happen if users mistakenly assume that for custom arguments,
                     // pkgs/by-name can't be used.
-                    write!(
+                    writedoc!(
                         f,
-                        "pkgs.{package_name}: This top-level package was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.",
+                        "
+                        - Attribute `pkgs.{package_name}` was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` in {}.
+                          While the manual `callPackage` is still needed, it's not necessary to move the package files.
+                        ",
                         structure::relative_file_for_package(package_name).display(),
+                        file.display(),
                         )
                 }
             },
-            NixpkgsProblem::NewPackageNotUsingByName { package_name, call_package_path, empty_arg } => {
+            NixpkgsProblem::NewPackageNotUsingByName { package_name, call_package_path, empty_arg, file } => {
                 let call_package_arg =
                     if let Some(path) = &call_package_path {
                         format!("./{}", path.display())
@@ -372,13 +382,18 @@ impl fmt::Display for NixpkgsProblem {
                     };
                 let extra =
                     if *empty_arg {
-                        "Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore."
+                        format!("Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {} is needed anymore.", file.display())
                     } else {
-                        "Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed."
+                        format!("Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {} is still needed.", file.display())
                     };
-                write!(
+                writedoc!(
                     f,
-                    "pkgs.{package_name}: This is a new top-level package of the form `callPackage {call_package_arg} {{ }}`. Please define it in {} instead. See `pkgs/by-name/README.md` for more details. {extra}",
+                    "
+                    - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`.
+                      Please define it in {} instead.
+                      See `pkgs/by-name/README.md` for more details.
+                      {extra}
+                    ",
                     structure::relative_file_for_package(package_name).display(),
                 )
             },
diff --git a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
index bdc1fbec4a27..28036d3689f1 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
@@ -6,6 +6,7 @@ use crate::nix_file::CallPackageArgumentInfo;
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::validation::{self, Validation, Validation::Success};
 use std::collections::HashMap;
+use std::path::PathBuf;
 
 /// The ratchet value for the entirety of Nixpkgs.
 #[derive(Default)]
@@ -145,24 +146,26 @@ impl ToNixpkgsProblem for ManualDefinition {
 pub enum UsesByName {}
 
 impl ToNixpkgsProblem for UsesByName {
-    type ToContext = CallPackageArgumentInfo;
+    type ToContext = (CallPackageArgumentInfo, PathBuf);
 
     fn to_nixpkgs_problem(
         name: &str,
         optional_from: Option<()>,
-        to: &Self::ToContext,
+        (to, file): &Self::ToContext,
     ) -> NixpkgsProblem {
         if let Some(()) = optional_from {
             NixpkgsProblem::MovedOutOfByName {
                 package_name: name.to_owned(),
                 call_package_path: to.relative_path.clone(),
                 empty_arg: to.empty_arg,
+                file: file.to_owned(),
             }
         } else {
             NixpkgsProblem::NewPackageNotUsingByName {
                 package_name: name.to_owned(),
                 call_package_path: to.relative_path.clone(),
                 empty_arg: to.empty_arg,
+                file: file.to_owned(),
             }
         }
     }
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected b/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected
index 8a2116aebcb9..3319cc271ac9 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/expected
@@ -1,5 +1,13 @@
-pkgs.foo1: This top-level package was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
-pkgs.foo2: This top-level package was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
-pkgs.foo3: This top-level package was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
-pkgs.foo4: This top-level package was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
+- Attribute `pkgs.foo1 was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { /* ... */ }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix.
+  Please move the package back and remove the manual `callPackage`.
+
+- Attribute `pkgs.foo2 was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { /* ... */ }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix.
+  Please move the package back and remove the manual `callPackage`.
+
+- Attribute `pkgs.foo3` was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix.
+  While the manual `callPackage` is still needed, it's not necessary to move the package files.
+
+- Attribute `pkgs.foo4` was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/move-to-non-by-name/all-packages.nix.
+  While the manual `callPackage` is still needed, it's not necessary to move the package files.
+
 This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected b/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected
index 31eff28f3c88..a7db19e5fd90 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/expected
@@ -1,5 +1,21 @@
-pkgs.new1: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new1/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
-pkgs.new2: This is a new top-level package of the form `callPackage ./without-config.nix { }`. Please define it in pkgs/by-name/ne/new2/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
-pkgs.new3: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new3/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed.
-pkgs.new4: This is a new top-level package of the form `callPackage ./with-config.nix { }`. Please define it in pkgs/by-name/ne/new4/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed.
+- Attribute `pkgs.new1` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
+  Please define it in pkgs/by-name/ne/new1/package.nix instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is `{ }`, no manual `callPackage` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is needed anymore.
+
+- Attribute `pkgs.new2` is a new top-level package using `pkgs.callPackage ./without-config.nix { /* ... */ }`.
+  Please define it in pkgs/by-name/ne/new2/package.nix instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is `{ }`, no manual `callPackage` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is needed anymore.
+
+- Attribute `pkgs.new3` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
+  Please define it in pkgs/by-name/ne/new3/package.nix instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is not `{ }`, the manual `callPackage` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is still needed.
+
+- Attribute `pkgs.new4` is a new top-level package using `pkgs.callPackage ./with-config.nix { /* ... */ }`.
+  Please define it in pkgs/by-name/ne/new4/package.nix instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is not `{ }`, the manual `callPackage` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/new-package-non-by-name/all-packages.nix is still needed.
+
 This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected b/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected
index d96d0d5638e5..2b3c2cdeee18 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected
@@ -6,7 +6,12 @@
   It is defined in all-packages.nix:2 as
 
     a = self.callPackage ./pkgs/by-name/a/a/package.nix { };
-pkgs.b: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/b/b/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
+
+- Attribute `pkgs.b` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
+  Please define it in pkgs/by-name/b/b/package.nix instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is `{ }`, no manual `callPackage` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/all-packages.nix is needed anymore.
+
 - Because pkgs/by-name/c/c exists, the attribute `pkgs.c` must be defined like
 
     c = callPackage ./pkgs/by-name/c/c/package.nix { /* ... */ };
@@ -15,5 +20,10 @@ pkgs.b: This is a new top-level package of the form `callPackage ... { }`. Pleas
   It is defined in all-packages.nix:4 as
 
     c = self.callPackage ./pkgs/by-name/c/c/package.nix { };
-pkgs.d: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/d/d/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
+
+- Attribute `pkgs.d` is a new top-level package using `pkgs.callPackage ... { /* ... */ }`.
+  Please define it in pkgs/by-name/d/d/package.nix instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is `{ }`, no manual `callPackage` in /home/tweagysil/src/nixpkgs/by-name-improv/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/all-packages.nix is needed anymore.
+
 This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch