This is an automated email from the ASF dual-hosted git repository.

henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git


The following commit(s) were added to refs/heads/master by this push:
     new d894074f JEXL-417: improve number narrowing; updated javadoc; updated 
test;
d894074f is described below

commit d894074fb485c8f23ca5111ad1470dc6018bc9fb
Author: Henri Biestro <hbies...@cloudera.com>
AuthorDate: Tue Nov 28 21:42:30 2023 +0100

    JEXL-417: improve number narrowing;
    updated javadoc;
    updated test;
---
 .../org/apache/commons/jexl3/JexlArithmetic.java   | 167 ++++++++++-----------
 .../org/apache/commons/jexl3/ArithmeticTest.java   |  12 +-
 2 files changed, 91 insertions(+), 88 deletions(-)

diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java 
b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index 90a1660f..585cf15d 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -41,15 +41,14 @@ import org.apache.commons.jexl3.introspection.JexlMethod;
  *
  * <p>The 5 base arithmetic operators (+, - , *, /, %) follow the same 
evaluation rules regarding their arguments.</p>
  * <ol>
- *   <li>If both are null, result is 0</li>
- *   <li>If either is a BigDecimal, coerce both to BigDecimal and perform 
operation</li>
- *   <li>If either is a floating point number, coerce both to Double and 
perform operation</li>
- *   <li>Else treat as BigInteger, perform operation and attempt to narrow 
result:
- *     <ol>
- *       <li>if both arguments can be narrowed to Integer, narrow result to 
Integer</li>
- *       <li>if both arguments can be narrowed to Long, narrow result to 
Long</li>
- *       <li>Else return result as BigInteger</li>
- *     </ol>
+ *   <li>If both are null, result is 0 if arithmetic (or operator) is 
non-strict, ArithmeticException is thrown
+ *   otherwise</li>
+ *   <li>If both arguments are numberable - any kind of integer including 
boolean -, coerce both to Long and coerce
+ *   result to the most precise argument class (boolean &lt; byte &lt; short 
&lt; int &lt; long);
+ *   if long operation would cause overflow, return a BigInteger</li>
+ *   <li>If either argument is a BigDecimal, coerce both to BigDecimal, 
operator returns BigDecimal</li>
+ *   <li>If either argument is a floating point number, coerce both to Double, 
operator returns Double</li>
+ *   <li>Else treat as BigInteger, perform operation and narrow result to the 
most precise argument class
  *   </li>
  * </ol>
  *
