Skip to content

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

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.

Sign up for GitHub

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

Merged
roberth merged 1 commit into from
Oct 28, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 30 additions & 4 deletions lib/modules.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' =
Expand Down Expand Up @@ -1201,10 +1224,13 @@ let
(
if type.merge ? v2 then
let
r = type.merge.v2 {
inherit loc;
defs = defsFinal;
};
# Check for v2 merge coherence
r = checkV2MergeCoherence loc type (
type.merge.v2 {
inherit loc;
defs = defsFinal;
}
);
in
r
// {
Expand Down
19 changes: 19 additions & 0 deletions lib/tests/modules.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ======
Expand Down
117 changes: 117 additions & 0 deletions lib/tests/modules/v2-check-coherence.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}
83 changes: 66 additions & 17 deletions lib/types.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -711,7 +734,10 @@ let
optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType
}";
descriptionClass = "composite";
check = isList;
check = {
__functor = _self: isList;
isV2MergeCoherent = true;
};
merge = {
__functor =
self: loc: defs:
Expand Down Expand Up @@ -824,7 +850,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:
Expand Down Expand Up @@ -1236,7 +1265,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;
Expand Down Expand Up @@ -1420,7 +1452,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:
Expand All @@ -1430,7 +1465,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;
Expand All @@ -1439,7 +1474,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;
Expand Down Expand Up @@ -1509,7 +1544,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:
Expand All @@ -1524,10 +1562,16 @@ let
// {
value =
let
merged = coercedType.merge.v2 {
inherit loc;
defs = [ def ];
};
merged =
if coercedType.merge ? v2 then
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
Expand All @@ -1540,10 +1584,12 @@ let
);
in
if finalType.merge ? v2 then
finalType.merge.v2 {
inherit loc;
defs = finalDefs;
}
checkV2MergeCoherence loc finalType (
finalType.merge.v2 {
inherit loc;
defs = finalDefs;
}
)
else
{
value = finalType.merge loc finalDefs;
Expand Down Expand Up @@ -1576,15 +1622,18 @@ 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:
(self.v2 { inherit loc defs; }).value;
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
Expand Down
Loading