Improve handling of space-deletion before #set.

Previously we deleted horizontal whitespace between a `$reference` and a following `#set`, and all whitespace between a `#directive` and a following `#set`. But this was wrong (meaning, not the way Apache Velocity does it). We should only ever delete horizontal whitespace before a `#set`, whether preceded by a `$reference` or a `#directive`. The confusion arose because a single newline is deleted _after_ a `#directive`, and then what remains before the `#set` may indeed only be horizontal whitespace.

Also fix a bug in `TemplateTest` where the same `VelocityEngine` instance was being used for multiple `compare` calls in the same test method. That was not right, because macros defined in one call persisted until the next and were not overridden by definitions of the same name.

RELNOTES=A bug with space deletion before `#set` has been fixed, to be more like Apache Velocity.
PiperOrigin-RevId: 502855503

src/main/java/com/google/escapevelocity/ConstantExpressionNode.java

@@ -50,11 +50,6 @@ Object evaluate(EvaluationContext context, boolean undefinedIsFalse) {
     return value;
   }
 
-  @Override
-  boolean isWhitespace() {
-    return value instanceof String && CharMatcher.whitespace().matchesAllOf((String) value);
-  }
-
   @Override
   boolean isHorizontalWhitespace() {
     return value instanceof String && HORIZONTAL_SPACE.matchesAllOf((String) value);

src/main/java/com/google/escapevelocity/Node.java

@@ -38,11 +38,6 @@ abstract class Node {
    */
   abstract void render(EvaluationContext context, StringBuilder output);
 
-  /** True if this node is just a span of whitespace in the text. */
-  boolean isWhitespace() {
-    return false;
-  }
-
   /** True if this node is just a span of horizontal whitespace in the text. */
   boolean isHorizontalWhitespace() {
     return false;

src/main/java/com/google/escapevelocity/SetSpacing.java

@@ -33,12 +33,26 @@ private SetSpacing() {}
    * of the given list is removed if it consists of space and if that space occurs in a context
    * where it is removed before {@code #set}. This hack is needed to match what appears to be
    * special treatment in Apache Velocity of spaces before {@code #set} directives. If you have
-   * <i>thing</i> <i>whitespace</i> {@code #set}, then the whitespace is deleted if the <i>thing</i>
-   * is a comment ({@code ##...\n}); a reference ({@code $x} or {@code $x.foo} etc); or another
-   * {@code #set}. Spaces are also removed before {@code #set} at the start of a macro definition,
-   * but that is implemented by calling {@link #removeInitialSpaceBeforeSet}.
+   * <i>thing</i> <i>horizontal whitespace</i> {@code #set}, then the whitespace is deleted if the
+   * <i>thing</i> is a comment ({@code ##...\n}); a reference ({@code $x} or {@code $x.foo} etc); or
+   * another {@code #set}. Spaces are also removed before {@code #set} at the start of a macro
+   * definition, but that is implemented by calling {@link #removeInitialSpaceBeforeSet}.
    *
-   * <p>The whitespace in question can include newlines, except when <i>thing</i> is a reference.
+   * <p>Newlines are already deleted after a directive or comment, so space will be deleted before
+   * the second {@code #set} here:
+   *
+   * <pre>
+   * #set ($x = 17)
+   *   #set ($y = 23)
+   * </pre>
+   *
+   * but not here:
+   *
+   * <pre>
+   * #set ($x = 17)
+   *
+   *   #set ($y = 23)
+   * </pre>
    */
   static boolean shouldRemoveLastNodeBeforeSet(List<Node> nodes) {
     if (nodes.isEmpty()) {
@@ -49,17 +63,18 @@ static boolean shouldRemoveLastNodeBeforeSet(List<Node> nodes) {
       return potentialSpaceBeforeSet.isHorizontalWhitespace();
     }
     Node beforeSpace = nodes.get(nodes.size() - 2);
-    if (beforeSpace instanceof ReferenceNode) {
+    if (beforeSpace instanceof ReferenceNode
+        || beforeSpace instanceof CommentNode
+        || beforeSpace instanceof DirectiveNode) {
       return potentialSpaceBeforeSet.isHorizontalWhitespace();
     }
-    if (beforeSpace instanceof CommentNode || beforeSpace instanceof DirectiveNode) {
-      return potentialSpaceBeforeSet.isWhitespace();
-    }
     return false;
   }
 
   static List<Node> removeInitialSpaceBeforeSet(List<Node> nodes) {
-    if (nodes.size() >= 2 && nodes.get(0).isWhitespace() && nodes.get(1) instanceof SetNode) {
+    if (nodes.size() >= 2
+        && nodes.get(0).isHorizontalWhitespace()
+        && nodes.get(1) instanceof SetNode) {
       return nodes.subList(1, nodes.size());
     }
     return nodes;

src/test/java/com/google/escapevelocity/TemplateTest.java

@@ -49,7 +49,6 @@
 import org.apache.velocity.runtime.RuntimeConstants;
 import org.apache.velocity.runtime.resource.loader.StringResourceLoader;
 import org.apache.velocity.runtime.resource.util.StringResourceRepository;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -64,7 +63,6 @@ public class TemplateTest {
   @Rule public TestName testName = new TestName();
   @Rule public Expect expect = Expect.create();
 
-  private VelocityEngine velocityEngine;
   private enum Version {V1, V2}
   private static final Version VERSION;
 
@@ -80,12 +78,6 @@ private enum Version {V1, V2}
     VERSION = version;
   }
 
-  @Before
-  public void initVelocityEngine() {
-    velocityEngine = newVelocityEngine();
-    velocityEngine.init();
-  }
-
   private VelocityEngine newVelocityEngine() {
     VelocityEngine engine = new VelocityEngine();
 
@@ -139,6 +131,8 @@ private void compare(String template, Supplier<? extends Map<String, ?>> varsSup
   }
 
   private String velocityRender(String template, Map<String, ?> vars) {
+    VelocityEngine velocityEngine = newVelocityEngine();
+    velocityEngine.init();
     VelocityContext velocityContext = new VelocityContext(new TreeMap<>(vars));
     StringWriter writer = new StringWriter();
     String templateName = testName.getMethodName();
@@ -158,6 +152,8 @@ private void expectException(
       String template,
       Map<String, ?> vars,
       String expectedMessageSubstring) {
+    VelocityEngine velocityEngine = newVelocityEngine();
+    velocityEngine.init();
     VelocityContext velocityContext = new VelocityContext(new TreeMap<>(vars));
     String templateName = testName.getMethodName();
     VelocityException velocityException =
@@ -1105,10 +1101,11 @@ public void breakDirective() {
   @Test
   public void setSpacing() {
     // The spacing in the output from #set is eccentric.
-    // If the #set is preceded by a reference, with only horizontal space intervening, that space
-    // is deleted. But if there are newlines, nothing is deleted.
-    // If the #set is preceded by a directive (for example another #set), with only whitespace
-    // intervening, that whitespace is deleted. That includes newlines.
+    // If the #set is preceded by a reference or a comment or a directive (for example another #set)
+    // with only horizontal space intervening, that space is deleted. But if there are newlines,
+    // nothing is deleted. But, a newline immediately after a directive or comment is already
+    // deleted, and if only horizontal space remains before the #set, it is deleted.
+    compare("  #set ($x = 1)"); // preceding space is deleted
     compare("x#set ($x = 0)x");
     compare("x #set ($x = 0)x");
     compare("x #set ($x = 0) x");
@@ -1139,6 +1136,40 @@ public void setSpacing() {
     compare("\n\n  #set ($x = 3)\n");
     compare(
         "  #foreach ($i in [1..3])\n  #set ($j = \"!$i!\")\n  #set ($k = $i + 1)\n  $j$k\n  #end");
+
+    compare(
+        "#set ($foo = 17)\n" //
+        + "#set ($bar = 23)\n"
+        + "\n"
+        + "#set ($baz = 5)\n"
+        + "hello");
+
+    compare(
+        "## comment\n" //
+        + "\n"
+        + "\n"
+        + "#set ($foo = 17)\n"
+        + "hello");
+
+    compare(
+      "## comment\n" //
+      + "\n"
+      + "  #set ($foo = 17)\n"
+      + "hello");
+
+    compare(
+        "#macro (m)\n\n\n\n" //
+        + "#set ($foo = 17)\n"
+        + "hello\n"
+        + "#end\n"
+        + "#m()\n");
+
+    compare(
+        "#macro (m)\n" //
+        + "  #set ($foo = 17)\n"
+        + "hello\n"
+        + "#end\n"
+        + "#m()\n");
   }