Merge pull request #1124 from HubSpot/legacy-text-merging

Legacy text merging

Author: jasmith-hs

src/main/java/com/hubspot/jinjava/LegacyOverrides.java

@@ -3,16 +3,28 @@
 /**
  * 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;
   private final boolean useSnakeCasePropertyNaming;
   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 +34,7 @@ private LegacyOverrides(Builder builder) {
     whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
     useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
     parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
+    allowAdjacentTextNodes = builder.allowAdjacentTextNodes;
   }
 
   public static Builder newBuilder() {
@@ -56,6 +69,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 +81,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 +101,8 @@ public static Builder from(LegacyOverrides legacyOverrides) {
         .withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
         .withParseWhitespaceControlStrictly(
           legacyOverrides.parseWhitespaceControlStrictly
-        );
+        )
+        .withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes);
     }
 
     public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
@@ -126,5 +145,10 @@ public Builder withParseWhitespaceControlStrictly(
       this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly;
       return this;
     }
+
+    public Builder withAllowAdjacentTextNodes(boolean allowAdjacentTextNodes) {
+      this.allowAdjacentTextNodes = allowAdjacentTextNodes;
+      return this;
+    }
   }
 }

src/main/java/com/hubspot/jinjava/tree/TreeParser.java

@@ -62,7 +62,18 @@ 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
+          ((TextToken) getLastSibling().getMaster()).mergeImageAndContent(
+              (TextToken) node.getMaster()
+            );
+        } else {
+          parent.getChildren().add(node);
+        }
       }
     }
 

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();

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;
@@ -231,13 +232,39 @@ 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";
     final Node tree = new TreeParser(interpreter, expression).buildTree();
     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(