tests.nixpkgs-check-by-name: Various minor improvements

Co-Authored-By: Philip Taron <philip.taron@gmail.com>

38 files changed, 262 insertions, 213 deletions

diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
index a8427a27ee43..a43a0cd743a7 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -1,5 +1,8 @@
+use crate::nix_file::CallPackageArgumentInfo;
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::ratchet;
+use crate::ratchet::RatchetState::Loose;
+use crate::ratchet::RatchetState::Tight;
 use crate::structure;
 use crate::utils;
 use crate::validation::ResultIteratorExt as _;
@@ -296,84 +299,27 @@ fn by_name(
                         let nix_file = nix_file_store.get(&location.file)?;
 
                         // The relative path of the Nix file, for error messages
-                        let relative_file =
-                            location.relative_file(nixpkgs_path).with_context(|| {
-                                format!(
-                                    "Failed to resolve location file of attribute {attribute_name}"
-                                )
-                            })?;
+                        let relative_location_file = location.relative_file(nixpkgs_path).with_context(|| {
+                            format!("Failed to resolve the file where attribute {attribute_name} is defined")
+                        })?;
 
                         // Figure out whether it's an attribute definition of the form `= callPackage <arg1> <arg2>`,
                         // returning the arguments if so.
-                        let (optional_syntactic_call_package, definition) = nix_file.call_package_argument_info_at(
-                            location.line,
-                            location.column,
-                            nixpkgs_path,
-                        ).with_context(|| format!("Failed to get the definition info for attribute {attribute_name}"))?;
-
-                        // At this point, we completed two different checks for whether it's a
-                        // `callPackage`
-                        match (is_semantic_call_package, optional_syntactic_call_package) {
-                            // Something like `<attr> = foo`
-                            (_, None) => NixpkgsProblem::NonSyntacticCallPackage {
-                                package_name: attribute_name.to_owned(),
-                                file: relative_file,
-                                line: location.line,
-                                column: location.column,
-                                definition,
-                            }
-                            .into(),
-                            // Something like `<attr> = pythonPackages.callPackage ...`
-                            (false, Some(_)) => NixpkgsProblem::NonToplevelCallPackage {
-                                package_name: attribute_name.to_owned(),
-                                file: relative_file,
-                                line: location.line,
-                                column: location.column,
-                                definition,
-                            }
-                            .into(),
-                            // Something like `<attr> = pkgs.callPackage ...`
-                            (true, Some(syntactic_call_package)) => {
-                                if let Some(path) = syntactic_call_package.relative_path {
-                                    if path == relative_package_file {
-                                        // Manual definitions with empty arguments are not allowed
-                                        // anymore
-                                        Success(if syntactic_call_package.empty_arg {
-                                            Loose(NixpkgsProblem::EmptyArgument {
-                                                package_name: attribute_name.to_owned(),
-                                                file: relative_file,
-                                                line: location.line,
-                                                column: location.column,
-                                                definition,
-                                            })
-                                        } else {
-                                            Tight
-                                        })
-                                    } else {
-                                        // Wrong path
-                                        NixpkgsProblem::WrongCallPackagePath {
-                                            package_name: attribute_name.to_owned(),
-                                            file: relative_file,
-                                            line: location.line,
-                                            column: location.column,
-                                            actual_path: path,
-                                            expected_path: relative_package_file,
-                                        }
-                                        .into()
-                                    }
-                                } else {
-                                    // No path
-                                    NixpkgsProblem::NonPath {
-                                        package_name: attribute_name.to_owned(),
-                                        file: relative_file,
-                                        line: location.line,
-                                        column: location.column,
-                                        definition,
-                                    }
-                                    .into()
-                                }
-                            }
-                        }
+                        let (optional_syntactic_call_package, definition) = nix_file
+                            .call_package_argument_info_at(location.line, location.column, nixpkgs_path)
+                            .with_context(|| {
+                                format!("Failed to get the definition info for attribute {attribute_name}")
+                            })?;
+
+                        by_name_override(
+                            attribute_name,
+                            relative_package_file,
+                            is_semantic_call_package,
+                            optional_syntactic_call_package,
+                            definition,
+                            location,
+                            relative_location_file,
+                        )
                     } else {
                         // If manual definitions don't have a location, it's likely `mapAttrs`'d
                         // over, e.g. if it's defined in aliases.nix.
@@ -401,6 +347,85 @@ fn by_name(
     )
 }
 
+/// Handles the case for packages in `pkgs/by-name` that are manually overridden, e.g. in
+/// all-packages.nix
+fn by_name_override(
+    attribute_name: &str,
+    expected_package_file: PathBuf,
+    is_semantic_call_package: bool,
+    optional_syntactic_call_package: Option<CallPackageArgumentInfo>,
+    definition: String,
+    location: Location,
+    relative_location_file: PathBuf,
+) -> validation::Validation<ratchet::RatchetState<ratchet::ManualDefinition>> {
+    // At this point, we completed two different checks for whether it's a
+    // `callPackage`
+    match (is_semantic_call_package, optional_syntactic_call_package) {
+        // Something like `<attr> = foo`
+        (_, None) => NixpkgsProblem::NonSyntacticCallPackage {
+            package_name: attribute_name.to_owned(),
+            file: relative_location_file,
+            line: location.line,
+            column: location.column,
+            definition,
+        }
+        .into(),
+        // Something like `<attr> = pythonPackages.callPackage ...`
+        (false, Some(_)) => NixpkgsProblem::NonToplevelCallPackage {
+            package_name: attribute_name.to_owned(),
+            file: relative_location_file,
+            line: location.line,
+            column: location.column,
+            definition,
+        }
+        .into(),
+        // Something like `<attr> = pkgs.callPackage ...`
+        (true, Some(syntactic_call_package)) => {
+            if let Some(actual_package_file) = syntactic_call_package.relative_path {
+                if actual_package_file != expected_package_file {
+                    // Wrong path
+                    NixpkgsProblem::WrongCallPackagePath {
+                        package_name: attribute_name.to_owned(),
+                        file: relative_location_file,
+                        line: location.line,
+                        actual_path: actual_package_file,
+                        expected_path: expected_package_file,
+                    }
+                    .into()
+                } else {
+                    // Manual definitions with empty arguments are not allowed
+                    // anymore, but existing ones should continue to be allowed
+                    let manual_definition_ratchet = if syntactic_call_package.empty_arg {
+                        // This is the state to migrate away from
+                        Loose(NixpkgsProblem::EmptyArgument {
+                            package_name: attribute_name.to_owned(),
+                            file: relative_location_file,
+                            line: location.line,
+                            column: location.column,
+                            definition,
+                        })
+                    } else {
+                        // This is the state to migrate to
+                        Tight
+                    };
+
+                    Success(manual_definition_ratchet)
+                }
+            } else {
+                // No path
+                NixpkgsProblem::NonPath {
+                    package_name: attribute_name.to_owned(),
+                    file: relative_location_file,
+                    line: location.line,
+                    column: location.column,
+                    definition,
+                }
+                .into()
+            }
+        }
+    }
+}
+
 /// Handles the evaluation result for an attribute _not_ in `pkgs/by-name`,
 /// turning it into a validation result.
 fn handle_non_by_name_attribute(
diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
index c92d0af64969..dcc9cb9e716d 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -93,27 +93,25 @@ pub fn process<W: io::Write>(
             for error in errors {
                 writeln!(error_writer, "{}", error.to_string().red())?
             }
-            writeln!(error_writer, "{}", "The base branch is broken and still has above problems with this PR, these need to be fixed first.\nConsider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.".yellow())?;
+            writeln!(error_writer, "{}", "The base branch is broken and still has above problems with this PR, which need to be fixed first.\nConsider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.".yellow())?;
             Ok(false)
         }
         (Failure(_), Success(_)) => {
-            // Base branch fails, but the PR fixes it
             writeln!(
                 error_writer,
                 "{}",
-                "The base branch was broken, but this PR fixes it, nice job!".green()
+                "The base branch is broken, but this PR fixes it. Nice job!".green()
             )?;
             Ok(true)
         }
         (Success(_), Failure(errors)) => {
-            // Base branch succeeds, the PR breaks it
             for error in errors {
                 writeln!(error_writer, "{}", error.to_string().red())?
             }
             writeln!(
                 error_writer,
                 "{}",
-                "This PR introduces the above problems, merging would break the base branch"
+                "This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break."
                     .yellow()
             )?;
             Ok(false)
@@ -125,7 +123,8 @@ pub fn process<W: io::Write>(
                     for error in errors {
                         writeln!(error_writer, "{}", error.to_string().red())?
                     }
-                    writeln!(error_writer, "{}", "This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch".yellow())?;
+                    writeln!(error_writer, "{}", "This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.".yellow())?;
+
                     Ok(false)
                 }
                 Success(()) => {
@@ -225,7 +224,7 @@ mod tests {
         test_nixpkgs(
             "case_sensitive",
             &path,
-            "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\nThis PR introduces the above problems, merging would break the base branch\n",
+            "pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\nThis PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.\n",
         )?;
 
         Ok(())
diff --git a/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs b/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
index 0176164e2cea..1cb5d1c0d08d 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/nix_file.rs
@@ -2,6 +2,7 @@
 
 use crate::utils::LineIndex;
 use anyhow::Context;
+use itertools::Either::{self, Left, Right};
 use rnix::ast;
 use rnix::ast::Expr;
 use rnix::ast::HasEntry;
@@ -86,8 +87,8 @@ pub struct CallPackageArgumentInfo {
 impl NixFile {
     /// Returns information about callPackage arguments for an attribute at a specific line/column
     /// index.
-    /// If the location is not of the form `<attr> = callPackage <arg1> <arg2>;`, `Ok((None, String))` is
-    /// returned, with String being how the definition looks like.
+    /// If the definition at the given location is not of the form `<attr> = callPackage <arg1> <arg2>;`,
+    /// `Ok((None, String))` is returned, with `String` being the definition itself.
     ///
     /// This function only returns `Err` for problems that can't be caused by the Nix contents,
     /// but rather problems in this programs code itself.
@@ -124,8 +125,8 @@ impl NixFile {
         relative_to: &Path,
     ) -> anyhow::Result<(Option<CallPackageArgumentInfo>, String)> {
         Ok(match self.attrpath_value_at(line, column)? {
-            Err(definition) => (None, definition),
-            Ok(attrpath_value) => {
+            Left(definition) => (None, definition),
+            Right(attrpath_value) => {
                 let definition = attrpath_value.to_string();
                 let attrpath_value =
                     self.attrpath_value_call_package_argument_info(attrpath_value, relative_to)?;
@@ -139,7 +140,7 @@ impl NixFile {
         &self,
         line: usize,
         column: usize,
-    ) -> anyhow::Result<Result<ast::AttrpathValue, String>> {
+    ) -> anyhow::Result<Either<String, ast::AttrpathValue>> {
         let index = self.line_index.fromlinecolumn(line, column);
 
         let token_at_offset = self
@@ -173,11 +174,11 @@ impl NixFile {
             // 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
-            return Ok(Err(node.to_string()));
+            return Ok(Left(node.to_string()));
         } else {
             // However, anything else is not expected and smells like a bug
             anyhow::bail!(
-                "Node in {} is neither an attribute node, nor an inherit node: {node:?}",
+                "Node in {} is neither an attribute node nor an inherit node: {node:?}",
                 self.path.display()
             )
         }
@@ -217,7 +218,9 @@ impl NixFile {
         // unwrap is fine because we confirmed that we can cast with the above check.
         // We could avoid this `unwrap` for a `clone`, since `cast` consumes the argument,
         // but we still need it for the error message when the cast fails.
-        Ok(Ok(ast::AttrpathValue::cast(attrpath_value_node).unwrap()))
+        Ok(Right(
+            ast::AttrpathValue::cast(attrpath_value_node).unwrap(),
+        ))
     }
 
     // Internal function mainly to make attrpath_value_at independently testable
@@ -446,24 +449,24 @@ mod tests {
 
         // These are builtins.unsafeGetAttrPos locations for the attributes
         let cases = [
-            (2, 3, Ok("foo = 1;")),
-            (3, 3, Ok(r#""bar" = 2;"#)),
-            (4, 3, Ok(r#"${"baz"} = 3;"#)),
-            (5, 3, Ok(r#""${"qux"}" = 4;"#)),
-            (8, 3, Ok("quux\n  # B\n  =\n  # C\n  5\n  # D\n  ;")),
-            (17, 7, Ok("quuux/**/=/**/5/**/;")),
-            (19, 10, Err("inherit toInherit;")),
-            (20, 22, Err("inherit (toInherit) toInherit;")),
+            (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;")),
         ];
 
         for (line, column, expected_result) in cases {
             let actual_result = nix_file
                 .attrpath_value_at(line, column)
                 .context(format!("line {line}, column {column}"))?
-                .map(|node| node.to_string());
+                .map_right(|node| node.to_string());
             let owned_expected_result = expected_result
                 .map(|x| x.to_string())
-                .map_err(|x| x.to_string());
+                .map_left(|x| x.to_string());
             assert_eq!(
                 actual_result, owned_expected_result,
                 "line {line}, column {column}"
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 fac789e3e03f..03203073fd4a 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/nixpkgs_problem.rs
@@ -68,7 +68,6 @@ pub enum NixpkgsProblem {
         package_name: String,
         file: PathBuf,
         line: usize,
-        column: usize,
         actual_path: PathBuf,
         expected_path: PathBuf,
     },
@@ -117,16 +116,24 @@ pub enum NixpkgsProblem {
         text: String,
         io_error: String,
     },
-    MovedOutOfByName {
+    MovedOutOfByNameEmptyArg {
+        package_name: String,
+        call_package_path: Option<PathBuf>,
+        file: PathBuf,
+    },
+    MovedOutOfByNameNonEmptyArg {
         package_name: String,
         call_package_path: Option<PathBuf>,
-        empty_arg: bool,
         file: PathBuf,
     },
-    NewPackageNotUsingByName {
+    NewPackageNotUsingByNameEmptyArg {
+        package_name: String,
+        call_package_path: Option<PathBuf>,
+        file: PathBuf,
+    },
+    NewPackageNotUsingByNameNonEmptyArg {
         package_name: String,
         call_package_path: Option<PathBuf>,
-        empty_arg: bool,
         file: PathBuf,
     },
     InternalCallPackageUsed {
@@ -203,8 +210,7 @@ impl fmt::Display for NixpkgsProblem {
 
                         {package_name} = callPackage ./{} {{ /* ... */ }};
 
-                      Notably the second argument must not be empty, which is not the case.
-                      It is defined in {}:{} as
+                      However, in this PR, the second argument is empty. See the definition in {}:{}:
 
                     {}
                     ",
@@ -222,8 +228,7 @@ impl fmt::Display for NixpkgsProblem {
 
                         {package_name} = callPackage ./{} {{ /* ... */ }};
 
-                      This is however not the case: A different `callPackage` is used.
-                      It is defined in {}:{} as
+                      However, in this PR, a different `callPackage` is used. See the definition in {}:{}:
 
                     {}
                     ",
@@ -241,8 +246,7 @@ impl fmt::Display for NixpkgsProblem {
 
                         {package_name} = callPackage ./{} {{ /* ... */ }};
 
-                      This is however not the case: The first callPackage argument is not right:
-                      It is defined in {}:{} as
+                      However, in this PR, the first `callPackage` argument is not a path. See the definition in {}:{}:
 
                     {}
                     ",
@@ -252,7 +256,7 @@ impl fmt::Display for NixpkgsProblem {
                     line,
                     indent_definition(*column, definition.clone()),
                 ),
-            NixpkgsProblem::WrongCallPackagePath { package_name, file, line, column, actual_path, expected_path } =>
+            NixpkgsProblem::WrongCallPackagePath { package_name, file, line, actual_path, expected_path } =>
                 writedoc! {
                     f,
                     "
@@ -260,14 +264,13 @@ impl fmt::Display for NixpkgsProblem {
 
                         {package_name} = callPackage {} {{ /* ... */ }};
 
-                      This is however not the case: The first `callPackage` argument is the wrong path.
-                      It is defined in {}:{}:{} as
+                      However, in this PR, the first `callPackage` argument is the wrong path. See the definition in {}:{}:
 
                         {package_name} = callPackage {} {{ /* ... */ }};
                     ",
                     structure::relative_dir_for_package(package_name).display(),
                     create_path_expr(file, expected_path),
-                    file.display(), line, column,
+                    file.display(), line,
                     create_path_expr(file, actual_path),
                 },
             NixpkgsProblem::NonSyntacticCallPackage { package_name, file, line, column, definition } => {
@@ -278,8 +281,7 @@ impl fmt::Display for NixpkgsProblem {
 
                         {package_name} = callPackage ./{} {{ /* ... */ }};
 
-                      This is however not the case.
-                      It is defined in {}:{} as
+                      However, in this PR, it isn't defined that way. See the definition in {}:{}
 
                     {}
                     ",
@@ -342,49 +344,67 @@ impl fmt::Display for NixpkgsProblem {
                     subpath.display(),
                     text,
                 ),
-            NixpkgsProblem::MovedOutOfByName { package_name, call_package_path, empty_arg, file } => {
+            NixpkgsProblem::MovedOutOfByNameEmptyArg { package_name, call_package_path, file } => {
+                let call_package_arg =
+                    if let Some(path) = &call_package_path {
+                        format!("./{}", path.display())
+                    } else {
+                        "...".into()
+                    };
+                writedoc!(
+                    f,
+                    "
+                    - 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(),
+                    )
+            },
+            NixpkgsProblem::MovedOutOfByNameNonEmptyArg { package_name, call_package_path, file } => {
                 let call_package_arg =
                     if let Some(path) = &call_package_path {
                         format!("./{}", path.display())
                     } else {
                         "...".into()
                     };
-                if *empty_arg {
-                    writedoc!(
-                        f,
-                        "
-                        - 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.
-                    writedoc!(
-                        f,
-                        "
-                        - 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(),
-                        )
-                }
+                // This can happen if users mistakenly assume that for custom arguments,
+                // pkgs/by-name can't be used.
+                writedoc!(
+                    f,
+                    "
+                    - 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, file } => {
+            NixpkgsProblem::NewPackageNotUsingByNameEmptyArg { package_name, call_package_path, file } => {
                 let call_package_arg =
                     if let Some(path) = &call_package_path {
                         format!("./{}", path.display())
                     } else {
                         "...".into()
                     };
-                let extra =
-                    if *empty_arg {
-                        format!("Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {} is needed anymore.", file.display())
+                writedoc!(
+                    f,
+                    "
+                    - 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.
+                      Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {} is needed anymore.
+                    ",
+                    structure::relative_file_for_package(package_name).display(),
+                    file.display(),
+                )
+            },
+            NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg { package_name, call_package_path, file } => {
+                let call_package_arg =
+                    if let Some(path) = &call_package_path {
+                        format!("./{}", path.display())
                     } else {
-                        format!("Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {} is still needed.", file.display())
+                        "...".into()
                     };
                 writedoc!(
                     f,
@@ -392,9 +412,10 @@ impl fmt::Display for NixpkgsProblem {
                     - 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}
+                      Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {} is still needed.
                     ",
                     structure::relative_file_for_package(package_name).display(),
+                    file.display(),
                 )
             },
             NixpkgsProblem::InternalCallPackageUsed { attr_name } =>
@@ -426,14 +447,13 @@ fn indent_definition(column: usize, definition: String) -> String {
 
 /// Creates a Nix path expression that when put into Nix file `from_file`, would point to the `to_file`.
 fn create_path_expr(from_file: impl AsRef<Path>, to_file: impl AsRef<Path>) -> String {
-    // FIXME: Clean these unwrap's up
-    // But these should never trigger because we only call this function with relative paths, and only
-    // with `from_file` being an actual file (so there's always a parent)
+    // These `expect` calls should never trigger because we only call this function with
+    // relative paths that have a parent. That's why we `expect` them!
     let from = RelativePath::from_path(&from_file)
-        .unwrap()
+        .expect("a relative path")
         .parent()
-        .unwrap();
-    let to = RelativePath::from_path(&to_file).unwrap();
+        .expect("a parent for this path");
+    let to = RelativePath::from_path(&to_file).expect("a path");
     let rel = from.relative(to);
     format!("./{rel}")
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
index 28036d3689f1..eed5071a5a2a 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/ratchet.rs
@@ -154,17 +154,29 @@ impl ToNixpkgsProblem for UsesByName {
         (to, file): &Self::ToContext,
     ) -> NixpkgsProblem {
         if let Some(()) = optional_from {
-            NixpkgsProblem::MovedOutOfByName {
+            if to.empty_arg {
+                NixpkgsProblem::MovedOutOfByNameEmptyArg {
+                    package_name: name.to_owned(),
+                    call_package_path: to.relative_path.clone(),
+                    file: file.to_owned(),
+                }
+            } else {
+                NixpkgsProblem::MovedOutOfByNameNonEmptyArg {
+                    package_name: name.to_owned(),
+                    call_package_path: to.relative_path.clone(),
+                    file: file.to_owned(),
+                }
+            }
+        } else if to.empty_arg {
+            NixpkgsProblem::NewPackageNotUsingByNameEmptyArg {
                 package_name: name.to_owned(),
                 call_package_path: to.relative_path.clone(),
-                empty_arg: to.empty_arg,
                 file: file.to_owned(),
             }
         } else {
-            NixpkgsProblem::NewPackageNotUsingByName {
+            NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg {
                 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/alt-callPackage/expected b/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected
index 2d05db0d4699..1d92e652200e 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/alt-callPackage/expected
@@ -2,9 +2,8 @@
 
     foo = callPackage ./pkgs/by-name/fo/foo/package.nix { /* ... */ };
 
-  This is however not the case: A different `callPackage` is used.
-  It is defined in all-packages.nix:5 as
+  However, in this PR, a different `callPackage` is used. See the definition in all-packages.nix:5:
 
     foo = self.alt.callPackage ({ }: self.someDrv) { };
 
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected b/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected
index feb6cce2fd04..e209e1855314 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/base-fixed/expected
@@ -1 +1 @@
-The base branch was broken, but this PR fixes it, nice job!
+The base branch is broken, but this PR fixes it. Nice job!
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected b/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected
index 4f0d08357d1f..c25f06b4150e 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/base-still-broken/expected
@@ -1,3 +1,3 @@
 pkgs/by-name/bar: This is a file, but it should be a directory.
-The base branch is broken and still has above problems with this PR, these need to be fixed first.
+The base branch is broken and still has above problems with this PR, which need to be fixed first.
 Consider reverting the PR that introduced these problems in order to prevent more failures of unrelated PRs.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected b/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected
index 6b937106b806..15b3e3ff6ede 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/broken-autocall/expected
@@ -1,2 +1,2 @@
 pkgs.foo: This attribute is not defined but it should be defined automatically as pkgs/by-name/fo/foo/package.nix
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected b/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected
index ec1918cace8c..3b0146cdc146 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/incorrect-shard/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected b/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected
index 9fef30d14f37..b3d0c6fc1a40 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/internalCallPackage/expected
@@ -1,2 +1,2 @@
 pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected b/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected
index aac1b186ca49..80f6e7dd5998 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/invalid-package-name/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo/fo@: Invalid package directory name "fo@", must be ASCII characters consisting of a-z, A-Z, 0-9, "-" or "_".
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected b/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected
index ce7c2695424c..7ca9ff8565bd 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/invalid-shard-name/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/A: Invalid directory name "A", must be at most 2 ASCII characters consisting of a-z, 0-9, "-" or "_".
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected b/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected
index 71d78019604b..8822e0cc24d4 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/manual-definition/expected
@@ -2,8 +2,7 @@
 
     noEval = callPackage ./pkgs/by-name/no/noEval/package.nix { /* ... */ };
 
-  Notably the second argument must not be empty, which is not the case.
-  It is defined in all-packages.nix:9 as
+  However, in this PR, the second argument is empty. See the definition in all-packages.nix:9:
 
     noEval = self.callPackage ./pkgs/by-name/no/noEval/package.nix { };
 
@@ -11,9 +10,8 @@
 
     onlyMove = callPackage ./pkgs/by-name/on/onlyMove/package.nix { /* ... */ };
 
-  Notably the second argument must not be empty, which is not the case.
-  It is defined in all-packages.nix:7 as
+  However, in this PR, the second argument is empty. See the definition in all-packages.nix:7:
 
     onlyMove = self.callPackage ./pkgs/by-name/on/onlyMove/package.nix { };
 
-This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch
+This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected b/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected
index 93c882b2bd3c..1b67704817cf 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/missing-package-nix/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo/foo: Missing required "package.nix" file.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
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 3319cc271ac9..0e5c07cc04c6 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,7 +1,7 @@
-- 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.
+- 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.
+- 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.
@@ -10,4 +10,4 @@
 - 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
+This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected b/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected
index 303884cb7944..0105b19078c7 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/multiple-failures/expected
@@ -11,4 +11,4 @@ pkgs/by-name/ba/foo: File package.nix at line 3 contains the path expression "..
 pkgs/by-name/ba/foo: File package.nix at line 4 contains the nix search path expression "<nixpkgs>" which may point outside the directory of that package.
 pkgs/by-name/ba/foo: File package.nix at line 5 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package.
 pkgs/by-name/fo/foo: Missing required "package.nix" file.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
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 a7db19e5fd90..9cfb996f381e 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
@@ -18,4 +18,4 @@
   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
+This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected b/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected
index 48acbec1210d..1705d22be798 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/non-attrs/expected
@@ -1,2 +1,2 @@
 pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected b/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected
index 48acbec1210d..1705d22be798 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/non-derivation/expected
@@ -1,2 +1,2 @@
 pkgs.nonDerivation: This attribute defined by pkgs/by-name/no/nonDerivation/package.nix is not a derivation
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/all-packages.nix b/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/all-packages.nix
index e31aa831b814..3e0ea20c2281 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/all-packages.nix
+++ b/pkgs/test/nixpkgs-check-by-name/tests/non-syntactical-callPackage-by-name/all-packages.nix
@@ -2,4 +2,5 @@ self: super: {
 
   bar = (x: x) self.callPackage ./pkgs/by-name/fo/foo/package.nix { someFlag = true; };
   foo = self.bar;
+
 }
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 f56585ac4678..e09e931bb658 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
@@ -2,9 +2,8 @@
 
     foo = callPackage ./pkgs/by-name/fo/foo/package.nix { /* ... */ };
 
-  This is however not the case.
-  It is defined in all-packages.nix:4 as
+  However, in this PR, it isn't defined that way. See the definition in all-packages.nix:4
 
     foo = self.bar;
 
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
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 571ec8e09a5c..16292c0c0eb1 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
@@ -2,9 +2,8 @@
 
     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
+  However, in this PR, the first `callPackage` argument is the wrong path. See the definition in all-packages.nix:2:
 
     nonDerivation = callPackage ./someDrv.nix { /* ... */ };
 
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected
index 257030e9a6c0..79f78e2437a0 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/override-empty-arg/expected
@@ -2,9 +2,8 @@
 
     nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ };
 
-  Notably the second argument must not be empty, which is not the case.
-  It is defined in all-packages.nix:2 as
+  However, in this PR, the second argument is empty. See the definition in all-packages.nix:2:
 
     nonDerivation = self.callPackage ./pkgs/by-name/no/nonDerivation/package.nix { };
 
-This PR introduces the above problems compared to the base branch, merging is discouraged, but would not break the base branch
+This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.
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 eb13c4df1e88..d91d58d629f2 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
@@ -2,9 +2,8 @@
 
     nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ };
 
-  This is however not the case.
-  It is defined in all-packages.nix:2 as
+  However, in this PR, it isn't defined that way. See the definition in all-packages.nix:2
 
     nonDerivation = self.someDrv;
 
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected
index 31679e55b7d6..807c440dd3d2 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/override-no-file/expected
@@ -2,9 +2,8 @@
 
     nonDerivation = callPackage ./pkgs/by-name/no/nonDerivation/package.nix { /* ... */ };
 
-  This is however not the case: The first callPackage argument is not right:
-  It is defined in all-packages.nix:2 as
+  However, in this PR, the first `callPackage` argument is not a path. See the definition in all-packages.nix:2:
 
     nonDerivation = self.callPackage ({ someDrv }: someDrv) { };
 
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected b/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected
index e7fd8242f5f4..4adbaf66edc0 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/override-non-path/expected
@@ -2,9 +2,8 @@
 
     foo = callPackage ./pkgs/by-name/fo/foo/package.nix { /* ... */ };
 
-  This is however not the case: The first callPackage argument is not right:
-  It is defined in all-packages.nix:3 as
+  However, in this PR, the first `callPackage` argument is not a path. See the definition in all-packages.nix:3:
 
     foo = self.callPackage ({ someDrv, someFlag }: someDrv) { someFlag = true; };
 
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected b/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected
index 864978b9b731..adee1d0137fa 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-dir-is-file/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo/foo: This path is a file, but it should be a directory.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected b/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected
index 82ad2ce85bbf..d03e1eceea26 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/package-nix-dir/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo/foo: "package.nix" must be a file.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected b/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected
index 0e0e876d5561..0bdb00f20cb9 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/ref-absolute/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "/foo" which cannot be resolved: No such file or directory (os error 2).
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected b/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected
index f414e4145674..2e4338ccc7ae 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/ref-escape/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "../." which may point outside the directory of that package.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected b/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected
index 7b47a786a8be..30125570794a 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/ref-nix-path/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/aa/aa: File package.nix at line 2 contains the nix search path expression "<nixpkgs>" which may point outside the directory of that package.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected b/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected
index 1905841a63df..6567439b77f8 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/ref-path-subexpr/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/aa/aa: File package.nix at line 2 contains the path expression "./${"test"}", which is not yet supported and may point outside the directory of that package.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected b/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected
index 4a02d99778a9..689cee41f1e3 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/shard-file/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo: This is a file, but it should be a directory.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
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 2b3c2cdeee18..8f574d9dddc3 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/sorted-order/expected
@@ -2,8 +2,7 @@
 
     a = callPackage ./pkgs/by-name/a/a/package.nix { /* ... */ };
 
-  Notably the second argument must not be empty, which is not the case.
-  It is defined in all-packages.nix:2 as
+  However, in this PR, the second argument is empty. See the definition in all-packages.nix:2:
 
     a = self.callPackage ./pkgs/by-name/a/a/package.nix { };
 
@@ -16,8 +15,7 @@
 
     c = callPackage ./pkgs/by-name/c/c/package.nix { /* ... */ };
 
-  Notably the second argument must not be empty, which is not the case.
-  It is defined in all-packages.nix:4 as
+  However, in this PR, the second argument is empty. See the definition in all-packages.nix:4:
 
     c = self.callPackage ./pkgs/by-name/c/c/package.nix { };
 
@@ -26,4 +24,4 @@
   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
+This PR introduces additional instances of discouraged patterns as listed above. Merging is discouraged but would not break the base branch.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected b/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected
index e6c183207e57..cd555c658a09 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/symlink-escape/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo/foo: Path package.nix is a symlink pointing to a path outside the directory of that package.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected b/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected
index 01de2702f7d5..1b06bcf4972b 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/symlink-invalid/expected
@@ -1,2 +1,2 @@
 pkgs/by-name/fo/foo: Path foo is a symlink which cannot be resolved: No such file or directory (os error 2).
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.
diff --git a/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected b/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected
index d66a3185e701..608843d93903 100644
--- a/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected
+++ b/pkgs/test/nixpkgs-check-by-name/tests/unknown-location/expected
@@ -1,2 +1,2 @@
 pkgs.foo: Cannot determine the location of this attribute using `builtins.unsafeGetAttrPos`.
-This PR introduces the above problems, merging would break the base branch
+This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.