Preserve identifier in EagerAstMethod like is done in EagerAstMacroFunction.

jasmith-hs · jasmith-hs · commit a84a2c919a2f · 2023-06-29T07:40:14.000-04:00
Also migrate both to directly use the EagerAstParameters.getPartiallyResolved to reconstruct the parameters. There's no reason that it wasn't already done this way
diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java
@@ -12,7 +12,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;
 
@@ -154,26 +153,23 @@ public String getPartiallyResolved(
     ) {
       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 (
+      deferredParsingException != null &&
+      deferredParsingException.getSourceNode() == params
+    ) {
+      paramString = deferredParsingException.getDeferredEvalResult();
+    } else {
+      paramString =
+        params.getPartiallyResolved(
+          bindings,
+          context,
+          deferredParsingException,
+          preserveIdentifier
         );
-      }
-      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
diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethod.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethod.java
@@ -2,10 +2,8 @@
 
 import com.hubspot.jinjava.el.ext.DeferredParsingException;
 import com.hubspot.jinjava.interpret.DeferredValueException;
-import com.hubspot.jinjava.util.EagerExpressionResolver;
 import de.odysseus.el.tree.Bindings;
 import de.odysseus.el.tree.impl.ast.AstMethod;
-import de.odysseus.el.tree.impl.ast.AstNode;
 import de.odysseus.el.tree.impl.ast.AstParameters;
 import de.odysseus.el.tree.impl.ast.AstProperty;
 import javax.el.ELContext;
@@ -90,16 +88,13 @@ public String getPartiallyResolved(
     ) {
       paramString = deferredParsingException.getDeferredEvalResult();
     } else {
-      try {
-        paramString =
-          EagerExpressionResolver.getValueAsJinjavaStringSafe(
-            ((AstNode) params).eval(bindings, context)
-          );
-        // remove brackets so they can get replaced with parentheses
-        paramString = paramString.substring(1, paramString.length() - 1);
-      } catch (DeferredParsingException e) {
-        paramString = e.getDeferredEvalResult();
-      }
+      paramString =
+        params.getPartiallyResolved(
+          bindings,
+          context,
+          deferredParsingException,
+          preserveIdentifier
+        );
     }
 
     return (propertyResult + String.format("(%s)", paramString));
diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java
@@ -70,7 +70,7 @@ public String getPartiallyResolved(
               context,
               node,
               deferredParsingException,
-              false
+              preserveIdentifier
             )
           )
       );
diff --git a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java
@@ -69,6 +69,16 @@ public void itPreservesIdentifier() {
     }
   }
 
+  @Test
+  public void itPreservesNonDeferredIdentifier() {
+    try {
+      interpreter.resolveELExpression("deferred.append(foo_map)", -1);
+      fail("Should throw DeferredParsingException");
+    } catch (DeferredParsingException e) {
+      assertThat(e.getDeferredEvalResult()).isEqualTo("deferred.append(foo_map)");
+    }
+  }
+
   @Test
   public void itPreservesAstDot() {
     try {

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ public String getPartiallyResolved(`
`70`	`70`	`context,`
`71`	`71`	`node,`
`72`	`72`	`deferredParsingException,`
`73`		`- false`
	`73`	`+ preserveIdentifier`
`74`	`74`	`)`
`75`	`75`	`)`
`76`	`76`	`);`