tests.nixpkgs-check-by-name: Make structural check a global function

2 files changed, 114 insertions, 134 deletions

diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs
index 7de6eadf6564f..6138284d2900d 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/main.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs
@@ -4,6 +4,7 @@ mod references;
 mod structure;
 mod utils;
 
+use crate::structure::check_structure;
 use anyhow::Context;
 use check_result::{flatten_check_results, write_check_result};
 use clap::{Parser, ValueEnum};
@@ -11,7 +12,6 @@ use colored::Colorize;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
-use structure::Nixpkgs;
 use utils::ErrorWriter;
 
 /// Program to check the validity of pkgs/by-name
@@ -88,9 +88,9 @@ pub fn check_nixpkgs<W: io::Write>(
             utils::BASE_SUBPATH
         );
     } else {
-        let nixpkgs = Nixpkgs::new(&nixpkgs_path, &mut error_writer)?;
+        let check_result = check_structure(&nixpkgs_path);
 
-        if error_writer.empty {
+        if let Some(nixpkgs) = write_check_result(&mut error_writer, check_result)? {
             // Only if we could successfully parse the structure, we do the semantic checks
             let check_result = flatten_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 d1655dc2e3fe3..24586c6b533cb 100644
--- a/pkgs/test/nixpkgs-check-by-name/src/structure.rs
+++ b/pkgs/test/nixpkgs-check-by-name/src/structure.rs
@@ -1,11 +1,10 @@
 use crate::check_result::{
-    flatten_check_results, pass, sequence_check_results, write_check_result, CheckError,
+    flatten_check_results, pass, sequence_check_results, CheckError, CheckResult,
 };
 use crate::utils;
-use crate::utils::{ErrorWriter, BASE_SUBPATH, PACKAGE_NIX_FILENAME};
+use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME};
 use lazy_static::lazy_static;
 use regex::Regex;
-use std::io;
 use std::path::{Path, PathBuf};
 
 lazy_static! {
@@ -42,155 +41,136 @@ impl Nixpkgs {
     }
 }
 
-impl Nixpkgs {
-    /// Read the structure of a Nixpkgs directory, displaying errors on the writer.
-    /// May return early with I/O errors.
-    pub fn new<W: io::Write>(
-        path: &Path,
-        error_writer: &mut ErrorWriter<W>,
-    ) -> anyhow::Result<Nixpkgs> {
-        let base_dir = path.join(BASE_SUBPATH);
-
-        let check_results = utils::read_dir_sorted(&base_dir)?
-            .into_iter()
-            .map(|shard_entry| {
-                let shard_path = shard_entry.path();
-                let shard_name = shard_entry.file_name().to_string_lossy().into_owned();
-                let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name);
-
-                if shard_name == "README.md" {
-                    // README.md is allowed to be a file and not checked
-                    pass(vec![])
-                } else if !shard_path.is_dir() {
-                    CheckError::ShardNonDir {
+/// Read the structure of a Nixpkgs directory, displaying errors on the writer.
+/// May return early with I/O errors.
+pub fn check_structure(path: &Path) -> CheckResult<Nixpkgs> {
+    let base_dir = path.join(BASE_SUBPATH);
+
+    let check_results = utils::read_dir_sorted(&base_dir)?
+        .into_iter()
+        .map(|shard_entry| {
+            let shard_path = shard_entry.path();
+            let shard_name = shard_entry.file_name().to_string_lossy().into_owned();
+            let relative_shard_path = Nixpkgs::relative_dir_for_shard(&shard_name);
+
+            if shard_name == "README.md" {
+                // README.md is allowed to be a file and not checked
+                pass(vec![])
+            } else if !shard_path.is_dir() {
+                CheckError::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 {
                         relative_shard_path: relative_shard_path.clone(),
+                        shard_name: shard_name.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 {
+                    pass(())
+                };
+
+                let entries = utils::read_dir_sorted(&shard_path)?;
+
+                let duplicate_check_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 {
                             relative_shard_path: relative_shard_path.clone(),
-                            shard_name: shard_name.clone(),
+                            first: l.file_name(),
+                            second: r.file_name(),
+                        }
+                        .into_result::<()>()
+                    });
+
+                let check_result = sequence_check_results(
+                    check_result,
+                    flatten_check_results(duplicate_check_results, |_| ()),
+                );
+
+                let check_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 {
+                            relative_package_dir: relative_package_dir.clone(),
                         }
                         .into_result()
                     } else {
-                        pass(())
-                    };
-
-                    let entries = utils::read_dir_sorted(&shard_path)?;
-
-                    let duplicate_check_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 {
-                                relative_shard_path: relative_shard_path.clone(),
-                                first: l.file_name(),
-                                second: r.file_name(),
-                            }
-                            .into_result::<()>()
-                        });
-
-                    let check_result = sequence_check_results(
-                        check_result,
-                        flatten_check_results(duplicate_check_results, |_| ()),
-                    );
-
-                    let check_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 {
+                        let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name);
+                        let name_check_result = if !package_name_valid {
+                            CheckError::InvalidPackageName {
                                 relative_package_dir: relative_package_dir.clone(),
+                                package_name: package_name.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 {
-                                    relative_package_dir: relative_package_dir.clone(),
-                                    package_name: package_name.clone(),
-                                }
-                                .into_result()
-                            } else {
-                                pass(())
-                            };
-
-                            let correct_relative_package_dir =
-                                Nixpkgs::relative_dir_for_package(&package_name);
-                            let shard_check_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 {
-                                            relative_package_dir: relative_package_dir.clone(),
-                                            correct_relative_package_dir:
-                                                correct_relative_package_dir.clone(),
-                                        }
-                                        .into_result()
-                                    } else {
-                                        pass(())
+                            pass(())
+                        };
+
+                        let correct_relative_package_dir =
+                            Nixpkgs::relative_dir_for_package(&package_name);
+                        let shard_check_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 {
+                                        relative_package_dir: relative_package_dir.clone(),
+                                        correct_relative_package_dir: correct_relative_package_dir
+                                            .clone(),
                                     }
+                                    .into_result()
                                 } else {
                                     pass(())
-                                };
-
-                            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(())
                             };
 
-                            flatten_check_results(
-                                [
-                                    name_check_result,
-                                    shard_check_result,
-                                    package_nix_check_result,
-                                ],
-                                |_| package_name.clone(),
-                            )
-                        }
-                    });
-
-                    sequence_check_results(
-                        check_result,
-                        flatten_check_results(check_results, |x| x),
-                    )
-                }
-            });
+                        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(())
+                        };
+
+                        flatten_check_results(
+                            [
+                                name_check_result,
+                                shard_check_result,
+                                package_nix_check_result,
+                            ],
+                            |_| package_name.clone(),
+                        )
+                    }
+                });
 
-        let check_result = flatten_check_results(check_results, |x| {
-            x.into_iter().flatten().collect::<Vec<_>>()
+                sequence_check_results(check_result, flatten_check_results(check_results, |x| x))
+            }
         });
 
-        if let Some(package_names) = write_check_result(error_writer, check_result)? {
-            Ok(Nixpkgs {
-                path: path.to_owned(),
-                package_names,
-            })
-        } else {
-            Ok(Nixpkgs {
-                path: path.to_owned(),
-                package_names: vec![],
-            })
-        }
-    }
+    flatten_check_results(check_results, |x| Nixpkgs {
+        path: path.to_owned(),
+        package_names: x.into_iter().flatten().collect::<Vec<_>>(),
+    })
 }