@@ -68,8 +67,8 @@ public class JexlArithmetic {
     /** Double.MAX_VALUE as BigDecimal. */
     protected static final BigDecimal BIGD_DOUBLE_MAX_VALUE = 
BigDecimal.valueOf(Double.MAX_VALUE);
 
-    /** Double.MIN_VALUE as BigDecimal. */
-    protected static final BigDecimal BIGD_DOUBLE_MIN_VALUE = 
BigDecimal.valueOf(Double.MIN_VALUE);
+    /** -Double.MAX_VALUE as BigDecimal. */
+    protected static final BigDecimal BIGD_DOUBLE_MIN_VALUE = 
BigDecimal.valueOf(-Double.MAX_VALUE);
 
     /** Long.MAX_VALUE as BigInteger. */
     protected static final BigInteger BIGI_LONG_MAX_VALUE = 
BigInteger.valueOf(Long.MAX_VALUE);
@@ -974,6 +973,18 @@ public class JexlArithmetic {
         return narrow == null || narrow.equals(source);
     }
 
+    /**
+     * Narrows a double to a float if there is no information loss.
+     * @param value the double value
+     * @param narrow the target narrow class
+     * @return the narrowed or initial number
+     */
+    private Number narrow(final Class<?> narrow, double value) {
+        return narrowAccept(narrow, Float.class) && (float) value == value
+            ? (float) value
+            : value;
+    }
+
     /**
      * Given a Number, return the value attempting to narrow it to a target 
class.
      *
@@ -982,67 +993,55 @@ public class JexlArithmetic {
      * @return the narrowed number or the source if no narrowing was possible
      */
     public Number narrowNumber(final Number original, final Class<?> narrow) {
-        if (original == null) {
-            return null;
-        }
-        Number result = original;
-        if (original instanceof BigDecimal) {
-            final BigDecimal bigd = (BigDecimal) original;
-            // if it is bigger than a double, it can not be narrowed
-            if (bigd.compareTo(BIGD_DOUBLE_MAX_VALUE) > 0
-                || bigd.compareTo(BIGD_DOUBLE_MIN_VALUE) < 0) {
-                return original;
-            }
-            try {
-                final long l = bigd.longValueExact();
-                // coerce to int when possible (int being so often used in 
method parms)
-                if (narrowAccept(narrow, Integer.class)
-                        && l <= Integer.MAX_VALUE
-                        && l >= Integer.MIN_VALUE) {
-                    return (int) l;
+        if (original != null) {
+            final long value;
+            if (original instanceof BigDecimal) {
+                final BigDecimal big = (BigDecimal) original;
+                try {
+                    // can it be represented as a long?
+                    value = big.longValueExact();
+                    // continue in sequence to try and further reduce
+                } catch (final ArithmeticException xa) {
+                    // if it is bigger than a double, it can not be narrowed
+                    if (big.compareTo(BIGD_DOUBLE_MAX_VALUE) > 0
+                        || big.compareTo(BIGD_DOUBLE_MIN_VALUE) < 0) {
+                        return original;
+                    }
+                    // represent as double
+                    return narrow(narrow, original.doubleValue());
                 }
-                if (narrowAccept(narrow, Long.class)) {
-                    return l;
+                // this continues with value as long
+            } else {
+                if (isFloatingPoint(original)) {
+                    final double doubleValue = original.doubleValue();
+                    // if it is not equivalent to a Long...
+                    if ((long) doubleValue != doubleValue) {
+                        return narrow(narrow, doubleValue);
+                    }
+                    // else it can be represented as a long
+                } else if (original instanceof BigInteger) {
+                    final BigInteger bigi = (BigInteger) original;
+                    // if it is bigger than a Long, it can not be narrowed
+                    if (!BigInteger.valueOf(bigi.longValue()).equals(bigi)) {
+                        return original;
+                    }
+                    // else it can be represented as a long
                 }
-            } catch (final ArithmeticException xa) {
-                // ignore, no exact value possible
+                value = original.longValue();
             }
-        }
-        if (original instanceof Double || original instanceof Float) {
-            final double value = original.doubleValue();
-            if (narrowAccept(narrow, Float.class)
-                    && value <= Float.MAX_VALUE
-                    && value >= Float.MIN_VALUE) {
-                result = result.floatValue();
+            // it can be represented as a long; determine the smallest 
possible numberable representation
+            if (narrowAccept(narrow, Byte.class) && (byte) value == value) {
+                // it will fit in a byte
+                return (byte) value;
             }
-            // else it fits in a double only
-        } else {
-            if (original instanceof BigInteger) {
-                final BigInteger bigi = (BigInteger) original;
-                // if it is bigger than a Long, it can not be narrowed
-                if (bigi.compareTo(BIGI_LONG_MAX_VALUE) > 0
-                        || bigi.compareTo(BIGI_LONG_MIN_VALUE) < 0) {
-                    return original;
-                }
+            if (narrowAccept(narrow, Short.class) && (short) value == value) {
+                return (short) value;
             }
-            final long value = original.longValue();
-            if (narrowAccept(narrow, Byte.class)
-                    && value <= Byte.MAX_VALUE
-                    && value >= Byte.MIN_VALUE) {
-                // it will fit in a byte
-                result = (byte) value;
-            } else if (narrowAccept(narrow, Short.class)
-                    && value <= Short.MAX_VALUE
-                    && value >= Short.MIN_VALUE) {
-                result = (short) value;
-            } else if (narrowAccept(narrow, Integer.class)
-                    && value <= Integer.MAX_VALUE
-                    && value >= Integer.MIN_VALUE) {
-                result = (int) value;
+            if (narrowAccept(narrow, Integer.class) && (int) value == value) {
+                return (int) value;
             }
-            // else it fits in a long
         }
-        return result;
+        return original;
     }
 
     /**
@@ -1056,25 +1055,23 @@ public class JexlArithmetic {
      *
      * @param lhs  the left-hand side operand that lead to the bigi result
      * @param rhs  the right-hand side operand that lead to the bigi result
-     * @param bigi the BigInteger to narrow
+     * @param big the BigInteger to narrow
      * @return an Integer or Long if narrowing is possible, the original 
BigInteger otherwise
      */
-    protected Number narrowBigInteger(final Object lhs, final Object rhs, 
final BigInteger bigi) {
-        //coerce to long if possible
-        if ((isNumberable(lhs) || isNumberable(rhs))
-            && bigi.compareTo(BIGI_LONG_MAX_VALUE) <= 0
-            && bigi.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
-            // coerce to int if possible
-            final long l = bigi.longValue();
-            // coerce to int when possible (int being so often used in method 
parms)
-            if (!(lhs instanceof Long || rhs instanceof Long)
-                && l <= Integer.MAX_VALUE
-                && l >= Integer.MIN_VALUE) {
-                return (int) l;
+    protected Number narrowBigInteger(final Object lhs, final Object rhs, 
final BigInteger big) {
+        if (isNumberable(lhs) || isNumberable(rhs)) {
+            try {
+                final long l = big.longValueExact();
+                // coerce to int when possible (int being so often used in 
method parms)
+                if ((int) l == l) {
+                    return (int) l;
+                }
+                return l;
+            } catch (final ArithmeticException xa) {
+                // ignore, no exact value possible
             }
-            return l;
         }
-        return bigi;
+        return big;
     }
 
     /**
@@ -1083,15 +1080,15 @@ public class JexlArithmetic {
      *
      * @param lhs  the left-hand side operand that lead to the bigd result
      * @param rhs  the right-hand side operand that lead to the bigd result
-     * @param bigd the BigDecimal to narrow
+     * @param big the BigDecimal to narrow
      * @return an Integer or Long if narrowing is possible, the original 
BigDecimal otherwise
      */
-    protected Number narrowBigDecimal(final Object lhs, final Object rhs, 
final BigDecimal bigd) {
+    protected Number narrowBigDecimal(final Object lhs, final Object rhs, 
final BigDecimal big) {
         if (isNumberable(lhs) || isNumberable(rhs)) {
             try {
-                final long l = bigd.longValueExact();
+                final long l = big.longValueExact();
                 // coerce to int when possible (int being so often used in 
method parms)
-                if (l <= Integer.MAX_VALUE && l >= Integer.MIN_VALUE) {
+                if ((int) l == l) {
                     return (int) l;
                 }
                 return l;
@@ -1099,7 +1096,7 @@ public class JexlArithmetic {
                 // ignore, no exact value possible
             }
         }
-        return bigd;
+        return big;
     }
 
     /**
diff --git a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java 
b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
index 51f0f69a..fa019b6c 100644
--- a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
@@ -1314,6 +1314,12 @@ public class ArithmeticTest extends JexlTestCase {
 
         evaluate = jexl.createExpression("math:abs(-42)").evaluate(null);
         Assert.assertEquals(42, evaluate);
+
+        evaluate =  jexl.createExpression("42B / 7").evaluate(null);
+        Assert.assertEquals(6, evaluate);
+
+        evaluate =  jexl.createExpression("42.7B / 7").evaluate(null);
+        Assert.assertEquals(BigDecimal.valueOf(6.1d), evaluate);
     }
 
     @Test
@@ -1595,9 +1601,9 @@ public class ArithmeticTest extends JexlTestCase {
     public void testNarrowBigDecimal() throws Exception {
         final List<String> ls = Arrays.asList("zero", "one", "two");
         asserter.setVariable("list", ls);
-        asserter.assertExpression("a -> list.get(a.intValue())", "zero", 
BigDecimal.ZERO);
-        asserter.assertExpression("a -> list.get(a.intValue())", "one", 
BigDecimal.ONE);
-        asserter.assertExpression("a -> list.get(2B.intValue())", "two");
+        asserter.assertExpression("a -> list.get(a)", "zero", BigDecimal.ZERO);
+        asserter.assertExpression("a -> list.get(a)", "one", BigDecimal.ONE);
+        asserter.assertExpression("a -> list.get(2B)", "two");
         BigDecimal bd42 = BigDecimal.valueOf(42);
         asserter.setVariable("bd10", BigDecimal.valueOf(10d));
         asserter.setVariable("bd420",BigDecimal.valueOf(420d));

Reply via email to