Divide by zero WIP

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

@@ -21,6 +21,7 @@
 import com.hubspot.jinjava.interpret.TemplateSyntaxException;
 import com.hubspot.jinjava.interpret.UnknownTokenException;
 import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory;
+import com.hubspot.jinjava.interpret.errorcategory.DivideByZeroException;
 import com.hubspot.jinjava.lib.fn.ELFunctionDefinition;
 import com.hubspot.jinjava.util.WhitespaceUtils;
 import de.odysseus.el.tree.TreeBuilderException;
@@ -102,6 +103,15 @@ public Object resolveExpression(String expression) {
       validateResult(result);
 
       return result;
+    } catch (DivideByZeroException e) {
+      // DivOperator lacks access to the interpreter, so couldn't generate an exception message that would
+      // be meaningful to the end-user. As there's no good way to get an interpreter down to DivOperator,
+      // generate a user-friendly TemplateError here.
+      TemplateError zeroDivisorError = TemplateError.fromException(e);
+      zeroDivisorError.setMessage(String.format("%s : %s", expression, e.getMessage()));
+      zeroDivisorError.setLineno(interpreter.getLineNumber());
+      zeroDivisorError.setStartPosition(interpreter.getPosition());
+      interpreter.addError(zeroDivisorError);
     } catch (PropertyNotFoundException e) {
       interpreter.addError(
         new TemplateError(

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

@@ -0,0 +1,44 @@
+package com.hubspot.jinjava.el.ext;
+
+import com.hubspot.jinjava.interpret.errorcategory.DivideByZeroException;
+import de.odysseus.el.misc.TypeConverter;
+import de.odysseus.el.tree.impl.Parser.ExtensionHandler;
+import de.odysseus.el.tree.impl.Parser.ExtensionPoint;
+import de.odysseus.el.tree.impl.Scanner;
+import de.odysseus.el.tree.impl.ast.AstBinary;
+import de.odysseus.el.tree.impl.ast.AstBinary.SimpleOperator;
+import de.odysseus.el.tree.impl.ast.AstNode;
+
+/**
+ * Created to allow for the detection and handling of divide-by-zero requests in EL expressions
+ * (see PR 473 @ https://github.com/HubSpot/jinjava/pull/473)
+ */
+public class DivOperator extends SimpleOperator {
+  public static final Scanner.ExtensionToken TOKEN = new Scanner.ExtensionToken("/");
+  public static final DivOperator OP = new DivOperator();
+
+  @Override
+  protected Object apply(TypeConverter converter, Object a, Object b) {
+    if (a == null || b == null) {
+      throw new DivideByZeroException("Division operator argument may not be null");
+    }
+
+    Number numA = (Number) a;
+    Number numB = (Number) b;
+    if (numB.doubleValue() == 0.0) {
+      throw new DivideByZeroException("Division operator divisor may not be zero");
+    }
+
+    return numA.doubleValue() / numB.doubleValue();
+  }
+
+  public static final ExtensionHandler HANDLER = new ExtensionHandler(
+    ExtensionPoint.MUL
+  ) {
+
+    @Override
+    public AstNode createAstNode(AstNode... children) {
+      return new AstBinary(children[0], children[1], OP);
+    }
+  };
+}

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

@@ -65,6 +65,7 @@ public class ExtendedParser extends Parser {
   static final Scanner.ExtensionToken LITERAL_DICT_END = new Scanner.ExtensionToken("}");
 
   static final Scanner.ExtensionToken TRUNC_DIV = TruncDivOperator.TOKEN;
+  static final Scanner.ExtensionToken REGULAR_DIV = DivOperator.TOKEN;
   static final Scanner.ExtensionToken POWER_OF = PowerOfOperator.TOKEN;
 
   static final Set<Symbol> VALID_SYMBOLS_FOR_EXP_TEST = Sets.newHashSet(
@@ -87,6 +88,7 @@ public class ExtendedParser extends Parser {
     ExtendedScanner.addKeyToken(PYFALSE);
 
     ExtendedScanner.addKeyToken(TruncDivOperator.TOKEN);
+    ExtendedScanner.addKeyToken(DivOperator.TOKEN);
     ExtendedScanner.addKeyToken(PowerOfOperator.TOKEN);
 
     ExtendedScanner.addKeyToken(CollectionMembershipOperator.TOKEN);
@@ -98,6 +100,7 @@ public ExtendedParser(Builder context, String input) {
     putExtensionHandler(NamedParameterOperator.TOKEN, NamedParameterOperator.HANDLER);
     putExtensionHandler(StringConcatOperator.TOKEN, StringConcatOperator.HANDLER);
     putExtensionHandler(TruncDivOperator.TOKEN, TruncDivOperator.HANDLER);
+    putExtensionHandler(DivOperator.TOKEN, DivOperator.HANDLER);
     putExtensionHandler(PowerOfOperator.TOKEN, PowerOfOperator.HANDLER);
 
     putExtensionHandler(

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

@@ -145,6 +145,10 @@ protected Token nextEval() throws ScanException {
     if (c1 == '~') {
       return StringConcatOperator.TOKEN;
     }
+    // Keep this comparison below that of all other tokens whose symbols begin with '/'
+    if (c1 == '/' && c2 != '/') {
+      return ExtendedParser.REGULAR_DIV;
+    }
 
     return super.nextEval();
   }

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

@@ -20,16 +20,30 @@ protected Object apply(TypeConverter converter, Object a, Object b) {
     boolean aNum = aInt || a instanceof Double || a instanceof Float;
     boolean bNum = bInt || b instanceof Double || b instanceof Float;
 
+    double bAsDouble = converter.convert(b, Double.class);
+    if (bAsDouble == 0.0) {
+      throw new IllegalArgumentException(
+        String.format(
+          "Divisor for // (truncated division) cannot be zero: '%s' (%s) and '%s' (%s)",
+          a,
+          a.getClass().getSimpleName(),
+          b,
+          b.getClass().getSimpleName()
+        )
+      );
+    }
+
     if (aInt && bInt) {
-      Long d = converter.convert(a, Long.class);
-      Long e = converter.convert(b, Long.class);
-      return Math.floorDiv(d, e);
+      Long aAsLong = converter.convert(a, Long.class);
+      Long bAsLong = converter.convert(b, Long.class);
+      return Math.floorDiv(aAsLong, bAsLong);
     }
+
     if (aNum && bNum) {
-      Double d = converter.convert(a, Double.class);
-      Double e = converter.convert(b, Double.class);
-      return Math.floor(d / e);
+      Double aAsDouble = converter.convert(a, Double.class);
+      return Math.floor(aAsDouble / bAsDouble);
     }
+
     throw new IllegalArgumentException(
       String.format(
         "Unsupported operand type(s) for //: '%s' (%s) and '%s' (%s)",

src/main/java/com/hubspot/jinjava/interpret/errorcategory/DivideByZeroException.java

@@ -0,0 +1,13 @@
+package com.hubspot.jinjava.interpret.errorcategory;
+
+public class DivideByZeroException extends RuntimeException {
+  private final String errorMessage;
+
+  public DivideByZeroException(String errorMessage) {
+    this.errorMessage = errorMessage;
+  }
+
+  public String getMessage() {
+    return errorMessage;
+  }
+}

src/test/java/com/hubspot/jinjava/el/ext/DivTest.java

@@ -0,0 +1,46 @@
+package com.hubspot.jinjava.el.ext;
+
+import static org.junit.Assert.assertEquals;
+
+import com.hubspot.jinjava.Jinjava;
+import com.hubspot.jinjava.interpret.RenderResult;
+import com.hubspot.jinjava.interpret.TemplateError;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
+import java.util.HashMap;
+import org.junit.Before;
+import org.junit.Test;
+
+public class DivTest {
+  private Jinjava jinja;
+
+  @Before
+  public void setUp() {
+    jinja = new Jinjava();
+  }
+
+  @Test
+  public void itDividesWithNonZeroLongDivisor() {
+    assertEquals(jinja.render("{% set x = 10 / 2%}{{x}}", new HashMap<>()), "5.0");
+  }
+
+  @Test
+  public void itDividesWithNonZeroDoubleDivisor() {
+    assertEquals(jinja.render("{% set x = 10.0 / 2.0%}{{x}}", new HashMap<>()), "5.0");
+  }
+
+  @Test
+  public void itErrorsOutWithZeroDivisor() {
+    RenderResult result = jinja.renderForResult(
+      "{% set x = (10.0 + 2.0 - 3.0/1.5) / 0.0%}{{x}}",
+      new HashMap<>()
+    );
+
+    assertEquals(result.getErrors().size(), 1);
+    TemplateError error = result.getErrors().get(0);
+    assertEquals(error.getSeverity(), ErrorType.FATAL);
+    assertEquals(error.getReason(), ErrorReason.EXCEPTION);
+    assertEquals(error.getMessage().contains("10.0 + 2.0 - 3.0/1.5"), true);
+    assertEquals(error.getMessage().contains("divisor may not be zero"), true);
+  }
+}

src/test/java/com/hubspot/jinjava/el/ext/TruncDivTest.java

@@ -6,6 +6,11 @@
 import com.google.common.collect.Maps;
 import com.hubspot.jinjava.Jinjava;
 import com.hubspot.jinjava.interpret.FatalTemplateErrorsException;
+import com.hubspot.jinjava.interpret.RenderResult;
+import com.hubspot.jinjava.interpret.TemplateError;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
+import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
+import java.util.HashMap;
 import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
@@ -28,6 +33,7 @@ public void testTruncDivInteger() {
     context.put("divisor", 2);
     context.put("negativeDividend", -5);
     context.put("negativeDivisor", -2);
+    context.put("zeroDivisor", 0);
 
     String[][] testCases = {
       { "{% set x = dividend // divisor %}{{x}}", "2" },
@@ -50,6 +56,29 @@ public void testTruncDivInteger() {
     }
   }
 
+  /**
+   * Test the truncated division operator "//" with divisor equal to zero
+   */
+  @Test
+  public void testTruncDivZeroDivisor() {
+    final String intTestCase = "{% set x = 10 // 0%}{{x}}";
+    final String doubleTestCase = "{% set x = 10 // 0.0%}{{x}}";
+    final String[] testCases = { intTestCase, doubleTestCase };
+
+    for (String testCase : testCases) {
+      RenderResult result = jinja.renderForResult(testCase, new HashMap<>());
+
+      assertEquals(result.getErrors().size(), 1);
+      TemplateError error = result.getErrors().get(0);
+      assertEquals(error.getSeverity(), ErrorType.FATAL);
+      assertEquals(error.getReason(), ErrorReason.EXCEPTION);
+      assertEquals(
+        error.getMessage().contains("Divisor for // (truncated division) cannot be zero"),
+        true
+      );
+    }
+  }
+
   /**
    * Test the truncated division operator "//" with fractional values
    */