tests.nixpkgs-check-by-name: Re-usable ratchet logic

This makes the attribute ratchet check logic more re-usable, which will
be used in a future commit.

It also renames the ratchet states to something more intuitive

pkgs/test/nixpkgs-check-by-name/src/eval.rs

@@ -130,6 +130,7 @@ pub fn check_values(
             let relative_package_file = structure::relative_file_for_package(&attribute_name);
             let absolute_package_file = nixpkgs_path.join(&relative_package_file);
 
+            use ratchet::RatchetState::*;
             use AttributeInfo::*;
             use ByNameAttribute::*;
             use CallPackageVariant::*;
@@ -166,7 +167,7 @@ pub fn check_values(
 
                     check_result.and(match &call_package_variant {
                         Auto => Success(ratchet::Package {
-                            empty_non_auto_called: ratchet::EmptyNonAutoCalled::Valid,
+                            empty_non_auto_called: Tight,
                         }),
                         Manual { path, empty_arg } => {
                             let correct_file = if let Some(call_package_path) = path {
@@ -179,9 +180,9 @@ pub fn check_values(
                                 Success(ratchet::Package {
                                     // Empty arguments for non-auto-called packages are not allowed anymore.
                                     empty_non_auto_called: if *empty_arg {
-                                        ratchet::EmptyNonAutoCalled::Invalid
+                                        Loose(ratchet::EmptyNonAutoCalled)
                                     } else {
-                                        ratchet::EmptyNonAutoCalled::Valid
+                                        Tight
                                     },
                                 })
                             } else {

pkgs/test/nixpkgs-check-by-name/src/ratchet.rs

@@ -30,7 +30,7 @@ impl Nixpkgs {
 /// The ratchet value for a single package in `pkgs/by-name`
 pub struct Package {
     /// The ratchet value for the check for non-auto-called empty arguments
-    pub empty_non_auto_called: EmptyNonAutoCalled,
+    pub empty_non_auto_called: RatchetState<EmptyNonAutoCalled>,
 }
 
 impl Package {
@@ -44,29 +44,61 @@ impl Package {
     }
 }
 
-/// The ratchet value of a single package in `pkgs/by-name`
+/// The ratchet state of a generic ratchet check.
+pub enum RatchetState<Context> {
+    /// The ratchet is loose, it can be tightened more.
+    /// In other words, this is the legacy state we're trying to move away from.
+    /// Introducing new instances is now allowed but previous instances will continue to be allowed.
+    /// The `Context` is context for error messages in case a new instance of this state is
+    /// introduced
+    Loose(Context),
+    /// The ratchet is tight, it can't be tightened any further.
+    /// This is either because we already use the latest state, or because the ratchet isn't
+    /// relevant.
+    Tight,
+}
+
+/// A trait for a specific ratchet check for an attribute.
+trait AttributeRatchet: Sized {
+    /// What error to produce in case the ratchet went in the wrong direction
+    fn to_error(name: &str, context: &Self, existed_before: bool) -> NixpkgsProblem;
+
+    /// Compare the previous ratchet state of an attribute to the new state.
+    /// The previous state may be `None` in case the attribute is new.
+    fn compare(
+        name: &str,
+        optional_from: Option<&RatchetState<Self>>,
+        to: &RatchetState<Self>,
+    ) -> Validation<()> {
+        // If we don't have a previous state, enforce a tight ratchet
+        let from = optional_from.unwrap_or(&RatchetState::Tight);
+        match (from, to) {
+            // Always okay to keep it tight or tighten the ratchet
+            (_, RatchetState::Tight) => Success(()),
+
+            // Grandfathering policy for a loose ratchet
+            (RatchetState::Loose { .. }, RatchetState::Loose { .. }) => Success(()),
+
+            // Loosening a ratchet is now allowed
+            (RatchetState::Tight, RatchetState::Loose(context)) => {
+                Self::to_error(name, context, optional_from.is_some()).into()
+            }
+        }
+    }
+}
+
+/// The ratchet value of an attribute
 /// for the non-auto-called empty argument check of a single.
 ///
 /// This checks that packages defined in `pkgs/by-name` cannot be overridden
 /// with an empty second argument like `callPackage ... { }`.
-#[derive(PartialEq, PartialOrd)]
+pub struct EmptyNonAutoCalled;
-pub enum EmptyNonAutoCalled {
-    Invalid,
-    Valid,
-}
 
-impl EmptyNonAutoCalled {
+impl AttributeRatchet for EmptyNonAutoCalled {
-    /// Validates the non-auto-called empty argument ratchet check for a single package defined in `pkgs/by-name`
+    fn to_error(name: &str, _context: &Self, _existed_before: bool) -> NixpkgsProblem {
-    fn compare(name: &str, optional_from: Option<&Self>, to: &Self) -> Validation<()> {
-        let from = optional_from.unwrap_or(&Self::Valid);
-        if to >= from {
-            Success(())
-        } else {
         NixpkgsProblem::WrongCallPackage {
             relative_package_file: structure::relative_file_for_package(name),
             package_name: name.to_owned(),
         }
-            .into()
-        }
     }
 }