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

Recursive type deprecation #99132

Merged
merged 5 commits into from
May 5, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
17 changes: 13 additions & 4 deletions lib/modules.nix
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,20 @@ rec {
# yield a value computed from the definitions
value = if opt ? apply then opt.apply res.mergedValue else res.mergedValue;

warnDeprecation =
warnIf (opt.type.deprecationMessage != null)
"The type `types.${opt.type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${opt.type.deprecationMessage}";
# Issue deprecation warnings recursively over all nested types of the
# given type. But don't recurse if a type with the same name was already
# visited before in order to prevent infinite recursion. So this only
# warns once per type name.
# Returns the new set of visited type names
recursiveWarn = visited: type:
let
maybeWarn = warnIf (type.deprecationMessage != null)
"The type `types.${type.name}' of option `${showOption loc}' defined in ${showFiles opt.declarations} is deprecated. ${type.deprecationMessage}";
in
if visited ? ${type.name} then visited
infinisil marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being strict in type.name turns out to be a significant problem, leading to incomprehensible errors when users define their own recursive types.
While this is technically a limitation of name, it manifests because of this use of it.

else lib.foldl' recursiveWarn (maybeWarn visited // { ${type.name} = null; }) (lib.attrValues type.nestedTypes);

in warnDeprecation opt //
in builtins.seq (recursiveWarn {} opt.type) opt //
{ value = builtins.addErrorContext "while evaluating the option `${showOption loc}':" value;
inherit (res.defsFinal') highestPrio;
definitions = map (def: def.value) res.defsFinal;
Expand Down
9 changes: 9 additions & 0 deletions lib/tests/modules.sh
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ checkConfigOutput "true" config.submodule.config ./declare-submoduleWith-noshort
## submoduleWith should merge all modules in one swoop
checkConfigOutput "true" config.submodule.inner ./declare-submoduleWith-modules.nix
checkConfigOutput "true" config.submodule.outer ./declare-submoduleWith-modules.nix
# Should also be able to evaluate the type name (which evaluates freeformType,
# which evaluates all the modules defined by the type)
checkConfigOutput "submodule" options.submodule.type.description ./declare-submoduleWith-modules.nix

## Paths should be allowed as values and work as expected
checkConfigOutput "true" config.submodule.enable ./declare-submoduleWith-path.nix
Expand Down Expand Up @@ -269,6 +272,12 @@ checkConfigError 'A definition for option .fun.\[function body\]. is not of type
checkConfigOutput "b a" config.result ./functionTo/list-order.nix
checkConfigOutput "a c" config.result ./functionTo/merging-attrs.nix

## Type deprecation
checkConfigError 'The type `types.simple'\'' of option `simple'\'' defined in .* is deprecated. simple shall not be used' config.simple ./type-deprecation.nix
checkConfigError 'The type `types.infinite'\'' of option `infinite'\'' defined in .* is deprecated. infinite shall not be used' config.infinite ./type-deprecation.nix
checkConfigError 'The type `types.left'\'' of option `nested'\'' defined in .* is deprecated. left shall not be used' config.nested ./type-deprecation.nix
checkConfigError 'The type `types.right'\'' of option `nested'\'' defined in .* is deprecated. right shall not be used' config.nested ./type-deprecation.nix

cat <<EOF
====== module tests ======
$pass Pass
Expand Down
4 changes: 1 addition & 3 deletions lib/tests/modules/declare-submoduleWith-modules.nix
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
default = false;
};
}
{
outer = true;
}
];
};
default = {};
Expand All @@ -25,6 +22,7 @@
})
{
inner = true;
outer = true;
}
];
}
39 changes: 39 additions & 0 deletions lib/tests/modules/type-deprecation.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{ lib, ... }: {

options.simple = lib.mkOption {
type = lib.mkOptionType {
name = "simple";
deprecationMessage = "simple shall not be used";
};
default = throw "";
};

options.infinite = lib.mkOption {
type =
let
t = lib.mkOptionType {
name = "infinite";
deprecationMessage = "infinite shall not be used";
};
r = lib.types.either t (lib.types.attrsOf r);
in r;
default = throw "";
};

options.nested = lib.mkOption {
type =
let
left = lib.mkOptionType {
name = "left";
deprecationMessage = "left shall not be used";
};
right = lib.mkOptionType {
name = "right";
deprecationMessage = "right shall not be used";
};
in lib.types.either left right;

default = throw "";
};

}
19 changes: 18 additions & 1 deletion lib/types.nix
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,13 @@ rec {
, # The deprecation message to display when this type is used by an option
# If null, the type isn't deprecated
deprecationMessage ? null
, # The types that occur in the definition of this type. This is used to
# issue deprecation warnings recursively. Can also be used to reuse
# nested types
nestedTypes ? {}
}:
{ _type = "option-type";
inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor deprecationMessage;
inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor deprecationMessage nestedTypes;
description = if description == null then name else description;
};

Expand Down Expand Up @@ -365,6 +369,7 @@ rec {
getSubModules = elemType.getSubModules;
substSubModules = m: listOf (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
nestedTypes.elemType = elemType;
};

nonEmptyListOf = elemType:
Expand All @@ -389,6 +394,7 @@ rec {
getSubModules = elemType.getSubModules;
substSubModules = m: attrsOf (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
nestedTypes.elemType = elemType;
};

# A version of attrsOf that's lazy in its values at the expense of
Expand All @@ -413,6 +419,7 @@ rec {
getSubModules = elemType.getSubModules;
substSubModules = m: lazyAttrsOf (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
nestedTypes.elemType = elemType;
};

# TODO: drop this in the future:
Expand All @@ -421,6 +428,7 @@ rec {
deprecationMessage = "Mixing lists with attribute values is no longer"
+ " possible; please use `types.attrsOf` instead. See"
+ " https://github.com/NixOS/nixpkgs/issues/1800 for the motivation.";
nestedTypes.elemType = elemType;
};

# Value of given type but with no merging (i.e. `uniq list`s are not concatenated).
Expand All @@ -433,6 +441,7 @@ rec {
getSubModules = elemType.getSubModules;
substSubModules = m: uniq (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
nestedTypes.elemType = elemType;
};

# Null or value of ...
Expand All @@ -451,6 +460,7 @@ rec {
getSubModules = elemType.getSubModules;
substSubModules = m: nullOr (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
nestedTypes.elemType = elemType;
};

functionTo = elemType: mkOptionType {
Expand Down Expand Up @@ -535,6 +545,9 @@ rec {
substSubModules = m: submoduleWith (attrs // {
modules = m;
});
nestedTypes = lib.optionalAttrs (freeformType != null) {
freeformType = freeformType;
};
functor = defaultFunctor name // {
type = types.submoduleWith;
payload = {
Expand Down Expand Up @@ -596,6 +609,8 @@ rec {
then functor.type mt1 mt2
else null;
functor = (defaultFunctor name) // { wrapped = [ t1 t2 ]; };
nestedTypes.left = t1;
nestedTypes.right = t2;
};

# Any of the types in the given list
Expand Down Expand Up @@ -627,6 +642,8 @@ rec {
substSubModules = m: coercedTo coercedType coerceFunc (finalType.substSubModules m);
typeMerge = t1: t2: null;
functor = (defaultFunctor name) // { wrapped = finalType; };
nestedTypes.coercedType = coercedType;
nestedTypes.finalType = finalType;
};

# Obsolete alternative to configOf. It takes its option
Expand Down