Preserve identifiers when reconstructing AstMethod

src/main/java/com/hubspot/jinjava/el/ext/DeferredParsingException.java

@@ -5,11 +5,13 @@
 public class DeferredParsingException extends DeferredValueException {
   private final String deferredEvalResult;
   private final Object sourceNode;
+  private final IdentifierPreservationStrategy identifierPreservationStrategy;
 
   public DeferredParsingException(String message) {
     super(message);
     this.deferredEvalResult = message;
     this.sourceNode = null;
+    this.identifierPreservationStrategy = IdentifierPreservationStrategy.RESOLVING;
   }
 
   public DeferredParsingException(Object sourceNode, String deferredEvalResult) {
@@ -22,6 +24,24 @@ public DeferredParsingException(Object sourceNode, String deferredEvalResult) {
     );
     this.deferredEvalResult = deferredEvalResult;
     this.sourceNode = sourceNode;
+    this.identifierPreservationStrategy = IdentifierPreservationStrategy.RESOLVING;
+  }
+
+  public DeferredParsingException(
+    Object sourceNode,
+    String deferredEvalResult,
+    IdentifierPreservationStrategy identifierPreservationStrategy
+  ) {
+    super(
+      String.format(
+        "%s could not be parsed more than: %s",
+        sourceNode.getClass(),
+        deferredEvalResult
+      )
+    );
+    this.deferredEvalResult = deferredEvalResult;
+    this.sourceNode = sourceNode;
+    this.identifierPreservationStrategy = identifierPreservationStrategy;
   }
 
   public String getDeferredEvalResult() {
@@ -31,4 +51,8 @@ public String getDeferredEvalResult() {
   public Object getSourceNode() {
     return sourceNode;
   }
+
+  public IdentifierPreservationStrategy getIdentifierPreservationStrategy() {
+    return identifierPreservationStrategy;
+  }
 }

src/main/java/com/hubspot/jinjava/el/ext/IdentifierPreservationStrategy.java

@@ -0,0 +1,20 @@
+package com.hubspot.jinjava.el.ext;
+
+public enum IdentifierPreservationStrategy {
+  PRESERVING(true),
+  RESOLVING(false);
+
+  public static IdentifierPreservationStrategy preserving(boolean preserveIdentifier) {
+    return preserveIdentifier ? PRESERVING : RESOLVING;
+  }
+
+  private final boolean preserving;
+
+  IdentifierPreservationStrategy(boolean preserving) {
+    this.preserving = preserving;
+  }
+
+  public boolean isPreserving() {
+    return preserving;
+  }
+}

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBinary.java

@@ -2,6 +2,7 @@
 
 import com.hubspot.jinjava.el.NoInvokeELContext;
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import com.hubspot.jinjava.el.ext.OrOperator;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstBinary;
@@ -48,15 +49,15 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     return (
       EvalResultHolder.reconstructNode(
         bindings,
         context,
         left,
         deferredParsingException,
-        false
+        IdentifierPreservationStrategy.RESOLVING
       ) +
       String.format(" %s ", operator.toString()) +
       EvalResultHolder.reconstructNode(
@@ -66,7 +67,7 @@ public String getPartiallyResolved(
           : context,
         right,
         deferredParsingException,
-        false
+        IdentifierPreservationStrategy.RESOLVING
       )
     );
   }

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBracket.java

@@ -1,6 +1,7 @@
 package com.hubspot.jinjava.el.ext.eager;
 
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstBracket;
 import de.odysseus.el.tree.impl.ast.AstNode;
@@ -63,7 +64,7 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     return String.format(
       "%s[%s]",
@@ -72,14 +73,14 @@ public String getPartiallyResolved(
         context,
         (EvalResultHolder) prefix,
         deferredParsingException,
-        preserveIdentifier
+        identifierPreservationStrategy
       ),
       EvalResultHolder.reconstructNode(
         bindings,
         context,
         (EvalResultHolder) property,
         deferredParsingException,
-        false
+        IdentifierPreservationStrategy.RESOLVING
       )
     );
   }

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstChoice.java

@@ -2,6 +2,7 @@
 
 import com.hubspot.jinjava.el.NoInvokeELContext;
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstChoice;
 import de.odysseus.el.tree.impl.ast.AstNode;
@@ -46,7 +47,12 @@ public Object eval(Bindings bindings, ELContext context) throws ELException {
       }
       throw new DeferredParsingException(
         this,
-        getPartiallyResolved(bindings, context, e, false)
+        getPartiallyResolved(
+          bindings,
+          context,
+          e,
+          IdentifierPreservationStrategy.RESOLVING
+        )
       );
     }
   }
@@ -72,31 +78,31 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     return (
       EvalResultHolder.reconstructNode(
         bindings,
         context,
         question,
         deferredParsingException,
-        false
+        IdentifierPreservationStrategy.RESOLVING
       ) +
       " ? " +
       EvalResultHolder.reconstructNode(
         bindings,
         new NoInvokeELContext(context),
         yes,
         deferredParsingException,
-        preserveIdentifier
+        identifierPreservationStrategy
       ) +
       " : " +
       EvalResultHolder.reconstructNode(
         bindings,
         new NoInvokeELContext(context),
         no,
         deferredParsingException,
-        preserveIdentifier
+        identifierPreservationStrategy
       )
     );
   }

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java

