tests.nixpkgs-check-by-name: Custom Validation type and improvements

Co-authored-by: Wanja Hentze

6 files changed, 293 insertions, 260 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
deleted file mode 100644
index a6538d778b6..00000000000
--- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs
+++ /dev/null
@@ -1,74 +0,0 @@
-use crate::nixpkgs_problem::NixpkgsProblem;
-use itertools::concat;
-use itertools::{
-    Either,
-    Either::{Left, Right},
-    Itertools,
-};
-
-/// A type alias representing the result of a check, either:
-/// - Err(anyhow::Error): A fatal failure, typically I/O errors.
-///   Such failures are not caused by the files in Nixpkgs.
-///   This hints at a bug in the code or a problem with the deployment.
-/// - Ok(Left(Vec<NixpkgsProblem>)): A non-fatal problem with the Nixpkgs files.
-///   Further checks can be run even with this result type.
-///   Such problems can be fixed by changing the Nixpkgs files.
-/// - Ok(Right(A)): A successful (potentially intermediate) result with an arbitrary value.
-///   No fatal errors have occurred and no problems have been found with Nixpkgs.
-pub type CheckResult<A> = anyhow::Result<Either<Vec<NixpkgsProblem>, A>>;
-
-/// Map a `CheckResult<I>` to a `CheckResult<O>` by applying a function to the
-/// potentially contained value in case of success.
-pub fn map<I, O>(check_result: CheckResult<I>, f: impl FnOnce(I) -> O) -> CheckResult<O> {
-    Ok(check_result?.map_right(f))
-}
-
-/// Create a successfull `CheckResult<A>` from a value `A`.
-pub fn ok<A>(value: A) -> CheckResult<A> {
-    Ok(Right(value))
-}
-
-impl NixpkgsProblem {
-    /// Create a `CheckResult<A>` from a single check problem
-    pub fn into_result<A>(self) -> CheckResult<A> {
-        Ok(Left(vec![self]))
-    }
-}
-
-/// Combine two check results, both of which need to be successful for the return value to be successful.
-/// The `NixpkgsProblem`s of both sides are returned concatenated.
-pub fn and<A>(first: CheckResult<()>, second: CheckResult<A>) -> CheckResult<A> {
-    match (first?, second?) {
-        (Right(_), Right(right_value)) => Ok(Right(right_value)),
-        (Left(errors), Right(_)) => Ok(Left(errors)),
-        (Right(_), Left(errors)) => Ok(Left(errors)),
-        (Left(errors_l), Left(errors_r)) => Ok(Left(concat([errors_l, errors_r]))),
-    }
-}
-
-/// Combine many checks results into a single one.
-/// All given check results need to be successful in order for the returned check result to be successful,
-/// in which case the returned check result value contains each a `Vec` of each individual result value.
-/// The `NixpkgsProblem`s of all results are returned concatenated.
-pub fn sequence<A>(check_results: impl IntoIterator<Item = CheckResult<A>>) -> CheckResult<Vec<A>> {
-    let (errors, values): (Vec<_>, Vec<_>) = check_results
-        .into_iter()
-        .collect::<anyhow::Result<Vec<_>>>()?
-        .into_iter()
-        .partition_map(|r| r);
-
-    // To combine the errors from the results we flatten all the error Vec's into a new Vec
-    // This is not very efficient, but doesn't matter because generally we should have no errors
-    let flattened_errors = errors.into_iter().flatten().collect::<Vec<_>>();
-
-    if flattened_errors.is_empty() {
-        Ok(Right(values))
-    } else {
-        Ok(Left(flattened_errors))
-    }
-}
-
-/// Like `sequence`, but replace the resulting value with ()
-pub fn sequence_<A>(check_results: impl IntoIterator<Item = CheckResult<A>>) -> CheckResult<()> {
-    map(sequence(check_results), |_| ())
-}
diff --git a/pkgs/test/nixpkgs-check-by-name/src/eval.rs b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
index 37fc783f3d3..e4f986748b4 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -1,7 +1,6 @@
-use crate::check_result;
-use crate::check_result::CheckResult;
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::structure;
+use crate::validation::{self, Validation::Success};
 use crate::Version;
 use std::path::Path;
 
