Merge pull request #282 from bkiers/fix/281-expresion-with-types

fix expression errors ignored

ruby/case_numeric_camparing_as_string.rb

@@ -2,13 +2,67 @@
 
 require_relative '_helpers.rb'
 
+
+
+pp render({}, "{%- assign value = 5.0 | round: 1 -%} {%- if value >= 4.0 and value <= 4.2 -%} Très bien {%- elsif value >= 4.3 and value <= 4.7 -%} Superbe {%- elsif value >= 4.8 and value <= 4.9 -%} Fabuleux {%- elsif value != 5.0 -%} Exceptionnel {% endif %}")
+
+
+first = 98
+second = true
+operator = '>'
+pp "{% if #{first} #{operator} #{second} %}true{% else %}false{% endif %}"
+pp render({}, "{% if #{first} #{operator} #{second} %}true{% else %}false{% endif %}")
+
 # pp render({}, "{% assign comparingValue = 98.0 %}{{ 98 > comparingValue }}")
 # assertEqual("true", render({}, "{% assign comparingValue = 98.0 %}{{ '98' == comparingValue }}"))
 
-assertRaise do
-  assertEqual("true", render({}, "{% if 98 > '98' %}true{% else %}false{% endif %}"))
-end
-assertEqual(" false ", render({}, "{% if null <= 0 %} true {% else %} false {% endif %}"))
-assertEqual("no", render({}, "{% if 42.1 >= false %}yes{% else %}no{% endif %}"))
-assertEqual("no", render({}, "{% if true > false %}yes{% else %}no{% endif %}"))
-assertEqual("no", render({}, "{% if true < false %}yes{% else %}no{% endif %}"))
+      # product of two operators of types:
+      # number
+      # string
+      # true
+      # false
+      # null
+      # with operators:
+      # >
+      # >=
+      # <
+      # <=
+      # ==
+      # !=
+
+      resArray = []
+      arr = [98, '97', true, false, nil]
+      ops = ['>', '>=', '<', '<=', '==', '!=']
+      arr.product(arr).product(ops).each do |pair|
+        operator = pair[1]
+        first = pair[0][0]
+        second = pair[0][1]
+        begin
+
+          if first.class == String then first = "'#{first}'" end
+          if first === nil then first = 'nil' end
+          if second.class == String then second = "'#{second}'" end
+          if second === nil then second = 'nil' end
+          res = render({}, "{% if #{first} #{operator} #{second} %}true{% else %}false{% endif %}")
+          if res === 'true' then resArray.push(1) else resArray.push(0) end
+          pp "#{first} #{operator} #{second} = #{res}"
+        rescue Exception => e
+          if first.class == String then first = "'#{first}'" end
+          if first === nil then first = 'nil' end
+          if second.class == String then second = "'#{second}'" end
+          if second === nil then second = 'nil' end
+          resArray.push('e')
+          pp "#{first} #{operator} #{second} = ERROR: #{e.message[/.*/]}"
+        end
+
+      end
+
+      pp resArray.join('')
+
+# assertRaise do
+#   assertEqual("true", render({}, "{% if 98 > '98' %}true{% else %}false{% endif %}"))
+# end
+# assertEqual(" false ", render({}, "{% if null <= 0 %} true {% else %} false {% endif %}"))
+# assertEqual("no", render({}, "{% if 42.1 >= false %}yes{% else %}no{% endif %}"))
+# assertEqual("no", render({}, "{% if true > false %}yes{% else %}no{% endif %}"))
+# assertEqual("no", render({}, "{% if true < false %}yes{% else %}no{% endif %}"))

src/main/java/liqp/LValue.java

@@ -275,11 +275,14 @@ public Number asNumber(Object value) throws NumberFormatException {
         return str.matches("\\d+") ? Long.valueOf(str) : Double.valueOf(str);
     }
 
