JEXL-412: improved ambiguity resolution for namespace funcall;

apache · Nov 3, 2023 · 4692b3c · 4692b3c
1 parent 578c708
commit 4692b3c
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 20 deletions.
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -37,6 +37,7 @@ New Features in 3.3.1:
 
 Bugs Fixed in 3.3.1:
 ===================
+* JEXL-412:     Ambiguous syntax between namespace function call and map object definition.
 * JEXL-410:     JexlFeatures: ctor does not enable all features
 * JEXL-409:     Disable LEXICAL should disable LEXICAL_SHADE
 * JEXL-405:     Recursive functions corrupt evaluation frame if reassigned

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -42,6 +42,9 @@
                 Allow 'trailing commas' or ellipsis while defining array, map and set literals
             </action>
             <!-- FIX -->
+            <action dev="henrib" type="fix" issue="JEXL-412" due-to="Xu Pengcheng" >
+                Ambiguous syntax between namespace function call and map object definition.
+            </action>action>
             <action dev="henrib" type="fix" issue="JEXL-410" due-to="sebb">
                 JexlFeatures: ctor does not enable all features
             </action>

diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
@@ -575,27 +575,51 @@ protected void declarePragma(final String key, final Object value) {
     }
 
     /**
-     * Checks whether a name identifies a declared namespace.
-     * @param token the namespace token
-     * @return true if the name qualifies a namespace
+     * Semantic check identifying whether a list of 4 tokens forms a namespace function call.
+     * <p>This is needed to disambiguate ternary operator, map entries and actual calls.</p>
+     * <p>Note that this check is performed before syntactic check so the expected parameters need to be
+     * verified.</p>
+     * @param ns the namespace token
+     * @param colon expected to be &quot;:&quot;
+     * @param fun the function name
+     * @param paren expected to be &quot;(&quot;
+     * @return true if the name qualifies a namespace function call
+     */
+    protected boolean isNamespaceFuncall(final Token ns, final Token colon, final Token fun, final Token paren) {
+        // let's make sure this is a namespace function call
+        if (!":".equals(colon.image)) {
+            return false;
+        }
+        if (!"(".equals(paren.image)) {
+            return false;
+        }
+        // if namespace name is shared with a variable name, use syntactic hint
+        final String name = ns.image;
+        if (isVariable(name)) {
+            // the namespace sticks to the colon as in 'ns:fun()' (vs 'ns : fun()')
+            return colon.beginColumn - 1 == ns.endColumn && isNamespace(name);
+        }
+        return true;
+    }
+
+    /**
+     * Checks whether a name is a declared namespace.
+     * @param name the namespace name
+     * @return true if declared, false otherwise
      */
-    protected boolean isDeclaredNamespace(final Token token, final Token colon) {
-        // syntactic hint, the namespace sticks to the colon
-        if (colon != null && ":".equals(colon.image) && colon.beginColumn - 1 == token.endColumn) {
+    private boolean isNamespace(String name) {
+        // templates
+        if ("jexl".equals(name) || "$jexl".equals(name)) {
             return true;
         }
-        // if name is shared with a variable name, use syntactic hint
-        final String name = token.image;
-        if (!isVariable(name)) {
-            final Set<String> ns = namespaces;
-            // declared through local pragma ?
-            if (ns != null && ns.contains(name)) {
-                return true;
-            }
-            // declared through engine features ?
-            if (getFeatures().namespaceTest().test(name)) {
-                return true;
-            }
+        final Set<String> ns = namespaces;
+        // declared through local pragma ?
+        if (ns != null && ns.contains(name)) {
+            return true;
+        }
+        // declared through engine features ?
+        if (getFeatures().namespaceTest().test(name)) {
+            return true;
         }
         return false;
     }

diff --git a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
@@ -914,7 +914,7 @@ void Arguments() #Arguments : {}
 
 void FunctionCallLookahead() #void : {}
 {
-    LOOKAHEAD(<IDENTIFIER> <COLON> <IDENTIFIER> <LPAREN>,  { isDeclaredNamespace(getToken(1), getToken(2)) }) <IDENTIFIER> <COLON> <IDENTIFIER> <LPAREN>
+    LOOKAHEAD(4, <IDENTIFIER> <COLON> <IDENTIFIER> <LPAREN>,  { isNamespaceFuncall(getToken(1), getToken(2), getToken(3), getToken(4)) }) <IDENTIFIER> <COLON> <IDENTIFIER> <LPAREN>
     |
     LOOKAHEAD(2) <IDENTIFIER> <LPAREN>
     |
@@ -923,7 +923,7 @@ void FunctionCallLookahead() #void : {}
 
 void FunctionCall() #void : {}
 {
-    LOOKAHEAD(<IDENTIFIER> <COLON> <IDENTIFIER> <LPAREN>,  { isDeclaredNamespace(getToken(1), getToken(2)) })  NamespaceIdentifier() Arguments() #FunctionNode(2)
+    LOOKAHEAD(4, <IDENTIFIER> <COLON> <IDENTIFIER> <LPAREN>,  { isNamespaceFuncall(getToken(1), getToken(2), getToken(3), getToken(4)) })  NamespaceIdentifier() Arguments() #FunctionNode(2)
     |
     LOOKAHEAD(<IDENTIFIER> <LPAREN>) Identifier(true) Arguments() #FunctionNode(2)
 }

diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
@@ -16,8 +16,10 @@
  */
 package org.apache.commons.jexl3;
 
+import java.math.BigDecimal;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Objects;
@@ -213,4 +215,45 @@ public void test406a() {
       Assert.assertEquals(src1, "1*2*3*4", result);
     }
   }
+
+  @Test
+  public void test407() {
+    // Java version
+    double r = 99.0d + 7.82d -99.0d -7.82d;
+    Assert.assertEquals(0d, r, 8.e-15); // Not zero, IEEE 754
+    // jexl
+    final JexlEngine jexl = new JexlBuilder().create();
+    JexlScript script = jexl.createScript("a + b - a - b", "a", "b");
+    // using doubles, same as Java
+    Number result = (Number) script.execute(null, 99.0d, 7.82d);
+    Assert.assertEquals(0d, result.doubleValue(), 8.e-15);
+    // using BigdDecimal, more precise, still not zero
+    result = (Number) script.execute(null, new BigDecimal(99.0d), new BigDecimal(7.82d));
+    Assert.assertEquals(0d, result.doubleValue(), 3.e-32);
+  }
+
+
+  @Test
+  public void test412() {
+    Map<Object,Object> ctl = new HashMap<>();
+    ctl.put("one", 1);
+    ctl.put("two", 2);
+    String fnsrc = "function f(x) { x }\n" +
+        "let one = 'one', two = 'two';\n" +
+        "{ one : f(1), two:f(2) }";
+    final JexlContext jc = new MapContext();
+    final String[] sources = {
+       fnsrc
+    };
+    final JexlEngine jexl = new JexlBuilder().create();
+    try {
+      final JexlScript e = jexl.createScript(fnsrc);
+      final Object o = e.execute(jc);
+      Assert.assertTrue(o instanceof Map);
+      Map<?,?> map = (Map<?, ?>) o;
+      Assert.assertEquals(map, ctl);
+    } catch(JexlException.Parsing xparse) {
+      Assert.fail(fnsrc + " : " + xparse.getMessage());
+    }
+  }
 }