Repository: PebbleTemplates/pebble
Pull request #560 (2020-12-11 15:38:31)

Special for loop variables class, to support all attribute resolvers
I have disabled accessing map entries as fields in my attribute resolver, so using for loop variables does not work.

I see no reason to not use a special class instead of a map. Probably has a tiny bit better performance as well and it is type-safe.


Patch:
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
index 6f156e71..07212831 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
@@ -49,6 +49,18 @@ public class ForNode extends AbstractRenderableNode {
     this.elseBody = elseBody;
   }
 
+  private static class LoopVariables {
+    public boolean first, last;
+    public LazyLength length;
+    public int index;
+    public LazyRevIndex revindex;
+
+    @Override
+    public String toString() {
+      return "{last=" + last + ", length=" + length + ", index=" + index + ", revindex=" + revindex + ", first=" + first + "}";
+    }
+  }
+
   @Override
   public void render(PebbleTemplateImpl self, Writer writer, EvaluationContextImpl context)
       throws IOException {
@@ -78,7 +90,7 @@ public class ForNode extends AbstractRenderableNode {
 
       int index = 0;
 
-      Map<String, Object> loop = null;
+      LoopVariables loop = null;
 
       boolean usingExecutorService = context.getExecutorService() != null;
 
@@ -91,23 +103,23 @@ public class ForNode extends AbstractRenderableNode {
          * would get it's own distinct copy of the context.
          */
         if (index == 0 || usingExecutorService) {
-          loop = new HashMap<>();
-          loop.put("first", index == 0);
-          loop.put("last", !iterator.hasNext());
-          loop.put("length", length);
+          loop = new LoopVariables();
+          loop.first = index == 0;
+          loop.last = !iterator.hasNext();
+          loop.length = length;
         } else if (index == 1) {
           // second iteration
-          loop.put("first", false);
+          loop.first = false;
         }
 
-        loop.put("revindex", new LazyRevIndex(index, length));
-        loop.put("index", index++);
+        loop.revindex = new LazyRevIndex(index, length);
+        loop.index = index++;
         scopeChain.put("loop", loop);
         scopeChain.put(this.variableName, iterator.next());
 
         // last iteration
         if (!iterator.hasNext()) {
-          loop.put("last", true);
+          loop.last = true;
         }
 
         this.body.render(self, writer, context);
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java b/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
index d0255ab6..04bdd9b7 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
@@ -244,6 +244,12 @@ public class PebbleTemplateImpl implements PebbleTemplate {
         .getTemplate(this.resolveRelativePath(name));
     for (Pair<String, String> pair : namedMacros) {
       Macro m = templateImpl.macros.get(pair.getRight());
+
+      if (m == null) {
+        throw new PebbleException(null, "Function or Macro [" + pair.getRight() + "] referenced by alias ["
+                + pair.getLeft() + "] does not exist.");
+      }
+
       this.registerMacro(pair.getLeft(), m);
     }
   }


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java b/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
index b9054334..074e518e 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
@@ -161,4 +161,22 @@ class TestMacroCalls {
           .startsWith("More than one named template can not share the same name"));
     }
   }
+
+  @Test
+  void testInvalidAliasReferencingUnknownMacro() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder().build();
+
+    try {
+      PebbleTemplate template = pebble
+          .getTemplate("templates/macros/invalid.from.unknownMacro.peb");
+      Writer writer = new StringWriter();
+      template.evaluate(writer);
+      fail("expected PebbleException");
+    } catch (PebbleException e) {
+      assertEquals(
+              "Function or Macro [iDontExist] referenced by alias [macro_test] does not exist.",
+              e.getPebbleMessage()
+      );
+    }
+  }
 }
diff --git a/pebble/src/test/resources/templates/macros/invalid.from.unknownMacro.peb b/pebble/src/test/resources/templates/macros/invalid.from.unknownMacro.peb
new file mode 100644
index 00000000..07b950c9
--- /dev/null
+++ b/pebble/src/test/resources/templates/macros/invalid.from.unknownMacro.peb
@@ -0,0 +1,4 @@
+Hello
+
+{% from "templates/macros/macro.peb" import iDontExist as macro_test %}
+Call 1: {{ macro_test() }}