-    public BigDecimal asStrictNumber(Number number) {
+    public BigDecimal asStrictNumber(Object number) {
         if (number == null) {
             return null;
         }
-        return new BigDecimal(number.toString());
+        if (number instanceof BigDecimal) {
+            return (BigDecimal) number;
+        }
+        return new BigDecimal(number.toString().trim());
     }
 
     // mimic ruby's `BigDecimal.to_f` with standard java capabilities
@@ -458,6 +461,13 @@ public boolean canBeDouble(Object value) {
         return String.valueOf(value).trim().matches("-?\\d+(\\.\\d*)?");
     }
 
+    public boolean canBeNumber(Object value) {
+        if (value == null) {
+            return false;
+        }
+        return canBeInteger(value) || canBeDouble(value);
+    }
+
     public boolean isMap(Object value) {
         return (value instanceof Map);
     }

src/main/java/liqp/TemplateParser.java

@@ -59,6 +59,7 @@ public enum ErrorMode {
     public final Insertions insertions;
     public final Filters filters;
     public final boolean evaluateInOutputTag;
+    public final boolean strictTypedExpressions;
     public final TemplateParser.ErrorMode errorMode;
     public final boolean liquidStyleInclude;
     public final boolean liquidStyleWhere;
@@ -131,6 +132,7 @@ public static class Builder {
         private List<Insertion> insertions = new ArrayList<>();
         private List<Filter> filters = new ArrayList<>();
         private Boolean evaluateInOutputTag;
+        private Boolean strictTypedExpressions;
         private TemplateParser.ErrorMode errorMode;
         private Boolean liquidStyleInclude;
         private Boolean liquidStyleWhere;
@@ -174,6 +176,7 @@ public Builder(TemplateParser parser) {
             this.limitMaxRenderTimeMillis = parser.limitMaxRenderTimeMillis;
             this.limitMaxTemplateSizeBytes = parser.limitMaxTemplateSizeBytes;
             this.evaluateInOutputTag = parser.evaluateInOutputTag;
+            this.strictTypedExpressions = parser.strictTypedExpressions;
             this.liquidStyleInclude = parser.liquidStyleInclude;
             this.liquidStyleWhere = parser.liquidStyleWhere;
 
@@ -232,6 +235,11 @@ public Builder withEvaluateInOutputTag(boolean evaluateInOutputTag) {
             return this;
         }
 
+        public Builder withStrictTypedExpressions(boolean strictTypedExpressions) {
+            this.strictTypedExpressions = strictTypedExpressions;
+            return this;
+        }
+
         @SuppressWarnings("hiding")
         public Builder withLiquidStyleInclude(boolean liquidStyleInclude) {
             this.liquidStyleInclude = liquidStyleInclude;
@@ -347,6 +355,11 @@ public TemplateParser build() {
                 evaluateInOutputTag = fl.isEvaluateInOutputTag();
             }
 
+            Boolean strictTypedExpressions = this.strictTypedExpressions;
+            if (strictTypedExpressions == null) {
+                strictTypedExpressions = fl.isStrictTypedExpressions();
+            }
+
             Boolean liquidStyleInclude = this.liquidStyleInclude;
             if (liquidStyleInclude == null) {
                 liquidStyleInclude = fl.isLiquidStyleInclude();
@@ -382,21 +395,24 @@ public TemplateParser build() {
             }
 
             return new TemplateParser(strictVariables, showExceptionsFromInclude, evaluateMode, renderTransformer, locale, defaultTimeZone, environmentMapConfigurator, errorMode, fl, stripSpacesAroundTags, stripSingleLine, mapper,
-                    allInsertions, finalFilters, snippetsFolderName, evaluateInOutputTag, liquidStyleInclude, liquidStyleWhere, limitMaxIterations, limitMaxSizeRenderedString, limitMaxRenderTimeMillis, limitMaxTemplateSizeBytes);
+                    allInsertions, finalFilters, snippetsFolderName, evaluateInOutputTag, strictTypedExpressions, liquidStyleInclude, liquidStyleWhere, limitMaxIterations, limitMaxSizeRenderedString, limitMaxRenderTimeMillis, limitMaxTemplateSizeBytes);
         }
     }
 
     TemplateParser(boolean strictVariables, boolean showExceptionsFromInclude, EvaluateMode evaluateMode,
                    RenderTransformer renderTransformer, Locale locale, ZoneId defaultTimeZone,
                    Consumer<Map<String, Object>> environmentMapConfigurator, ErrorMode errorMode, Flavor flavor, boolean stripSpacesAroundTags, boolean stripSingleLine,
-                   ObjectMapper mapper, Insertions insertions, Filters filters, String snippetsFolderName, boolean evaluateInOutputTag, boolean liquidStyleInclude, Boolean liquidStyleWhere, int maxIterations, int maxSizeRenderedString, long maxRenderTimeMillis, long maxTemplateSizeBytes) {
+                   ObjectMapper mapper, Insertions insertions, Filters filters, String snippetsFolderName, boolean evaluateInOutputTag,
+                   boolean strictTypedExpressions,
+                   boolean liquidStyleInclude, Boolean liquidStyleWhere, int maxIterations, int maxSizeRenderedString, long maxRenderTimeMillis, long maxTemplateSizeBytes) {
         this.flavor = flavor;
         this.stripSpacesAroundTags = stripSpacesAroundTags;
         this.stripSingleLine = stripSingleLine;
         this.mapper = mapper;
         this.insertions = insertions;
         this.filters = filters;
         this.evaluateInOutputTag = evaluateInOutputTag;
+        this.strictTypedExpressions = strictTypedExpressions;
         this.errorMode = errorMode;
         this.liquidStyleInclude = liquidStyleInclude;
         this.liquidStyleWhere = liquidStyleWhere;

src/main/java/liqp/filters/Round.java

@@ -1,5 +1,6 @@
 package liqp.filters;
 
+import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.text.DecimalFormat;
 
@@ -31,6 +32,6 @@ public Object apply(Object value, Object... params) {
         DecimalFormat formatter = new DecimalFormat(formatBuilder.toString());
         formatter.setRoundingMode(RoundingMode.HALF_UP);
 
-        return formatter.format(number);
+        return new BigDecimal(formatter.format(number));
     }
 }

src/main/java/liqp/nodes/ComparingExpressionNode.java

@@ -3,13 +3,18 @@
 import liqp.LValue;
 import liqp.TemplateContext;
 
+import java.math.BigDecimal;
+import java.util.Optional;
+
 public abstract class ComparingExpressionNode extends LValue implements LNode {
     protected final LNode lhs;
     protected final LNode rhs;
+    private final boolean relative;
 
-    public ComparingExpressionNode(LNode lhs, LNode rhs) {
+    public ComparingExpressionNode(LNode lhs, LNode rhs, boolean realtive) {
         this.lhs = lhs;
         this.rhs = rhs;
+        this.relative = realtive;
     }
 
     @Override
@@ -48,8 +53,63 @@ public Object render(TemplateContext context) {
         if (b instanceof Number) {
             b = asStrictNumber((Number)b);
         }
-        return doCompare(a, b);
+        boolean strictTypedExpressions;
+        if (context != null && context.getParser() != null) {
+            // this variable always set except for tests
+            strictTypedExpressions = context.getParser().strictTypedExpressions;
+        } else {
+            strictTypedExpressions = true;
+        }
+        if (relative) {
+            Optional<Object> common = relativeCompareCommonRules(a, b, strictTypedExpressions);
+            if (common.isPresent()) {
+                return common.get();
+            }
+            if (!strictTypedExpressions) {
+                if (a instanceof Boolean) {
+                    a = booleanToNumber((Boolean) a);
+                }
+                if (b instanceof Boolean) {
+                    b = booleanToNumber((Boolean) b);
+                }
+                if (a == null) {
+                    a = BigDecimal.ZERO;
+                }
+                if (b == null) {
+                    b = BigDecimal.ZERO;
+                }
+            }
+        }
+        if (!strictTypedExpressions) {
+            if ((a instanceof Number && canBeNumber(b)) || (b instanceof Number && canBeNumber(a))) {
+                a = asStrictNumber(a);
+                b = asStrictNumber(b);
+            }
+        }
+        return doCompare(a, b, strictTypedExpressions);
+    }
+
+    private Object booleanToNumber(Boolean a) {
+        if  (Boolean.TRUE.equals(a)) {
+            return BigDecimal.ONE;
+        } else {
+            return BigDecimal.ZERO;
+        }
     }
 
-    abstract Object doCompare(Object a, Object b);
+    abstract Object doCompare(Object a, Object b, boolean strictTypedExpressions);
+
+    protected Optional<Object> relativeCompareCommonRules(Object a, Object b, boolean strictTypedExpressions) {
+        if (strictTypedExpressions) {
+            if (a instanceof Boolean || b instanceof Boolean) {
+                // relative comparing with boolean should be false all the time
+                return Optional.of(false);
+            }
+            if (a == null || b == null) {
+                // relative comparing with null should be false all the time
+                return Optional.of(false);
+            }
+        }
+        return Optional.empty();
+    }
 }

src/main/java/liqp/nodes/EqNode.java

@@ -7,11 +7,11 @@
 public class EqNode extends ComparingExpressionNode {
 
     public EqNode(LNode lhs, LNode rhs) {
-        super(lhs, rhs);
+        super(lhs, rhs, false);
     }
 
     @Override
-    Object doCompare(Object a, Object b) {
+    Object doCompare(Object a, Object b, boolean strictTypedExpressions) {
 
         if (a == null) {
             return b == null;

src/main/java/liqp/nodes/GtEqNode.java

@@ -3,26 +3,12 @@
 public class GtEqNode extends ComparingExpressionNode {
 
     public GtEqNode(LNode lhs, LNode rhs) {
-        super(lhs, rhs);
+        super(lhs, rhs, true);
     }
 
     @SuppressWarnings("unchecked")
     @Override
-    Object doCompare(Object a, Object b) {
-        if (a == null) {
-            return b == null;
-        }
-        if (b == null) {
-            return false;
-        }
-
-        if (a instanceof Boolean) {
-            return false;
-        }
-        if (b instanceof Boolean) {
-            return false;
-        }
-
+    Object doCompare(Object a, Object b, boolean strictTypedExpressions) {
         if (a instanceof Comparable && a.getClass().isInstance(b)) {
             return ((Comparable<Object>) a).compareTo(b) >= 0;
         } else if (b instanceof Comparable && b.getClass().isInstance(a)) {

src/main/java/liqp/nodes/GtNode.java

@@ -1,28 +1,16 @@
 package liqp.nodes;
 
+import java.util.Optional;
+
 public class GtNode extends ComparingExpressionNode {
 
     public GtNode(LNode lhs, LNode rhs) {
-        super(lhs, rhs);
+        super(lhs, rhs, true);
     }
 
     @SuppressWarnings("unchecked")
     @Override
-    Object doCompare(Object a, Object b) {
-
-        if (a == null) {
-            return b == null;
-        }
-        if (b == null) {
-            return false;
-        }
-
-        if (a instanceof Boolean) {
-            return false;
-        }
-        if (b instanceof Boolean) {
-            return false;
-        }
+    Object doCompare(Object a, Object b, boolean strictTypedExpressions) {
 
         if (a instanceof Comparable && a.getClass().isInstance(b)) {
             return ((Comparable<Object>) a).compareTo(b) > 0;

src/main/java/liqp/nodes/LtEqNode.java

@@ -1,27 +1,15 @@
 package liqp.nodes;
 
+import java.util.Optional;
+
 public class LtEqNode extends ComparingExpressionNode {
     public LtEqNode(LNode lhs, LNode rhs) {
-        super(lhs, rhs);
+        super(lhs, rhs, true);
     }
 
     @SuppressWarnings("unchecked")
     @Override
-    Object doCompare(Object a, Object b) {
-
-        if (a == null) {
-            return b == null;
-        }
-        if (b == null) {
-            return false;
-        }
-
-        if (a instanceof Boolean) {
-            return false;
-        }
-        if (b instanceof Boolean) {
-            return false;
-        }
+    Object doCompare(Object a, Object b, boolean strictTypedExpressions) {
 
         if (a instanceof Comparable && a.getClass().isInstance(b)) {
             return ((Comparable<Object>) a).compareTo(b) <= 0;