tests.nixpkgs-check-by-name: Intermediate NonDerivation error

4 files changed, 43 insertions, 40 deletions

diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs
index 32d43edc153..4f1ee9f674f 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs
@@ -6,6 +6,10 @@ use std::io;
 use std::path::PathBuf;
 
 pub enum CheckError {
+    NonDerivation {
+        relative_package_file: PathBuf,
+        package_name: String,
+    },
     OutsideSymlink {
         relative_package_dir: PathBuf,
         subpath: PathBuf,
@@ -56,6 +60,12 @@ impl CheckError {
 impl fmt::Display for CheckError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
+            CheckError::NonDerivation { relative_package_file, package_name } =>
+                write!(
+                    f,
+                    "pkgs.{package_name}: This attribute defined by {} is not a derivation",
+                    relative_package_file.display()
+                ),
             CheckError::OutsideSymlink { relative_package_dir, subpath } =>
                 write!(
                     f,
diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
index 26836501c97..695d52c0a8f 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -1,3 +1,4 @@
+use crate::check_result::{pass, write_check_result, CheckError};
 use crate::structure;
 use crate::utils::ErrorWriter;
 use crate::Version;
@@ -144,12 +145,16 @@ pub fn check_values<W: io::Write>(
                 continue;
             }
 
-            if !attribute_info.is_derivation {
-                error_writer.write(&format!(
-                    "pkgs.{package_name}: This attribute defined by {} is not a derivation",
-                    relative_package_file.display()
-                ))?;
-            }
+            let check_result = if !attribute_info.is_derivation {
+                CheckError::NonDerivation {
+                    relative_package_file: relative_package_file.clone(),
+                    package_name: package_name.clone(),
+                }
+                .into_result()
+            } else {
+                pass(())
+            };
+            write_check_result(error_writer, check_result)?;
         } else {
             error_writer.write(&format!(
                 "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
index ee98c5a1a89..80ed29646f7 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -5,6 +5,7 @@ mod structure;
 mod utils;
 
 use anyhow::Context;
+use check_result::write_check_result;
 use clap::{Parser, ValueEnum};
 use colored::Colorize;
 use std::io;
@@ -92,7 +93,7 @@ pub fn check_nixpkgs<W: io::Write>(
         if error_writer.empty {
             // Only if we could successfully parse the structure, we do the semantic checks
             eval::check_values(version, &mut error_writer, &nixpkgs, eval_accessible_paths)?;
-            references::check_references(&mut error_writer, &nixpkgs)?;
+            write_check_result(&mut error_writer, references::check_references(&nixpkgs))?;
         }
     }
     Ok(error_writer.empty)
diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs
index f72074483b1..b932b556f54 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/references.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs
@@ -1,20 +1,16 @@
-use crate::check_result::{
-    flatten_check_results, pass, write_check_result, CheckError, CheckResult,
-};
+use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult};
 use crate::structure::Nixpkgs;
 use crate::utils;
-use crate::utils::{ErrorWriter, LineIndex};
+use crate::utils::LineIndex;
 
 use anyhow::Context;
 use rnix::{Root, SyntaxKind::NODE_PATH};
 use std::ffi::OsStr;
 use std::fs::read_to_string;
-use std::io;
 use std::path::{Path, PathBuf};
 
 /// Small helper so we don't need to pass in the same arguments to all functions
-struct PackageContext<'a, W: io::Write> {
-    error_writer: &'a mut ErrorWriter<W>,
+struct PackageContext<'a> {
     /// The package directory relative to Nixpkgs, such as `pkgs/by-name/fo/foo`
     relative_package_dir: &'a PathBuf,
     /// The absolute package directory
@@ -23,36 +19,32 @@ struct PackageContext<'a, W: io::Write> {
 
 /// Check that every package directory in pkgs/by-name doesn't link to outside that directory.
 /// Both symlinks and Nix path expressions are checked.
-pub fn check_references<W: io::Write>(
-    error_writer: &mut ErrorWriter<W>,
-    nixpkgs: &Nixpkgs,
-) -> anyhow::Result<()> {
+pub fn check_references(nixpkgs: &Nixpkgs) -> CheckResult<()> {
     // Check the directories for each package separately
-    for package_name in &nixpkgs.package_names {
+    let check_results = nixpkgs.package_names.iter().map(|package_name| {
         let relative_package_dir = Nixpkgs::relative_dir_for_package(package_name);
-        let mut context = PackageContext {
-            error_writer,
+        let context = PackageContext {
             relative_package_dir: &relative_package_dir,
             absolute_package_dir: &nixpkgs.path.join(&relative_package_dir),
         };
 
         // The empty argument here is the subpath under the package directory to check
         // An empty one means the package directory itself
-        check_path(&mut context, Path::new("")).context(format!(
+        check_path(&context, Path::new("")).context(format!(
             "While checking the references in package directory {}",
             relative_package_dir.display()
-        ))?;
-    }
-    Ok(())
+        ))
+    });
+    flatten_check_results(check_results, |_| ())
 }
 
 /// Checks for a specific path to not have references outside
-fn check_path<W: io::Write>(context: &mut PackageContext<W>, subpath: &Path) -> anyhow::Result<()> {
+fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
     let path = context.absolute_package_dir.join(subpath);
 
     if path.is_symlink() {
         // Check whether the symlink resolves to outside the package directory
-        let check_result = match path.canonicalize() {
+        match path.canonicalize() {
             Ok(target) => {
                 // No need to handle the case of it being inside the directory, since we scan through the
                 // entire directory recursively anyways
@@ -72,18 +64,18 @@ fn check_path<W: io::Write>(context: &mut PackageContext<W>, subpath: &Path) ->
                 io_error,
             }
             .into_result(),
-        };
-        write_check_result(context.error_writer, check_result)?;
+        }
     } else if path.is_dir() {
         // Recursively check each entry
-        for entry in utils::read_dir_sorted(&path)? {
+        let check_results = utils::read_dir_sorted(&path)?.into_iter().map(|entry| {
             let entry_subpath = subpath.join(entry.file_name());
             check_path(context, &entry_subpath)
-                .context(format!("Error while recursing into {}", subpath.display()))?
-        }
+                .context(format!("Error while recursing into {}", subpath.display()))
+        });
+        flatten_check_results(check_results, |_| ())
     } else if path.is_file() {
         // Only check Nix files
-        let check_result = if let Some(ext) = path.extension() {
+        if let Some(ext) = path.extension() {
             if ext == OsStr::new("nix") {
                 check_nix_file(context, subpath).context(format!(
                     "Error while checking Nix file {}",
@@ -94,21 +86,16 @@ fn check_path<W: io::Write>(context: &mut PackageContext<W>, subpath: &Path) ->
             }
         } else {
             pass(())
-        };
-        write_check_result(context.error_writer, check_result)?;
+        }
     } else {
         // This should never happen, git doesn't support other file types
         anyhow::bail!("Unsupported file type for path {}", subpath.display());
     }
-    Ok(())
 }
 
 /// Check whether a nix file contains path expression references pointing outside the package
 /// directory
-fn check_nix_file<W: io::Write>(
-    context: &mut PackageContext<W>,
-    subpath: &Path,
-) -> CheckResult<()> {
+fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
     let path = context.absolute_package_dir.join(subpath);
     let parent_dir = path.parent().context(format!(
         "Could not get parent of path {}",