Repository: PebbleTemplates/pebble
Pull request #394 (2018-08-27 03:12:38)

Make NOT operator more tollerant to argument type


Patch:
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/IfNode.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/IfNode.java
index bfda5ef9..288bfe94 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/IfNode.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/IfNode.java
@@ -14,10 +14,13 @@ import com.mitchellbosecke.pebble.node.expression.Expression;
 import com.mitchellbosecke.pebble.template.EvaluationContextImpl;
 import com.mitchellbosecke.pebble.template.PebbleTemplateImpl;
 import com.mitchellbosecke.pebble.utils.Pair;
+
 import java.io.IOException;
 import java.io.Writer;
 import java.util.List;
 
+import static com.mitchellbosecke.pebble.utils.TypeUtils.compatibleCast;
+
 public class IfNode extends AbstractRenderableNode {
 
   private final List<Pair<Expression<?>, BodyNode>> conditionsWithBodies;
@@ -49,16 +52,10 @@ public class IfNode extends AbstractRenderableNode {
         Object result = conditionalExpression.evaluate(self, context);
 
         if (result != null) {
-          if (result instanceof Boolean) {
-            satisfied = (Boolean) result;
-          }
-          else if (result instanceof Number) {
-            Number number = (Number) result;
-            satisfied = number.intValue() != 0;
-          }
-          else if (result instanceof String) {
-            String str = (String) result;
-            satisfied = !str.isEmpty();
+          if (result instanceof Boolean
+                  || result instanceof Number
+                  || result instanceof String) {
+            satisfied = compatibleCast(result, Boolean.class);
           } else {
             throw new PebbleException(
                       null,
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpression.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpression.java
index bef70dd9..3682d1ec 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpression.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpression.java
@@ -12,21 +12,34 @@ import com.mitchellbosecke.pebble.error.PebbleException;
 import com.mitchellbosecke.pebble.template.EvaluationContextImpl;
 import com.mitchellbosecke.pebble.template.PebbleTemplateImpl;
 
+import static com.mitchellbosecke.pebble.utils.TypeUtils.compatibleCast;
+
 public class UnaryNotExpression extends UnaryExpression {
 
   @Override
-  public Object evaluate(PebbleTemplateImpl self, EvaluationContextImpl context) {
-    Boolean result = (Boolean) this.getChildExpression().evaluate(self, context);
-    if (context.isStrictVariables()) {
-      if (result == null) {
-        throw new PebbleException(null,
-            "null value given to not() and strict variables is set to true", this.getLineNumber(),
-            self.getName());
+  public Boolean evaluate(PebbleTemplateImpl self, EvaluationContextImpl context) {
+    Object result = this.getChildExpression().evaluate(self, context);
+    if (result != null) {
+      if (result instanceof Boolean
+              || result instanceof Number
+              || result instanceof String) {
+        return !compatibleCast(result, Boolean.class);
       }
-      return !result;
-    } else {
-      return result == null || !result;
+      throw new PebbleException(
+              null,
+              String.format(
+                      "Unsupported value type %s. Expected Boolean, String, Number in \"if\" statement",
+                      result.getClass().getSimpleName()),
+              this.getLineNumber(),
+              self.getName());
+
     }
+    // input is null
+    if (context.isStrictVariables()) {
+      throw new PebbleException(null,
+              "null value given to not() and strict variables is set to true", this.getLineNumber(),
+              self.getName());
+    }
+    return true;
   }
-
 }
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/utils/TypeUtils.java b/pebble/src/main/java/com/mitchellbosecke/pebble/utils/TypeUtils.java
index 5047b316..94315951 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/utils/TypeUtils.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/utils/TypeUtils.java
@@ -24,40 +24,50 @@ public class TypeUtils {
     return result;
   }
 
-  public static Object compatibleCast(Object value, Class<?> type) {
+  @SuppressWarnings("unchecked")
+  public static <T> T compatibleCast(Object value, Class<T> type) {
     if (value == null || type == null || type.isAssignableFrom(value.getClass())) {
-      return value;
+      return (T) value;
     }
     if (value instanceof Number) {
       Number number = (Number) value;
-      if (type == byte.class || type == Byte.class) {
-        return number.byteValue();
-      }
-      if (type == short.class || type == Short.class) {
-        return number.shortValue();
-      }
       if (type == int.class || type == Integer.class) {
-        return number.intValue();
+        return (T) (Integer) number.intValue();
       }
       if (type == long.class || type == Long.class) {
-        return number.longValue();
+        return (T) (Long) number.longValue();
+      }
+      if (type == double.class || type == Double.class) {
+        return (T) (Double) number.doubleValue();
       }
       if (type == float.class || type == Float.class) {
-        return number.floatValue();
+        return (T) (Float) number.floatValue();
       }
-      if (type == double.class || type == Double.class) {
-        return number.doubleValue();
+      if (type == byte.class || type == Byte.class) {
+        return (T) (Byte) number.byteValue();
+      }
+      if (type == short.class || type == Short.class) {
+        return (T) (Short) number.shortValue();
       }
       if (type == BigInteger.class) {
-        return BigInteger.valueOf(number.longValue());
+        return (T) BigInteger.valueOf(number.longValue());
       }
       if (type == BigDecimal.class) {
-        return BigDecimal.valueOf(number.doubleValue());
+        return (T) BigDecimal.valueOf(number.doubleValue());
       }
       if (type == Date.class) {
-        return new Date(number.longValue());
+        return (T) new Date(number.longValue());
+      }
+      if (type == Boolean.class) {
+        return (T) (Boolean) (number.intValue() != 0);
+      }
+    }
+    if (value instanceof String) {
+      String str = (String) value;
+      if (type == Boolean.class) {
+        return (T) (Boolean) !str.isEmpty();
       }
     }
-    return value;
+    return (T) value;
   }
 }


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpressionTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpressionTest.java
new file mode 100644
index 00000000..8db36077
--- /dev/null
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/node/expression/UnaryNotExpressionTest.java
@@ -0,0 +1,100 @@
+package com.mitchellbosecke.pebble.node.expression;
+
+import com.mitchellbosecke.pebble.PebbleEngine;
+import com.mitchellbosecke.pebble.error.PebbleException;
+import com.mitchellbosecke.pebble.loader.StringLoader;
+import com.mitchellbosecke.pebble.template.PebbleTemplate;
+
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.io.Writer;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+public class UnaryNotExpressionTest {
+
+  private static final String templateSource = "{% if not value %}yes{% else %}no{% endif %}";
+
+  private String render(Object foobar) throws IOException {
+    return render(false, foobar);
+  }
+
+  private String render(boolean strict, Object value) throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder()
+            .loader(new StringLoader())
+            .strictVariables(strict)
+            .build();
+
+    PebbleTemplate template = pebble.getTemplate(templateSource);
+    Map<String, Object> context = new HashMap<>();
+    context.put("value", value);
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer, context);
+    return writer.toString();
+  }
+
+  @Test
+  public void testIfNotNull() throws IOException {
+    assertEquals("Not Null should be interpreted as TRUE",
+            "yes", render(null));
+  }
+
+  @Test(expected = PebbleException.class)
+  public void testIfNullInStrictMode() throws IOException {
+    render(true, null);
+  }
+
+  @Test
+  public void testIfNotTrue() throws IOException {
+    assertEquals("Not true should be interpreted as FALSE",
+            "no", render(true));
+  }
+
+  @Test
+  public void testIfNotFalse() throws IOException {
+    assertEquals("Not false should be interpreted as TRUE",
+            "yes", render(false));
+  }
+
+  @Test
+  public void testIfNotIntegerDifferentThanZero() throws IOException {
+    assertEquals("Not Integer one should be interpreted as FALSE",
+            "no", render(1));
+  }
+
+  @Test
+  public void testIfNotIntegerZero() throws IOException {
+    assertEquals("Not Integer Zero should be interpreted as TRUE",
+            "yes", render(0));
+  }
+
+  @Test
+  public void testIfNotFloatDifferentThanZero() throws IOException {
+    assertEquals("Not float different than zero should be interpreted as FALSE",
+            "no", render(1.1));
+  }
+
+  @Test
+  public void testIfNotFloatZero() throws IOException {
+    assertEquals("Not float zero should be interpreted as TRUE",
+            "yes", render(0.0));
+  }
+
+  @Test
+  public void testIfNotString() throws IOException {
+    assertEquals("Not string should be interpreted as FALSE",
+            "no", render("not empty string"));
+  }
+
+  @Test
+  public void testIfNotEmptyString() throws IOException {
+    assertEquals("Not Empty string should be interpreted as TRUE",
+            "yes", render(""));
+  }
+
+}