Merge pull request #1125 from HubSpot/revert-whitespace-changes

Revert whitespace changes

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

@@ -3,28 +3,16 @@
 /**
  * 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;
@@ -34,7 +22,6 @@ private LegacyOverrides(Builder builder) {
     whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
     useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
     parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
-    allowAdjacentTextNodes = builder.allowAdjacentTextNodes;
   }
 
   public static Builder newBuilder() {
@@ -69,10 +56,6 @@ public boolean isParseWhitespaceControlStrictly() {
     return parseWhitespaceControlStrictly;
   }
 
-  public boolean isAllowAdjacentTextNodes() {
-    return allowAdjacentTextNodes;
-  }
-
   public static class Builder {
     private boolean evaluateMapKeys = false;
     private boolean iterateOverMapKeys = false;
@@ -81,7 +64,6 @@ public static class Builder {
     private boolean whitespaceRequiredWithinTokens = false;
     private boolean useNaturalOperatorPrecedence = false;
     private boolean parseWhitespaceControlStrictly = false;
-    private boolean allowAdjacentTextNodes = false;
 
     private Builder() {}
 
@@ -101,8 +83,7 @@ public static Builder from(LegacyOverrides legacyOverrides) {
         .withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
         .withParseWhitespaceControlStrictly(
           legacyOverrides.parseWhitespaceControlStrictly
-        )
-        .withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes);
+        );
     }
 
     public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
@@ -145,10 +126,5 @@ 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,15 +62,9 @@ public Node buildTree() {
       Node node = nextNode();
 
       if (node != null) {
-        if (
-          node instanceof TextNode &&
-          getLastSibling() instanceof TextNode &&
-          !interpreter.getConfig().getLegacyOverrides().isAllowAdjacentTextNodes()
-        ) {
+        if (node instanceof TextNode && getLastSibling() instanceof TextNode) {
           // merge adjacent text nodes so whitespace control properly applies
-          ((TextToken) getLastSibling().getMaster()).mergeImageAndContent(
-              (TextToken) node.getMaster()
-            );
+          getLastSibling().getMaster().mergeImageAndContent(node.getMaster());
         } else {
           parent.getChildren().add(node);
         }
@@ -102,12 +96,6 @@ public Node buildTree() {
 
   private Node nextNode() {
     Token token = scanner.next();
-    if (token.isLeftTrim()) {
-      final Node lastSibling = getLastSibling();
-      if (lastSibling instanceof TextNode) {
-        lastSibling.getMaster().setRightTrim(true);
-      }
-    }
 
     if (token.getType() == symbols.getFixed()) {
       if (token instanceof UnclosedToken) {
@@ -182,7 +170,7 @@ private Node text(TextToken textToken) {
     final Node lastSibling = getLastSibling();
 
     // if last sibling was a tag and has rightTrimAfterEnd, strip whitespace
-    if (lastSibling != null && isRightTrim(lastSibling)) {
+    if (lastSibling instanceof TagNode && isRightTrim((TagNode) lastSibling)) {
       textToken.setLeftTrim(true);
     }
 
@@ -198,21 +186,18 @@ private Node text(TextToken textToken) {
     return n;
   }
 
-  private boolean isRightTrim(Node lastSibling) {
-    if (lastSibling instanceof TagNode) {
-      return (
-          ((TagNode) lastSibling).getEndName() == null ||
-          (
-            ((TagNode) lastSibling).getTag() instanceof FlexibleTag &&
-            !((FlexibleTag) ((TagNode) lastSibling).getTag()).hasEndTag(
-                (TagToken) lastSibling.getMaster()
-              )
-          )
+  private boolean isRightTrim(TagNode lastSibling) {
+    return (
+        lastSibling.getEndName() == null ||
+        (
+          lastSibling.getTag() instanceof FlexibleTag &&
+          !((FlexibleTag) lastSibling.getTag()).hasEndTag(
+              (TagToken) lastSibling.getMaster()
+            )
         )
-        ? lastSibling.getMaster().isRightTrim()
-        : lastSibling.getMaster().isRightTrimAfterEnd();
-    }
-    return lastSibling.getMaster().isRightTrim();
+      )
+      ? lastSibling.getMaster().isRightTrim()
+      : lastSibling.getMaster().isRightTrimAfterEnd();
   }
 
   private Node expression(ExpressionToken expressionToken) {
@@ -257,6 +242,14 @@ private Node tag(TagToken tagToken) {
     if (tag instanceof EndTag) {
       endTag(tag, tagToken);
       return null;
+    } else {
+      // if a tag has left trim, mark the last sibling to trim right whitespace
+      if (tagToken.isLeftTrim()) {
+        final Node lastSibling = getLastSibling();
+        if (lastSibling instanceof TextNode) {
+          lastSibling.getMaster().setRightTrim(true);
+        }
+      }
     }
 
     TagNode node = new TagNode(tag, tagToken, symbols);
@@ -275,6 +268,16 @@ private Node tag(TagToken tagToken) {
   }
 
   private void endTag(Tag tag, TagToken tagToken) {
+    final Node lastSibling = getLastSibling();
+
+    if (
+      parent instanceof TagNode &&
+      tagToken.isLeftTrim() &&
+      lastSibling instanceof TextNode
+    ) {
+      lastSibling.getMaster().setRightTrim(true);
+    }
+
     if (parent.getMaster() != null) { // root node
       parent.getMaster().setRightTrimAfterEnd(tagToken.isRightTrim());
     }

src/main/java/com/hubspot/jinjava/tree/output/OutputList.java

@@ -3,14 +3,11 @@
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.OutputTooBigException;
 import com.hubspot.jinjava.interpret.TemplateError;
-import com.hubspot.jinjava.tree.parse.TokenScannerSymbols;
 import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
 import java.util.LinkedList;
 import java.util.List;
 
 public class OutputList {
-  public static final String PREVENT_ACCIDENTAL_EXPRESSIONS =
-    "PREVENT_ACCIDENTAL_EXPRESSIONS";
   private final List<OutputNode> nodes = new LinkedList<>();
   private final List<BlockPlaceholderOutputNode> blocks = new LinkedList<>();
   private final long maxOutputSize;
@@ -51,72 +48,6 @@ public List<BlockPlaceholderOutputNode> getBlocks() {
   public String getValue() {
     LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize);
 
-    return JinjavaInterpreter
-      .getCurrentMaybe()
-      .map(JinjavaInterpreter::getConfig)
-      .filter(
-        config ->
-          config
-            .getFeatures()
-            .getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS)
-            .isActive(null)
-      )
-      .map(
-        config -> joinNodesWithoutAddingExpressions(val, config.getTokenScannerSymbols())
-      )
-      .orElseGet(() -> joinNodes(val));
-  }
-
-  private String joinNodesWithoutAddingExpressions(
-    LengthLimitingStringBuilder val,
-    TokenScannerSymbols tokenScannerSymbols
-  ) {
-    String separator = getWhitespaceSeparator(tokenScannerSymbols);
-    String prev = null;
-    String cur;
-    for (OutputNode node : nodes) {
-      try {
-        cur = node.getValue();
-        if (
-          prev != null &&
-          prev.length() > 0 &&
-          prev.charAt(prev.length() - 1) == tokenScannerSymbols.getExprStartChar()
-        ) {
-          if (
-            cur.length() > 0 &&
-            TokenScannerSymbols.isNoteTagOrExprChar(tokenScannerSymbols, cur.charAt(0))
-          ) {
-            val.append(separator);
-          }
-        }
-        prev = cur;
-        val.append(node.getValue());
-      } catch (OutputTooBigException e) {
-        JinjavaInterpreter
-          .getCurrent()
-          .addError(TemplateError.fromOutputTooBigException(e));
-        return val.toString();
-      }
-    }
-
-    return val.toString();
-  }
-
-  private static String getWhitespaceSeparator(TokenScannerSymbols tokenScannerSymbols) {
-    @SuppressWarnings("StringBufferReplaceableByString")
-    String separator = new StringBuilder()
-      .append('\n')
-      .append(tokenScannerSymbols.getPrefixChar())
-      .append(tokenScannerSymbols.getNoteChar())
-      .append(tokenScannerSymbols.getTrimChar())
-      .append(' ')
-      .append(tokenScannerSymbols.getNoteChar())
-      .append(tokenScannerSymbols.getExprEndChar())
-      .toString();
-    return separator;
-  }
-
-  private String joinNodes(LengthLimitingStringBuilder val) {
     for (OutputNode node : nodes) {
       try {
         val.append(node.getValue());

src/main/java/com/hubspot/jinjava/tree/parse/NoteToken.java

@@ -15,8 +15,6 @@
  **********************************************************************/
 package com.hubspot.jinjava.tree.parse;
 
