JEXL-425, JEXL-426, JEXL-427 : added flag for logical expressions 3.4…

… compatibility; - javadoc, nitpicks, light code refactoring;
apache · Aug 30, 2024 · ca83d6b · ca83d6b
1 parent c793d2a
commit ca83d6b
Show file tree

Hide file tree

Showing 18 changed files with 369 additions and 242 deletions.
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -29,8 +29,17 @@
     <body>
         <release version="3.4.1" date="YYYY-MM-DD" description="This is a feature and maintenance release. Java 8 or later is required.">
           <!-- FIX -->
+            <action dev="henrib" type="fix" issue="JEXL-425" due-to="Xu Pengcheng">
+                Multiline format literals does not always return string
+            </action>
           <action type="fix" dev="ggregory" due-to="Gary Gregory">Replace NumberParser use of Locale.ENGLISH with Locale.ROOT.</action>
           <!-- ADD -->
+            <action dev="henrib" type="add" issue="JEXL-427" due-to="Xu Pengcheng">
+                Avoid coercing logical expressions to boolean
+            </action>
+            <action dev="henrib" type="add" issue="JEXL-426" due-to="Xu Pengcheng">
+                Enable pass-by-reference for Captured Variables
+            </action>
           <!-- UPDATE -->
           <action type="update" dev="ggregory">Bump commons-logging:commons-logging from 1.3.2 to 1.3.4 #267, #280.</action>
           <action type="update" dev="ggregory">Bump org.codehaus.mojo:animal-sniffer-maven-plugin from 1.23 to 1.24 #266.</action>

diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
@@ -187,15 +187,15 @@ public boolean antish() {
     }
 
     /**
-         * Sets whether the engine will resolve antish variable names.
-         *
-         * @param flag true means antish resolution is enabled, false disables it
-         * @return this builder
-         */
-        public JexlBuilder antish(final boolean flag) {
-            options.setAntish(flag);
-            return this;
-        }
+     * Sets whether the engine will resolve antish variable names.
+     *
+     * @param flag true means antish resolution is enabled, false disables it
+     * @return this builder
+     */
+    public JexlBuilder antish(final boolean flag) {
+        options.setAntish(flag);
+        return this;
+    }
 
     /** @return the arithmetic */
     public JexlArithmetic arithmetic() {
@@ -216,6 +216,16 @@ public JexlBuilder arithmetic(final JexlArithmetic a) {
         return this;
     }
 
+    /**
+     * Sets whether logical expressions (&quot;&quot; , ||) coerce their result to boolean.
+     * @param flag true or false
+     * @return this builder
+     */
+    public JexlBuilder booleanLogical(final boolean flag) {
+      options.setBooleanLogical(flag);
+      return this;
+    }
+
     /**
      * @return the cache size
      */
@@ -431,7 +441,12 @@ public JexlBuilder imports(final String... imports) {
         return imports(Arrays.asList(imports));
     }
 
-    /** @return whether lexical scope is enabled */
+    /**
+     * @see JexlOptions#isLexical()
+     * @return whether lexical scope is enabled
+     * @deprecated 3.4.1
+     */
+    @Deprecated
     public boolean lexical() {
         return options.isLexical();
     }
@@ -448,7 +463,12 @@ public JexlBuilder lexical(final boolean flag) {
         return this;
     }
 
-    /** @return whether lexical shading is enabled */
+    /**
+     * @see JexlOptions#isLexicalShade()
+     * @return whether lexical shading is enabled
+     * @deprecated 3.4.1
+     */
+    @Deprecated
     public boolean lexicalShade() {
         return options.isLexicalShade();
     }
@@ -546,9 +566,11 @@ public JexlBuilder namespaces(final Map<String, Object> ns) {
         return this;
     }
 
-    /** @return the current set of options */
+    /**
+     * @return the current set of options
+     */
     public JexlOptions options() {
-        return options;
+      return options;
     }
 
     /** @return the permissions */
@@ -675,15 +697,16 @@ public JexlBuilder strict(final boolean flag) {
         return this;
     }
 
+    /**
+     * @see JexlOptions#setStrictInterpolation(boolean)
+     * @param flag strict interpolation flag
+     * @return this builder
+     */
     public JexlBuilder strictInterpolation(final boolean flag) {
         options.setStrictInterpolation(flag);
         return this;
     }
 
-    public boolean strictInterpolation() {
-        return options.isStrictInterpolation();
-    }
-
     /** @return the uberspect */
     public JexlUberspect uberspect() {
         return this.uberspect;

diff --git a/src/main/java/org/apache/commons/jexl3/JexlFeatures.java b/src/main/java/org/apache/commons/jexl3/JexlFeatures.java
@@ -59,6 +59,7 @@
  * <li>Pragma anywhere: whether pragma, that are <em>not</em> statements and handled before execution begins,
  * can appear anywhere in the source or before any statements - ie at the beginning of a script.</li>
  * <li>Const Capture: whether variables captured by lambdas are read-only (aka const, same as Java) or read-write.</li>
+ * <li>Reference Capture: whether variables captured by lambdas are pass-by-reference or pass-by-value.</li>
  * </ul>
  * @since 3.2
  */
@@ -341,7 +342,7 @@ public JexlFeatures comparatorNames(final boolean flag) {
     }
 
     /**
-     * Sets whether lambda captured-variables are const or not.
+     * Sets whether lambda captured-variables are constant or mutable.
      * <p>
      * When disabled, lambda-captured variables are implicitly converted to read-write local variable (let),
      * when enabled, those are implicitly converted to read-only local variables (const).
@@ -355,7 +356,7 @@ public JexlFeatures constCapture(final boolean flag) {
     }
 
     /**
-     * Sets whether lambda captured-variables are references or not.
+     * Sets whether lambda captured-variables are references or values.
      * <p>When variables are pass-by-reference, side-effects are visible from inner lexical scopes
      * to outer-scope.</p>
      * <p>

diff --git a/src/main/java/org/apache/commons/jexl3/JexlOptions.java b/src/main/java/org/apache/commons/jexl3/JexlOptions.java
@@ -38,12 +38,15 @@
  * <li>sharedInstance: whether these options can be modified at runtime during execution (expert)</li>
  * <li>constCapture: whether captured variables will throw an error if an attempt is made to change their value</li>
  * <li>strictInterpolation: whether interpolation strings always return a string or attempt to parse and return integer</li>
+ * <li>booleanLogical: whether logical expressions (&quot;&quot; , ||) coerce their result to boolean</li>
  * </ul>
  * The sensible default is cancellable, strict and strictArithmetic.
  * <p>This interface replaces the now deprecated JexlEngine.Options.
  * @since 3.2
  */
 public final class JexlOptions {
+    /** The boolean logical flag. */
+    private static final int BOOLEAN_LOGICAL = 10;
     /** The interpolation string bit. */
     private static final int STRICT_INTERPOLATION= 9;
     /** The const capture bit. */
@@ -67,10 +70,12 @@ public final class JexlOptions {
     /** The flag names ordered. */
     private static final String[] NAMES = {
         "cancellable", "strict", "silent", "safe", "lexical", "antish",
-        "lexicalShade", "sharedInstance", "constCapture", "strictInterpolation"
+        "lexicalShade", "sharedInstance", "constCapture", "strictInterpolation",
+        "booleanShortCircuit"
     };
     /** Default mask .*/
     private static int DEFAULT = 1 /*<< CANCELLABLE*/ | 1 << STRICT | 1 << ANTISH | 1 << SAFE;
+
     /**
      * Checks the value of a flag in the mask.
      * @param ordinal the flag ordinal
@@ -80,6 +85,7 @@ public final class JexlOptions {
     private static boolean isSet(final int ordinal, final int mask) {
         return (mask & 1 << ordinal) != 0;
     }
+
     /**
      * Parses flags by name.
      * <p>A '+flag' or 'flag' will set flag as true, '-flag' set as false.
@@ -115,6 +121,7 @@ public static int parseFlags(final int initial, final String... flags) {
         }
         return mask;
     }
+
     /**
      * Sets the value of a flag in a mask.
      * @param ordinal the flag ordinal
@@ -125,6 +132,7 @@ public static int parseFlags(final int initial, final String... flags) {
     private static int set(final int ordinal, final int mask, final boolean value) {
         return value? mask | 1 << ordinal : mask & ~(1 << ordinal);
     }
+
     /**
      * Sets the default (static, shared) option flags.
      * <p>
@@ -140,8 +148,10 @@ private static int set(final int ordinal, final int mask, final boolean value) {
     public static void setDefaultFlags(final String...flags) {
         DEFAULT = parseFlags(DEFAULT, flags);
     }
+
     /** The arithmetic math context. */
     private MathContext mathContext;
+
     /** The arithmetic math scale. */
     private int mathScale = Integer.MIN_VALUE;
 
@@ -212,6 +222,17 @@ public boolean isAntish() {
         return isSet(ANTISH, flags);
     }
 
+    /**
+     * Gets whether logical expressions (&quot;&quot; , ||) coerce their result to boolean; if set,
+     * an expression like (3 &quot;&quot; 4 &quot;&quot; 5) will evaluate to true. If not, it will evaluate to 5.
+     * To preserve strict compatibility with 3.4, set the flag to true.
+     * @return true if short-circuit logicals coerce their result to boolean, false otherwise
+     * @since 3.4.1
+     */
+    public boolean isBooleanLogical() {
+        return isSet(BOOLEAN_LOGICAL, flags);
+    }
+
     /**
      * Checks whether evaluation will throw JexlException.Cancel (true) or
      * return null (false) if interrupted.
@@ -339,6 +360,14 @@ public void setAntish(final boolean flag) {
         flags = set(ANTISH, flags, flag);
     }
 
+    /**
+     * Sets whether logical expressions (&quot;&quot; , ||) coerce their result to boolean.
+     * @param flag true or false
+     */
+    public void setBooleanLogical(final boolean flag) {
+        flags = set(BOOLEAN_LOGICAL, flags, flag);
+    }
+
     /**
      * Sets whether the engine will throw JexlException.Cancel (true) or return
      * null (false) when interrupted during evaluation.
@@ -473,10 +502,15 @@ public void setStrictArithmetic(final boolean stricta) {
 
     /**
      * Sets the strict interpolation flag.
-     * @param flag true or false
-     */
-    public void setStrictInterpolation(final boolean flag) {
-        flags = set(STRICT_INTERPOLATION, flags, flag);
+     * <p>When strict, interpolation strings composed only of an expression (ie `${...}`) are evaluated
+     * as strings; when not strict, integer results are left untouched.</p>
+     * This can affect the results of expressions like <code>map.`${key}`</code> when key is
+     * an integer (or a string); it is almost always possible to use <code>map[key]</code> to ensure
+     * that the key type is not altered.
+     * @param strict true or false
+     */
+    public void setStrictInterpolation(final boolean strict) {
+        flags = set(STRICT_INTERPOLATION, flags, strict);
     }
 
     @Override public String toString() {

diff --git a/src/main/java/org/apache/commons/jexl3/internal/Frame.java b/src/main/java/org/apache/commons/jexl3/internal/Frame.java
@@ -65,7 +65,7 @@ Frame assign(final Object... values) {
     }
 
     /**
-     * Creates a new from of this frame&quote;s class.
+     * Creates a new from of this frame&quot;s class.
      * @param s the scope
      * @param r the arguments
      * @param c the number of curried parameters
@@ -149,6 +149,9 @@ Object[] nocycleStack(final Closure closure) {
     }
 }
 
+/**
+ * Pass-by-reference frame.
+ */
 class ReferenceFrame extends Frame {
     ReferenceFrame(Scope s, Object[] r, int c) {
         super(s, r, c);
@@ -160,42 +163,40 @@ Frame newFrame(final Scope s, final Object[] r, final int c) {
     }
 
     @Override
-    CaptureReference capture(int s) {
-        synchronized(stack) {
-            Object o = stack[s];
-            if (o instanceof CaptureReference) {
-                return (CaptureReference) o;
-            } else {
-                CaptureReference captured = new CaptureReference(o);
-                stack[s] = captured;
-                return captured;
-            }
+    CaptureReference capture(final int s) {
+        final Object o = stack[s];
+        if (o instanceof CaptureReference) {
+            return (CaptureReference) o;
+        } else {
+            // change the type of the captured register, wrap the value in a reference
+            CaptureReference captured = new CaptureReference(o);
+            stack[s] = captured;
+            return captured;
         }
     }
 
     @Override
     Object get(final int s) {
-        synchronized(stack) {
-            Object o = stack[s];
-            return o instanceof CaptureReference ? ((CaptureReference) o).get() : o;
-        }
+        final Object o = stack[s];
+        return o instanceof CaptureReference ? ((CaptureReference) o).get() : o;
     }
 
     @Override
     void set(final int r, final Object value) {
-        synchronized (stack) {
-            Object o = stack[r];
-            if (o instanceof CaptureReference) {
-                if (value != Scope.UNDEFINED && value != Scope.UNDECLARED) {
-                    ((CaptureReference) o).set(value);
-                }
-            } else {
-                stack[r] = value;
+        final Object o = stack[r];
+        if (o instanceof CaptureReference) {
+            if (value != Scope.UNDEFINED && value != Scope.UNDECLARED) {
+                ((CaptureReference) o).set(value);
             }
+        } else {
+            stack[r] = value;
         }
     }
 }
 
+/**
+ * Captured variable reference.
+ */
 class CaptureReference extends AtomicReference<Object> {
     CaptureReference(Object o) {
         super(o);