@@ -3,6 +3,7 @@
 import com.hubspot.jinjava.el.ext.AstDict;
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
 import com.hubspot.jinjava.el.ext.ExtendedParser;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.util.EagerExpressionResolver;
 import de.odysseus.el.tree.Bindings;
@@ -34,7 +35,7 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     JinjavaInterpreter interpreter = (JinjavaInterpreter) context
       .getELResolver()
@@ -52,7 +53,9 @@ public String getPartiallyResolved(
               context,
               (EvalResultHolder) key,
               deferredParsingException,
-              !interpreter.getConfig().getLegacyOverrides().isEvaluateMapKeys()
+              IdentifierPreservationStrategy.preserving(
+                !interpreter.getConfig().getLegacyOverrides().isEvaluateMapKeys()
+              )
             )
           );
         } else {
@@ -69,7 +72,7 @@ public String getPartiallyResolved(
               context,
               (EvalResultHolder) value,
               deferredParsingException,
-              preserveIdentifier
+              identifierPreservationStrategy
             )
           );
         } else {

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDot.java

@@ -1,6 +1,7 @@
 package com.hubspot.jinjava.el.ext.eager;
 
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstDot;
 import de.odysseus.el.tree.impl.ast.AstNode;
@@ -52,11 +53,17 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException e,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     return String.format(
       "%s.%s",
-      EvalResultHolder.reconstructNode(bindings, context, base, e, preserveIdentifier),
+      EvalResultHolder.reconstructNode(
+        bindings,
+        context,
+        base,
+        e,
+        identifierPreservationStrategy
+      ),
       property
     );
   }

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstIdentifier.java

@@ -1,6 +1,7 @@
 package com.hubspot.jinjava.el.ext.eager;
 
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstIdentifier;
 import javax.el.ELContext;
@@ -43,7 +44,7 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     return getName();
   }

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstList.java

@@ -2,6 +2,7 @@
 
 import com.hubspot.jinjava.el.ext.AstList;
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstParameters;
 import java.util.StringJoiner;
@@ -45,7 +46,7 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     StringJoiner joiner = new StringJoiner(", ");
     for (int i = 0; i < elements.getCardinality(); i++) {
@@ -55,7 +56,7 @@ public String getPartiallyResolved(
           context,
           (EvalResultHolder) elements.getChild(i),
           deferredParsingException,
-          preserveIdentifier
+          identifierPreservationStrategy
         )
       );
     }

src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java

@@ -3,6 +3,7 @@
 import com.hubspot.jinjava.el.ext.AstMacroFunction;
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
 import com.hubspot.jinjava.el.ext.ExtendedParser;
+import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
 import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
 import com.hubspot.jinjava.interpret.DeferredValueException;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
@@ -12,7 +13,6 @@
 import java.lang.reflect.Array;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.StringJoiner;
 import javax.el.ELContext;
 import javax.el.ELException;
 
@@ -54,7 +54,13 @@ public Object eval(Bindings bindings, ELContext context) {
       );
       throw new DeferredParsingException(
         this,
-        getPartiallyResolved(bindings, context, e, true) // Need this to always be true because the macro function may modify the identifier
+        getPartiallyResolved(
+          bindings,
+          context,
+          e,
+          IdentifierPreservationStrategy.PRESERVING
+        ), // Need this to always be true because the macro function may modify the identifier
+        IdentifierPreservationStrategy.PRESERVING
       );
     }
   }
@@ -146,34 +152,28 @@ public String getPartiallyResolved(
     Bindings bindings,
     ELContext context,
     DeferredParsingException deferredParsingException,
-    boolean preserveIdentifier
+    IdentifierPreservationStrategy identifierPreservationStrategy
   ) {
     if (
       deferredParsingException != null &&
       deferredParsingException.getSourceNode() instanceof MacroFunction
     ) {
       return deferredParsingException.getDeferredEvalResult();
     }
-    StringBuilder sb = new StringBuilder();
-    sb.append(getName());
-    try {
-      StringJoiner paramString = new StringJoiner(", ");
-      for (int i = 0; i < ((AstParameters) params).getCardinality(); i++) {
-        paramString.add(
-          EvalResultHolder.reconstructNode(
-            bindings,
-            context,
-            (EvalResultHolder) ((AstParameters) params).getChild(i),
-            deferredParsingException,
-            preserveIdentifier
-          )
+    String paramString;
+    if (EvalResultHolder.exceptionMatchesNode(deferredParsingException, params)) {
+      paramString = deferredParsingException.getDeferredEvalResult();
+    } else {
+      paramString =
+        params.getPartiallyResolved(
+          bindings,
+          context,
+          deferredParsingException,
+          identifierPreservationStrategy
         );
-      }
-      sb.append(String.format("(%s)", paramString));
-    } catch (DeferredParsingException dpe) {
-      sb.append(String.format("(%s)", dpe.getDeferredEvalResult()));
     }
-    return sb.toString();
+
+    return (getName() + String.format("(%s)", paramString));
   }
 
   @Override