-import org.apache.commons.lang3.StringUtils;
-
 public class NoteToken extends Token {
   private static final long serialVersionUID = -3859011447900311329L;
 
@@ -39,9 +37,6 @@ public int getType() {
    */
   @Override
   protected void parse() {
-    if (StringUtils.isNotEmpty(image)) {
-      handleTrim(image.substring(2, image.length() - 2));
-    }
     content = "";
   }
 

src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java

@@ -29,13 +29,6 @@ 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/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;
   }

src/main/java/com/hubspot/jinjava/tree/parse/TokenScannerSymbols.java

@@ -122,10 +122,4 @@ public String getClosingComment() {
     }
     return closingComment;
   }
-
-  public static boolean isNoteTagOrExprChar(TokenScannerSymbols symbols, char c) {
-    return (
-      c == symbols.getNote() || c == symbols.getTag() || c == symbols.getExprStartChar()
-    );
-  }
 }

src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java

@@ -8,19 +8,15 @@
 import com.google.common.collect.Lists;
 import com.hubspot.jinjava.Jinjava;
 import com.hubspot.jinjava.JinjavaConfig;
-import com.hubspot.jinjava.features.FeatureConfig;
-import com.hubspot.jinjava.features.FeatureStrategies;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
-import com.hubspot.jinjava.mode.EagerExecutionMode;
 import com.hubspot.jinjava.mode.PreserveRawExecutionMode;
 import com.hubspot.jinjava.objects.date.FormattedDate;
 import com.hubspot.jinjava.objects.date.StrftimeFormatter;
 import com.hubspot.jinjava.tree.TextNode;
 import com.hubspot.jinjava.tree.output.BlockInfo;
