From 35b2551b6bd37dd2912babb8300aa98d9cedff82 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Mon, 30 Oct 2023 15:45:16 -0400 Subject: [PATCH 1/3] Add back legacy behaviour of merging adjacent text nodes with the option to override the legacy behaviour --- .../com/hubspot/jinjava/LegacyOverrides.java | 15 +++++++++++++- .../com/hubspot/jinjava/tree/TreeParser.java | 11 +++++++++- .../com/hubspot/jinjava/tree/parse/Token.java | 5 +++++ .../hubspot/jinjava/tree/TreeParserTest.java | 20 +++++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java index 5ff01314f..c4fce4429 100644 --- a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java +++ b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java @@ -13,6 +13,7 @@ public class LegacyOverrides { private final boolean whitespaceRequiredWithinTokens; private final boolean useNaturalOperatorPrecedence; private final boolean parseWhitespaceControlStrictly; + private final boolean allowAdjacentTextNodes; private LegacyOverrides(Builder builder) { evaluateMapKeys = builder.evaluateMapKeys; @@ -22,6 +23,7 @@ private LegacyOverrides(Builder builder) { whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens; useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence; parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly; + allowAdjacentTextNodes = builder.allowAdjacentTextNodes; } public static Builder newBuilder() { @@ -56,6 +58,10 @@ public boolean isParseWhitespaceControlStrictly() { return parseWhitespaceControlStrictly; } + public boolean isAllowAdjacentTextNodes() { + return allowAdjacentTextNodes; + } + public static class Builder { private boolean evaluateMapKeys = false; private boolean iterateOverMapKeys = false; @@ -64,6 +70,7 @@ public static class Builder { private boolean whitespaceRequiredWithinTokens = false; private boolean useNaturalOperatorPrecedence = false; private boolean parseWhitespaceControlStrictly = false; + private boolean allowAdjacentTextNodes = false; private Builder() {} @@ -83,7 +90,8 @@ public static Builder from(LegacyOverrides legacyOverrides) { .withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence) .withParseWhitespaceControlStrictly( legacyOverrides.parseWhitespaceControlStrictly - ); + ) + .withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes); } public Builder withEvaluateMapKeys(boolean evaluateMapKeys) { @@ -126,5 +134,10 @@ public Builder withParseWhitespaceControlStrictly( this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly; return this; } + + public Builder withAllowAdjacentTextNodes(boolean allowAdjacentTextNodes) { + this.allowAdjacentTextNodes = allowAdjacentTextNodes; + return this; + } } } diff --git a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java index bc6f62557..ebcd5a0d7 100644 --- a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java +++ b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java @@ -62,7 +62,16 @@ public Node buildTree() { Node node = nextNode(); if (node != null) { - parent.getChildren().add(node); + if ( + node instanceof TextNode && + getLastSibling() instanceof TextNode && + !interpreter.getConfig().getLegacyOverrides().isAllowAdjacentTextNodes() + ) { + // merge adjacent text nodes so whitespace control properly applies + getLastSibling().getMaster().mergeImageAndContent(node.getMaster()); + } else { + parent.getChildren().add(node); + } } } diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/Token.java b/src/main/java/com/hubspot/jinjava/tree/parse/Token.java index 487987626..dda5bd63b 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/Token.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/Token.java @@ -53,6 +53,11 @@ public String getImage() { return image; } + public void mergeImageAndContent(Token otherToken) { + this.image = image + otherToken.image; + this.content = content + otherToken.content; + } + public int getLineNumber() { return lineNumber; } diff --git a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index b31e82cc3..a30031d93 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -6,6 +6,7 @@ import com.hubspot.jinjava.BaseInterpretingTest; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; +import com.hubspot.jinjava.LegacyOverrides; import com.hubspot.jinjava.interpret.TemplateError.ErrorType; import java.nio.charset.StandardCharsets; import org.junit.Test; @@ -238,6 +239,25 @@ public void itTrimsExpressions() { assertThat(interpreter.render(tree)).isEqualTo("ABC"); } + @Test + public void itDoesNotMergeAdjacentTextNodesWhenLegacyOverrideIsApplied() { + String expression = "A\n{%- if true -%}\n{# comment #}\nB{% endif %}"; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(tree)).isEqualTo("AB"); + interpreter = + new Jinjava( + JinjavaConfig + .newBuilder() + .withLegacyOverrides( + LegacyOverrides.newBuilder().withAllowAdjacentTextNodes(true).build() + ) + .build() + ) + .newInterpreter(); + final Node overriddenTree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(overriddenTree)).isEqualTo("A\nB"); + } + Node parse(String fixture) { try { return new TreeParser( From 3874722532fcf53873dc1bb573ad85861b2f240c Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Mon, 30 Oct 2023 15:50:07 -0400 Subject: [PATCH 2/3] Add option for LegacyOverrides.ALL --- .../java/com/hubspot/jinjava/LegacyOverrides.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java index c4fce4429..5ac395397 100644 --- a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java +++ b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java @@ -3,9 +3,20 @@ /** * This class allows Jinjava to be configured to override legacy behaviour. * LegacyOverrides.NONE signifies that none of the legacy functionality will be overridden. + * LegacyOverrides.ALL signifies that all new functionality will be used; avoid legacy "bugs". */ public class LegacyOverrides { public static final LegacyOverrides NONE = new LegacyOverrides.Builder().build(); + public static final LegacyOverrides ALL = new LegacyOverrides.Builder() + .withEvaluateMapKeys(true) + .withIterateOverMapKeys(true) + .withUsePyishObjectMapper(true) + .withUseSnakeCasePropertyNaming(true) + .withWhitespaceRequiredWithinTokens(true) + .withUseNaturalOperatorPrecedence(true) + .withParseWhitespaceControlStrictly(true) + .withAllowAdjacentTextNodes(true) + .build(); private final boolean evaluateMapKeys; private final boolean iterateOverMapKeys; private final boolean usePyishObjectMapper; From a66b0ab9b387d7802778d1a0501add688368353a Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Mon, 30 Oct 2023 16:40:24 -0400 Subject: [PATCH 3/3] Make legacy merging compatible with note trimming --- src/main/java/com/hubspot/jinjava/tree/TreeParser.java | 4 +++- .../java/com/hubspot/jinjava/tree/parse/TextToken.java | 7 +++++++ src/main/java/com/hubspot/jinjava/tree/parse/Token.java | 5 ----- src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java | 7 +++++++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java index ebcd5a0d7..409b3db1f 100644 --- a/src/main/java/com/hubspot/jinjava/tree/TreeParser.java +++ b/src/main/java/com/hubspot/jinjava/tree/TreeParser.java @@ -68,7 +68,9 @@ public Node buildTree() { !interpreter.getConfig().getLegacyOverrides().isAllowAdjacentTextNodes() ) { // merge adjacent text nodes so whitespace control properly applies - getLastSibling().getMaster().mergeImageAndContent(node.getMaster()); + ((TextToken) getLastSibling().getMaster()).mergeImageAndContent( + (TextToken) node.getMaster() + ); } else { parent.getChildren().add(node); } diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java b/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java index dc3d64b42..edfbb4d3e 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java @@ -29,6 +29,13 @@ public TextToken( super(image, lineNumber, startPosition, symbols); } + public void mergeImageAndContent(TextToken otherToken) { + String thisOutput = output(); + String otherTokenOutput = otherToken.output(); + this.image = thisOutput + otherTokenOutput; + this.content = image; + } + @Override public int getType() { return getSymbols().getFixed(); diff --git a/src/main/java/com/hubspot/jinjava/tree/parse/Token.java b/src/main/java/com/hubspot/jinjava/tree/parse/Token.java index dda5bd63b..487987626 100644 --- a/src/main/java/com/hubspot/jinjava/tree/parse/Token.java +++ b/src/main/java/com/hubspot/jinjava/tree/parse/Token.java @@ -53,11 +53,6 @@ public String getImage() { return image; } - public void mergeImageAndContent(Token otherToken) { - this.image = image + otherToken.image; - this.content = content + otherToken.content; - } - public int getLineNumber() { return lineNumber; } diff --git a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java index a30031d93..7a665abc8 100644 --- a/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/TreeParserTest.java @@ -232,6 +232,13 @@ public void itTrimsNotes() { assertThat(interpreter.render(tree)).isEqualTo("AB"); } + @Test + public void itMergesTextNodesWhileRespectingTrim() { + String expression = "{% print 'A' -%}\n{#- note -#}\nB\n{%- print 'C' %}"; + final Node tree = new TreeParser(interpreter, expression).buildTree(); + assertThat(interpreter.render(tree)).isEqualTo("ABC"); + } + @Test public void itTrimsExpressions() { String expression = "A\n{{- 'B' -}}\nC";