@@ -46,7 +45,7 @@ pub fn check_values(
     nixpkgs_path: &Path,
     package_names: Vec<String>,
     eval_accessible_paths: Vec<&Path>,
-) -> CheckResult<()> {
+) -> validation::Result<()> {
     // Write the list of packages we need to check into a temporary JSON file.
     // This can then get read by the Nix evaluation.
     let attrs_file = NamedTempFile::new().context("Failed to create a temporary file")?;
@@ -112,52 +111,54 @@ pub fn check_values(
             String::from_utf8_lossy(&result.stdout)
         ))?;
 
-    check_result::sequence_(package_names.iter().map(|package_name| {
-        let relative_package_file = structure::relative_file_for_package(package_name);
-        let absolute_package_file = nixpkgs_path.join(&relative_package_file);
+    Ok(validation::sequence_(package_names.iter().map(
+        |package_name| {
+            let relative_package_file = structure::relative_file_for_package(package_name);
+            let absolute_package_file = nixpkgs_path.join(&relative_package_file);
 
-        if let Some(attribute_info) = actual_files.get(package_name) {
-            let valid = match &attribute_info.variant {
-                AttributeVariant::AutoCalled => true,
-                AttributeVariant::CallPackage { path, empty_arg } => {
-                    let correct_file = if let Some(call_package_path) = path {
-                        absolute_package_file == *call_package_path
-                    } else {
-                        false
-                    };
-                    // Only check for the argument to be non-empty if the version is V1 or
-                    // higher
-                    let non_empty = if version >= Version::V1 {
-                        !empty_arg
-                    } else {
-                        true
-                    };
-                    correct_file && non_empty
-                }
-                AttributeVariant::Other => false,
-            };
+            if let Some(attribute_info) = actual_files.get(package_name) {
+                let valid = match &attribute_info.variant {
+                    AttributeVariant::AutoCalled => true,
+                    AttributeVariant::CallPackage { path, empty_arg } => {
+                        let correct_file = if let Some(call_package_path) = path {
+                            absolute_package_file == *call_package_path
+                        } else {
+                            false
+                        };
+                        // Only check for the argument to be non-empty if the version is V1 or
+                        // higher
+                        let non_empty = if version >= Version::V1 {
+                            !empty_arg
+                        } else {
+                            true
+                        };
+                        correct_file && non_empty
+                    }
+                    AttributeVariant::Other => false,
+                };
 
-            if !valid {
-                NixpkgsProblem::WrongCallPackage {
-                    relative_package_file: relative_package_file.clone(),
-                    package_name: package_name.clone(),
+                if !valid {
+                    NixpkgsProblem::WrongCallPackage {
+                        relative_package_file: relative_package_file.clone(),
+                        package_name: package_name.clone(),
+                    }
+                    .into()
+                } else if !attribute_info.is_derivation {
+                    NixpkgsProblem::NonDerivation {
+                        relative_package_file: relative_package_file.clone(),
+                        package_name: package_name.clone(),
+                    }
+                    .into()
+                } else {
+                    Success(())
                 }
-                .into_result()
-            } else if !attribute_info.is_derivation {
-                NixpkgsProblem::NonDerivation {
+            } else {
+                NixpkgsProblem::UndefinedAttr {
                     relative_package_file: relative_package_file.clone(),
                     package_name: package_name.clone(),
                 }
-                .into_result()
-            } else {
-                check_result::ok(())
-            }
-        } else {
-            NixpkgsProblem::UndefinedAttr {
-                relative_package_file: relative_package_file.clone(),
-                package_name: package_name.clone(),
+                .into()
             }
-            .into_result()
-        }
-    }))
+        },
+    )))
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
index 1f52d81c36c..4cabf8f446f 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -1,18 +1,16 @@
-mod check_result;
 mod eval;
 mod nixpkgs_problem;
 mod references;
 mod structure;
 mod utils;
+mod validation;
 
 use crate::structure::check_structure;
+use crate::validation::Validation::Failure;
+use crate::validation::Validation::Success;
 use anyhow::Context;
 use clap::{Parser, ValueEnum};
 use colored::Colorize;
-use itertools::{
-    Either,
-    Either::{Left, Right},
-};
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
@@ -86,26 +84,26 @@ pub fn check_nixpkgs<W: io::Write>(
             "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
             utils::BASE_SUBPATH
         );
-        check_result::ok(())
+        Success(())
     } else {
         match check_structure(&nixpkgs_path)? {
-            Left(errors) => Ok(Left(errors)),
-            Right(package_names) =>
+            Failure(errors) => Failure(errors),
+            Success(package_names) =>
             // Only if we could successfully parse the structure, we do the evaluation checks
             {
-                eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths)
+                eval::check_values(version, &nixpkgs_path, package_names, eval_accessible_paths)?
             }
         }
     };
 
-    match check_result? {
-        Either::Left(errors) => {
+    match check_result {
+        Failure(errors) => {
             for error in errors {
                 writeln!(error_writer, "{}", error.to_string().red())?
             }
             Ok(false)
         }
-        Either::Right(_) => Ok(true),
+        Success(_) => Ok(true),
     }
 }
 
diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs
index 37837a54ddc..c437ebd222f 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/references.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs
@@ -1,8 +1,7 @@
-use crate::check_result;
-use crate::check_result::CheckResult;
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::utils;
 use crate::utils::LineIndex;
+use crate::validation::{self, ResultIteratorExt, Validation::Success};
 
 use anyhow::Context;
 use rnix::{Root, SyntaxKind::NODE_PATH};
@@ -23,7 +22,7 @@ struct PackageContext<'a> {
 pub fn check_references(
     relative_package_dir: &Path,
     absolute_package_dir: &Path,
-) -> CheckResult<()> {
+) -> validation::Result<()> {
     let context = PackageContext {
         relative_package_dir: &relative_package_dir.to_path_buf(),
         absolute_package_dir: &absolute_package_dir.to_path_buf(),
@@ -38,10 +37,10 @@ pub fn check_references(
 }
 
 /// Checks for a specific path to not have references outside
-fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
+fn check_path(context: &PackageContext, subpath: &Path) -> validation::Result<()> {
     let path = context.absolute_package_dir.join(subpath);
 
-    if path.is_symlink() {
+    Ok(if path.is_symlink() {
         // Check whether the symlink resolves to outside the package directory
         match path.canonicalize() {
             Ok(target) => {
@@ -52,9 +51,9 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                         relative_package_dir: context.relative_package_dir.clone(),
                         subpath: subpath.to_path_buf(),
                     }
-                    .into_result()
+                    .into()
                 } else {
-                    check_result::ok(())
+                    Success(())
                 }
             }
             Err(io_error) => NixpkgsProblem::UnresolvableSymlink {
@@ -62,15 +61,20 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                 subpath: subpath.to_path_buf(),
                 io_error,
             }
-            .into_result(),
+            .into(),
         }
     } else if path.is_dir() {
         // Recursively check each entry
-        check_result::sequence_(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()))
-        }))
+        validation::sequence_(
+            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()))
+                })
+                .collect_vec()?,
+        )
     } else if path.is_file() {
         // Only check Nix files
         if let Some(ext) = path.extension() {
@@ -78,22 +82,22 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                 check_nix_file(context, subpath).context(format!(
                     "Error while checking Nix file {}",
                     subpath.display()
-                ))
+                ))?
             } else {
-                check_result::ok(())
+                Success(())
             }
         } else {
-            check_result::ok(())
+            Success(())
         }
     } else {
         // This should never happen, git doesn't support other file types
         anyhow::bail!("Unsupported file type for path {}", subpath.display());
-    }
+    })
 }
 
 /// Check whether a nix file contains path expression references pointing outside the package
 /// directory
-fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
+fn check_nix_file(context: &PackageContext, subpath: &Path) -> validation::Result<()> {
     let path = context.absolute_package_dir.join(subpath);
     let parent_dir = path.parent().context(format!(
         "Could not get parent of path {}",
@@ -105,71 +109,75 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
 
     let root = Root::parse(&contents);
     if let Some(error) = root.errors().first() {
-        return NixpkgsProblem::CouldNotParseNix {
+        return Ok(NixpkgsProblem::CouldNotParseNix {
             relative_package_dir: context.relative_package_dir.clone(),
             subpath: subpath.to_path_buf(),
             error: error.clone(),
         }
-        .into_result();
+        .into());
     }
 
     let line_index = LineIndex::new(&contents);
 
-    check_result::sequence_(root.syntax().descendants().map(|node| {
-        let text = node.text().to_string();
-        let line = line_index.line(node.text_range().start().into());
+    Ok(validation::sequence_(root.syntax().descendants().map(
+        |node| {
+            let text = node.text().to_string();
+            let line = line_index.line(node.text_range().start().into());
 
-        if node.kind() != NODE_PATH {
-            // We're only interested in Path expressions
-            check_result::ok(())
-        } else if node.children().count() != 0 {
-            // Filters out ./foo/${bar}/baz
-            // TODO: We can just check ./foo
-            NixpkgsProblem::PathInterpolation {
-                relative_package_dir: context.relative_package_dir.clone(),
-                subpath: subpath.to_path_buf(),
-                line,
-                text,
-            }
-            .into_result()
-        } else if text.starts_with('<') {
-            // Filters out search paths like <nixpkgs>
-            NixpkgsProblem::SearchPath {
-                relative_package_dir: context.relative_package_dir.clone(),
-                subpath: subpath.to_path_buf(),
-                line,
-                text,
-            }
-            .into_result()
-        } else {
-            // Resolves the reference of the Nix path
-            // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz`
-            match parent_dir.join(Path::new(&text)).canonicalize() {
-                Ok(target) => {
-                    // Then checking if it's still in the package directory
-                    // No need to handle the case of it being inside the directory, since we scan through the
-                    // entire directory recursively anyways
-                    if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
-                        NixpkgsProblem::OutsidePathReference {
-                            relative_package_dir: context.relative_package_dir.clone(),
-                            subpath: subpath.to_path_buf(),
-                            line,
-                            text,
-                        }
-                        .into_result()
-                    } else {
-                        check_result::ok(())
-                    }
+            if node.kind() != NODE_PATH {
+                // We're only interested in Path expressions
+                Success(())
+            } else if node.children().count() != 0 {
+                // Filters out ./foo/${bar}/baz
+                // TODO: We can just check ./foo
+                NixpkgsProblem::PathInterpolation {
+                    relative_package_dir: context.relative_package_dir.clone(),
+                    subpath: subpath.to_path_buf(),
+                    line,
+                    text,
                 }
-                Err(e) => NixpkgsProblem::UnresolvablePathReference {
+                .into()
+            } else if text.starts_with('<') {
+                // Filters out search paths like <nixpkgs>
+                NixpkgsProblem::SearchPath {
                     relative_package_dir: context.relative_package_dir.clone(),
                     subpath: subpath.to_path_buf(),
                     line,
                     text,
-                    io_error: e,
                 }
-                .into_result(),
+                .into()
+            } else {
+                // Resolves the reference of the Nix path
+                // turning `../baz` inside `/foo/bar/default.nix` to `/foo/baz`
+                match parent_dir.join(Path::new(&text)).canonicalize() {
+                    Ok(target) => {
+                        // Then checking if it's still in the package directory
+                        // No need to handle the case of it being inside the directory, since we scan through the
+                        // entire directory recursively anyways
+                        if let Err(_prefix_error) =
+                            target.strip_prefix(context.absolute_package_dir)
+                        {
+                            NixpkgsProblem::OutsidePathReference {
+                                relative_package_dir: context.relative_package_dir.clone(),
+                                subpath: subpath.to_path_buf(),
+                                line,
+                                text,
+                            }
+                            .into()
+                        } else {
+                            Success(())
+                        }
+                    }
+                    Err(e) => NixpkgsProblem::UnresolvablePathReference {
+                        relative_package_dir: context.relative_package_dir.clone(),
+                        subpath: subpath.to_path_buf(),
+                        line,
+                        text,
+                        io_error: e,
+                    }
+                    .into(),
+                }
             }
-        }
-    }))
+        },
+    )))
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
index b69f6211c0a..4051ca037c9 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
@@ -1,9 +1,8 @@
-use crate::check_result;
-use crate::check_result::CheckResult;
 use crate::nixpkgs_problem::NixpkgsProblem;
 use crate::references;
 use crate::utils;
 use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME};
+use crate::validation::{self, ResultIteratorExt, Validation::Success};
 use itertools::concat;
 use lazy_static::lazy_static;
 use regex::Regex;
@@ -35,24 +34,25 @@ pub fn relative_file_for_package(package_name: &str) -> PathBuf {
 
 /// Check the structure of Nixpkgs, returning the attribute names that are defined in
 /// `pkgs/by-name`
-pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
+pub fn check_structure(path: &Path) -> validation::Result<Vec<String>> {
     let base_dir = path.join(BASE_SUBPATH);
 
     let shard_results = utils::read_dir_sorted(&base_dir)?
         .into_iter()
-        .map(|shard_entry| {
+        .map(|shard_entry| -> validation::Result<_> {
             let shard_path = shard_entry.path();
             let shard_name = shard_entry.file_name().to_string_lossy().into_owned();
             let relative_shard_path = relative_dir_for_shard(&shard_name);
 
-            if shard_name == "README.md" {
+            Ok(if shard_name == "README.md" {
                 // README.md is allowed to be a file and not checked
-                check_result::ok(vec![])
+
+                Success(vec![])
             } else if !shard_path.is_dir() {
                 NixpkgsProblem::ShardNonDir {
                     relative_shard_path: relative_shard_path.clone(),
                 }
-                .into_result()
+                .into()
                 // we can't check for any other errors if it's a file, since there's no subdirectories to check
             } else {
                 let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name);
@@ -61,9 +61,9 @@ pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
                         relative_shard_path: relative_shard_path.clone(),
                         shard_name: shard_name.clone(),
                     }
-                    .into_result()
+                    .into()
                 } else {
-                    check_result::ok(())
+                    Success(())
                 };
 
                 let entries = utils::read_dir_sorted(&shard_path)?;
@@ -80,21 +80,25 @@ pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
                             first: l.file_name(),
                             second: r.file_name(),
                         }
-                        .into_result::<()>()
+                        .into()
                     });
 
-                let result = check_result::and(result, check_result::sequence_(duplicate_results));
+                let result = result.and(validation::sequence_(duplicate_results));
 
-                let package_results = entries.into_iter().map(|package_entry| {
-                    check_package(path, &shard_name, shard_name_valid, package_entry)
-                });
+                let package_results = entries
+                    .into_iter()
+                    .map(|package_entry| {
+                        check_package(path, &shard_name, shard_name_valid, package_entry)
+                    })
+                    .collect_vec()?;
 
-                check_result::and(result, check_result::sequence(package_results))
-            }
-        });
+                result.and(validation::sequence(package_results))
+            })
+        })
+        .collect_vec()?;
 
     // Combine the package names conatained within each shard into a longer list
-    check_result::map(check_result::sequence(shard_results), concat)
+    Ok(validation::sequence(shard_results).map(concat))
 }
 
 fn check_package(
@@ -102,16 +106,16 @@ fn check_package(
     shard_name: &str,
     shard_name_valid: bool,
     package_entry: DirEntry,
-) -> CheckResult<String> {
+) -> validation::Result<String> {
     let package_path = package_entry.path();
     let package_name = package_entry.file_name().to_string_lossy().into_owned();
     let relative_package_dir = PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}"));
 
-    if !package_path.is_dir() {
+    Ok(if !package_path.is_dir() {
         NixpkgsProblem::PackageNonDir {
             relative_package_dir: relative_package_dir.clone(),
         }
-        .into_result()
+        .into()
     } else {
         let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name);
         let result = if !package_name_valid {
@@ -119,54 +123,48 @@ fn check_package(
                 relative_package_dir: relative_package_dir.clone(),
                 package_name: package_name.clone(),
             }
-            .into_result()
+            .into()
         } else {
-            check_result::ok(())
+            Success(())
         };
 
         let correct_relative_package_dir = relative_dir_for_package(&package_name);
-        let result = check_result::and(
-            result,
-            if relative_package_dir != correct_relative_package_dir {
-                // Only show this error if we have a valid shard and package name
-                // Because if one of those is wrong, you should fix that first
-                if shard_name_valid && package_name_valid {
-                    NixpkgsProblem::IncorrectShard {
-                        relative_package_dir: relative_package_dir.clone(),
-                        correct_relative_package_dir: correct_relative_package_dir.clone(),
-                    }
-                    .into_result()
-                } else {
-                    check_result::ok(())
+        let result = result.and(if relative_package_dir != correct_relative_package_dir {
+            // Only show this error if we have a valid shard and package name
+            // Because if one of those is wrong, you should fix that first
+            if shard_name_valid && package_name_valid {
+                NixpkgsProblem::IncorrectShard {
+                    relative_package_dir: relative_package_dir.clone(),
+                    correct_relative_package_dir: correct_relative_package_dir.clone(),
                 }
+                .into()
             } else {
-                check_result::ok(())
-            },
-        );
+                Success(())
+            }
+        } else {
+            Success(())
+        });
 
         let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME);
-        let result = check_result::and(
-            result,
-            if !package_nix_path.exists() {
-                NixpkgsProblem::PackageNixNonExistent {
-                    relative_package_dir: relative_package_dir.clone(),
-                }
-                .into_result()
-            } else if package_nix_path.is_dir() {
-                NixpkgsProblem::PackageNixDir {
-                    relative_package_dir: relative_package_dir.clone(),
-                }
-                .into_result()
-            } else {
-                check_result::ok(())
-            },
-        );
+        let result = result.and(if !package_nix_path.exists() {
+            NixpkgsProblem::PackageNixNonExistent {
+                relative_package_dir: relative_package_dir.clone(),
+            }
+            .into()
+        } else if package_nix_path.is_dir() {
+            NixpkgsProblem::PackageNixDir {
+                relative_package_dir: relative_package_dir.clone(),
+            }
+            .into()
+        } else {
+            Success(())
+        });
 
-        let result = check_result::and(
-            result,
-            references::check_references(&relative_package_dir, &path.join(&relative_package_dir)),
-        );
+        let result = result.and(references::check_references(
+            &relative_package_dir,
+            &path.join(&relative_package_dir),
+        )?);
 
-        check_result::map(result, |_| package_name.clone())
-    }
+        result.map(|_| package_name.clone())
+    })
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/validation.rs b/pkgs/test/nixpkgs-check-by-name/src/validation.rs
new file mode 100644
index 00000000000..e7279385152
--- /dev/null
+++ b/pkgs/test/nixpkgs-check-by-name/src/validation.rs
@@ -0,0 +1,102 @@
+use crate::nixpkgs_problem::NixpkgsProblem;
+use itertools::concat;
+use itertools::{
+    Either::{Left, Right},
+    Itertools,
+};
+use Validation::*;
+
+/// The validation result of a check.
+/// Instead of exiting at the first failure,
+/// this type can accumulate multiple failures.
+/// This can be achieved using the functions `and`, `sequence` and `sequence_`
+///
+/// This leans on https://hackage.haskell.org/package/validation
+pub enum Validation<A> {
+    Failure(Vec<NixpkgsProblem>),
+    Success(A),
+}
+
+impl<A> From<NixpkgsProblem> for Validation<A> {
+    /// Create a `Validation<A>` from a single check problem
+    fn from(value: NixpkgsProblem) -> Self {
+        Failure(vec![value])
+    }
+}
+
+/// A type alias representing the result of a check, either:
+/// - Err(anyhow::Error): A fatal failure, typically I/O errors.
+///   Such failures are not caused by the files in Nixpkgs.
+///   This hints at a bug in the code or a problem with the deployment.
+/// - Ok(Failure(Vec<NixpkgsProblem>)): A non-fatal validation problem with the Nixpkgs files.
+///   Further checks can be run even with this result type.
+///   Such problems can be fixed by changing the Nixpkgs files.
+/// - Ok(Success(A)): A successful (potentially intermediate) result with an arbitrary value.
+///   No fatal errors have occurred and no validation problems have been found with Nixpkgs.
+pub type Result<A> = anyhow::Result<Validation<A>>;
+
+pub trait ResultIteratorExt<A, E>: Sized + Iterator<Item = std::result::Result<A, E>> {
+    fn collect_vec(self) -> std::result::Result<Vec<A>, E>;
+}
+
+impl<I, A, E> ResultIteratorExt<A, E> for I
+where
+    I: Sized + Iterator<Item = std::result::Result<A, E>>,
+{
+    /// A convenience version of `collect` specialised to a vector
+    fn collect_vec(self) -> std::result::Result<Vec<A>, E> {
+        self.collect()
+    }
+}
+
+impl<A> Validation<A> {
+    /// Map a `Validation<A>` to a `Validation<B>` by applying a function to the
+    /// potentially contained value in case of success.
+    pub fn map<B>(self, f: impl FnOnce(A) -> B) -> Validation<B> {
+        match self {
+            Failure(err) => Failure(err),
+            Success(value) => Success(f(value)),
+        }
+    }
+}
+
+impl Validation<()> {
+    /// Combine two validations, both of which need to be successful for the return value to be successful.
+    /// The `NixpkgsProblem`s of both sides are returned concatenated.
+    pub fn and<A>(self, other: Validation<A>) -> Validation<A> {
+        match (self, other) {
+            (Success(_), Success(right_value)) => Success(right_value),
+            (Failure(errors), Success(_)) => Failure(errors),
+            (Success(_), Failure(errors)) => Failure(errors),
+            (Failure(errors_l), Failure(errors_r)) => Failure(concat([errors_l, errors_r])),
+        }
+    }
+}
+
+/// Combine many validations into a single one.
+/// All given validations need to be successful in order for the returned validation to be successful,
+/// in which case the returned validation value contains a `Vec` of each individual value.
+/// Otherwise the `NixpkgsProblem`s of all validations are returned concatenated.
+pub fn sequence<A>(check_results: impl IntoIterator<Item = Validation<A>>) -> Validation<Vec<A>> {
+    let (errors, values): (Vec<Vec<NixpkgsProblem>>, Vec<A>) = check_results
+        .into_iter()
+        .partition_map(|validation| match validation {
+            Failure(err) => Left(err),
+            Success(value) => Right(value),
+        });
+
+    // To combine the errors from the results we flatten all the error Vec's into a new Vec
+    // This is not very efficient, but doesn't matter because generally we should have no errors
+    let flattened_errors = errors.into_iter().concat();
+
+    if flattened_errors.is_empty() {
+        Success(values)
+    } else {
+        Failure(flattened_errors)
+    }
+}
+
+/// Like `sequence`, but without any containing value, for convenience
+pub fn sequence_(validations: impl IntoIterator<Item = Validation<()>>) -> Validation<()> {
+    sequence(validations).map(|_| ())
+}