You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2022/05/18 17:47:26 UTC

[commons-jexl] branch master updated: JEXL-369: const variables require assignment at declaration time; - simplified code accordingly;

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 be7e4cb5 JEXL-369: const variables require assignment at declaration time; - simplified code accordingly;
be7e4cb5 is described below

commit be7e4cb50a63e2dde647f983c45411315b5827a3
Author: henrib <he...@apache.org>
AuthorDate: Wed May 18 19:47:20 2022 +0200

    JEXL-369: const variables require assignment at declaration time;
    - simplified code accordingly;
---
 .../commons/jexl3/internal/LexicalScope.java       | 25 +-----------
 .../apache/commons/jexl3/parser/ASTIdentifier.java | 10 -----
 .../commons/jexl3/parser/JexlLexicalNode.java      | 10 -----
 .../apache/commons/jexl3/parser/JexlParser.java    | 31 ++-------------
 .../org/apache/commons/jexl3/parser/Parser.jjt     |  2 +-
 .../java/org/apache/commons/jexl3/LambdaTest.java  | 46 +++++++++++++++-------
 6 files changed, 37 insertions(+), 87 deletions(-)

diff --git a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
index 5551551a..4be5d755 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
@@ -34,7 +34,7 @@ public class LexicalScope {
      * Bits per symbol.
      * Declared, const, defined.
      */
-    protected static final int BITS_PER_SYMBOL = 3;
+    protected static final int BITS_PER_SYMBOL = 2;
     /**
      * Number of symbols.
      */
@@ -133,18 +133,6 @@ public class LexicalScope {
         return isSet(bit);
     }
 
-    /**
-     * Checks whether a const symbol has been defined, ie has a value.
-     *
-     * @param symbol the symbol
-     * @return true if defined, false otherwise
-     */
-    public boolean isDefined(final int symbol) {
-        final int bit = (symbol << BITS_PER_SYMBOL) | 2;
-        return isSet(bit);
-
-    }
-
     /**
      * Adds a symbol in this scope.
      *
@@ -171,17 +159,6 @@ public class LexicalScope {
         return set(bit);
     }
 
-    /**
-     * Defines a constant in this scope.
-     *
-     * @param symbol the symbol
-     * @return true if registered, false if symbol was already registered
-     */
-    public boolean defineSymbol(final int symbol) {
-        final int bit = (symbol << BITS_PER_SYMBOL) | 2;
-        return set(bit);
-    }
-
     /**
      * Clear all symbols.
      *
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifier.java b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifier.java
index 88748750..944b5915 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifier.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifier.java
@@ -34,8 +34,6 @@ public class ASTIdentifier extends JexlNode {
     private static final int LEXICAL = 3;
     /** The const variable flag. */
     private static final int CONST = 4;
-    /** The defined variable flag. */
-    private static final int DEFINED = 5;
 
     ASTIdentifier(final int id) {
         super(id);
@@ -127,14 +125,6 @@ public class ASTIdentifier extends JexlNode {
         flags = set(CONST, flags, f);
     }
 
-    public boolean isDefined() {
-        return isSet(DEFINED, flags);
-    }
-
-    public void setDefined(final boolean f) {
-        flags = set(DEFINED, flags, f);
-    }
-
     public String getName() {
         return name;
     }
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlLexicalNode.java b/src/main/java/org/apache/commons/jexl3/parser/JexlLexicalNode.java
index 475f11cc..17eff4f6 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlLexicalNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlLexicalNode.java
@@ -46,21 +46,11 @@ public class JexlLexicalNode extends JexlNode implements JexlParser.LexicalUnit
         return lexicalScope != null && lexicalScope.isConstant(symbol);
     }
 
-    @Override
-    public boolean isDefined(final int symbol) {
-        return  lexicalScope != null && lexicalScope.isDefined(symbol);
-    }
-
     @Override
     public void setConstant(int symbol) {
         lexicalScope.addConstant(symbol);
     }
 
-    @Override
-    public void setDefined(int symbol) {
-        lexicalScope.defineSymbol(symbol);
-    }
-
     @Override
     public int getSymbolCount() {
         return lexicalScope == null? 0 : lexicalScope.getSymbolCount();
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
index d790ac75..3f0ff1ba 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
@@ -99,7 +99,6 @@ public abstract class JexlParser extends StringParser {
          */
         boolean declareSymbol(int symbol);
         void setConstant(int symbol);
-        void setDefined(int symbol);
 
         /**
          * Checks whether a symbol is declared in this lexical unit.
@@ -108,7 +107,6 @@ public abstract class JexlParser extends StringParser {
          */
         boolean hasSymbol(int symbol);
         boolean isConstant(int symbol);
-        boolean isDefined(int symbol);
 
         /**
          * @return the number of local variables declared in this unit
@@ -337,7 +335,6 @@ public abstract class JexlParser extends StringParser {
                         // track if const is defined or not
                         if (unit.isConstant(symbol)) {
                             identifier.setConstant(true);
-                            identifier.setDefined(unit.isDefined(symbol));
                         }
                     } else if (info instanceof JexlNode.Info) {
                         declared = isSymbolDeclared((JexlNode.Info) info, symbol);
@@ -412,7 +409,6 @@ public abstract class JexlParser extends StringParser {
         if (declareSymbol(symbol)) {
             scope.addLexical(symbol);
             block.setConstant(symbol);
-            block.setDefined(symbol);
         } else {
             if (getFeatures().isLexical()) {
                 throw new JexlException(variable, name + ": variable is already declared");
@@ -488,7 +484,6 @@ public abstract class JexlParser extends StringParser {
             scope.addLexical(symbol);
             if (constant) {
                 block.setConstant(symbol);
-                block.setDefined(symbol); // worst case is no argument, parameter will bind to null
             }
         }
     }
@@ -620,32 +615,12 @@ public abstract class JexlParser extends StringParser {
             if (!lv.isLeftValue()) {
                 throw new JexlException.Assignment(lv.jexlInfo(), null).clean();
             }
-            if (lv instanceof ASTIdentifier) {
+            if (lv instanceof ASTIdentifier && !(lv instanceof ASTVar)) {
                 ASTIdentifier var = (ASTIdentifier) lv;
                 int symbol = var.getSymbol();
                 boolean isconst = symbol >= 0 && block != null && block.isConstant(symbol);
-                if (isconst) {
-                    if (!(var instanceof ASTVar)) { // if not a declaration...
-                        if (block.isDefined(symbol)) {
-                            throw new JexlException.Assignment(var.jexlInfo(), var.getName()).clean();
-                        } else {
-                            block.setDefined(symbol);
-                        }
-                    } else {
-                        block.setDefined(symbol);
-                    }
-                }
-            }
-        } else {
-            // control that a const is defined before usage
-            int nchildren = node.jjtGetNumChildren();
-            for(int c = 0; c < nchildren; ++c) {
-                JexlNode child = node.jjtGetChild(c);
-                if (child instanceof ASTIdentifier) {
-                    ASTIdentifier var = (ASTIdentifier) child;
-                    if (var.isConstant() && !var.isDefined()) {
-                        throw new JexlException.Parsing(info, var.getName() + ": constant is not defined").clean();
-                    }
+                if (isconst) { // if not a declaration...
+                    throw new JexlException.Parsing(var.jexlInfo(), var.getName() +": const assignment.").clean();
                 }
             }
         }
diff --git a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
index af16272a..753bad00 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
+++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
@@ -445,7 +445,7 @@ void Var() #void : {}
     |
     <LET> DeclareVar(true, false) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))?
     |
-    <CONST> DeclareVar(true, true) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))?
+    <CONST> DeclareVar(true, true) <assign> Expression() #Assignment(2)
 }
 
 void DeclareVar(boolean lexical, boolean constant) #Var :
diff --git a/src/test/java/org/apache/commons/jexl3/LambdaTest.java b/src/test/java/org/apache/commons/jexl3/LambdaTest.java
index 70d823f0..31f27401 100644
--- a/src/test/java/org/apache/commons/jexl3/LambdaTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LambdaTest.java
@@ -438,7 +438,7 @@ public class LambdaTest extends JexlTestCase {
     }
 
     @Test
-    public void testConst0() {
+    public void testConst0a() {
         final JexlFeatures f = new JexlFeatures();
         final JexlEngine jexl = new JexlBuilder().strict(true).create();
         final JexlScript script = jexl.createScript(
@@ -448,37 +448,55 @@ public class LambdaTest extends JexlTestCase {
         Assert.assertEquals(22, result);
     }
 
+    @Test
+    public void testConstb0() {
+        final JexlFeatures f = new JexlFeatures();
+        final JexlEngine jexl = new JexlBuilder().strict(true).create();
+        final JexlScript script = jexl.createScript(
+                "{ const x = 10; }{ const x = 20; }");
+        final JexlContext jc = new MapContext();
+        final Object result = script.execute(jc);
+        Assert.assertEquals(20, result);
+    }
+
     @Test
     public void testConst1() {
         final JexlFeatures f = new JexlFeatures();
         final JexlEngine jexl = new JexlBuilder().strict(true).create();
         try {
             final JexlScript script = jexl.createScript(
-                    "const x;  x + 1");
-            Assert.fail("should fail, x is not defined");
+                    "const foo;  foo");
+            Assert.fail("should fail, const foo must be followed by assign.");
         } catch(JexlException.Parsing xparse) {
-            Assert.assertTrue(xparse.getMessage().contains("x"));
+            Assert.assertTrue(xparse.getMessage().contains("const"));
         }
     }
 
     @Test
-    public void testConst2() {
+    public void testConst2a() {
         final JexlFeatures f = new JexlFeatures();
         final JexlEngine jexl = new JexlBuilder().strict(true).create();
-            final JexlScript script = jexl.createScript( "const x;  x = 1");
-            Object result = script.execute(null);
-            Assert.assertEquals(1, result);
+        for(String op : Arrays.asList("=", "+=", "-=", "/=", "*=", "%=", "<<=", ">>=", ">>>=", "^=", "&=", "|=")) {
+            try {
+                final JexlScript script = jexl.createScript("const foo = 42;  foo "+op+" 1;");
+                Assert.fail("should fail, const precludes assignment");
+            } catch (JexlException.Parsing xparse) {
+                Assert.assertTrue(xparse.getMessage().contains("foo"));
+            }
+        }
     }
 
     @Test
-    public void testConst3() {
+    public void testConst2b() {
         final JexlFeatures f = new JexlFeatures();
         final JexlEngine jexl = new JexlBuilder().strict(true).create();
-        try {
-            final JexlScript script = jexl.createScript( "const x;  x = 1; x = 2;");
-            Assert.fail("should fail, x is not defined");
-        } catch(JexlException.Parsing xparse) {
-            Assert.assertTrue(xparse.getMessage().contains("x"));
+        for(String op : Arrays.asList("=", "+=", "-=", "/=", "*=", "%=", "<<=", ">>=", ">>>=", "^=", "&=", "|=")) {
+            try {
+                final JexlScript script = jexl.createScript("{ const foo = 42; } { let foo  = 0; foo " + op + " 1; }");
+                Assert.assertNotNull(script);
+            } catch(JexlException.Parsing xparse) {
+                Assert.fail(xparse.toString());
+            }
         }
     }