Allow null-returning do expressions with failOnUnknownTokens

src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java

@@ -5,6 +5,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.hubspot.jinjava.Jinjava;
 import com.hubspot.jinjava.el.ext.NamedParameter;
+import com.hubspot.jinjava.el.ext.eager.EagerExtendedSyntaxBuilder;
 import com.hubspot.jinjava.interpret.CollectionTooBigException;
 import com.hubspot.jinjava.interpret.DeferredValueException;
 import com.hubspot.jinjava.interpret.DisabledException;
@@ -25,6 +26,7 @@
 import com.hubspot.jinjava.objects.serialization.PyishObjectMapper;
 import com.hubspot.jinjava.util.WhitespaceUtils;
 import de.odysseus.el.tree.TreeBuilderException;
+import de.odysseus.el.tree.impl.Builder.Feature;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
@@ -41,6 +43,7 @@ public class ExpressionResolver {
 
   private final JinjavaInterpreter interpreter;
   private final ExpressionFactory expressionFactory;
+  private final JinjavaInterpreterResolver interpreterResolver;
   private final ReturnTypeValidatingJinjavaInterpreterResolver resolver;
   private final JinjavaELContext elContext;
   private final ObjectUnwrapper objectUnwrapper;
@@ -55,10 +58,11 @@ public ExpressionResolver(JinjavaInterpreter interpreter, Jinjava jinjava) {
         ? jinjava.getEagerExpressionFactory()
         : jinjava.getExpressionFactory();
 
+    this.interpreterResolver = new JinjavaInterpreterResolver(interpreter);
     this.resolver =
       new ReturnTypeValidatingJinjavaInterpreterResolver(
         interpreter.getConfig().getReturnTypeValidator(),
-        new JinjavaInterpreterResolver(interpreter)
+        interpreterResolver
       );
     this.elContext = new JinjavaELContext(interpreter, resolver);
     for (ELFunctionDefinition fn : jinjava.getGlobalContext().getAllFunctions()) {
@@ -118,8 +122,17 @@ private Object resolveExpression(String expression, boolean addToResolvedExpress
         elExpression,
         Object.class
       );
+      interpreterResolver.resetMethodInvoked();
       Object result = valueExp.getValue(elContext);
-      if (result == null && interpreter.getConfig().isFailOnUnknownTokens()) {
+      boolean nullReturnedByMethod =
+        result == null &&
+        interpreterResolver.wasMethodInvoked() &&
+        expressionIsMethodInvocation(elExpression);
+      if (
+        result == null &&
+        interpreter.getConfig().isFailOnUnknownTokens() &&
+        !nullReturnedByMethod
+      ) {
         throw new UnknownTokenException(
           expression,
           interpreter.getLineNumber(),
@@ -227,6 +240,16 @@ private Object resolveExpression(String expression, boolean addToResolvedExpress
     return null;
   }
 
+  private boolean expressionIsMethodInvocation(String expression) {
+    ExtendedSyntaxBuilder builder = interpreter
+        .getConfig()
+        .getExecutionMode()
+        .useEagerParser()
+      ? new EagerExtendedSyntaxBuilder(Feature.METHOD_INVOCATIONS, Feature.VARARGS)
+      : new ExtendedSyntaxBuilder(Feature.METHOD_INVOCATIONS, Feature.VARARGS);
+    return builder.build(expression).getRoot().isMethodInvocation();
+  }
+
   private void handleELException(String expression, ELException e) {
     if (e.getCause() != null && e.getCause() instanceof DeferredValueException) {
       throw (DeferredValueException) e.getCause();

src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java

@@ -74,6 +74,7 @@ public class JinjavaInterpreterResolver extends SimpleResolver {
 
   private final JinjavaInterpreter interpreter;
   private final ObjectUnwrapper objectUnwrapper;
+  private boolean methodInvoked;
 
   public JinjavaInterpreterResolver(JinjavaInterpreter interpreter) {
     super(interpreter.getConfig().getElResolver());
@@ -89,21 +90,23 @@ public Object invoke(
     Class<?>[] paramTypes,
     Object[] params
   ) {
+    methodInvoked = false;
     try {
       Object methodProperty = getValue(context, base, method, false);
       if (methodProperty instanceof AbstractCallableMethod) {
         Object result = interpreter.getContext().isValidationMode()
           ? ""
           : ((AbstractCallableMethod) methodProperty).evaluate(params);
         context.setPropertyResolved(true);
+        methodInvoked = true;
         return result;
       }
     } catch (IllegalArgumentException e) {
       // failed to access property, continue with method calls
     }
 
     try {
-      return interpreter.getContext().isValidationMode()
+      Object result = interpreter.getContext().isValidationMode()
         ? ""
         : super.invoke(
           context,
@@ -112,18 +115,29 @@ public Object invoke(
           paramTypes,
           generateMethodParams(method, params)
         );
+      methodInvoked = true;
+      return result;
     } catch (IllegalArgumentException e) {
       return null;
     }
   }
 
+  void resetMethodInvoked() {
+    methodInvoked = false;
+  }
+
+  boolean wasMethodInvoked() {
+    return methodInvoked;
+  }
+
   /**
    * {@inheritDoc}
    *
    * If the base object is null, the property will be looked up in the context.
    */
   @Override
   public Object getValue(ELContext context, Object base, Object property) {
+    methodInvoked = false;
     return getValue(context, base, property, true);
   }
 

src/test/java/com/hubspot/jinjava/tree/FailOnUnknownTokensTest.java

@@ -8,6 +8,7 @@
 import com.hubspot.jinjava.JinjavaConfig;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.UnknownTokenException;
+import com.hubspot.jinjava.mode.EagerExecutionMode;
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.Before;
@@ -47,6 +48,43 @@ public void itReplacesTokensWithDefaultValues() {
       .isEqualTo("mary has a lamb and eats apple");
   }
 
+  @Test
+  public void itAllowsNullReturningDoExpressions() {
+    String template = "{% set object = {} %}{% do object.update({}) %}done";
+
+    assertThat(jinjava.render(template, new HashMap<>())).isEqualTo("done");
+  }
+
+  @Test
+  public void itAllowsNullReturningDoExpressionsInEagerMode() {
+    Jinjava eagerJinjava = new Jinjava(
+      BaseJinjavaTest
+        .newConfigBuilder()
+        .withFailOnUnknownTokens(true)
+        .withExecutionMode(EagerExecutionMode.instance())
+        .build()
+    );
+    String template = "{% set object = {} %}{% do object.update({}) %}done";
+
+    assertThat(eagerJinjava.render(template, new HashMap<>())).isEqualTo("done");
+  }
+
+  @Test
+  public void itRejectsUnknownMethodsInDoExpressions() {
+    String template = "{% set object = {} %}{% do object.missing() %}";
+
+    assertThatThrownBy(() -> jinjava.render(template, new HashMap<>()))
+      .hasMessageContaining("Cannot find method missing");
+  }
+
+  @Test
+  public void itRejectsUnknownPropertiesAfterNullReturningMethods() {
+    String template = "{% set object = {} %}{% do object.update({}).missing %}";
+
+    assertThatThrownBy(() -> jinjava.render(template, new HashMap<>()))
+      .hasMessageContaining("Unknown token found: object.update({}).missing");
+  }
+
   @Test
   public void itReplacesTokensInContextButThrowsExceptionForOthers() {
     final JinjavaConfig config = BaseJinjavaTest