Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Error Types #24

Merged
merged 19 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a169008
Remove superfluous error field relative_package_file
willbush Mar 22, 2024
d12e15a
Rename package_name field in InvalidPackageName error
willbush Mar 22, 2024
0bb6405
Derive relative_package_dir from the package name for PackageNonDir
willbush Mar 22, 2024
f8bdeab
Refactor ratchet problems into a struct with common fields
willbush Mar 22, 2024
5c502a2
Remove unnecessary braces
willbush Mar 23, 2024
9391e85
Group shard problems into a struct for shared context
willbush Mar 23, 2024
686cbbc
Fix clippy warnings
willbush Mar 23, 2024
94244b0
Refactor nix file problems into a struct with common fields
willbush Mar 23, 2024
7eaa754
Refactor ByNameOveride problems into a struct with common fields
willbush Mar 24, 2024
d32d4cf
Refactor check_nix_file to use to_problem closure
willbush Mar 24, 2024
797f201
Refactor ByName problems into a struct with common fields
willbush Mar 24, 2024
e5aa63d
Refactor Path related problems into a struct with common fields
willbush Mar 24, 2024
b0d270e
Refactor Package related problems into a struct with common fields
willbush Mar 24, 2024
81e45fa
Refactor some nixpkgs_problem imports
willbush Mar 24, 2024
25adca4
Fix clone of `node` by moving variables above usage of `cast`
willbush Mar 26, 2024
2ed5984
Change most to_problem closures into to_validation
willbush Mar 26, 2024
c57206d
Replace RatchetErrorKind with is_new and is_empty bools
willbush Mar 26, 2024
bcc472d
Rename RatchetError to TopLevelPackageError
willbush Mar 26, 2024
591be8e
Add comments to NixpkgsProblem struct sub-types
willbush Mar 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 32 additions & 62 deletions src/eval.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::nix_file::CallPackageArgumentInfo;
use crate::nixpkgs_problem::NixpkgsProblem;
use crate::nixpkgs_problem::{
ByNameError, ByNameErrorKind, ByNameOverrideError, ByNameOverrideErrorKind, NixpkgsProblem,
};
use crate::ratchet;
use crate::ratchet::RatchetState::Loose;
use crate::ratchet::RatchetState::Tight;
Expand Down Expand Up @@ -213,7 +215,12 @@ fn by_name(
use ratchet::RatchetState::*;
use ByNameAttribute::*;

let relative_package_file = structure::relative_file_for_package(attribute_name);
let to_problem = |kind| {
willbush marked this conversation as resolved.
Show resolved Hide resolved
NixpkgsProblem::ByName(ByNameError {
attribute_name: attribute_name.to_owned(),
kind,
})
};

// At this point we know that `pkgs/by-name/fo/foo/package.nix` has to exists.
// This match decides whether the attribute `foo` is defined accordingly
Expand All @@ -223,11 +230,7 @@ fn by_name(
Missing => {
// This indicates a bug in the `pkgs/by-name` overlay, because it's supposed to
// automatically defined attributes in `pkgs/by-name`
NixpkgsProblem::UndefinedAttr {
relative_package_file: relative_package_file.to_owned(),
package_name: attribute_name.to_owned(),
}
.into()
to_problem(ByNameErrorKind::UndefinedAttr).into()
willbush marked this conversation as resolved.
Show resolved Hide resolved
}
// The attribute exists
Existing(AttributeInfo {
Expand All @@ -241,11 +244,7 @@ fn by_name(
//
// We can't know whether the attribute is automatically or manually defined for sure,
// and while we could check the location, the error seems clear enough as is.
NixpkgsProblem::NonDerivation {
relative_package_file: relative_package_file.to_owned(),
package_name: attribute_name.to_owned(),
}
.into()
to_problem(ByNameErrorKind::NonDerivation).into()
}
// The attribute exists
Existing(AttributeInfo {
Expand All @@ -261,11 +260,7 @@ fn by_name(
let is_derivation_result = if is_derivation {
Success(())
} else {
NixpkgsProblem::NonDerivation {
relative_package_file: relative_package_file.to_owned(),
package_name: attribute_name.to_owned(),
}
.into()
to_problem(ByNameErrorKind::NonDerivation).into()
};

// If the definition looks correct
Expand All @@ -277,10 +272,7 @@ fn by_name(
if let Some(_location) = location {
// Such an automatic definition should definitely not have a location
// Having one indicates that somebody is using `_internalCallByNamePackageFile`,
NixpkgsProblem::InternalCallPackageUsed {
attr_name: attribute_name.to_owned(),
}
.into()
to_problem(ByNameErrorKind::InternalCallPackageUsed).into()
} else {
Success(Tight)
}
Expand Down Expand Up @@ -312,7 +304,6 @@ fn by_name(

by_name_override(
attribute_name,
relative_package_file,
is_semantic_call_package,
optional_syntactic_call_package,
definition,
Expand All @@ -323,10 +314,7 @@ fn by_name(
// If manual definitions don't have a location, it's likely `mapAttrs`'d
// over, e.g. if it's defined in aliases.nix.
// We can't verify whether its of the expected `callPackage`, so error out
NixpkgsProblem::CannotDetermineAttributeLocation {
attr_name: attribute_name.to_owned(),
}
.into()
to_problem(ByNameErrorKind::CannotDetermineAttributeLocation).into()
}
}
};
Expand All @@ -350,59 +338,48 @@ fn by_name(
/// all-packages.nix
fn by_name_override(
attribute_name: &str,
expected_package_file: RelativePathBuf,
is_semantic_call_package: bool,
optional_syntactic_call_package: Option<CallPackageArgumentInfo>,
definition: String,
location: Location,
relative_location_file: RelativePathBuf,
) -> validation::Validation<ratchet::RatchetState<ratchet::ManualDefinition>> {
// At this point, we completed two different checks for whether it's a
// `callPackage`
match (is_semantic_call_package, optional_syntactic_call_package) {
// Something like `<attr> = foo`
(_, None) => NixpkgsProblem::NonSyntacticCallPackage {
let to_problem = |kind| {
NixpkgsProblem::ByNameOverride(ByNameOverrideError {
package_name: attribute_name.to_owned(),
file: relative_location_file,
line: location.line,
column: location.column,
definition,
}
.into(),
kind,
})
};

// At this point, we completed two different checks for whether it's a
// `callPackage`
match (is_semantic_call_package, optional_syntactic_call_package) {
// Something like `<attr> = foo`
(_, None) => to_problem(ByNameOverrideErrorKind::NonSyntacticCallPackage).into(),
// Something like `<attr> = pythonPackages.callPackage ...`
(false, Some(_)) => NixpkgsProblem::NonToplevelCallPackage {
package_name: attribute_name.to_owned(),
file: relative_location_file,
line: location.line,
column: location.column,
definition,
}
.into(),
(false, Some(_)) => to_problem(ByNameOverrideErrorKind::NonToplevelCallPackage).into(),
// Something like `<attr> = pkgs.callPackage ...`
(true, Some(syntactic_call_package)) => {
if let Some(actual_package_file) = syntactic_call_package.relative_path {
let expected_package_file = structure::relative_file_for_package(attribute_name);

if actual_package_file != expected_package_file {
// Wrong path
NixpkgsProblem::WrongCallPackagePath {
package_name: attribute_name.to_owned(),
file: relative_location_file,
line: location.line,
to_problem(ByNameOverrideErrorKind::WrongCallPackagePath {
actual_path: actual_package_file,
expected_path: expected_package_file,
}
})
.into()
} else {
// Manual definitions with empty arguments are not allowed
// anymore, but existing ones should continue to be allowed
let manual_definition_ratchet = if syntactic_call_package.empty_arg {
// This is the state to migrate away from
Loose(NixpkgsProblem::EmptyArgument {
package_name: attribute_name.to_owned(),
file: relative_location_file,
line: location.line,
column: location.column,
definition,
})
Loose(to_problem(ByNameOverrideErrorKind::EmptyArgument))
} else {
// This is the state to migrate to
Tight
Expand All @@ -412,14 +389,7 @@ fn by_name_override(
}
} else {
// No path
NixpkgsProblem::NonPath {
package_name: attribute_name.to_owned(),
file: relative_location_file,
line: location.line,
column: location.column,
definition,
}
.into()
to_problem(ByNameOverrideErrorKind::NonPath).into()
}
}
}
Expand Down
Loading