better template error messages for invalid assignments in expressions

CHANGES.md

@@ -2,7 +2,7 @@
 
 ### Version 2.1.1 ([Maven Central](http://search.maven.org/#search%7Cga%7C1%7Cg%3A%22com.hubspot.jinjava%22%20AND%20v%3A%222.1.1%22)) ###
 
-* 
+* Better error messages for invalid assignment in expression 
 
 ### Version 2.1.0 ([Maven Central](http://search.maven.org/#search%7Cga%7C1%7Cg%3A%22com.hubspot.jinjava%22%20AND%20v%3A%222.1.0%22)) ###
 

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

@@ -11,13 +11,15 @@
 
 import org.apache.commons.lang3.StringUtils;
 
+import com.hubspot.jinjava.el.ext.NamedParameter;
 import com.hubspot.jinjava.interpret.InterpretException;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.TemplateError;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
 import com.hubspot.jinjava.interpret.TemplateSyntaxException;
 import com.hubspot.jinjava.lib.fn.ELFunctionDefinition;
+
 import de.odysseus.el.tree.TreeBuilderException;
 
 /**
@@ -44,7 +46,8 @@ public ExpressionResolver(JinjavaInterpreter interpreter, ExpressionFactory expr
   /**
    * Resolve expression against current context.
    *
-   * @param expression Jinja expression.
+   * @param expression
+   *          Jinja expression.
    * @return Value of expression.
    */
   public Object resolveExpression(String expression) {
@@ -55,7 +58,11 @@ public Object resolveExpression(String expression) {
     try {
       String elExpression = "#{" + expression.trim() + "}";
       ValueExpression valueExp = expressionFactory.createValueExpression(elContext, elExpression, Object.class);
-      return valueExp.getValue(elContext);
+      Object result = valueExp.getValue(elContext);
+
+      validateResult(result);
+
+      return result;
 
     } catch (PropertyNotFoundException e) {
       interpreter.addError(new TemplateError(ErrorType.WARNING, ErrorReason.UNKNOWN, e.getMessage(), "", interpreter.getLineNumber(), e));
@@ -72,11 +79,19 @@ public Object resolveExpression(String expression) {
     return "";
   }
 
+  private void validateResult(Object result) {
+    if (result instanceof NamedParameter) {
+      throw new ELException("Unexpected '=' operator (use {% set %} tag for variable assignment)");
+    }
+  }
+
   /**
    * Resolve property of bean.
    *
-   * @param object Bean.
-   * @param propertyNames Names of properties to resolve recursively.
+   * @param object
+   *          Bean.
+   * @param propertyNames
+   *          Names of properties to resolve recursively.
    * @return Value of property.
    */
   public Object resolveProperty(Object object, List<String> propertyNames) {

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

@@ -93,8 +93,7 @@ protected AstNode expr(boolean required) throws ScanException, ParseException {
         consumeToken(COLON);
         AstNode b = expr(true);
         v = createAstChoice(v, a, b);
-      }
-      else {
+      } else {
         consumeToken();
         AstNode cond = expr(true);
         AstNode elseNode = new AstNull();
@@ -270,8 +269,7 @@ protected AstNode literal() throws ScanException, ParseException {
     case EXTENSION:
       if (getToken() == LITERAL_DICT_START) {
         v = dict();
-      }
-      else if (getToken() == LITERAL_DICT_END) {
+      } else if (getToken() == LITERAL_DICT_END) {
         return null;
       }
       break;
@@ -324,16 +322,14 @@ protected AstNode value() throws ScanException, ParseException {
           AstNode rangeMax = expr(true);
           consumeToken(RBRACK);
           v = createAstRangeBracket(v, property, rangeMax, lvalue, strict);
-        }
-        else if (nextToken.getSymbol() == RBRACK) {
+        } else if (nextToken.getSymbol() == RBRACK) {
           AstBracket bracket = createAstBracket(v, property, lvalue, strict);
           if (getToken().getSymbol() == LPAREN && context.isEnabled(METHOD_INVOCATIONS)) {
             v = createAstMethod(bracket, params());
           } else {
             v = bracket;
           }
-        }
-        else {
+        } else {
           fail(RBRACK);
         }
 
@@ -357,8 +353,7 @@ else if (nextToken.getSymbol() == RBRACK) {
             v = createAstMethod(filterProperty, new AstParameters(filterParams)); // function("filter:" + filterName, new AstParameters(filterParams));
 
           } while ("|".equals(getToken().getImage()));
-        }
-        else if ("is".equals(getToken().getImage()) && lookahead(0).getSymbol() == IDENTIFIER) {
+        } else if ("is".equals(getToken().getImage()) && lookahead(0).getSymbol() == IDENTIFIER) {
           consumeToken(); // 'is'
           String exptestName = consumeToken().getImage();
           List<AstNode> exptestParams = Lists.newArrayList(v, interpreter());

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

@@ -1,5 +1,7 @@
 package com.hubspot.jinjava.el.ext;
 
+import com.hubspot.jinjava.interpret.InterpretException;
+
 import de.odysseus.el.tree.impl.Parser.ExtensionHandler;
 import de.odysseus.el.tree.impl.Parser.ExtensionPoint;
 import de.odysseus.el.tree.impl.Scanner;
@@ -13,6 +15,9 @@ public class NamedParameterOperator {
   public static final ExtensionHandler HANDLER = new ExtensionHandler(ExtensionPoint.ADD) {
     @Override
     public AstNode createAstNode(AstNode... children) {
+      if (!(children[0] instanceof AstIdentifier)) {
+        throw new InterpretException("Expected IDENTIFIER, found " + children[0].toString());
+      }
       AstIdentifier name = (AstIdentifier) children[0];
       return new AstNamedParameter(name, children[1]);
     }

src/test/java/com/hubspot/jinjava/el/ExtendedSyntaxBuilderTest.java

@@ -212,6 +212,20 @@ public void listRangeSyntax() throws Exception {
     assertThat(val("mylist[2]")).isEqualTo(3);
   }
 
+  @Test
+  public void invalidNestedAssignmentExpr() throws Exception {
+    assertThat(val("content.template_path = 'Custom/Email/Responsive/testing.html'")).isEqualTo("");
+    assertThat(interpreter.getErrors()).isNotEmpty();
+    assertThat(interpreter.getErrors().get(0).getMessage()).containsIgnoringCase("identifier");
+  }
+
+  @Test
+  public void invalidIdentifierAssignmentExpr() throws Exception {
+    assertThat(val("content = 'Custom/Email/Responsive/testing.html'")).isEqualTo("");
+    assertThat(interpreter.getErrors()).isNotEmpty();
+    assertThat(interpreter.getErrors().get(0).getMessage()).containsIgnoringCase("'='");
+  }
+
   private Object val(String expr) {
     return interpreter.resolveELExpression(expr, -1);
   }