lib/modules: Report error for unsupported t // { check = ...; } (#454964)

lib/modules.nix

@@ -1126,6 +1126,29 @@ let
       __toString = _: showOption loc;
     };
 
+  # Check that a type with v2 merge has a coherent check attribute.
+  # Throws an error if the type uses an ad-hoc `type // { check }` override.
+  # Returns the last argument like `seq`, allowing usage: checkV2MergeCoherence loc type expr
+  checkV2MergeCoherence =
+    loc: type: result:
+    if type.check.isV2MergeCoherent or false then
+      result
+    else
+      throw ''
+        The option `${showOption loc}' has a type `${type.description}' that uses
+        an ad-hoc `type // { check = ...; }' override, which is incompatible with
+        the v2 merge mechanism.
+
+        Please use `lib.types.addCheck` instead of `type // { check }' to add
+        custom validation. For example:
+
+          lib.types.addCheck baseType (value: /* your check */)
+
+        instead of:
+
+          baseType // { check = value: /* your check */; }
+      '';
+
   # Merge definitions of a value of a given type.
   mergeDefinitions = loc: type: defs: rec {
     defsFinal' =
@@ -1201,10 +1224,13 @@ let
         (
           if type.merge ? v2 then
             let
-              r = type.merge.v2 {
+              # Check for v2 merge coherence
-                inherit loc;
+              r = checkV2MergeCoherence loc type (
-                defs = defsFinal;
+                type.merge.v2 {
-              };
+                  inherit loc;
+                  defs = defsFinal;
+                }
+              );
             in
             r
             // {

lib/tests/modules.sh

@@ -806,6 +806,25 @@ checkConfigError 'A definition for option .* is not of type .signed integer.*' c
 checkConfigOutput '^true$' config.v2checkedPass ./add-check.nix
 checkConfigError 'A definition for option .* is not of type .attribute set of signed integer.*' config.v2checkedFail ./add-check.nix
 
+# v2 merge check coherence
+# Tests checkV2MergeCoherence call in modules.nix (mergeDefinitions for lazyAttrsOf)
+checkConfigError 'ad-hoc.*override.*incompatible' config.adhocFail.foo ./v2-check-coherence.nix
+# Tests checkV2MergeCoherence call in modules.nix (mergeDefinitions for lazyAttrsOf)
+checkConfigError 'ad-hoc.*override.*incompatible' config.adhocOuterFail.bar ./v2-check-coherence.nix
+# Tests checkV2MergeCoherence call in types.nix (either t1)
+checkConfigError 'ad-hoc.*override.*incompatible' config.eitherLeftFail ./v2-check-coherence.nix
+# Tests checkV2MergeCoherence call in types.nix (either t2)
+checkConfigError 'ad-hoc.*override.*incompatible' config.eitherRightFail ./v2-check-coherence.nix
+# Tests checkV2MergeCoherence call in types.nix (coercedTo coercedType)
+checkConfigError 'ad-hoc.*override.*incompatible' config.coercedFromFail.bar ./v2-check-coherence.nix
+# Tests checkV2MergeCoherence call in types.nix (coercedTo finalType)
+checkConfigError 'ad-hoc.*override.*incompatible' config.coercedToFail.foo ./v2-check-coherence.nix
+# Tests checkV2MergeCoherence call in types.nix (addCheck elemType)
+checkConfigError 'ad-hoc.*override.*incompatible' config.addCheckNested.foo ./v2-check-coherence.nix
+checkConfigError 'Please use.*lib.types.addCheck.*instead' config.adhocFail.foo ./v2-check-coherence.nix
+checkConfigError 'A definition for option .* is not of type .*' config.addCheckFail.bar.baz ./v2-check-coherence.nix
+checkConfigOutput '^true$' config.result ./v2-check-coherence.nix
+
 
 cat <<EOF
 ====== module tests ======

lib/tests/modules/v2-check-coherence.nix

@@ -0,0 +1,117 @@
+# Tests for v2 merge check coherence
+{ lib, ... }:
+let
+  inherit (lib) types mkOption;
+
+  # The problematic pattern: overriding check with //
+  # This inner type should reject everything (check always returns false)
+  adhocOverrideType = (types.lazyAttrsOf types.raw) // {
+    check = _: false;
+  };
+
+  # Using addCheck is the correct way to add custom checks
+  properlyCheckedType = types.addCheck (types.lazyAttrsOf types.raw) (v: v ? foo);
+
+  # Using addCheck with a check that will fail
+  failingCheckedType = types.addCheck (types.lazyAttrsOf types.raw) (v: v ? foo);
+
+  # Ad-hoc override on outer type
+  adhocOuterType = types.lazyAttrsOf types.int // {
+    check = _: false;
+  };
+
+  # Ad-hoc override on left side of either
+  adhocEitherLeft = types.lazyAttrsOf types.raw // {
+    check = _: false;
+  };
+
+  # Ad-hoc override on coercedType in coercedTo
+  adhocCoercedFrom = types.lazyAttrsOf types.raw // {
+    check = _: false;
+  };
+
+  # Ad-hoc override on finalType in coercedTo
+  adhocCoercedTo = types.lazyAttrsOf types.raw // {
+    check = _: false;
+  };
+
+  # Ad-hoc override wrapped in addCheck
+  adhocAddCheck = types.addCheck (types.lazyAttrsOf types.raw // { check = _: false; }) (v: true);
+in
+{
+  # Test 1: Ad-hoc check override in nested type should be detected
+  options.adhocFail = mkOption {
+    type = types.lazyAttrsOf adhocOverrideType;
+    default = { };
+  };
+  config.adhocFail = {
+    foo = { };
+  };
+
+  # Test 1b: Ad-hoc check override in outer type should be detected
+  options.adhocOuterFail = mkOption {
+    type = adhocOuterType;
+    default = { };
+  };
+  config.adhocOuterFail.bar = 42;
+
+  # Test 1c: Ad-hoc check override on left side of either type
+  options.eitherLeftFail = mkOption {
+    type = types.either adhocEitherLeft types.int;
+  };
+  config.eitherLeftFail.foo = { };
+
+  # Test 1d: Ad-hoc check override on right side of either type
+  options.eitherRightFail = mkOption {
+    type = types.either types.int (types.lazyAttrsOf types.raw // { check = _: false; });
+  };
+  config.eitherRightFail.foo = { };
+
+  # Test 1e: Ad-hoc check override on coercedType in coercedTo
+  options.coercedFromFail = mkOption {
+    type = types.coercedTo adhocCoercedFrom (x: { bar = 1; }) (types.lazyAttrsOf types.int);
+  };
+  config.coercedFromFail = {
+    foo = { };
+  };
+
+  # Test 1f: Ad-hoc check override on finalType in coercedTo
+  options.coercedToFail = mkOption {
+    type = types.coercedTo types.str (x: { }) adhocCoercedTo;
+  };
+  config.coercedToFail.foo = { };
+
+  # Test 1g: Ad-hoc check override wrapped in addCheck
+  options.addCheckNested = mkOption {
+    type = adhocAddCheck;
+  };
+  config.addCheckNested.foo = { };
+
+  # Test 2: Using addCheck should work correctly
+  options.addCheckPass = mkOption {
+    type = types.lazyAttrsOf properlyCheckedType;
+    default = { };
+  };
+  config.addCheckPass.bar.foo = "value";
+
+  # Test 3: addCheck should validate values properly
+  options.addCheckFail = mkOption {
+    type = types.lazyAttrsOf failingCheckedType;
+    default = { };
+  };
+  config.addCheckFail.bar.baz = "value"; # Missing required 'foo' attribute
+
+  # Test 4: Normal v2 types should work without coherence errors
+  options.normalPass = mkOption {
+    type = types.lazyAttrsOf (types.attrsOf types.int);
+    default = { };
+  };
+  config.normalPass.foo.bar = 42;
+
+  # Success assertion - only checks things that should succeed
+  options.result = mkOption {
+    type = types.bool;
+    default = false;
+  };
+  config.result = true;
+}

lib/types.nix

@@ -106,6 +106,29 @@ let
     in
     if invalidDefs != [ ] then { message = "Definition values: ${showDefs invalidDefs}"; } else null;
 
+  # Check that a type with v2 merge has a coherent check attribute.
+  # Throws an error if the type uses an ad-hoc `type // { check }` override.
+  # Returns the last argument like `seq`, allowing usage: checkV2MergeCoherence loc type expr
+  checkV2MergeCoherence =
+    loc: type: result:
+    if type.check.isV2MergeCoherent or false then
+      result
+    else
+      throw ''
+        The option `${showOption loc}' has a type `${type.description}' that uses
+        an ad-hoc `type // { check = ...; }' override, which is incompatible with
+        the v2 merge mechanism.
+
+        Please use `lib.types.addCheck` instead of `type // { check }' to add
+        custom validation. For example:
+
+          lib.types.addCheck baseType (value: /* your check */)
+
+        instead of:
+
+          baseType // { check = value: /* your check */; }
+      '';
+
   outer_types = rec {
     isType = type: x: (x._type or "") == type;
 
@@ -694,7 +717,10 @@ let
             optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType
           }";
           descriptionClass = "composite";
-          check = isList;
+          check = {
+            __functor = _self: isList;
+            isV2MergeCoherent = true;
+          };
           merge = {
             __functor =
               self: loc: defs:
@@ -807,7 +833,10 @@ let
             (if lazy then "lazy attribute set" else "attribute set")
             + " of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}";
           descriptionClass = "composite";
-          check = isAttrs;
+          check = {
+            __functor = _self: isAttrs;
+            isV2MergeCoherent = true;
+          };
           merge = {
             __functor =
               self: loc: defs:
@@ -1219,7 +1248,10 @@ let
 
           name = "submodule";
 
-          check = x: isAttrs x || isFunction x || path.check x;
+          check = {
+            __functor = _self: x: isAttrs x || isFunction x || path.check x;
+            isV2MergeCoherent = true;
+          };
         in
         mkOptionType {
           inherit name;
@@ -1403,7 +1435,10 @@ let
                 ) t2
               }";
           descriptionClass = "conjunction";
-          check = x: t1.check x || t2.check x;
+          check = {
+            __functor = _self: x: t1.check x || t2.check x;
+            isV2MergeCoherent = true;
+          };
           merge = {
             __functor =
               self: loc: defs:
@@ -1413,7 +1448,7 @@ let
               let
                 t1CheckedAndMerged =
                   if t1.merge ? v2 then
-                    t1.merge.v2 { inherit loc defs; }
+                    checkV2MergeCoherence loc t1 (t1.merge.v2 { inherit loc defs; })
                   else
                     {
                       value = t1.merge loc defs;
@@ -1422,7 +1457,7 @@ let
                     };
                 t2CheckedAndMerged =
                   if t2.merge ? v2 then
-                    t2.merge.v2 { inherit loc defs; }
+                    checkV2MergeCoherence loc t2 (t2.merge.v2 { inherit loc defs; })
                   else
                     {
                       value = t2.merge loc defs;
@@ -1492,7 +1527,10 @@ let
           description = "${optionDescriptionPhrase (class: class == "noun") finalType} or ${
             optionDescriptionPhrase (class: class == "noun") coercedType
           } convertible to it";
-          check = x: (coercedType.check x && finalType.check (coerceFunc x)) || finalType.check x;
+          check = {
+            __functor = _self: x: (coercedType.check x && finalType.check (coerceFunc x)) || finalType.check x;
+            isV2MergeCoherent = true;
+          };
           merge = {
             __functor =
               self: loc: defs:
@@ -1507,10 +1545,16 @@ let
                     // {
                       value =
                         let
-                          merged = coercedType.merge.v2 {
+                          merged =
-                            inherit loc;
+                            if coercedType.merge ? v2 then
-                            defs = [ def ];
+                              checkV2MergeCoherence loc coercedType (
-                          };
+                                coercedType.merge.v2 {
+                                  inherit loc;
+                                  defs = [ def ];
+                                }
+                              )
+                            else
+                              null;
                         in
                         if coercedType.merge ? v2 then
                           if merged.headError == null then coerceFunc def.value else def.value
@@ -1523,10 +1567,12 @@ let
                 );
               in
               if finalType.merge ? v2 then
-                finalType.merge.v2 {
+                checkV2MergeCoherence loc finalType (
-                  inherit loc;
+                  finalType.merge.v2 {
-                  defs = finalDefs;
+                    inherit loc;
-                }
+                    defs = finalDefs;
+                  }
+                )
               else
                 {
                   value = finalType.merge loc finalDefs;
@@ -1559,7 +1605,10 @@ let
         if elemType.merge ? v2 then
           elemType
           // {
-            check = x: elemType.check x && check x;
+            check = {
+              __functor = _self: x: elemType.check x && check x;
+              isV2MergeCoherent = true;
+            };
             merge = {
               __functor =
                 self: loc: defs:
@@ -1567,7 +1616,7 @@ let
               v2 =
                 { loc, defs }:
                 let
-                  orig = elemType.merge.v2 { inherit loc defs; };
+                  orig = checkV2MergeCoherence loc elemType (elemType.merge.v2 { inherit loc defs; });
                   headError' = if orig.headError != null then orig.headError else checkDefsForError check loc defs;
                 in
                 orig