Merge pull request #1126 from HubSpot/legacy-override-for-whitespace-control

Re-add note/expression whitespace control behind legacy override

Author: jasmith-hs

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

@@ -3,16 +3,30 @@
 /**
  * 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)
+    .withUseTrimmingForNotesAndExpressions(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 final boolean useTrimmingForNotesAndExpressions;
 
   private LegacyOverrides(Builder builder) {
     evaluateMapKeys = builder.evaluateMapKeys;
@@ -22,6 +36,8 @@ private LegacyOverrides(Builder builder) {
     whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
     useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
     parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
+    allowAdjacentTextNodes = builder.allowAdjacentTextNodes;
+    useTrimmingForNotesAndExpressions = builder.useTrimmingForNotesAndExpressions;
   }
 
   public static Builder newBuilder() {
@@ -56,6 +72,14 @@ public boolean isParseWhitespaceControlStrictly() {
     return parseWhitespaceControlStrictly;
   }
 
+  public boolean isAllowAdjacentTextNodes() {
+    return allowAdjacentTextNodes;
+  }
+
+  public boolean isUseTrimmingForNotesAndExpressions() {
+    return useTrimmingForNotesAndExpressions;
+  }
+
   public static class Builder {
     private boolean evaluateMapKeys = false;
     private boolean iterateOverMapKeys = false;
@@ -64,6 +88,8 @@ public static class Builder {
     private boolean whitespaceRequiredWithinTokens = false;
     private boolean useNaturalOperatorPrecedence = false;
     private boolean parseWhitespaceControlStrictly = false;
+    private boolean allowAdjacentTextNodes = false;
+    private boolean useTrimmingForNotesAndExpressions = false;
 
     private Builder() {}
 
@@ -83,6 +109,10 @@ public static Builder from(LegacyOverrides legacyOverrides) {
         .withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
         .withParseWhitespaceControlStrictly(
           legacyOverrides.parseWhitespaceControlStrictly
+        )
+        .withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes)
+        .withUseTrimmingForNotesAndExpressions(
+          legacyOverrides.useTrimmingForNotesAndExpressions
         );
     }
 
@@ -126,5 +156,17 @@ public Builder withParseWhitespaceControlStrictly(
       this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly;
       return this;
     }
+
+    public Builder withAllowAdjacentTextNodes(boolean allowAdjacentTextNodes) {
+      this.allowAdjacentTextNodes = allowAdjacentTextNodes;
+      return this;
+    }
+
+    public Builder withUseTrimmingForNotesAndExpressions(
+      boolean useTrimmingForNotesAndExpressions
+    ) {
+      this.useTrimmingForNotesAndExpressions = useTrimmingForNotesAndExpressions;
+      return this;
+    }
   }
 }

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

@@ -17,6 +17,7 @@
 
 import com.google.common.collect.Iterators;
 import com.google.common.collect.PeekingIterator;
+import com.hubspot.jinjava.JinjavaConfig;
 import com.hubspot.jinjava.interpret.DisabledException;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.MissingEndTagException;
@@ -62,9 +63,15 @@ public Node buildTree() {
       Node node = nextNode();
 
       if (node != null) {
-        if (node instanceof TextNode && getLastSibling() instanceof TextNode) {
+        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());
+          ((TextToken) getLastSibling().getMaster()).mergeImageAndContent(
+              (TextToken) node.getMaster()
+            );
         } else {
           parent.getChildren().add(node);
         }
@@ -96,6 +103,12 @@ public Node buildTree() {
 
   private Node nextNode() {
     Token token = scanner.next();
+    if (token.isLeftTrim() && isTrimmingEnabledForToken(token, interpreter.getConfig())) {
+      final Node lastSibling = getLastSibling();
+      if (lastSibling instanceof TextNode) {
+        lastSibling.getMaster().setRightTrim(true);
+      }
+    }
 
     if (token.getType() == symbols.getFixed()) {
       if (token instanceof UnclosedToken) {
@@ -170,7 +183,11 @@ private Node text(TextToken textToken) {
     final Node lastSibling = getLastSibling();
 
     // if last sibling was a tag and has rightTrimAfterEnd, strip whitespace
-    if (lastSibling instanceof TagNode && isRightTrim((TagNode) lastSibling)) {
+    if (
+      lastSibling != null &&
+      isRightTrim(lastSibling) &&
+      isTrimmingEnabledForToken(lastSibling.getMaster(), interpreter.getConfig())
+    ) {
       textToken.setLeftTrim(true);
     }
 
@@ -186,18 +203,21 @@ private Node text(TextToken textToken) {
     return n;
   }
 
-  private boolean isRightTrim(TagNode lastSibling) {
-    return (
-        lastSibling.getEndName() == null ||
-        (
-          lastSibling.getTag() instanceof FlexibleTag &&
-          !((FlexibleTag) lastSibling.getTag()).hasEndTag(
-              (TagToken) lastSibling.getMaster()
-            )
+  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()
+              )
+          )
         )
-      )
-      ? lastSibling.getMaster().isRightTrim()
-      : lastSibling.getMaster().isRightTrimAfterEnd();
+        ? lastSibling.getMaster().isRightTrim()
+        : lastSibling.getMaster().isRightTrimAfterEnd();
+    }
+    return lastSibling.getMaster().isRightTrim();
   }
 
   private Node expression(ExpressionToken expressionToken) {
@@ -242,14 +262,6 @@ 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);
@@ -268,16 +280,6 @@ 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());
     }
@@ -318,4 +320,11 @@ private void endTag(Tag tag, TagToken tagToken) {
       );
     }
   }
+
+  private boolean isTrimmingEnabledForToken(Token token, JinjavaConfig jinjavaConfig) {
+    if (token instanceof TagToken || token instanceof TextToken) {
+      return true;
+    }
+    return jinjavaConfig.getLegacyOverrides().isUseTrimmingForNotesAndExpressions();
+  }
 }

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

@@ -3,11 +3,14 @@
 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;
@@ -48,6 +51,72 @@ 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,6 +15,8 @@
  **********************************************************************/
 package com.hubspot.jinjava.tree.parse;
 
+import org.apache.commons.lang3.StringUtils;
+
 public class NoteToken extends Token {
   private static final long serialVersionUID = -3859011447900311329L;
 
@@ -37,6 +39,9 @@ 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,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/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;
   }

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

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