-import com.hubspot.jinjava.tree.output.OutputList;
 import com.hubspot.jinjava.tree.parse.TextToken;
 import com.hubspot.jinjava.tree.parse.TokenScannerSymbols;
 import java.time.ZoneId;
@@ -507,53 +503,4 @@ public void itFiltersDuplicateErrors() {
 
     assertThat(interpreter.getErrors()).containsExactly(error1, error2);
   }
-
-  @Test
-  public void itPreventsAccidentalExpressions() {
-    String makeExpression = "if (true) {\n{%- print deferred -%}\n}";
-    String makeTag = "if (true) {\n{%- print '% print 123 %' -%}\n}";
-    String makeNote = "if (true) {\n{%- print '# note #' -%}\n}";
-    jinjava.getGlobalContext().put("deferred", DeferredValue.instance());
-
-    JinjavaInterpreter normalInterpreter = new JinjavaInterpreter(
-      jinjava,
-      jinjava.getGlobalContext(),
-      JinjavaConfig.newBuilder().withExecutionMode(EagerExecutionMode.instance()).build()
-    );
-    JinjavaInterpreter preventingInterpreter = new JinjavaInterpreter(
-      jinjava,
-      jinjava.getGlobalContext(),
-      JinjavaConfig
-        .newBuilder()
-        .withFeatureConfig(
-          FeatureConfig
-            .newBuilder()
-            .add(OutputList.PREVENT_ACCIDENTAL_EXPRESSIONS, FeatureStrategies.ACTIVE)
-            .build()
-        )
-        .withExecutionMode(EagerExecutionMode.instance())
-        .build()
-    );
-    JinjavaInterpreter.pushCurrent(normalInterpreter);
-    try {
-      assertThat(normalInterpreter.render(makeExpression))
-        .isEqualTo("if (true) {{% print deferred %}}");
-      assertThat(normalInterpreter.render(makeTag))
-        .isEqualTo("if (true) {% print 123 %}");
-      assertThat(normalInterpreter.render(makeNote)).isEqualTo("if (true) {# note #}");
-    } finally {
-      JinjavaInterpreter.popCurrent();
-    }
-    JinjavaInterpreter.pushCurrent(preventingInterpreter);
-    try {
-      assertThat(preventingInterpreter.render(makeExpression))
-        .isEqualTo("if (true) {\n" + "{#- #}{% print deferred %}}");
-      assertThat(preventingInterpreter.render(makeTag))
-        .isEqualTo("if (true) {\n" + "{#- #}% print 123 %}");
-      assertThat(preventingInterpreter.render(makeNote))
-        .isEqualTo("if (true) {\n" + "{#- #}# note #}");
-    } finally {
-      JinjavaInterpreter.popCurrent();
-    }
-  }
 }