From 27a94369a9a76c7e41220060b85b984e3dc6da4e Mon Sep 17 00:00:00 2001 From: piegames Date: Sun, 23 Nov 2025 19:46:02 +0100 Subject: [PATCH] nix::parser::State::addAttr: Deep attribute merging Closes #845 Change-Id: Ie14d0e5a7a9fb778325c4ad30d1e1bd73c60b4f9 --- lix/libexpr/parser/state.hh | 17 ++++++++++++---- .../parser-merge-attrs/eval-okay-deep.out.exp | 1 + .../lang/parser-merge-attrs/in-deep.nix | 20 +++++++++++++++++++ .../parse-fail-err2.err.exp | 2 +- 4 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 tests/functional2/lang/parser-merge-attrs/eval-okay-deep.out.exp create mode 100644 tests/functional2/lang/parser-merge-attrs/in-deep.nix diff --git a/lix/libexpr/parser/state.hh b/lix/libexpr/parser/state.hh index cf3d807a5..d9624214e 100644 --- a/lix/libexpr/parser/state.hh +++ b/lix/libexpr/parser/state.hh @@ -214,11 +214,20 @@ inline void State::mergeAttrs(AttrPath & attrPath, ExprSet * source, ExprSet * t target->inheritFromExprs = std::make_unique>>(); } for (auto & [insertKey, insertDef] : source->attrs) { - if (auto collision = target->attrs.find(insertKey); - collision != target->attrs.end()) // Attr already defined in target, error. - { + if (auto collision = target->attrs.find(insertKey); collision != target->attrs.end()) { + // Attr already defined in target, recurse merge if possible otherwise error. + auto * collisionInsert = dynamic_cast(insertDef.e.get()); + auto * collisionTarget = dynamic_cast(collision->second.e.get()); + if (!collisionInsert || !collisionTarget) { + attrPath.push_back(AttrName(insertDef.pos, insertKey)); + return dupAttr(attrPath, insertDef.pos, collision->second.pos); + } + + // Push insertKey to the attrPath for error propagation (pop afterwards), then recurse + // merge attrPath.push_back(AttrName(insertDef.pos, insertKey)); - return dupAttr(attrPath, insertDef.pos, collision->second.pos); + mergeAttrs(attrPath, collisionInsert, collisionTarget); + attrPath.pop_back(); } if (insertDef.kind == ExprAttrs::AttrDef::Kind::InheritedFrom) { auto & sel = dynamic_cast(*insertDef.e); diff --git a/tests/functional2/lang/parser-merge-attrs/eval-okay-deep.out.exp b/tests/functional2/lang/parser-merge-attrs/eval-okay-deep.out.exp new file mode 100644 index 000000000..436854b60 --- /dev/null +++ b/tests/functional2/lang/parser-merge-attrs/eval-okay-deep.out.exp @@ -0,0 +1 @@ +{ foo = { bar = { baz = 1; qux = 2; }; }; } diff --git a/tests/functional2/lang/parser-merge-attrs/in-deep.nix b/tests/functional2/lang/parser-merge-attrs/in-deep.nix new file mode 100644 index 000000000..f78b940ad --- /dev/null +++ b/tests/functional2/lang/parser-merge-attrs/in-deep.nix @@ -0,0 +1,20 @@ +# Deep recursive attr merges +# Regression test for https://git.lix.systems/lix-project/lix/issues/845 / https://github.com/NixOS/nix/issues/11268 + +# Also this is an eval test and not a parser test because the printed value is easier to inspect than the AST, +# though we are primarily testing parser functionality here +let + reference = { a.b.c = 1; a.b.d = 2; }; +in +# Test cases courtesy of @rhendric +assert { a = { b = { c = 1; }; }; a = { b = { d = 2; }; }; } == reference; +assert { a.b = { c = 1; }; a.b = { d = 2; }; } == reference; +assert { a = { b.c = 1; }; a = { b.d = 2; }; } == reference; +assert { a = { b = { c = 1; }; }; a.b.d = 2; } == reference; +assert { a.b.c = 1; a = { b = { d = 2; }; }; } == reference; +{ + foo.bar.baz = 1; + foo = { + bar.qux = 2; + }; +} diff --git a/tests/functional2/lang/parser-merge-attrs/parse-fail-err2.err.exp b/tests/functional2/lang/parser-merge-attrs/parse-fail-err2.err.exp index cfcf40eaa..985fe140f 100644 --- a/tests/functional2/lang/parser-merge-attrs/parse-fail-err2.err.exp +++ b/tests/functional2/lang/parser-merge-attrs/parse-fail-err2.err.exp @@ -1,4 +1,4 @@ -error: attribute 'x.y' already defined at /pwd/in.nix:2:3 +error: attribute 'x.y.y' already defined at /pwd/in.nix:2:3 at /pwd/in.nix:3:9: 2| x.y.y = 3; 3| x = { y.y= 3; z = 3; };