tests.nixpkgs-check-by-name: Redesign and document check_result functions
This commit is contained in:
parent
3d60440799
commit
eac0b69063
5 changed files with 147 additions and 125 deletions
|
@ -1,13 +1,17 @@
|
||||||
use crate::utils::PACKAGE_NIX_FILENAME;
|
use crate::utils::PACKAGE_NIX_FILENAME;
|
||||||
use itertools::concat;
|
use itertools::concat;
|
||||||
use itertools::{Either, Itertools};
|
use itertools::{
|
||||||
|
Either,
|
||||||
|
Either::{Left, Right},
|
||||||
|
Itertools,
|
||||||
|
};
|
||||||
use rnix::parser::ParseError;
|
use rnix::parser::ParseError;
|
||||||
use std::ffi::OsString;
|
use std::ffi::OsString;
|
||||||
use std::fmt;
|
use std::fmt;
|
||||||
use std::io;
|
use std::io;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|
||||||
pub enum CheckError {
|
pub enum CheckProblem {
|
||||||
ShardNonDir {
|
ShardNonDir {
|
||||||
relative_shard_path: PathBuf,
|
relative_shard_path: PathBuf,
|
||||||
},
|
},
|
||||||
|
@ -90,97 +94,97 @@ pub enum CheckError {
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
impl CheckError {
|
impl CheckProblem {
|
||||||
pub fn into_result<A>(self) -> CheckResult<A> {
|
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 {
|
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
|
||||||
match self {
|
match self {
|
||||||
CheckError::ShardNonDir { relative_shard_path } =>
|
CheckProblem::ShardNonDir { relative_shard_path } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: This is a file, but it should be a directory.",
|
"{}: This is a file, but it should be a directory.",
|
||||||
relative_shard_path.display(),
|
relative_shard_path.display(),
|
||||||
),
|
),
|
||||||
CheckError::InvalidShardName { relative_shard_path, shard_name } =>
|
CheckProblem::InvalidShardName { relative_shard_path, shard_name } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".",
|
"{}: Invalid directory name \"{shard_name}\", must be at most 2 ASCII characters consisting of a-z, 0-9, \"-\" or \"_\".",
|
||||||
relative_shard_path.display()
|
relative_shard_path.display()
|
||||||
),
|
),
|
||||||
CheckError::PackageNonDir { relative_package_dir } =>
|
CheckProblem::PackageNonDir { relative_package_dir } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: This path is a file, but it should be a directory.",
|
"{}: This path is a file, but it should be a directory.",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
),
|
),
|
||||||
CheckError::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
|
CheckProblem::CaseSensitiveDuplicate { relative_shard_path, first, second } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Duplicate case-sensitive package directories {first:?} and {second:?}.",
|
"{}: Duplicate case-sensitive package directories {first:?} and {second:?}.",
|
||||||
relative_shard_path.display(),
|
relative_shard_path.display(),
|
||||||
),
|
),
|
||||||
CheckError::InvalidPackageName { relative_package_dir, package_name } =>
|
CheckProblem::InvalidPackageName { relative_package_dir, package_name } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".",
|
"{}: Invalid package directory name \"{package_name}\", must be ASCII characters consisting of a-z, A-Z, 0-9, \"-\" or \"_\".",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
),
|
),
|
||||||
CheckError::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
|
CheckProblem::IncorrectShard { relative_package_dir, correct_relative_package_dir } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Incorrect directory location, should be {} instead.",
|
"{}: Incorrect directory location, should be {} instead.",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
correct_relative_package_dir.display(),
|
correct_relative_package_dir.display(),
|
||||||
),
|
),
|
||||||
CheckError::PackageNixNonExistent { relative_package_dir } =>
|
CheckProblem::PackageNixNonExistent { relative_package_dir } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.",
|
"{}: Missing required \"{PACKAGE_NIX_FILENAME}\" file.",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
),
|
),
|
||||||
CheckError::PackageNixDir { relative_package_dir } =>
|
CheckProblem::PackageNixDir { relative_package_dir } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.",
|
"{}: \"{PACKAGE_NIX_FILENAME}\" must be a file.",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
),
|
),
|
||||||
CheckError::UndefinedAttr { relative_package_file, package_name } =>
|
CheckProblem::UndefinedAttr { relative_package_file, package_name } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
|
"pkgs.{package_name}: This attribute is not defined but it should be defined automatically as {}",
|
||||||
relative_package_file.display()
|
relative_package_file.display()
|
||||||
),
|
),
|
||||||
CheckError::WrongCallPackage { relative_package_file, package_name } =>
|
CheckProblem::WrongCallPackage { relative_package_file, package_name } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
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.",
|
"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()
|
relative_package_file.display()
|
||||||
),
|
),
|
||||||
CheckError::NonDerivation { relative_package_file, package_name } =>
|
CheckProblem::NonDerivation { relative_package_file, package_name } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"pkgs.{package_name}: This attribute defined by {} is not a derivation",
|
"pkgs.{package_name}: This attribute defined by {} is not a derivation",
|
||||||
relative_package_file.display()
|
relative_package_file.display()
|
||||||
),
|
),
|
||||||
CheckError::OutsideSymlink { relative_package_dir, subpath } =>
|
CheckProblem::OutsideSymlink { relative_package_dir, subpath } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Path {} is a symlink pointing to a path outside the directory of that package.",
|
"{}: Path {} is a symlink pointing to a path outside the directory of that package.",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
subpath.display(),
|
subpath.display(),
|
||||||
),
|
),
|
||||||
CheckError::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
|
CheckProblem::UnresolvableSymlink { relative_package_dir, subpath, io_error } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: Path {} is a symlink which cannot be resolved: {io_error}.",
|
"{}: Path {} is a symlink which cannot be resolved: {io_error}.",
|
||||||
relative_package_dir.display(),
|
relative_package_dir.display(),
|
||||||
subpath.display(),
|
subpath.display(),
|
||||||
),
|
),
|
||||||
CheckError::CouldNotParseNix { relative_package_dir, subpath, error } =>
|
CheckProblem::CouldNotParseNix { relative_package_dir, subpath, error } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: File {} could not be parsed by rnix: {}",
|
"{}: File {} could not be parsed by rnix: {}",
|
||||||
|
@ -188,7 +192,7 @@ impl fmt::Display for CheckError {
|
||||||
subpath.display(),
|
subpath.display(),
|
||||||
error,
|
error,
|
||||||
),
|
),
|
||||||
CheckError::PathInterpolation { relative_package_dir, subpath, line, text } =>
|
CheckProblem::PathInterpolation { relative_package_dir, subpath, line, text } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: File {} at line {line} contains the path expression \"{}\", which is not yet supported and may point outside the directory of that package.",
|
"{}: 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(),
|
subpath.display(),
|
||||||
text
|
text
|
||||||
),
|
),
|
||||||
CheckError::SearchPath { relative_package_dir, subpath, line, text } =>
|
CheckProblem::SearchPath { relative_package_dir, subpath, line, text } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: File {} at line {line} contains the nix search path expression \"{}\" which may point outside the directory of that package.",
|
"{}: 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(),
|
subpath.display(),
|
||||||
text
|
text
|
||||||
),
|
),
|
||||||
CheckError::OutsidePathReference { relative_package_dir, subpath, line, text } =>
|
CheckProblem::OutsidePathReference { relative_package_dir, subpath, line, text } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: File {} at line {line} contains the path expression \"{}\" which may point outside the directory of that package.",
|
"{}: 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(),
|
subpath.display(),
|
||||||
text,
|
text,
|
||||||
),
|
),
|
||||||
CheckError::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
|
CheckProblem::UnresolvablePathReference { relative_package_dir, subpath, line, text, io_error } =>
|
||||||
write!(
|
write!(
|
||||||
f,
|
f,
|
||||||
"{}: File {} at line {line} contains the path expression \"{}\" which cannot be resolved: {io_error}.",
|
"{}: 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> {
|
/// A type alias representing the result of a check, either:
|
||||||
Ok(Either::Right(value))
|
/// - 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?) {
|
match (first?, second?) {
|
||||||
(Either::Right(_), Either::Right(right_value)) => pass(right_value),
|
(Right(_), Right(right_value)) => Ok(Right(right_value)),
|
||||||
(Either::Left(errors), Either::Right(_)) => Ok(Either::Left(errors)),
|
(Left(errors), Right(_)) => Ok(Left(errors)),
|
||||||
(Either::Right(_), Either::Left(errors)) => Ok(Either::Left(errors)),
|
(Right(_), Left(errors)) => Ok(Left(errors)),
|
||||||
(Either::Left(errors_l), Either::Left(errors_r)) => {
|
(Left(errors_l), Left(errors_r)) => Ok(Left(concat([errors_l, errors_r]))),
|
||||||
Ok(Either::Left(concat([errors_l, errors_r])))
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn flatten_check_results<I, O>(
|
/// Combine many checks results into a single one.
|
||||||
check_results: impl IntoIterator<Item = CheckResult<I>>,
|
/// All given check results need to be successful in order for the returned check result to be successful,
|
||||||
value_transform: impl Fn(Vec<I>) -> O,
|
/// in which case the returned check result value contains each a `Vec` of each individual result value.
|
||||||
) -> CheckResult<O> {
|
/// 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
|
let (errors, values): (Vec<_>, Vec<_>) = check_results
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.collect::<anyhow::Result<Vec<_>>>()?
|
.collect::<anyhow::Result<Vec<_>>>()?
|
||||||
|
@ -256,8 +277,13 @@ pub fn flatten_check_results<I, O>(
|
||||||
let flattened_errors = errors.into_iter().flatten().collect::<Vec<_>>();
|
let flattened_errors = errors.into_iter().flatten().collect::<Vec<_>>();
|
||||||
|
|
||||||
if flattened_errors.is_empty() {
|
if flattened_errors.is_empty() {
|
||||||
Ok(Either::Right(value_transform(values)))
|
Ok(Right(values))
|
||||||
} else {
|
} 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), |_| ())
|
||||||
|
}
|
||||||
|
|
|
@ -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::structure;
|
||||||
use crate::Version;
|
use crate::Version;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
|
@ -110,7 +111,7 @@ pub fn check_values(
|
||||||
String::from_utf8_lossy(&result.stdout)
|
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 relative_package_file = structure::relative_file_for_package(package_name);
|
||||||
let absolute_package_file = nixpkgs_path.join(&relative_package_file);
|
let absolute_package_file = nixpkgs_path.join(&relative_package_file);
|
||||||
|
|
||||||
|
@ -136,27 +137,26 @@ pub fn check_values(
|
||||||
};
|
};
|
||||||
|
|
||||||
if !valid {
|
if !valid {
|
||||||
CheckError::WrongCallPackage {
|
CheckProblem::WrongCallPackage {
|
||||||
relative_package_file: relative_package_file.clone(),
|
relative_package_file: relative_package_file.clone(),
|
||||||
package_name: package_name.clone(),
|
package_name: package_name.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else if !attribute_info.is_derivation {
|
} else if !attribute_info.is_derivation {
|
||||||
CheckError::NonDerivation {
|
CheckProblem::NonDerivation {
|
||||||
relative_package_file: relative_package_file.clone(),
|
relative_package_file: relative_package_file.clone(),
|
||||||
package_name: package_name.clone(),
|
package_name: package_name.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
CheckError::UndefinedAttr {
|
CheckProblem::UndefinedAttr {
|
||||||
relative_package_file: relative_package_file.clone(),
|
relative_package_file: relative_package_file.clone(),
|
||||||
package_name: package_name.clone(),
|
package_name: package_name.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
}
|
}
|
||||||
});
|
}))
|
||||||
flatten_check_results(check_results, |_| ())
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -6,11 +6,12 @@ mod utils;
|
||||||
|
|
||||||
use crate::structure::check_structure;
|
use crate::structure::check_structure;
|
||||||
use anyhow::Context;
|
use anyhow::Context;
|
||||||
use check_result::pass;
|
|
||||||
use clap::{Parser, ValueEnum};
|
use clap::{Parser, ValueEnum};
|
||||||
use colored::Colorize;
|
use colored::Colorize;
|
||||||
use itertools::Either;
|
use itertools::{
|
||||||
use itertools::Either::{Left, Right};
|
Either,
|
||||||
|
Either::{Left, Right},
|
||||||
|
};
|
||||||
use std::io;
|
use std::io;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use std::process::ExitCode;
|
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.",
|
"Given Nixpkgs path does not contain a {} subdirectory, no check necessary.",
|
||||||
utils::BASE_SUBPATH
|
utils::BASE_SUBPATH
|
||||||
);
|
);
|
||||||
pass(())
|
check_result::ok(())
|
||||||
} else {
|
} else {
|
||||||
match check_structure(&nixpkgs_path)? {
|
match check_structure(&nixpkgs_path)? {
|
||||||
Left(errors) => Ok(Left(errors)),
|
Left(errors) => Ok(Left(errors)),
|
||||||
|
|
|
@ -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;
|
||||||
use crate::utils::LineIndex;
|
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
|
// No need to handle the case of it being inside the directory, since we scan through the
|
||||||
// entire directory recursively anyways
|
// entire directory recursively anyways
|
||||||
if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
|
if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
|
||||||
CheckError::OutsideSymlink {
|
CheckProblem::OutsideSymlink {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(io_error) => CheckError::UnresolvableSymlink {
|
Err(io_error) => CheckProblem::UnresolvableSymlink {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
io_error,
|
io_error,
|
||||||
|
@ -64,12 +65,11 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
|
||||||
}
|
}
|
||||||
} else if path.is_dir() {
|
} else if path.is_dir() {
|
||||||
// Recursively check each entry
|
// 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());
|
let entry_subpath = subpath.join(entry.file_name());
|
||||||
check_path(context, &entry_subpath)
|
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() {
|
} else if path.is_file() {
|
||||||
// Only check Nix files
|
// Only check Nix files
|
||||||
if let Some(ext) = path.extension() {
|
if let Some(ext) = path.extension() {
|
||||||
|
@ -79,10 +79,10 @@ fn check_path(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
|
||||||
subpath.display()
|
subpath.display()
|
||||||
))
|
))
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// This should never happen, git doesn't support other file types
|
// 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);
|
let root = Root::parse(&contents);
|
||||||
if let Some(error) = root.errors().first() {
|
if let Some(error) = root.errors().first() {
|
||||||
return CheckError::CouldNotParseNix {
|
return CheckProblem::CouldNotParseNix {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
error: error.clone(),
|
error: error.clone(),
|
||||||
|
@ -114,17 +114,17 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
|
||||||
|
|
||||||
let line_index = LineIndex::new(&contents);
|
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 text = node.text().to_string();
|
||||||
let line = line_index.line(node.text_range().start().into());
|
let line = line_index.line(node.text_range().start().into());
|
||||||
|
|
||||||
if node.kind() != NODE_PATH {
|
if node.kind() != NODE_PATH {
|
||||||
// We're only interested in Path expressions
|
// We're only interested in Path expressions
|
||||||
pass(())
|
check_result::ok(())
|
||||||
} else if node.children().count() != 0 {
|
} else if node.children().count() != 0 {
|
||||||
// Filters out ./foo/${bar}/baz
|
// Filters out ./foo/${bar}/baz
|
||||||
// TODO: We can just check ./foo
|
// TODO: We can just check ./foo
|
||||||
CheckError::PathInterpolation {
|
CheckProblem::PathInterpolation {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
line,
|
line,
|
||||||
|
@ -133,7 +133,7 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
|
||||||
.into_result()
|
.into_result()
|
||||||
} else if text.starts_with('<') {
|
} else if text.starts_with('<') {
|
||||||
// Filters out search paths like <nixpkgs>
|
// Filters out search paths like <nixpkgs>
|
||||||
CheckError::SearchPath {
|
CheckProblem::SearchPath {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
line,
|
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
|
// No need to handle the case of it being inside the directory, since we scan through the
|
||||||
// entire directory recursively anyways
|
// entire directory recursively anyways
|
||||||
if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
|
if let Err(_prefix_error) = target.strip_prefix(context.absolute_package_dir) {
|
||||||
CheckError::OutsidePathReference {
|
CheckProblem::OutsidePathReference {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
line,
|
line,
|
||||||
|
@ -157,10 +157,10 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(e) => CheckError::UnresolvablePathReference {
|
Err(e) => CheckProblem::UnresolvablePathReference {
|
||||||
relative_package_dir: context.relative_package_dir.clone(),
|
relative_package_dir: context.relative_package_dir.clone(),
|
||||||
subpath: subpath.to_path_buf(),
|
subpath: subpath.to_path_buf(),
|
||||||
line,
|
line,
|
||||||
|
@ -170,6 +170,5 @@ fn check_nix_file(context: &PackageContext, subpath: &Path) -> CheckResult<()> {
|
||||||
.into_result(),
|
.into_result(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
}))
|
||||||
flatten_check_results(check_results, |_| ())
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,6 +1,5 @@
|
||||||
use crate::check_result::{
|
use crate::check_result;
|
||||||
flatten_check_results, pass, sequence_check_results, CheckError, CheckResult,
|
use crate::check_result::{CheckProblem, CheckResult};
|
||||||
};
|
|
||||||
use crate::references;
|
use crate::references;
|
||||||
use crate::utils;
|
use crate::utils;
|
||||||
use crate::utils::{BASE_SUBPATH, PACKAGE_NIX_FILENAME};
|
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>> {
|
pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
|
||||||
let base_dir = path.join(BASE_SUBPATH);
|
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()
|
.into_iter()
|
||||||
.map(|shard_entry| {
|
.map(|shard_entry| {
|
||||||
let shard_path = shard_entry.path();
|
let shard_path = shard_entry.path();
|
||||||
|
@ -45,35 +44,35 @@ pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
|
||||||
|
|
||||||
if shard_name == "README.md" {
|
if shard_name == "README.md" {
|
||||||
// README.md is allowed to be a file and not checked
|
// README.md is allowed to be a file and not checked
|
||||||
pass(vec![])
|
check_result::ok(vec![])
|
||||||
} else if !shard_path.is_dir() {
|
} else if !shard_path.is_dir() {
|
||||||
CheckError::ShardNonDir {
|
CheckProblem::ShardNonDir {
|
||||||
relative_shard_path: relative_shard_path.clone(),
|
relative_shard_path: relative_shard_path.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
// we can't check for any other errors if it's a file, since there's no subdirectories to check
|
// we can't check for any other errors if it's a file, since there's no subdirectories to check
|
||||||
} else {
|
} else {
|
||||||
let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name);
|
let shard_name_valid = SHARD_NAME_REGEX.is_match(&shard_name);
|
||||||
let check_result = if !shard_name_valid {
|
let result = if !shard_name_valid {
|
||||||
CheckError::InvalidShardName {
|
CheckProblem::InvalidShardName {
|
||||||
relative_shard_path: relative_shard_path.clone(),
|
relative_shard_path: relative_shard_path.clone(),
|
||||||
shard_name: shard_name.clone(),
|
shard_name: shard_name.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
};
|
};
|
||||||
|
|
||||||
let entries = utils::read_dir_sorted(&shard_path)?;
|
let entries = utils::read_dir_sorted(&shard_path)?;
|
||||||
|
|
||||||
let duplicate_check_results = entries
|
let duplicate_results = entries
|
||||||
.iter()
|
.iter()
|
||||||
.zip(entries.iter().skip(1))
|
.zip(entries.iter().skip(1))
|
||||||
.filter(|(l, r)| {
|
.filter(|(l, r)| {
|
||||||
l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase()
|
l.file_name().to_ascii_lowercase() == r.file_name().to_ascii_lowercase()
|
||||||
})
|
})
|
||||||
.map(|(l, r)| {
|
.map(|(l, r)| {
|
||||||
CheckError::CaseSensitiveDuplicate {
|
CheckProblem::CaseSensitiveDuplicate {
|
||||||
relative_shard_path: relative_shard_path.clone(),
|
relative_shard_path: relative_shard_path.clone(),
|
||||||
first: l.file_name(),
|
first: l.file_name(),
|
||||||
second: r.file_name(),
|
second: r.file_name(),
|
||||||
|
@ -81,90 +80,87 @@ pub fn check_structure(path: &Path) -> CheckResult<Vec<String>> {
|
||||||
.into_result::<()>()
|
.into_result::<()>()
|
||||||
});
|
});
|
||||||
|
|
||||||
let check_result = sequence_check_results(
|
let result = check_result::and(result, check_result::sequence_(duplicate_results));
|
||||||
check_result,
|
|
||||||
flatten_check_results(duplicate_check_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_path = package_entry.path();
|
||||||
let package_name = package_entry.file_name().to_string_lossy().into_owned();
|
let package_name = package_entry.file_name().to_string_lossy().into_owned();
|
||||||
let relative_package_dir =
|
let relative_package_dir =
|
||||||
PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}"));
|
PathBuf::from(format!("{BASE_SUBPATH}/{shard_name}/{package_name}"));
|
||||||
|
|
||||||
if !package_path.is_dir() {
|
if !package_path.is_dir() {
|
||||||
CheckError::PackageNonDir {
|
CheckProblem::PackageNonDir {
|
||||||
relative_package_dir: relative_package_dir.clone(),
|
relative_package_dir: relative_package_dir.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name);
|
let package_name_valid = PACKAGE_NAME_REGEX.is_match(&package_name);
|
||||||
let name_check_result = if !package_name_valid {
|
let result = if !package_name_valid {
|
||||||
CheckError::InvalidPackageName {
|
CheckProblem::InvalidPackageName {
|
||||||
relative_package_dir: relative_package_dir.clone(),
|
relative_package_dir: relative_package_dir.clone(),
|
||||||
package_name: package_name.clone(),
|
package_name: package_name.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
};
|
};
|
||||||
|
|
||||||
let correct_relative_package_dir = relative_dir_for_package(&package_name);
|
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 {
|
if relative_package_dir != correct_relative_package_dir {
|
||||||
// Only show this error if we have a valid shard and package name
|
// 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
|
// Because if one of those is wrong, you should fix that first
|
||||||
if shard_name_valid && package_name_valid {
|
if shard_name_valid && package_name_valid {
|
||||||
CheckError::IncorrectShard {
|
CheckProblem::IncorrectShard {
|
||||||
relative_package_dir: relative_package_dir.clone(),
|
relative_package_dir: relative_package_dir.clone(),
|
||||||
correct_relative_package_dir: correct_relative_package_dir
|
correct_relative_package_dir: correct_relative_package_dir
|
||||||
.clone(),
|
.clone(),
|
||||||
}
|
}
|
||||||
.into_result()
|
.into_result()
|
||||||
} else {
|
} else {
|
||||||
pass(())
|
check_result::ok(())
|
||||||
}
|
}
|
||||||
} else {
|
} 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 reference_check_result = references::check_references(
|
|
||||||
&relative_package_dir,
|
|
||||||
&path.join(&relative_package_dir),
|
|
||||||
);
|
);
|
||||||
|
|
||||||
flatten_check_results(
|
let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME);
|
||||||
[
|
let result = check_result::and(
|
||||||
name_check_result,
|
result,
|
||||||
shard_check_result,
|
if !package_nix_path.exists() {
|
||||||
package_nix_check_result,
|
CheckProblem::PackageNixNonExistent {
|
||||||
reference_check_result,
|
relative_package_dir: relative_package_dir.clone(),
|
||||||
],
|
}
|
||||||
|_| package_name.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 result = check_result::and(
|
||||||
|
result,
|
||||||
|
references::check_references(
|
||||||
|
&relative_package_dir,
|
||||||
|
&path.join(&relative_package_dir),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
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<_>>()
|
x.into_iter().flatten().collect::<Vec<_>>()
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue