From af166146ff881734bbefbb44284cbc7317728a69 Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 24 Nov 2025 19:19:43 +0100 Subject: [PATCH] nix::parser::State::addAttr: Forbid dynamic attrs in recursive attrsets Co-authored-by: Commentator2.0 Change-Id: I92656b3b27f551bf286abc0d680c4a1c542337d7 --- doc/manual/rl-next/deprecated-features.md | 3 +- lix/libexpr/parser/state.hh | 30 +++++++++++++++++++ .../rec-set-dynamic-attrs.md | 12 ++++++++ lix/libutil/meson.build | 1 + .../lang/attrs-__overrides/test.toml | 3 +- .../functional2/lang/attrs-dynamic-rec/in.nix | 5 ++++ .../lang/attrs-dynamic-rec/parse-fail.err.exp | 6 ++++ .../lang/attrs-dynamic-rec/parse-okay.out.exp | 18 +++++++++++ .../lang/attrs-dynamic-rec/test.toml | 8 +++++ .../functional2/lang/attrs-dynamic/test.toml | 10 +++++++ .../lang/parser-output-json/test.toml | 3 ++ .../lang/rec-set-overrides/test.toml | 4 +-- tests/nixpkgs/eval.nix | 1 + tests/nixpkgs/lib.nix | 1 + 14 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 lix/libutil/deprecated-features/rec-set-dynamic-attrs.md create mode 100644 tests/functional2/lang/attrs-dynamic-rec/in.nix create mode 100644 tests/functional2/lang/attrs-dynamic-rec/parse-fail.err.exp create mode 100644 tests/functional2/lang/attrs-dynamic-rec/parse-okay.out.exp create mode 100644 tests/functional2/lang/attrs-dynamic-rec/test.toml create mode 100644 tests/functional2/lang/attrs-dynamic/test.toml create mode 100644 tests/functional2/lang/parser-output-json/test.toml diff --git a/doc/manual/rl-next/deprecated-features.md b/doc/manual/rl-next/deprecated-features.md index 1273d01a6..bd3183457 100644 --- a/doc/manual/rl-next/deprecated-features.md +++ b/doc/manual/rl-next/deprecated-features.md @@ -1,7 +1,7 @@ --- synopsis: 'more deprecated features' issues: [] -cls: [2092, 2310, 2311, 4638] +cls: [2092, 2310, 2311, 4638, 4652] category: Breaking Changes credits: [piegames, commentator2.0] --- @@ -12,3 +12,4 @@ You can opt in into the old behavior with `--extra-deprecated-features` or any e - `broken-string-escape` "escaped" characters without a properly defined escape sequence evaluate to "themselves". This is in most cases unintended behaviour, both for writing regexes, and using legacy or uncommon escape sequences like `\f`. The user will now be warned, if those are present. - `floating-without-zero` so far, one was able to declare a float using something like `.123`. This can cause confusion about accessing attributes. Floating point numbers must now always include the leading zero, i.e. `0.123` - `rec-set-merges` Attribute sets like `{ foo = {}; foo.bar = 42;}` implicitly merge at parse time, however if one of them is marked as recursive but not the others then the recursive attribute may get lost (order-dependent). Therefore, merging attrs with mixed-`rec` is now forbidden. +- `rec-set-dynamic-attrs` Dynamic attributes have weird semantics in the presence of recursive attrsets (they evaluate *after* the rest of the set). This is now forbidden. diff --git a/lix/libexpr/parser/state.hh b/lix/libexpr/parser/state.hh index fd8056a80..c0645187c 100644 --- a/lix/libexpr/parser/state.hh +++ b/lix/libexpr/parser/state.hh @@ -41,6 +41,7 @@ struct State void badEscapeFound(const PosIdx pos, char found, std::string escape); void nulFound(const PosIdx pos); void recSetMergeFound(const AttrPath & attrPath, const PosIdx pos); + void recSetDynamicAttrFound(const PosIdx pos); void addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr e, const PosIdx pos); void mergeAttrs(AttrPath & attrPath, ExprSet * source, ExprSet * target); void validateLambdaAttrs(AttrsPattern & pattern, PosIdx pos = noPos); @@ -183,6 +184,19 @@ inline void State::recSetMergeFound(const AttrPath & attrPath, const PosIdx pos) .pos = positions[pos], }); } +// Added 2025-11-24 +inline void State::recSetDynamicAttrFound(const PosIdx pos) +{ + throw ParseError({ + .msg = HintFmt( + "dynamic attributes are not allowed within recursive attrsets, because they would be " + "evaluated separately from the other recursive attributes. Use %s to disable this " + "error.", + "--extra-deprecated-features rec-set-dynamic-attrs" + ), + .pos = positions[pos], + }); +} inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr e, const PosIdx pos) { @@ -196,6 +210,14 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ AttrName & attr = *i; if (attr.isDynamic()) { + /* We don't want to insert dynamic attributes into recursive sets, because that has + * fucky semantics */ + if (ExprSet * set = dynamic_cast(attrs); + !featureSettings.isEnabled(Dep::RecSetDynamicAttrs) && set && set->recursive) + { + recSetDynamicAttrFound(pos); + } + // Simply insert an empty attrset (but dynamic) auto & next = attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::make_unique(), pos); attrs = static_cast(next.valueExpr.get()); @@ -237,6 +259,14 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ AttrName & attr = *i; if (attr.isDynamic()) { + /* We don't want to insert dynamic attributes into recursive sets, because that has + * fucky semantics */ + if (ExprSet * set = dynamic_cast(attrs); + !featureSettings.isEnabled(Dep::RecSetDynamicAttrs) && set && set->recursive) + { + recSetDynamicAttrFound(pos); + } + attrs->dynamicAttrs.emplace_back(std::move(attr.expr), std::move(e), pos); } else if (ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(attr.symbol); j != attrs->attrs.end()) diff --git a/lix/libutil/deprecated-features/rec-set-dynamic-attrs.md b/lix/libutil/deprecated-features/rec-set-dynamic-attrs.md new file mode 100644 index 000000000..f1855327d --- /dev/null +++ b/lix/libutil/deprecated-features/rec-set-dynamic-attrs.md @@ -0,0 +1,12 @@ +--- +name: rec-set-dynamic-attrs +internalName: RecSetDynamicAttrs +timeline: + - date: 2026-01-30 + release: 2.95.0 + cls: [4652] + message: Introduced as a parser error. +--- +Dynamic attrs (attrs with interpolation in the key) are deprecated in `rec` attrsets. +This is because the recursive dynamics fundamentally do not work with dynamic attributes, +which is why dynamic attributes are currently evaluated *after* the recursive attributes, and *without* the recursive semantics. diff --git a/lix/libutil/meson.build b/lix/libutil/meson.build index 1550be895..a12cdb52e 100644 --- a/lix/libutil/meson.build +++ b/lix/libutil/meson.build @@ -181,6 +181,7 @@ deprecated_feature_definitions = files( 'deprecated-features/floating-without-zero.md', 'deprecated-features/nix-path-shadow.md', 'deprecated-features/nul-bytes.md', + 'deprecated-features/rec-set-dynamic-attrs.md', 'deprecated-features/rec-set-merges.md', 'deprecated-features/rec-set-overrides.md', 'deprecated-features/shadow-internal-symbols.md', diff --git a/tests/functional2/lang/attrs-__overrides/test.toml b/tests/functional2/lang/attrs-__overrides/test.toml index 9e43e02b0..c1aaddc08 100644 --- a/tests/functional2/lang/attrs-__overrides/test.toml +++ b/tests/functional2/lang/attrs-__overrides/test.toml @@ -1,9 +1,10 @@ [[test]] name = "warning" runner = "eval-okay" +flags = ["--extra-deprecated-features", "rec-set-dynamic-attrs"] matrix = true [[test]] runner = "eval-okay" -flags = [ "--extra-deprecated-features", "rec-set-overrides" ] +flags = [ "--extra-deprecated-features", "rec-set-overrides rec-set-dynamic-attrs" ] matrix = true diff --git a/tests/functional2/lang/attrs-dynamic-rec/in.nix b/tests/functional2/lang/attrs-dynamic-rec/in.nix new file mode 100644 index 000000000..62e1c34e0 --- /dev/null +++ b/tests/functional2/lang/attrs-dynamic-rec/in.nix @@ -0,0 +1,5 @@ +rec { + yep = "bar"; + foo = yep; + ${foo} = "awa"; +} diff --git a/tests/functional2/lang/attrs-dynamic-rec/parse-fail.err.exp b/tests/functional2/lang/attrs-dynamic-rec/parse-fail.err.exp new file mode 100644 index 000000000..e4fc5b43f --- /dev/null +++ b/tests/functional2/lang/attrs-dynamic-rec/parse-fail.err.exp @@ -0,0 +1,6 @@ +error: dynamic attributes are not allowed within recursive attrsets, because they would be evaluated separately from the other recursive attributes. Use --extra-deprecated-features rec-set-dynamic-attrs to disable this error. + at /pwd/in.nix:4:5: + 3| foo = yep; + 4| ${foo} = "awa"; + | ^ + 5| } diff --git a/tests/functional2/lang/attrs-dynamic-rec/parse-okay.out.exp b/tests/functional2/lang/attrs-dynamic-rec/parse-okay.out.exp new file mode 100644 index 000000000..6d4bfc59a --- /dev/null +++ b/tests/functional2/lang/attrs-dynamic-rec/parse-okay.out.exp @@ -0,0 +1,18 @@ +_type: ExprSet +attrs: + foo: + _type: ExprVar + value: yep + yep: + _type: ExprLiteral + value: bar + valueType: String +dynamicAttrs: + - name: + _type: ExprVar + value: foo + value: + _type: ExprLiteral + value: awa + valueType: String +recursive: true diff --git a/tests/functional2/lang/attrs-dynamic-rec/test.toml b/tests/functional2/lang/attrs-dynamic-rec/test.toml new file mode 100644 index 000000000..fc324f7f8 --- /dev/null +++ b/tests/functional2/lang/attrs-dynamic-rec/test.toml @@ -0,0 +1,8 @@ +[[test]] +runner = "parse-fail" +matrix = true + +[[test]] +runner = "parse-okay" +flags = ["--extra-deprecated-features", "rec-set-dynamic-attrs"] +matrix = true diff --git a/tests/functional2/lang/attrs-dynamic/test.toml b/tests/functional2/lang/attrs-dynamic/test.toml new file mode 100644 index 000000000..0b28b2ec5 --- /dev/null +++ b/tests/functional2/lang/attrs-dynamic/test.toml @@ -0,0 +1,10 @@ +[[test]] +runner = "eval-okay" +flags = ["--extra-deprecated-features", "rec-set-dynamic-attrs"] +matrix = true +in = ["in.nix", "in-2.nix", "in-bare.nix", "in-merge.nix", "in-null.nix"] + +[[test]] +runner = "eval-fail" +matrix = true +in = ["in-duplicate.nix", "in-incomplete-state.nix", "in-infrec.nix"] diff --git a/tests/functional2/lang/parser-output-json/test.toml b/tests/functional2/lang/parser-output-json/test.toml new file mode 100644 index 000000000..6604fabd7 --- /dev/null +++ b/tests/functional2/lang/parser-output-json/test.toml @@ -0,0 +1,3 @@ +[[test]] +runner = "parse-okay" +flags = [ "--extra-deprecated-features", "rec-set-dynamic-attrs" ] diff --git a/tests/functional2/lang/rec-set-overrides/test.toml b/tests/functional2/lang/rec-set-overrides/test.toml index 6daa78872..274d462c4 100644 --- a/tests/functional2/lang/rec-set-overrides/test.toml +++ b/tests/functional2/lang/rec-set-overrides/test.toml @@ -1,12 +1,12 @@ [[test]] runner = "parse-okay" -flags = [ "--extra-deprecated-features", "rec-set-overrides", "--extra-deprecated-features", "rec-set-merges" ] +flags = [ "--extra-deprecated-features", "rec-set-overrides rec-set-merges rec-set-dynamic-attrs" ] # Same as above but with deprecation warning [[test]] name = "parse-okay-deprecation-warning" runner = "parse-okay" -flags = [ "--extra-deprecated-features", "rec-set-merges" ] +flags = [ "--extra-deprecated-features", "rec-set-merges rec-set-dynamic-attrs" ] [[test]] runner = "eval-fail" diff --git a/tests/nixpkgs/eval.nix b/tests/nixpkgs/eval.nix index 643754815..10cca95dc 100644 --- a/tests/nixpkgs/eval.nix +++ b/tests/nixpkgs/eval.nix @@ -14,6 +14,7 @@ let "broken-string-indentation" "broken-string-escape" "rec-set-merges" + "rec-set-dynamic-attrs" ]; in diff --git a/tests/nixpkgs/lib.nix b/tests/nixpkgs/lib.nix index 3f37f8183..46112a82a 100644 --- a/tests/nixpkgs/lib.nix +++ b/tests/nixpkgs/lib.nix @@ -23,6 +23,7 @@ let "broken-string-indentation" "broken-string-escape" "rec-set-merges" + "rec-set-dynamic-attrs" ]; env.NIX_CONFIG = "extra-deprecated-features = ${concatStringsSep " " deprecatedFeatures}";