Repository: PebbleTemplates/pebble
Pull request #375 (2018-07-26 17:09:12)

Macro local scope (#371)


Patch:
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/MacroNode.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/MacroNode.java
index ae36e982..7a20c429 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/MacroNode.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/MacroNode.java
@@ -69,7 +69,7 @@ public class MacroNode extends AbstractRenderableNode {
         ScopeChain scopeChain = context.getScopeChain();
 
         // scope for default arguments
-        scopeChain.pushScope();
+        scopeChain.pushLocalScope();
         for (NamedArgumentNode arg: MacroNode.this.getArgs().getNamedArgs()) {
           Expression<?> valueExpression = arg.getValueExpression();
           if (valueExpression == null) {
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/template/Scope.java b/pebble/src/main/java/com/mitchellbosecke/pebble/template/Scope.java
index 2b034aed..890d1d2d 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/template/Scope.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/template/Scope.java
@@ -19,6 +19,12 @@ import java.util.Map;
  */
 public class Scope {
 
+  /**
+   * A "local" scope ensures that the search for a particular variable will end at this scope
+   * whether or not it was found.
+   */
+  private final boolean local;
+
   /**
    * The map of variables known at this scope
    */
@@ -28,9 +34,11 @@ public class Scope {
    * Constructor
    *
    * @param backingMap The backing map of variables
+   * @param local    Whether this scope is local or not
    */
-  public Scope(Map<String, Object> backingMap) {
+  public Scope(Map<String, Object> backingMap, boolean local) {
     this.backingMap = backingMap == null ? new HashMap<>() : backingMap;
+    this.local = local;
   }
 
   /**
@@ -47,7 +55,7 @@ public class Scope {
    */
   public Scope shallowCopy() {
     Map<String, Object> backingMapCopy = new HashMap<>(this.backingMap);
-    return new Scope(backingMapCopy);
+    return new Scope(backingMapCopy, this.local);
   }
 
   /**
@@ -80,4 +88,12 @@ public class Scope {
     return this.backingMap.containsKey(key);
   }
 
+  /**
+   * Returns whether or not this scope is "local".
+   *
+   * @return boolean stating whether this scope is local or not.
+   */
+  public boolean isLocal() {
+    return this.local;
+  }
 }
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/template/ScopeChain.java b/pebble/src/main/java/com/mitchellbosecke/pebble/template/ScopeChain.java
index f1f9daf3..1d4b0c9f 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/template/ScopeChain.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/template/ScopeChain.java
@@ -38,7 +38,7 @@ public class ScopeChain {
    * @param map The map of variables used to initialize a scope.
    */
   public ScopeChain(Map<String, Object> map) {
-    Scope scope = new Scope(new HashMap<>(map));
+    Scope scope = new Scope(new HashMap<>(map), false);
     this.stack.push(scope);
   }
 
@@ -75,7 +75,15 @@ public class ScopeChain {
    * @param map The known variables of this scope.
    */
   public void pushScope(Map<String, Object> map) {
-    Scope scope = new Scope(map);
+    Scope scope = new Scope(map, false);
+    this.stack.push(scope);
+  }
+
+  /**
+   * Adds a new local scope to the scope chain
+   */
+  public void pushLocalScope() {
+    Scope scope = new Scope(new HashMap<>(), true);
     this.stack.push(scope);
   }
 
@@ -117,7 +125,7 @@ public class ScopeChain {
     }
 
     if (this.stack.size() > 1) {
-      if (scope.containsKey(key)) {
+      if (scope.isLocal() || scope.containsKey(key)) {
         // key could be defined with null and override another value below in the stack
         return null;
       }
@@ -126,10 +134,11 @@ public class ScopeChain {
       iterator.next();
 
       while (iterator.hasNext()) {
-        result = iterator.next().get(key);
+        scope = iterator.next();
+        result = scope.get(key);
         if (result != null) {
           return result;
-        } else if (scope.containsKey(key)) {
+        } else if (scope.isLocal() || scope.containsKey(key)) {
           // null value
           return null;
         }
@@ -157,6 +166,9 @@ public class ScopeChain {
     if (scope.containsKey(key)) {
       return true;
     }
+    if (scope.isLocal()) {
+      return false;
+    }
 
     Iterator<Scope> iterator = this.stack.iterator();
 
@@ -169,6 +181,9 @@ public class ScopeChain {
       if (scope.containsKey(key)) {
         return true;
       }
+      if (scope.isLocal()) {
+        return false;
+      }
     }
 
     return false;
@@ -198,7 +213,7 @@ public class ScopeChain {
      * create an iterator, etc. This is solely for performance.
      */
     Scope scope = this.stack.getFirst();
-    if (scope.containsKey(key)) {
+    if (scope.isLocal() || scope.containsKey(key)) {
       scope.put(key, value);
       return;
     }
@@ -211,7 +226,7 @@ public class ScopeChain {
     while (iterator.hasNext()) {
       scope = iterator.next();
 
-      if (scope.containsKey(key)) {
+      if (scope.isLocal() || scope.containsKey(key)) {
         scope.put(key, value);
         return;
       }


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/MacroTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/MacroTest.java
index 2adb22f3..6c28d057 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/MacroTest.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/MacroTest.java
@@ -128,18 +128,6 @@ public class MacroTest {
     assertEquals("HELLO" + LINE_SEPARATOR, writer.toString());
   }
 
-  @Test
-  public void testMacroHasAccessToGlobalVariables() throws PebbleException, IOException {
-    PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
-        .strictVariables(false).build();
-    PebbleTemplate template = pebble
-        .getTemplate("{% set foo = 'bar' %}{{ test() }}{% macro test() %}{{ foo }}{% endmacro %}");
-
-    Writer writer = new StringWriter();
-    template.evaluate(writer);
-    assertEquals("bar", writer.toString());
-  }
-
   @Test
   public void testImportFile() throws PebbleException, IOException {
     PebbleEngine pebble = new PebbleEngine.Builder().strictVariables(false).build();
@@ -179,4 +167,18 @@ public class MacroTest {
     template.evaluate(writer, context);
     assertEquals("ajax macro" + LINE_SEPARATOR, writer.toString());
   }
+
+  @Test
+  public void testSetVariableInsideMacro() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().strictVariables(false).build();
+    PebbleTemplate template = pebble.getTemplate("templates/macros/setVariableBase.peb");
+
+    Map<String, Object> context = new HashMap<>();
+    context.put("unit", "tank");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer, context);
+
+    assertEquals("tankinfantrytank", writer.toString());
+  }
 }
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/ScopeChainTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/ScopeChainTest.java
index f51bce60..ef012fe8 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/ScopeChainTest.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/ScopeChainTest.java
@@ -9,6 +9,9 @@
 package com.mitchellbosecke.pebble;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import com.mitchellbosecke.pebble.error.PebbleException;
 import com.mitchellbosecke.pebble.template.ScopeChain;
@@ -27,5 +30,70 @@ public class ScopeChainTest {
     assertEquals("value2", scopeChain.get("key"));
     scopeChain.popScope();
     assertEquals("value2", scopeChain.get("key"));
+    scopeChain.pushLocalScope();
+    scopeChain.set("key", "value3");
+    assertEquals("value3", scopeChain.get("key"));
+    scopeChain.popScope();
+    assertEquals("value2", scopeChain.get("key"));
+  }
+
+  @Test
+  public void testGetValueWithLocalScopeFirstInChainAndValueInAnotherScope() {
+    ScopeChain scopeChain = new ScopeChain();
+    scopeChain.pushScope();
+    scopeChain.set("key", "value");
+
+    scopeChain.pushLocalScope();
+    scopeChain.set("key2", "value2");
+
+    assertNull(scopeChain.get("key"));
+    assertEquals("value2", scopeChain.get("key2"));
+  }
+
+  @Test
+  public void testGetValueWithLocalScopeNotFirstInChainAndValueInAnotherScope() {
+    ScopeChain scopeChain = new ScopeChain();
+    scopeChain.pushScope();
+    scopeChain.set("key", "value");
+
+    scopeChain.pushLocalScope();
+    scopeChain.set("key2", "value2");
+
+    scopeChain.pushScope();
+    scopeChain.set("key3", "value3");
+
+    assertNull(scopeChain.get("key"));
+    assertEquals("value2", scopeChain.get("key2"));
+    assertEquals("value3", scopeChain.get("key3"));
+  }
+
+  @Test
+  public void testContainsKeyWithLocalScopeFirstInChainAndValueInAnotherScope() {
+    ScopeChain scopeChain = new ScopeChain();
+    scopeChain.pushScope();
+    scopeChain.set("key", "value");
+
+    scopeChain.pushLocalScope();
+    scopeChain.set("key2", "value2");
+
+    assertFalse(scopeChain.containsKey("key"));
+    assertTrue(scopeChain.containsKey("key2"));
+  }
+
+  @Test
+  public void testContainsKeyWithLocalScopeNotFirstInChainAndValueInAnotherScope() {
+    ScopeChain scopeChain = new ScopeChain();
+    scopeChain.pushScope();
+    scopeChain.set("key", "value");
+
+    scopeChain.pushLocalScope();
+    scopeChain.set("key2", "value2");
+
+    scopeChain.pushScope();
+    scopeChain.set("key3", "value3");
+
+    assertFalse(scopeChain.containsKey("key"));
+    assertTrue(scopeChain.containsKey("key2"));
+    assertTrue(scopeChain.containsKey("key3"));
   }
 }
diff --git a/pebble/src/test/resources/templates/macros/setVariableBase.peb b/pebble/src/test/resources/templates/macros/setVariableBase.peb
new file mode 100644
index 00000000..22d4a5c4
--- /dev/null
+++ b/pebble/src/test/resources/templates/macros/setVariableBase.peb
@@ -0,0 +1,4 @@
+{{ unit }}
+{% import 'templates/macros/setVariableMacro.peb' %}
+{{ infantry() }}
+{{ unit }}
\ No newline at end of file
diff --git a/pebble/src/test/resources/templates/macros/setVariableMacro.peb b/pebble/src/test/resources/templates/macros/setVariableMacro.peb
new file mode 100644
index 00000000..bba83d52
--- /dev/null
+++ b/pebble/src/test/resources/templates/macros/setVariableMacro.peb
@@ -0,0 +1,4 @@
+{% macro infantry() %}
+{% set unit = 'infantry' %}
+{{ unit }}
+{% endmacro %}
\ No newline at end of file