Refactor ratchet problems into a struct with common fields

NixOS · Mar 22, 2024 · f8bdeab · f8bdeab
1 parent 0bb6405
commit f8bdeab
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 110 deletions.
diff --git a/src/nixpkgs_problem.rs b/src/nixpkgs_problem.rs
@@ -115,26 +115,7 @@ pub enum NixpkgsProblem {
  text: String,
  io_error: String,
  },
- MovedOutOfByNameEmptyArg {
- package_name: String,
- call_package_path: Option<RelativePathBuf>,
- file: RelativePathBuf,
- },
- MovedOutOfByNameNonEmptyArg {
- package_name: String,
- call_package_path: Option<RelativePathBuf>,
- file: RelativePathBuf,
- },
- NewPackageNotUsingByNameEmptyArg {
- package_name: String,
- call_package_path: Option<RelativePathBuf>,
- file: RelativePathBuf,
- },
- NewPackageNotUsingByNameNonEmptyArg {
- package_name: String,
- call_package_path: Option<RelativePathBuf>,
- file: RelativePathBuf,
- },
+ RatchetProblem(RatchetError),
  InternalCallPackageUsed {
  attr_name: String,
  },
@@ -143,6 +124,22 @@ pub enum NixpkgsProblem {
  },
 }
 
+#[derive(Clone)]
+pub struct RatchetError {
+ pub package_name: String,
+ pub call_package_path: Option<RelativePathBuf>,
+ pub file: RelativePathBuf,
+ pub kind: RatchetErrorKind,
+}
+
+#[derive(Clone)]
+pub enum RatchetErrorKind {
+ MovedOutOfByNameEmptyArg,
+ MovedOutOfByNameNonEmptyArg,
+ NewPackageNotUsingByNameEmptyArg,
+ NewPackageNotUsingByNameNonEmptyArg,
+}
+
 impl fmt::Display for NixpkgsProblem {
  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
  match self {
@@ -319,75 +316,64 @@ impl fmt::Display for NixpkgsProblem {
  f,
  "{relative_package_dir}: File {subpath} at line {line} contains the path expression \"{text}\" which cannot be resolved: {io_error}.",
  ),
- NixpkgsProblem::MovedOutOfByNameEmptyArg { package_name, call_package_path, file } => {
- let call_package_arg =
- if let Some(path) = &call_package_path {
- format!("./{path}")
- } else {
- "...".into()
- };
- let relative_package_file = structure::relative_file_for_package(package_name);
- writedoc!(
- f,
- "
- - Attribute `pkgs.{package_name}` was previously defined in {relative_package_file}, but is now manually defined as `callPackage {call_package_arg} {{ /* ... */ }}` in {file}.
- Please move the package back and remove the manual `callPackage`.
- ",
- )
- },
- NixpkgsProblem::MovedOutOfByNameNonEmptyArg { package_name, call_package_path, file } => {
+ NixpkgsProblem::RatchetProblem(RatchetError {
+ package_name,
+ call_package_path,
+ file,
+ kind,
+ }) => {
  let call_package_arg =
  if let Some(path) = &call_package_path {
  format!("./{}", path)
  } else {
  "...".into()
  };
  let relative_package_file = structure::relative_file_for_package(package_name);
- // This can happen if users mistakenly assume that for custom arguments,
- // pkgs/by-name can't be used.
- writedoc!(
- f,
- "
- - Attribute `pkgs.{package_name}` was previously defined in {relative_package_file}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` in {file}.
- While the manual `callPackage` is still needed, it's not necessary to move the package files.
- ",
- )
- },
- NixpkgsProblem::NewPackageNotUsingByNameEmptyArg { package_name, call_package_path, file } => {
- let call_package_arg =
- if let Some(path) = &call_package_path {
- format!("./{}", path)
- } else {
- "...".into()
- };
- let relative_package_file = structure::relative_file_for_package(package_name);
- writedoc!(
- f,
- "
- - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`.
-  Please define it in {relative_package_file} instead.
- See `pkgs/by-name/README.md` for more details.
- Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {file} is needed anymore.
- ",
- )
- },
- NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg { package_name, call_package_path, file } => {
- let call_package_arg =
- if let Some(path) = &call_package_path {
- format!("./{}", path)
- } else {
-  "...".into()
- };
- let relative_package_file = structure::relative_file_for_package(package_name);
- writedoc!(
- f,
- "
- - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`.
- Please define it in {relative_package_file} instead.
- See `pkgs/by-name/README.md` for more details.
- Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {file} is still needed.
- ",
- )
+
+ match kind {
+  RatchetErrorKind::MovedOutOfByNameEmptyArg => {
+  writedoc!(
+  f,
+  "
+  - Attribute `pkgs.{package_name}` was previously defined in {relative_package_file}, but is now manually defined as `callPackage {call_package_arg} {{ /* ... */ }}` in {file}.
+  Please move the package back and remove the manual `callPackage`.
+  ",
+  )
+  },
+  RatchetErrorKind::MovedOutOfByNameNonEmptyArg => {
+  // This can happen if users mistakenly assume that for custom arguments,
+ // pkgs/by-name can't be used.
+  writedoc!(
+  f,
+  "
+  - Attribute `pkgs.{package_name}` was previously defined in {relative_package_file}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` in {file}.
+  While the manual `callPackage` is still needed, it's not necessary to move the package files.
+  ",
+  )
+ },
+ RatchetErrorKind::NewPackageNotUsingByNameEmptyArg => {
+  writedoc!(
+  f,
+  "
+  - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`.
+  Please define it in {relative_package_file} instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is `{{ }}`, no manual `callPackage` in {file} is needed anymore.
+  ",
+ )
+ },
+ RatchetErrorKind::NewPackageNotUsingByNameNonEmptyArg => {
+  writedoc!(
+  f,
+  "
+  - Attribute `pkgs.{package_name}` is a new top-level package using `pkgs.callPackage {call_package_arg} {{ /* ... */ }}`.
+  Please define it in {relative_package_file} instead.
+  See `pkgs/by-name/README.md` for more details.
+  Since the second `callPackage` argument is not `{{ }}`, the manual `callPackage` in {file} is still needed.
+  ",
+  )
+ },
+ }
  },
  NixpkgsProblem::InternalCallPackageUsed { attr_name } =>
  write!(

diff --git a/src/ratchet.rs b/src/ratchet.rs
@@ -3,7 +3,7 @@
 //! Each type has a `compare` method that validates the ratchet checks for that item.
 
 use crate::nix_file::CallPackageArgumentInfo;
-use crate::nixpkgs_problem::NixpkgsProblem;
+use crate::nixpkgs_problem::{NixpkgsProblem, RatchetError, RatchetErrorKind};
 use crate::validation::{self, Validation, Validation::Success};
 use relative_path::RelativePathBuf;
 use std::collections::HashMap;
@@ -153,32 +153,16 @@ impl ToNixpkgsProblem for UsesByName {
  optional_from: Option<()>,
  (to, file): &Self::ToContext,
  ) -> NixpkgsProblem {
- if let Some(()) = optional_from {
- if to.empty_arg {
- NixpkgsProblem::MovedOutOfByNameEmptyArg {
- package_name: name.to_owned(),
- call_package_path: to.relative_path.clone(),
- file: file.to_owned(),
- }
- } else {
- NixpkgsProblem::MovedOutOfByNameNonEmptyArg {
- package_name: name.to_owned(),
- call_package_path: to.relative_path.clone(),
- file: file.to_owned(),
- }
- }
- } else if to.empty_arg {
- NixpkgsProblem::NewPackageNotUsingByNameEmptyArg {
- package_name: name.to_owned(),
- call_package_path: to.relative_path.clone(),
- file: file.to_owned(),
- }
- } else {
- NixpkgsProblem::NewPackageNotUsingByNameNonEmptyArg {
- package_name: name.to_owned(),
- call_package_path: to.relative_path.clone(),
- file: file.to_owned(),
- }
- }
+ NixpkgsProblem::RatchetProblem(RatchetError {
+ package_name: name.to_owned(),
+ call_package_path: to.relative_path.clone(),
+ file: file.to_owned(),
+ kind: match (optional_from, to.empty_arg) {
+ (Some(()), true) => RatchetErrorKind::MovedOutOfByNameEmptyArg,
+ (Some(()), false) => RatchetErrorKind::MovedOutOfByNameNonEmptyArg,
+ (None, true) => RatchetErrorKind::NewPackageNotUsingByNameEmptyArg,
+ (None, false) => RatchetErrorKind::NewPackageNotUsingByNameNonEmptyArg,
+ },
+ })
  }
 }