Include wiki links in error output (#200)

Author: philiptaron

src/main.rs

@@ -201,7 +201,7 @@ mod tests {
         test_nixpkgs(
             "case_sensitive",
             path,
-            "- pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\".\n\
+            "- pkgs/by-name/fo: Duplicate case-sensitive package directories \"foO\" and \"foo\". (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-111)\n\
             This PR introduces the problems listed above. Please fix them before merging, \
             otherwise the base branch would break.\n",
         );

src/problem/mod.rs

@@ -1,5 +1,6 @@
+use std::fmt;
+
 use derive_enum_from_into::EnumFrom;
-use derive_more::Display;
 use relative_path::RelativePath;
 
 pub mod npv_100;
@@ -36,7 +37,9 @@ pub mod npv_161;
 pub mod npv_162;
 pub mod npv_163;
 
-#[derive(Clone, Display, EnumFrom)]
+const WIKI_BASE_URL: &str = "https://github.com/NixOS/nixpkgs-vet/wiki";
+
+#[derive(Clone, EnumFrom)]
 pub enum Problem {
     /// NPV-100: attribute is not defined but it should be defined automatically
     ByNameUndefinedAttribute(npv_100::ByNameUndefinedAttribute),
@@ -133,6 +136,86 @@ pub enum Problem {
     ),
 }
 
+impl Problem {
+    /// Returns the NPV error code for this problem (e.g. "NPV-100").
+    pub fn npv_code(&self) -> &'static str {
+        match self {
+            Self::ByNameUndefinedAttribute(..) => "NPV-100",
+            Self::ByNameNonDerivation(..) => "NPV-101",
+            Self::ByNameInternalCallPackageUsed(..) => "NPV-102",
+            Self::ByNameCannotDetermineAttributeLocation(..) => "NPV-103",
+            Self::ByNameOverrideOfNonSyntacticCallPackage(..) => "NPV-104",
+            Self::ByNameOverrideOfNonTopLevelPackage(..) => "NPV-105",
+            Self::ByNameOverrideContainsWrongCallPackagePath(..) => "NPV-106",
+            Self::ByNameOverrideContainsEmptyArgument(..) => "NPV-107",
+            Self::ByNameOverrideContainsEmptyPath(..) => "NPV-108",
+            Self::ByNameShardIsNotDirectory(..) => "NPV-109",
+            Self::ByNameShardIsInvalid(..) => "NPV-110",
+            Self::ByNameShardIsCaseSensitiveDuplicate(..) => "NPV-111",
+            Self::NixEvalError(..) => "NPV-120",
+            Self::NixFileContainsPathInterpolation(..) => "NPV-121",
+            Self::NixFileContainsSearchPath(..) => "NPV-122",
+            Self::NixFileContainsPathOutsideDirectory(..) => "NPV-123",
+            Self::NixFileContainsUnresolvablePath(..) => "NPV-124",
+            Self::PackageContainsSymlinkPointingOutside(..) => "NPV-125",
+            Self::PackageContainsUnresolvableSymlink(..) => "NPV-126",
+            Self::NixFileContainsAbsolutePath(..) => "NPV-127",
+            Self::NixFileContainsHomeRelativePath(..) => "NPV-128",
+            Self::PackageDirectoryIsNotDirectory(..) => "NPV-140",
+            Self::InvalidPackageDirectoryName(..) => "NPV-141",
+            Self::PackageInWrongShard(..) => "NPV-142",
+            Self::PackageNixMissing(..) => "NPV-143",
+            Self::PackageNixIsNotFile(..) => "NPV-144",
+            Self::TopLevelPackageMovedOutOfByName(..) => "NPV-160",
+            Self::TopLevelPackageMovedOutOfByNameWithCustomArguments(..) => "NPV-161",
+            Self::NewTopLevelPackageShouldBeByName(..) => "NPV-162",
+            Self::NewTopLevelPackageShouldBeByNameWithCustomArgument(..) => "NPV-163",
+        }
+    }
+
+    /// Returns the wiki URL for this problem's documentation.
+    pub fn wiki_url(&self) -> String {
+        format!("{WIKI_BASE_URL}/{}", self.npv_code())
+    }
+}
+
+impl fmt::Display for Problem {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::ByNameUndefinedAttribute(inner) => inner.fmt(f),
+            Self::ByNameNonDerivation(inner) => inner.fmt(f),
+            Self::ByNameInternalCallPackageUsed(inner) => inner.fmt(f),
+            Self::ByNameCannotDetermineAttributeLocation(inner) => inner.fmt(f),
+            Self::ByNameOverrideOfNonSyntacticCallPackage(inner) => inner.fmt(f),
+            Self::ByNameOverrideOfNonTopLevelPackage(inner) => inner.fmt(f),
+            Self::ByNameOverrideContainsWrongCallPackagePath(inner) => inner.fmt(f),
+            Self::ByNameOverrideContainsEmptyArgument(inner) => inner.fmt(f),
+            Self::ByNameOverrideContainsEmptyPath(inner) => inner.fmt(f),
+            Self::ByNameShardIsNotDirectory(inner) => inner.fmt(f),
+            Self::ByNameShardIsInvalid(inner) => inner.fmt(f),
+            Self::ByNameShardIsCaseSensitiveDuplicate(inner) => inner.fmt(f),
+            Self::NixEvalError(inner) => inner.fmt(f),
+            Self::NixFileContainsPathInterpolation(inner) => inner.fmt(f),
+            Self::NixFileContainsSearchPath(inner) => inner.fmt(f),
+            Self::NixFileContainsPathOutsideDirectory(inner) => inner.fmt(f),
+            Self::NixFileContainsUnresolvablePath(inner) => inner.fmt(f),
+            Self::PackageContainsSymlinkPointingOutside(inner) => inner.fmt(f),
+            Self::PackageContainsUnresolvableSymlink(inner) => inner.fmt(f),
+            Self::NixFileContainsAbsolutePath(inner) => inner.fmt(f),
+            Self::NixFileContainsHomeRelativePath(inner) => inner.fmt(f),
+            Self::PackageDirectoryIsNotDirectory(inner) => inner.fmt(f),
+            Self::InvalidPackageDirectoryName(inner) => inner.fmt(f),
+            Self::PackageInWrongShard(inner) => inner.fmt(f),
+            Self::PackageNixMissing(inner) => inner.fmt(f),
+            Self::PackageNixIsNotFile(inner) => inner.fmt(f),
+            Self::TopLevelPackageMovedOutOfByName(inner) => inner.fmt(f),
+            Self::TopLevelPackageMovedOutOfByNameWithCustomArguments(inner) => inner.fmt(f),
+            Self::NewTopLevelPackageShouldBeByName(inner) => inner.fmt(f),
+            Self::NewTopLevelPackageShouldBeByNameWithCustomArgument(inner) => inner.fmt(f),
+        }
+    }
+}
+
 fn indent_definition(column: usize, definition: &str) -> String {
     // The entire code should be indented 4 spaces
     textwrap::indent(

src/status.rs

@@ -42,13 +42,33 @@ impl Status {
         // These all respect the NO_COLOR environment variable even if `use_color` is true.
         let maybe_green = |s: &str| if use_color { s.green() } else { s.into() };
         let maybe_yellow = |s: &str| if use_color { s.yellow() } else { s.into() };
-        let maybe_red = |s: &str| if use_color { s.red() } else { s.into() };
-
-        // If there are errors, print them all out first in red.
+        // Print each error with its wiki link.
         if let Some(errors) = self.errors() {
             for error in errors {
-                let error = format!("{error}\n");
-                fmt::Display::fmt(&maybe_red(&error), f)?;
+                let code = error.npv_code();
+                let url = error.wiki_url();
+
+                if use_color {
+                    let error_str = format!("{error}");
+                    // OSC 8 hyperlink: \e]8;;URL\e\\TEXT\e]8;;\e\\
+                    let link = format!("\x1b]8;;{url}\x1b\\{code}\x1b]8;;\x1b\\");
+
+                    // Most errors follow "- {path}: {message}". When we can identify
+                    // that pattern, make the path bold and the message red. Otherwise
+                    // fall back to coloring the entire error red.
+                    if let Some(rest) = error_str.strip_prefix("- ")
+                        && let Some((location, message)) = rest.split_once(": ")
+                        && !location.contains('\n')
+                    {
+                        writeln!(f, "- {}: {} ({})", location.bold(), message.red(), link)?;
+                        continue;
+                    }
+
+                    // Fallback for messages that don't match the simple pattern.
+                    writeln!(f, "{} ({})", error_str.red(), link)?;
+                } else {
+                    writeln!(f, "{error} ({url})")?;
+                }
             }
         }
 

tests/alt-callPackage/expected

@@ -5,5 +5,5 @@
   However, in this PR, a different `callPackage` is used. See the definition in pkgs/top-level/all-packages.nix:5:
 
     foo = self.alt.callPackage ({ }: self.someDrv) { };
-
+ (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-105)
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.

tests/base-still-broken/expected

@@ -1,3 +1,3 @@
-- pkgs/by-name/bar: This is a file, but it should be a directory.
+- pkgs/by-name/bar: This is a file, but it should be a directory. (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-109)
 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.

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
+- pkgs.foo: This attribute is not defined but it should be defined automatically as pkgs/by-name/fo/foo/package.nix (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-100)
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.

tests/by-name-failure/expected

@@ -1,4 +1,4 @@
 trace: This should be on stderr!
 @REDACTED@error: This is an error!@REDACTED@
-- Nix evaluation failed for some package in `pkgs/by-name`, see error above
+- Nix evaluation failed for some package in `pkgs/by-name`, see error above (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-120)
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.

tests/by-name-numprefix/expected

@@ -1,3 +1,3 @@
-- pkgs/by-name/10: Invalid directory name "10", must be at most 2 ASCII characters, starting with a-z or "_", consisting of a-z, 0-9, "-" or "_".
-- pkgs/by-name/10/10foo: Invalid package directory name "10foo", must start with a letter (a-z, A-Z) or "_", followed by ASCII characters a-z, A-Z, 0-9, "-" or "_".
+- pkgs/by-name/10: Invalid directory name "10", must be at most 2 ASCII characters, starting with a-z or "_", consisting of a-z, 0-9, "-" or "_". (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-110)
+- pkgs/by-name/10/10foo: Invalid package directory name "10foo", must start with a letter (a-z, A-Z) or "_", followed by ASCII characters a-z, A-Z, 0-9, "-" or "_". (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-141)
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.

tests/incorrect-shard/expected

@@ -1,2 +1,2 @@
-- pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead.
+- pkgs/by-name/aa/FOO: Incorrect directory location, should be pkgs/by-name/fo/FOO instead. (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-142)
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.

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.
+- pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use. (https://github.com/NixOS/nixpkgs-vet/wiki/NPV-102)
 This PR introduces the problems listed above. Please fix them before merging, otherwise the base branch would break.