tests.nixpkgs-check-by-name: Redesign and document check_result functions

5 files changed, 144 insertions, 122 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 e5f14152d5acd..db2377c7204b2 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs
@@ -1,13 +1,17 @@
 use crate::utils::PACKAGE_NIX_FILENAME;
 use itertools::concat;
-use itertools::{Either, Itertools};
+use itertools::{
+    Either,
+    Either::{Left, Right},
+    Itertools,
+};
 use rnix::parser::ParseError;
 use std::ffi::OsString;
 use std::fmt;
 use std::io;
 use std::path::PathBuf;
 
-pub enum CheckError {
+pub enum CheckProblem {
     ShardNonDir {
         relative_shard_path: PathBuf,
     },
@@ -90,97 +94,97 @@ pub enum CheckError {
     },
 }
 
-impl CheckError {
+impl CheckProblem {
     pub fn into_result<A>(self) -> CheckResult<A> {
-        Ok(Either::Left(vec![self]))
+        Ok(Left(vec![self]))
     }
 }
 
-impl fmt::Display for CheckError {
+impl fmt::Display for CheckProblem {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            CheckError::ShardNonDir { relative_shard_path } =>
+            CheckProblem::ShardNonDir { relative_shard_path } =>
                 write!(
                     f,
                     "{}: This is a file, but it should be a directory.",
                     relative_shard_path.display(),
                 ),
-            CheckError::InvalidShardName { relative_shard_path, shard_name } =>
+            CheckProblem::InvalidShardName { relative_shard_path, shard_name } =>
                 write!(
                     f,
                     "{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".",
                     relative_shard_path.display()
                 ),
-            CheckError::PackageNonDir { relative_package_dir } =>
+            CheckProblem::PackageNonDir { relative_package_dir } =>
                 write!(
                     f,
                     "{}: This path is a file, but it should be a directory.",
                     relative_package_dir.display(),
                 ),
-            CheckError::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
+            CheckProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
                 write!(
                     f,
                     "{}: Duplicate case-sensitive package directories {first:?} and {second:?}.",
                     relative_shard_path.display(),
                 ),
-            CheckError::InvalidPackageName { relative_package_dir, package_name } =>
+            CheckProblem::InvalidPackageName { relative_package_dir, package_name } =>
                 write!(
                     f,
                     "{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".",
                     relative_package_dir.display(),
                 ),
-            CheckError::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
+            CheckProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
                 write!(
                     f,
                     "{}: Incorrect directory location, should be {} instead.",
                     relative_package_dir.display(),
                     correct_relative_package_dir.display(),
                 ),
-            CheckError::PackageNixNonExistent { relative_package_dir } =>
+            CheckProblem::PackageNixNonExistent { relative_package_dir } =>
                 write!(
                     f,
                     "{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.",
                     relative_package_dir.display(),
                 ),
-            CheckError::PackageNixDir { relative_package_dir } =>
+            CheckProblem::PackageNixDir { relative_package_dir } =>
                 write!(
                     f,
                     "{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.",
                     relative_package_dir.display(),
                 ),
-            CheckError::UndefinedAttr { relative_package_file, package_name } =>
+            CheckProblem::UndefinedAttr { relative_package_file, package_name } =>
                 write!(
                     f,
                     "pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
                     relative_package_file.display()
                 ),
-            CheckError::WrongCallPackage { relative_package_file, package_name } =>
+            CheckProblem::WrongCallPackage { relative_package_file, package_name } =>
                 write!(
                     f,
                     "pkgs.{package_name}: This attribute is manually defined (most likely in pkgs/top-level/all-packages.nix), which is only allowed if the definition is of the form `pkgs.callPackage {} {{ ... }}` with a non-empty second argument.",
                     relative_package_file.display()
                 ),
-            CheckError::NonDerivation { relative_package_file, package_name } =>
+            CheckProblem::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 } =>
+            CheckProblem::OutsideSymlink { relative_package_dir, subpath } =>
                 write!(
                     f,
                     "{}: Path {} is a symlink pointing to a path outside the directory of that package.",
                     relative_package_dir.display(),
                     subpath.display(),
                 ),
-            CheckError::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
+            CheckProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
                 write!(
                     f,
                     "{}: Path {} is a symlink which cannot be resolved: {io_error}.",
                     relative_package_dir.display(),
                     subpath.display(),
                 ),
-            CheckError::CouldNotParseNix { relative_package_dir, subpath, error } =>
+            CheckProblem::CouldNotParseNix { relative_package_dir, subpath, error } =>
                 write!(
                     f,
                     "{}: File {} could not be parsed by rnix: {}",
@@ -188,7 +192,7 @@ impl fmt::Display for CheckError {
                     subpath.display(),
                     error,
                 ),
-            CheckError::PathInterpolation { relative_package_dir, subpath, line, text } =>
+            CheckProblem::PathInterpolation { relative_package_dir, subpath, line, text } =>
                 write!(
                     f,
                     "{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.",
@@ -196,7 +200,7 @@ impl fmt::Display for CheckError {
                     subpath.display(),
                     text
                 ),
-            CheckError::SearchPath { relative_package_dir, subpath, line, text } =>
+            CheckProblem::SearchPath { relative_package_dir, subpath, line, text } =>
                 write!(
                     f,
                     "{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.",
@@ -204,7 +208,7 @@ impl fmt::Display for CheckError {
                     subpath.display(),
                     text
                 ),
-            CheckError::OutsidePathReference { relative_package_dir, subpath, line, text } =>
+            CheckProblem::OutsidePathReference { relative_package_dir, subpath, line, text } =>
                 write!(
                     f,
                     "{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.",
@@ -212,7 +216,7 @@ impl fmt::Display for CheckError {
                     subpath.display(),
                     text,
                 ),
-            CheckError::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
+            CheckProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
                 write!(
                     f,
                     "{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.",
@@ -224,27 +228,44 @@ impl fmt::Display for CheckError {
     }
 }
 
-pub fn pass<A>(value: A) -> CheckResult<A> {
-    Ok(Either::Right(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(Left(Vec<CheckProblem>)): 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<CheckProblem>, 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))
 }
 
-pub type CheckResult<A> = anyhow::Result<Either<Vec<CheckError>, A>>;
+/// Create a successfull `CheckResult<A>` from a value `A`.
+pub fn ok<A>(value: A) -> CheckResult<A> {
+    Ok(Right(value))
+}
 
-pub fn sequence_check_results<A>(first: CheckResult<()>, second: CheckResult<A>) -> CheckResult<A> {
+/// Combine two check results, both of which need to be successful for the return value to be successful.
+/// The `CheckProblem`s of both sides are returned concatenated.
+pub fn and<A>(first: CheckResult<()>, second: CheckResult<A>) -> CheckResult<A> {
     match (first?, second?) {
-        (Either::Right(_), Either::Right(right_value)) => pass(right_value),
-        (Either::Left(errors), Either::Right(_)) => Ok(Either::Left(errors)),
-        (Either::Right(_), Either::Left(errors)) => Ok(Either::Left(errors)),
-        (Either::Left(errors_l), Either::Left(errors_r)) => {
-            Ok(Either::Left(concat([errors_l, errors_r])))
-        }
+        (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]))),
     }
 }
 
-pub fn flatten_check_results<I, O>(
-    check_results: impl IntoIterator<Item = CheckResult<I>>,
-    value_transform: impl Fn(Vec<I>) -> O,
-) -> CheckResult<O> {
+/// 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 `CheckProblem`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<_>>>()?
@@ -256,8 +277,13 @@ pub fn flatten_check_results<I, O>(
     let flattened_errors = errors.into_iter().flatten().collect::<Vec<_>>();
 
     if flattened_errors.is_empty() {
-        Ok(Either::Right(value_transform(values)))
+        Ok(Right(values))
     } else {
-        Ok(Either::Left(flattened_errors))
+        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 487ec7cab7300..a76abba05d964 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/eval.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/eval.rs
@@ -1,4 +1,5 @@
-use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult};
+use crate::check_result;
+use crate::check_result::{CheckProblem, CheckResult};
 use crate::structure;
 use crate::Version;
 use std::path::Path;
@@ -110,7 +111,7 @@ pub fn check_values(
             String::from_utf8_lossy(&result.stdout)
         ))?;
 
-    let check_results = package_names.iter().map(|package_name| {
+    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);
 
@@ -136,27 +137,26 @@ pub fn check_values(
             };
 
             if !valid {
-                CheckError::WrongCallPackage {
+                CheckProblem::WrongCallPackage {
                     relative_package_file: relative_package_file.clone(),
                     package_name: package_name.clone(),
                 }
                 .into_result()
             } else if !attribute_info.is_derivation {
-                CheckError::NonDerivation {
+                CheckProblem::NonDerivation {
                     relative_package_file: relative_package_file.clone(),
                     package_name: package_name.clone(),
                 }
                 .into_result()
             } else {
-                pass(())
+                check_result::ok(())
             }
         } else {
-            CheckError::UndefinedAttr {
+            CheckProblem::UndefinedAttr {
                 relative_package_file: relative_package_file.clone(),
                 package_name: package_name.clone(),
             }
             .into_result()
         }
-    });
-    flatten_check_results(check_results, |_| ())
+    }))
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
index 537276a177d50..11fedde741309 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -6,11 +6,12 @@ mod utils;
 
 use crate::structure::check_structure;
 use anyhow::Context;
-use check_result::pass;
 use clap::{Parser, ValueEnum};
 use colored::Colorize;
-use itertools::Either;
-use itertools::Either::{Left, Right};
+use itertools::{
+    Either,
+    Either::{Left, Right},
+};
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
@@ -84,7 +85,7 @@ pub fn check_nixpkgs<W: io::Write>(
             "Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
             utils::BASE_SUBPATH
         );
-        pass(())
+        check_result::ok(())
     } else {
         match check_structure(&nixpkgs_path)? {
             Left(errors) => Ok(Left(errors)),
diff --git a/pkgs/test/nixpkgs-check-by-name/src/references.rs b/pkgs/test/nixpkgs-check-by-name/src/references.rs
index 91f898fe3bfd4..a6bc53d717d79 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/references.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/references.rs
@@ -1,4 +1,5 @@
-use crate::check_result::{flatten_check_results, pass, CheckError, CheckResult};
+use crate::check_result;
+use crate::check_result::{CheckProblem, CheckResult};
 use crate::utils;
 use crate::utils::LineIndex;
 
@@ -46,16 +47,16 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                 // 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) {
-                    CheckError::OutsideSymlink {
+                    CheckProblem::OutsideSymlink {
                         relative_package_dir: context.relative_package_dir.clone(),
                         subpath: subpath.to_path_buf(),
                     }
                     .into_result()
                 } else {
-                    pass(())
+                    check_result::ok(())
                 }
             }
-            Err(io_error) => CheckError::UnresolvableSymlink {
+            Err(io_error) => CheckProblem::UnresolvableSymlink {
                 relative_package_dir: context.relative_package_dir.clone(),
                 subpath: subpath.to_path_buf(),
                 io_error,
@@ -64,12 +65,11 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
         }
     } else if path.is_dir() {
         // Recursively check each entry
-        let check_results = utils::read_dir_sorted(&path)?.into_iter().map(|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()))
-        });
-        flatten_check_results(check_results, |_| ())
+        }))
     } else if path.is_file() {
         // Only check Nix files
         if let Some(ext) = path.extension() {
@@ -79,10 +79,10 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                     subpath.display()
                 ))
             } else {
-                pass(())
+                check_result::ok(())
             }
         } else {
-            pass(())
+            check_result::ok(())
         }
     } else {
         // This should never happen, git doesn't support other file types
@@ -104,7 +104,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
 
     let root = Root::parse(&contents);
     if let Some(error) = root.errors().first() {
-        return CheckError::CouldNotParseNix {
+        return CheckProblem::CouldNotParseNix {
             relative_package_dir: context.relative_package_dir.clone(),
             subpath: subpath.to_path_buf(),
             error: error.clone(),
@@ -114,17 +114,17 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
 
     let line_index = LineIndex::new(&contents);
 
-    let check_results = root.syntax().descendants().map(|node| {
+    check_result::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
-            pass(())
+            check_result::ok(())
         } else if node.children().count() != 0 {
             // Filters out ./foo/${bar}/baz
             // TODO: We can just check ./foo
-            CheckError::PathInterpolation {
+            CheckProblem::PathInterpolation {
                 relative_package_dir: context.relative_package_dir.clone(),
                 subpath: subpath.to_path_buf(),
                 line,
@@ -133,7 +133,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
             .into_result()
         } else if text.starts_with('<') {
             // Filters out search paths like <nixpkgs>
-            CheckError::SearchPath {
+            CheckProblem::SearchPath {
                 relative_package_dir: context.relative_package_dir.clone(),
                 subpath: subpath.to_path_buf(),
                 line,
@@ -149,7 +149,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                     // 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) {
-                        CheckError::OutsidePathReference {
+                        CheckProblem::OutsidePathReference {
                             relative_package_dir: context.relative_package_dir.clone(),
                             subpath: subpath.to_path_buf(),
                             line,
@@ -157,10 +157,10 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                         }
                         .into_result()
                     } else {
-                        pass(())
+                        check_result::ok(())
                     }
                 }
-                Err(e) => CheckError::UnresolvablePathReference {
+                Err(e) => CheckProblem::UnresolvablePathReference {
                     relative_package_dir: context.relative_package_dir.clone(),
                     subpath: subpath.to_path_buf(),
                     line,
@@ -170,6 +170,5 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
                 .into_result(),
             }
         }
-    });
-    flatten_check_results(check_results, |_| ())
+    }))
 }
diff --git a/pkgs/test/nixpkgs-check-by-name/src/structure.rs b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
index c8fbc17fcb392..90a552a388637 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
@@ -1,6 +1,5 @@
-use crate::check_result::{
-    flatten_check_results, pass, sequence_check_results, CheckError, CheckResult,
-};
+use crate::check_result;
+use crate::check_result::{CheckProblem, CheckResult};
 use crate::references;
 use crate::utils;
 use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME};
@@ -36,7 +35,7 @@ pub fn relative_file_for_package(package_name: &str) -> PathBuf {
 pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
     let base_dir = path.join(BASE_SUBPATH);
 
-    let check_results = utils::read_dir_sorted(&base_dir)?
+    let shard_results = utils::read_dir_sorted(&base_dir)?
         .into_iter()
         .map(|shard_entry| {
             let shard_path = shard_entry.path();
@@ -45,35 +44,35 @@ pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
 
             if shard_name == "README.md" {
                 // README.md is allowed to be a file and not checked
-                pass(vec![])
+                check_result::ok(vec![])
             } else if !shard_path.is_dir() {
-                CheckError::ShardNonDir {
+                CheckProblem::ShardNonDir {
                     relative_shard_path: relative_shard_path.clone(),
                 }
                 .into_result()
                 // 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);
-                let check_result = if !shard_name_valid {
-                    CheckError::InvalidShardName {
+                let result = if !shard_name_valid {
+                    CheckProblem::InvalidShardName {
                         relative_shard_path: relative_shard_path.clone(),
                         shard_name: shard_name.clone(),
                     }
                     .into_result()
                 } else {
-                    pass(())
+                    check_result::ok(())
                 };
 
                 let entries = utils::read_dir_sorted(&shard_path)?;
 
-                let duplicate_check_results = entries
+                let duplicate_results = entries
                     .iter()
                     .zip(entries.iter().skip(1))
                     .filter(|(l, r)| {
                         l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase()
                     })
                     .map(|(l, r)| {
-                        CheckError::CaseSensitiveDuplicate {
+                        CheckProblem::CaseSensitiveDuplicate {
                             relative_shard_path: relative_shard_path.clone(),
                             first: l.file_name(),
                             second: r.file_name(),
@@ -81,90 +80,87 @@ pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
                         .into_result::<()>()
                     });
 
-                let check_result = sequence_check_results(
-                    check_result,
-                    flatten_check_results(duplicate_check_results, |_| ()),
-                );
+                let result = check_result::and(result, check_result::sequence_(duplicate_results));
 
-                let check_results = entries.into_iter().map(|package_entry| {
+                let package_results = entries.into_iter().map(|package_entry| {
                     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() {
-                        CheckError::PackageNonDir {
+                        CheckProblem::PackageNonDir {
                             relative_package_dir: relative_package_dir.clone(),
                         }
                         .into_result()
                     } else {
                         let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name);
-                        let name_check_result = if !package_name_valid {
-                            CheckError::InvalidPackageName {
+                        let result = if !package_name_valid {
+                            CheckProblem::InvalidPackageName {
                                 relative_package_dir: relative_package_dir.clone(),
                                 package_name: package_name.clone(),
                             }
                             .into_result()
                         } else {
-                            pass(())
+                            check_result::ok(())
                         };
 
                         let correct_relative_package_dir = relative_dir_for_package(&package_name);
-                        let shard_check_result =
+                        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 {
-                                    CheckError::IncorrectShard {
+                                    CheckProblem::IncorrectShard {
                                         relative_package_dir: relative_package_dir.clone(),
                                         correct_relative_package_dir: correct_relative_package_dir
                                             .clone(),
                                     }
                                     .into_result()
                                 } else {
-                                    pass(())
+                                    check_result::ok(())
                                 }
                             } else {
-                                pass(())
-                            };
+                                check_result::ok(())
+                            },
+                        );
 
                         let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME);
-                        let package_nix_check_result = if !package_nix_path.exists() {
-                            CheckError::PackageNixNonExistent {
-                                relative_package_dir: relative_package_dir.clone(),
-                            }
-                            .into_result()
-                        } else if package_nix_path.is_dir() {
-                            CheckError::PackageNixDir {
-                                relative_package_dir: relative_package_dir.clone(),
-                            }
-                            .into_result()
-                        } else {
-                            pass(())
-                        };
+                        let result = check_result::and(
+                            result,
+                            if !package_nix_path.exists() {
+                                CheckProblem::PackageNixNonExistent {
+                                    relative_package_dir: relative_package_dir.clone(),
+                                }
+                                .into_result()
+                            } else if package_nix_path.is_dir() {
+                                CheckProblem::PackageNixDir {
+                                    relative_package_dir: relative_package_dir.clone(),
+                                }
+                                .into_result()
+                            } else {
+                                check_result::ok(())
+                            },
+                        );
 
-                        let reference_check_result = references::check_references(
-                            &relative_package_dir,
-                            &path.join(&relative_package_dir),
+                        let result = check_result::and(
+                            result,
+                            references::check_references(
+                                &relative_package_dir,
+                                &path.join(&relative_package_dir),
+                            ),
                         );
 
-                        flatten_check_results(
-                            [
-                                name_check_result,
-                                shard_check_result,
-                                package_nix_check_result,
-                                reference_check_result,
-                            ],
-                            |_| package_name.clone(),
-                        )
+                        check_result::map(result, |_| package_name.clone())
                     }
                 });
 
-                sequence_check_results(check_result, flatten_check_results(check_results, |x| x))
+                check_result::and(result, check_result::sequence(package_results))
             }
         });
 
-    flatten_check_results(check_results, |x| {
+    check_result::map(check_result::sequence(shard_results), |x| {
         x.into_iter().flatten().collect::<Vec<_>>()
     })
 }