From 4a9f941d0f24bc92d9e357215238a7c2e6c9f696 Mon Sep 17 00:00:00 2001 From: jasmith-hs Date: Fri, 10 Jul 2026 11:54:45 -0400 Subject: [PATCH] fix: reconstruct namespace root of dotted `{% set ns.attr %}` across scopes In eager execution, a dotted namespace assignment such as `{% set ns.attr = x %}` registered the root `ns` only as a *set* deferred base. Set bases are deferred only within the same scope (the `isInSameScope` macro-stack check), so when the assignment runs across a macro boundary or inside a child interpreter context, the deferral of `ns` never reached the scope that owns it. Phase 1 then read the namespace unmutated and produced output that diverged from a full render. Register the namespace root as a *used* deferred base as well (only when the root resolves to a `Namespace`), reusing the same unconditional propagation and value-reconstruction path that already makes `{% do ns.update(...) %}` correct. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tag/eager/EagerBlockSetTagStrategy.java | 1 + .../tag/eager/EagerInlineSetTagStrategy.java | 1 + .../lib/tag/eager/EagerSetTagStrategy.java | 14 +++++ .../java/com/hubspot/jinjava/EagerTest.java | 36 +++++++++++ .../lib/tag/eager/EagerSetTagTest.java | 59 +++++++++++++++++++ .../test.expected.expected.jinja | 1 + .../test.expected.jinja | 6 ++ .../test.jinja | 4 ++ .../test.expected.expected.jinja | 1 + .../test.expected.jinja | 13 ++++ .../test.jinja | 5 ++ 11 files changed, 141 insertions(+) create mode 100644 src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.expected.jinja create mode 100644 src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.jinja create mode 100644 src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.jinja create mode 100644 src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.expected.jinja create mode 100644 src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.jinja create mode 100644 src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.jinja diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerBlockSetTagStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerBlockSetTagStrategy.java index 66952147e..e812ef967 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerBlockSetTagStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerBlockSetTagStrategy.java @@ -166,6 +166,7 @@ protected Triple getPrefixTokenAndSuffix( interpreter, DeferredToken .builderFromImage(joiner.toString(), tagNode.getMaster()) + .addUsedDeferredWords(getNamespaceRootsForDottedSet(variables, interpreter)) .addSetDeferredWords(Stream.of(variables)) .build() ) diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerInlineSetTagStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerInlineSetTagStrategy.java index 2c05052e8..d6e359e3c 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerInlineSetTagStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerInlineSetTagStrategy.java @@ -97,6 +97,7 @@ public Triple getPrefixTokenAndSuffix( DeferredToken .builderFromImage(joiner.toString(), tagNode.getMaster()) .addUsedDeferredWords(eagerExecutionResult.getResult().getDeferredWords()) + .addUsedDeferredWords(getNamespaceRootsForDottedSet(variables, interpreter)) .addSetDeferredWords(Arrays.stream(variables).map(String::trim)) .build() ) diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java index 9af7f2263..1747d004f 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java @@ -6,6 +6,7 @@ import com.hubspot.jinjava.interpret.MetaContextVariables; import com.hubspot.jinjava.lib.tag.SetTag; import com.hubspot.jinjava.loader.RelativePathResolver; +import com.hubspot.jinjava.objects.Namespace; import com.hubspot.jinjava.tree.TagNode; import com.hubspot.jinjava.util.EagerReconstructionUtils; import com.hubspot.jinjava.util.PrefixToPreserveState; @@ -26,6 +27,19 @@ protected EagerSetTagStrategy(SetTag setTag) { this.setTag = setTag; } + protected static Stream getNamespaceRootsForDottedSet( + String[] variables, + JinjavaInterpreter interpreter + ) { + return Arrays + .stream(variables) + .map(String::trim) + .filter(variable -> variable.contains(".")) + .map(variable -> variable.substring(0, variable.indexOf('.'))) + .distinct() + .filter(root -> interpreter.getContext().get(root) instanceof Namespace); + } + public String run(TagNode tagNode, JinjavaInterpreter interpreter) { int eqPos = tagNode.getHelpers().indexOf('='); String[] variables; diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index 2cd4590b0..5ad3a4e9a 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -929,6 +929,42 @@ public void itHandlesDeferredInNamespaceSecondPass() { ); } + @Test + public void itReconstructsNamespaceForSetTagsAcrossScope() { + expectedTemplateInterpreter.assertExpectedOutput( + "reconstructs-namespace-for-set-tags-across-scope/test" + ); + } + + @Test + public void itReconstructsNamespaceForSetTagsAcrossScopeSecondPass() { + localContext.put("deferred", "resolved"); + expectedTemplateInterpreter.assertExpectedOutput( + "reconstructs-namespace-for-set-tags-across-scope/test.expected" + ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "reconstructs-namespace-for-set-tags-across-scope/test.expected" + ); + } + + @Test + public void itReconstructsNamespaceForSetTagsInForLoop() { + expectedTemplateInterpreter.assertExpectedOutput( + "reconstructs-namespace-for-set-tags-in-for-loop/test" + ); + } + + @Test + public void itReconstructsNamespaceForSetTagsInForLoopSecondPass() { + localContext.put("deferred", 2); + expectedTemplateInterpreter.assertExpectedOutput( + "reconstructs-namespace-for-set-tags-in-for-loop/test.expected" + ); + expectedTemplateInterpreter.assertExpectedNonEagerOutput( + "reconstructs-namespace-for-set-tags-in-for-loop/test.expected" + ); + } + @Test public void itHandlesClashingNameInMacro() { expectedTemplateInterpreter.assertExpectedOutput( diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java index 73f484d4d..0e6830a4c 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagTest.java @@ -9,7 +9,9 @@ import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; import com.hubspot.jinjava.lib.tag.SetTagTest; import com.hubspot.jinjava.mode.EagerExecutionMode; +import com.hubspot.jinjava.objects.Namespace; import com.hubspot.jinjava.tree.parse.TagToken; +import java.util.HashMap; import java.util.Optional; import java.util.stream.Collectors; import org.junit.After; @@ -98,6 +100,63 @@ public void itHandlesMultipleVars() { .containsExactlyInAnyOrder("deferred", "range"); } + @Test + public void itDefersNamespaceRootAsUsedWordForDottedSet() { + context.put("ns", new Namespace()); + context.setDeferredExecutionMode(true); + interpreter.render("{% set ns.enabled = deferred %}"); + Optional maybeDeferredToken = context + .getDeferredTokens() + .stream() + .filter(e -> ((TagToken) e.getToken()).getTagName().equals(tag.getName())) + .findAny(); + assertThat(maybeDeferredToken).isPresent(); + assertThat(maybeDeferredToken.get().getSetDeferredWords()) + .containsExactly("ns.enabled"); + assertThat(maybeDeferredToken.get().getUsedDeferredWords()).contains("ns"); + } + + @Test + public void itDefersNamespaceRootAsUsedWordForDottedBlockSet() { + context.put("ns", new Namespace()); + context.setDeferredExecutionMode(true); + interpreter.render("{% set ns.enabled %}{{ deferred }}{% endset %}"); + Optional maybeDeferredToken = context + .getDeferredTokens() + .stream() + .filter(e -> e.getToken() instanceof TagToken) + .filter(e -> ((TagToken) e.getToken()).getTagName().equals(tag.getName())) + .findAny(); + assertThat(maybeDeferredToken).isPresent(); + assertThat(maybeDeferredToken.get().getSetDeferredWords()) + .containsExactly("ns.enabled"); + assertThat(maybeDeferredToken.get().getUsedDeferredWords()).contains("ns"); + } + + @Test + public void itDoesNotDeferNonNamespaceRootAsUsedWordForDottedSet() { + context.put("someMap", new HashMap()); + context.setDeferredExecutionMode(true); + interpreter.render("{% set someMap.field = deferred %}"); + Optional maybeDeferredToken = context + .getDeferredTokens() + .stream() + .filter(e -> ((TagToken) e.getToken()).getTagName().equals(tag.getName())) + .findAny(); + assertThat(maybeDeferredToken).isPresent(); + assertThat(maybeDeferredToken.get().getUsedDeferredWords()).doesNotContain("someMap"); + } + + @Test + public void itDoesNotThrowWhenNamespaceHoldsUnserializableValue() { + Namespace ns = new Namespace(); + ns.put("obj", new Object()); + context.put("ns", ns); + context.setDeferredExecutionMode(true); + interpreter.render("{% set ns.enabled = deferred %}"); + assertThat(interpreter.getErrors()).isEmpty(); + } + @Test public void itLimitsLength() { StringBuilder tooLong = new StringBuilder(); diff --git a/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.expected.jinja b/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.expected.jinja new file mode 100644 index 000000000..2ab19ae60 --- /dev/null +++ b/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.expected.jinja @@ -0,0 +1 @@ +resolved diff --git a/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.jinja b/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.jinja new file mode 100644 index 000000000..cbc662993 --- /dev/null +++ b/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.expected.jinja @@ -0,0 +1,6 @@ +{% set ns = namespace({'enabled': false} ) %}\ +{% set __macro_setit_1985651457_temp_variable_0__ %}\ +{% set ns.enabled = deferred %}\ +{% endset %}\ +{{ __macro_setit_1985651457_temp_variable_0__ }} +{{ ns.enabled }} diff --git a/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.jinja b/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.jinja new file mode 100644 index 000000000..1abf8c3a2 --- /dev/null +++ b/src/test/resources/eager/reconstructs-namespace-for-set-tags-across-scope/test.jinja @@ -0,0 +1,4 @@ +{%- set ns = namespace(enabled=false) %} +{%- macro setit() %}{% set ns.enabled = deferred %}{% endmacro %} +{{- setit() -}} +{{ ns.enabled }} diff --git a/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.expected.jinja b/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.expected.jinja new file mode 100644 index 000000000..27ba77dda --- /dev/null +++ b/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.expected.jinja @@ -0,0 +1 @@ +true diff --git a/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.jinja b/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.jinja new file mode 100644 index 000000000..8395227b0 --- /dev/null +++ b/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.expected.jinja @@ -0,0 +1,13 @@ +{% set ns = namespace({'found': false} ) %}\ +{% for __ignored__ in [0] %}\ +{% if 1 == deferred %}\ +{% set ns.found = true %}\ +{% endif %}\ +{% if 2 == deferred %}\ +{% set ns.found = true %}\ +{% endif %}\ +{% if 3 == deferred %}\ +{% set ns.found = true %}\ +{% endif %}\ +{% endfor %} +{{ ns.found }} diff --git a/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.jinja b/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.jinja new file mode 100644 index 000000000..2039fc59f --- /dev/null +++ b/src/test/resources/eager/reconstructs-namespace-for-set-tags-in-for-loop/test.jinja @@ -0,0 +1,5 @@ +{%- set ns = namespace(found=false) %} +{%- for item in [1, 2, 3] %} +{%- if item == deferred %}{% set ns.found = true %}{% endif %} +{%- endfor %} +{{ ns.found }}