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

lib.types: init attrsWith #344216

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
29 changes: 18 additions & 11 deletions lib/options.nix
Original file line number Diff line number Diff line change
Expand Up @@ -427,21 +427,28 @@ rec {
Placeholders will not be quoted as they are not actual values:
(showOption ["foo" "*" "bar"]) == "foo.*.bar"
(showOption ["foo" "<name>" "bar"]) == "foo.<name>.bar"
(showOption ["foo" "<myPlaceholder>" "bar"]) == "foo.<myPlaceholder>.bar"
*/
showOption = parts: let
# If the part is a named placeholder of the form "<...>" don't escape it.
# Required for compatibility with: namedAttrsOf
# Can lead to misleading escaping if somebody uses literally "<...>" in their option names.
# This is the trade-off to allow for named placeholders in option names.
Copy link
Member

Choose a reason for hiding this comment

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

Right, we actually have two types of option paths: abstract ones containing placeholders, and real ones that are just data.
Treating them the same isn't quite correct, and merging a workaround could complicate a real fix.
Also note that some of these path items are not module options but attrsOf attributes, and "*" will occur as an attribute name in certain configurations to represent a "pattern" that matches everything; for example when an HTTP server doesn't responds for an unknown virtual host, etc.

We could solve this in at least two ways

a. Leave the "option path" type as is, but use two functions

  • leave showOption as is, suitable for concrete option paths
  • add showAbstractOption, which implements these new rules
    b. Only improve the representation
  • represent abstract path items with a value like { _type = "optionPathPlaceholder"; metaVariable = "name"; __toString = this: "<${this.metaVariable}>"; }

I think (b) may have good backward compatibility and it solves the problem for module options; not just attrsOf keys.
__toString is mostly for compatibility with existing code that kind of works when a placeholder string is passed. This happens when generating option docs.

Copy link
Member

Choose a reason for hiding this comment

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

This is pre-existing tech debt, so while this is a good opportunity to fix it before making it slightly worse, perhaps this should be done in a follow-up PR instead.

Copy link
Contributor Author

@hsjobeki hsjobeki Sep 28, 2024

Choose a reason for hiding this comment

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

I'd prefer to do this in a follow up PR if you are okay with it. Should i remove the changes from this one?

isNamedPlaceholder = builtins.match "\<(.*)\>";
# "<function body>" # functionTo
# "<name>" # attrsOf submoule
# "<customName>" # attrsWith { name = "customName"; elemType = submoule; }
# We assume that these are "special values" and not real configuration data.
# If it is real configuration data, it is rendered incorrectly.
specialIdentifiers = [
"*" # listOf (submodule {})
];
escapeOptionPart = part:
let
# We assume that these are "special values" and not real configuration data.
# If it is real configuration data, it is rendered incorrectly.
specialIdentifiers = [
"<name>" # attrsOf (submodule {})
"*" # listOf (submodule {})
"<function body>" # functionTo
];
in if builtins.elem part specialIdentifiers
then part
else lib.strings.escapeNixIdentifier part;
if builtins.elem part specialIdentifiers || isNamedPlaceholder part != null
then part
else lib.strings.escapeNixIdentifier part;
in (concatStringsSep ".") (map escapeOptionPart parts);

showFiles = files: concatStringsSep " and " (map (f: "`${f}'") files);

showDefs = defs: concatMapStrings (def:
Expand Down
38 changes: 38 additions & 0 deletions lib/tests/misc.nix
Original file line number Diff line number Diff line change
Expand Up @@ -1851,6 +1851,44 @@ runTests {
expected = [ [ "_module" "args" ] [ "foo" ] [ "foo" "<name>" "bar" ] [ "foo" "bar" ] ];
};

testAttrsWithName = {
expr = let
eval = evalModules {
modules = [
{
options = {
foo = lib.mkOption {
type = lib.types.attrsWith {
name = "MyCustomPlaceholder";
elemType = lib.types.submodule {
options.bar = lib.mkOption {
type = lib.types.int;
default = 42;
};
};
};
};
};
}
];
};
opt = eval.options.foo;
in
(opt.type.getSubOptions opt.loc).bar.loc;
expected = [
"foo"
"<MyCustomPlaceholder>"
"bar"
];
};

testShowOptionWithPlaceholder = {
# <name>, *, should now be escaped. It is used as a placeholder by convention.
# Other symbols should be escaped. `{}`
expr = lib.showOption ["<name>" "<myName>" "*" "{foo}"];
expected = "<name>.<myName>.*.\"{foo}\"";
};

hsjobeki marked this conversation as resolved.
Show resolved Hide resolved
testCartesianProductOfEmptySet = {
Copy link
Member

Choose a reason for hiding this comment

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

thought:
Most of the tests are in lib/tests/modules{,.sh}.
We should put all module system tests in one place. (but not in this PR)

expr = cartesianProduct {};
expected = [ {} ];
Expand Down
101 changes: 69 additions & 32 deletions lib/types.nix
Original file line number Diff line number Diff line change
Expand Up @@ -568,48 +568,85 @@ rec {
substSubModules = m: nonEmptyListOf (elemType.substSubModules m);
};

attrsOf = elemType: mkOptionType rec {
name = "attrsOf";
description = "attribute set of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}";
descriptionClass = "composite";
check = isAttrs;
merge = loc: defs:
mapAttrs (n: v: v.value) (filterAttrs (n: v: v ? value) (zipAttrsWith (name: defs:
(mergeDefinitions (loc ++ [name]) elemType defs).optionalValue
)
# Push down position info.
(map (def: mapAttrs (n: v: { inherit (def) file; value = v; }) def.value) defs)));
emptyValue = { value = {}; };
getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<name>"]);
getSubModules = elemType.getSubModules;
substSubModules = m: attrsOf (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
nestedTypes.elemType = elemType;
};
attrsOf = elemType: attrsWith { inherit elemType; };

# A version of attrsOf that's lazy in its values at the expense of
# conditional definitions not working properly. E.g. defining a value with
# `foo.attr = mkIf false 10`, then `foo ? attr == true`, whereas with
# attrsOf it would correctly be `false`. Accessing `foo.attr` would throw an
# error that it's not defined. Use only if conditional definitions don't make sense.
lazyAttrsOf = elemType: mkOptionType rec {
name = "lazyAttrsOf";
description = "lazy attribute set of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}";
lazyAttrsOf = elemType: attrsWith { inherit elemType; lazy = true; };

# base type for lazyAttrsOf and attrsOf
attrsWith = {
elemType,
name ? "name",
lazy ? false,
}:
let
typeName = if lazy then "lazyAttrsOf" else "attrsOf";
# Push down position info.
pushPositions = map (def: mapAttrs (n: v: { inherit (def) file; value = v; }) def.value);
in
mkOptionType {
name = typeName;
description = (if lazy then "lazy attribute set" else "attribute set") + " of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}";
descriptionClass = "composite";
check = isAttrs;
merge = loc: defs:
zipAttrsWith (name: defs:
let merged = mergeDefinitions (loc ++ [name]) elemType defs;
# mergedValue will trigger an appropriate error when accessed
in merged.optionalValue.value or elemType.emptyValue.value or merged.mergedValue
)
# Push down position info.
(map (def: mapAttrs (n: v: { inherit (def) file; value = v; }) def.value) defs);
merge = if lazy then (
# Lazy merge Function
loc: defs:
zipAttrsWith (name: defs:
let merged = mergeDefinitions (loc ++ [name]) elemType defs;
# mergedValue will trigger an appropriate error when accessed
in merged.optionalValue.value or elemType.emptyValue.value or merged.mergedValue
)
# Push down position info.
(pushPositions defs)
) else (
# Non-lazy merge Function
loc: defs:
mapAttrs (n: v: v.value) (filterAttrs (n: v: v ? value) (zipAttrsWith (name: defs:
(mergeDefinitions (loc ++ [name]) elemType (defs)).optionalValue
)
# Push down position info.
(pushPositions defs)))
);
emptyValue = { value = {}; };
getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<name>"]);
getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<${name}>"]);
getSubModules = elemType.getSubModules;
substSubModules = m: lazyAttrsOf (elemType.substSubModules m);
functor = (defaultFunctor name) // { wrapped = elemType; };
substSubModules = m: attrsWith { elemType = elemType.substSubModules m; inherit name lazy; };
functor = defaultFunctor "attrsWith" // {
wrapped = elemType;
type = t: attrsWith { elemType = t; inherit name lazy; };
payload = {
inherit elemType name lazy;
};
binOp = lhs: rhs:
let
elemType = lhs.elemType.typeMerge rhs.elemType.functor;
name =
if lhs.name == rhs.name then
lhs.name
else if lhs.name == "name" then
rhs.name
else if rhs.name == "name" then
lhs.name
else
null;
lazy =
if lhs.lazy == rhs.lazy then
lhs.lazy
else
null;
in
if elemType == null || name == null || lazy == null then
null
else
{
inherit elemType name lazy;
};
};
nestedTypes.elemType = elemType;
};

Expand Down
19 changes: 19 additions & 0 deletions nixos/doc/manual/development/option-types.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,25 @@ Composed types are types that take a type as parameter. `listOf
returned instead for the same `mkIf false` definition.
:::

`types.attrsWith` *`attrs`*

: An attribute set of where all the values are of *`attrs.elemType`* type.
hsjobeki marked this conversation as resolved.
Show resolved Hide resolved

`attrs.elemType` (`type`, required )
: The expected type of all attribute values.

`attrs.lazy` (`Bool`, default: `false` )
: If set to `true` attributes will be evaluated lazily. See also: `types.lazyAttrsOf`

`attrs.name` (`String`, default: `name` )
: Placeholder string in documentation for the attribute names.
The default value `name` results in the placeholder `<name>`

::: {.note}
This is the underlying implementation of `types.attrsOf` and `types.lazyAttrsOf`
:::


`types.uniq` *`t`*

: Ensures that type *`t`* cannot be merged. It is used to ensure option
Expand Down