nix::parser::State::addAttr: Forbid dynamic attrs in recursive attrsets

Co-authored-by: Commentator2.0 <lix@crystal-cavern.systems> Change-Id: I92656b3b27f551bf286abc0d680c4a1c542337d7
2026-01-31 15:32:27 +01:00
parent 51dcc6ac0d
commit af166146ff
14 changed files with 101 additions and 4 deletions
@@ -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.
@@ -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<Expr> 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<Expr> 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<ExprSet *>(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<ExprSet>(), pos);
            attrs = static_cast<ExprSet *>(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<ExprSet *>(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())
@@ -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.
@@ -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',
@@ -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
@@ -0,0 +1,5 @@
+rec {
+    yep = "bar";
+    foo = yep;
+    ${foo} = "awa";
+}
@@ -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| }
@@ -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
@@ -0,0 +1,8 @@
+[[test]]
+runner = "parse-fail"
+matrix = true
+
+[[test]]
+runner = "parse-okay"
+flags = ["--extra-deprecated-features", "rec-set-dynamic-attrs"]
+matrix = true
@@ -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"]
@@ -0,0 +1,3 @@
+[[test]]
+runner = "parse-okay"
+flags = [ "--extra-deprecated-features", "rec-set-dynamic-attrs" ]
@@ -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"
@@ -14,6 +14,7 @@ let
    "broken-string-indentation"
    "broken-string-escape"
    "rec-set-merges"
+    "rec-set-dynamic-attrs"
  ];
 in

@@ -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}";