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 2023/02/28 17:56:40 UTC
[commons-jexl] branch master updated: JEXL-393: fix determination of variable const-ness;
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 ed414765 JEXL-393: fix determination of variable const-ness;
new 89f6b414 Merge remote-tracking branch 'origin/master'
ed414765 is described below
commit ed41476588a2e28c661394ccf6827185c9a9a65c
Author: henrib <he...@apache.org>
AuthorDate: Tue Feb 28 18:56:31 2023 +0100
JEXL-393: fix determination of variable const-ness;
---
.../org/apache/commons/jexl3/internal/Scope.java | 12 +++++-
.../org/apache/commons/jexl3/parser/ASTBlock.java | 4 --
.../commons/jexl3/parser/ASTForeachStatement.java | 4 +-
.../commons/jexl3/parser/JexlLexicalNode.java | 7 +---
.../apache/commons/jexl3/parser/JexlParser.java | 47 ++++++++++++++++++++--
.../org/apache/commons/jexl3/Issues300Test.java | 19 +++++++++
.../java/org/apache/commons/jexl3/LexicalTest.java | 39 ++++++++++++++----
7 files changed, 108 insertions(+), 24 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Scope.java b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
index 1572468f..34928110 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java
@@ -239,7 +239,7 @@ public final class Scope {
}
/**
- * Gets the captured index of a given symbol, ie the target index of a symbol in a child frame.
+ * Gets the captured index of a given symbol, ie the target index of a symbol in a child scope.
* @param symbol the symbol index
* @return the target symbol index or null if the symbol is not captured
*/
@@ -255,6 +255,16 @@ public final class Scope {
return null;
}
+ /**
+ * Gets the index of a captured symbol, ie the source index of a symbol in a parent scope.
+ * @param symbol the symbol index
+ * @return the source symbol index or -1 if the symbol is not captured
+ */
+ public int getCaptureDeclaration(final int symbol) {
+ Integer declared = capturedVariables != null? capturedVariables.get(symbol) : null;
+ return declared != null? declared.intValue() : -1;
+ }
+
/**
* Gets this script captured symbols names, i.e. local variables defined in outer scopes and used
* by this scope.
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTBlock.java b/src/main/java/org/apache/commons/jexl3/parser/ASTBlock.java
index d0d82a71..c65004ad 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTBlock.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTBlock.java
@@ -20,10 +20,6 @@ package org.apache.commons.jexl3.parser;
* Declares a block.
*/
public class ASTBlock extends JexlLexicalNode {
-
- /**
- *
- */
private static final long serialVersionUID = 1L;
public ASTBlock(final int id) {
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java b/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java
index cc8880fb..70408346 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java
@@ -20,10 +20,8 @@ package org.apache.commons.jexl3.parser;
* Declares a for each loop.
*/
public class ASTForeachStatement extends JexlLexicalNode {
- /**
- *
- */
private static final long serialVersionUID = 1L;
+ /** for(:)=0 vs for(;;)=1|2|4 form. */
private int loopForm;
void setLoopForm(final int form) {
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 0ba3e2e1..da82733e 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlLexicalNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlLexicalNode.java
@@ -22,11 +22,9 @@ import org.apache.commons.jexl3.internal.LexicalScope;
* Base class for AST nodes behaving as lexical units.
* @since 3.2
*/
-public class JexlLexicalNode extends JexlNode implements JexlParser.LexicalUnit {
- /**
- *
- */
+public abstract class JexlLexicalNode extends JexlNode implements JexlParser.LexicalUnit {
private static final long serialVersionUID = 1L;
+ /** The local lexical scope, local information about let/const. */
private LexicalScope lexicalScope = null;
public JexlLexicalNode(final int id) {
@@ -70,7 +68,6 @@ public class JexlLexicalNode extends JexlNode implements JexlParser.LexicalUnit
return lexicalScope;
}
-
@Override
public void jjtClose() {
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 e771da58..5016d129 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
@@ -30,6 +30,7 @@ import java.util.ArrayDeque;
import java.util.Arrays;
import java.util.Deque;
import java.util.HashSet;
+import java.util.IdentityHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
@@ -87,6 +88,10 @@ public abstract class JexlParser extends StringParser {
* Stack of lexical blocks.
*/
protected final Deque<LexicalUnit> blocks = new ArrayDeque<>();
+ /**
+ * The map of lexical to functional blocks.
+ */
+ protected final Map<LexicalUnit, Scope> blockScopes = new IdentityHashMap<>();
/**
* A lexical unit is the container defining local symbols and their
@@ -260,6 +265,7 @@ public abstract class JexlParser extends StringParser {
* @param unit the new lexical unit
*/
protected void pushUnit(final LexicalUnit unit) {
+ blockScopes.put(unit, scope);
if (block != null) {
blocks.push(block);
}
@@ -272,6 +278,7 @@ public abstract class JexlParser extends StringParser {
*/
protected void popUnit(final LexicalUnit unit) {
if (block == unit){
+ blockScopes.remove(unit);
if (!blocks.isEmpty()) {
block = blocks.pop();
} else {
@@ -629,7 +636,10 @@ public abstract class JexlParser extends StringParser {
* @param node the node
*/
protected void jjtreeOpenNodeScope(final JexlNode node) {
- // nothing
+// if (node instanceof ASTBlock || node instanceof ASTForeachStatement) {
+// final LexicalUnit unit = (LexicalUnit) node;
+// unit.setScope(scope);
+// }
}
/**
@@ -662,9 +672,7 @@ public abstract class JexlParser extends StringParser {
}
if (lv instanceof ASTIdentifier && !(lv instanceof ASTVar)) {
final ASTIdentifier var = (ASTIdentifier) lv;
- final int symbol = var.getSymbol();
- final boolean isconst = symbol >= 0 && block != null && block.isConstant(symbol);
- if (isconst) { // if constant, fail...
+ if (isConstant(var.getSymbol())) { // if constant, fail...
JexlInfo xinfo = lv.jexlInfo();
xinfo = info.at(xinfo.getLine(), xinfo.getColumn());
throw new JexlException.Assignment(xinfo, var.getName()).clean();
@@ -675,6 +683,37 @@ public abstract class JexlParser extends StringParser {
featureController.controlNode(node);
}
+ /**
+ * Checks whether a symbol has been declared as a const in the current stack of lexical units.
+ * @param symbol the symbol
+ * @return true if constant, false otherwise
+ */
+ private boolean isConstant(int symbol) {
+ if (symbol >= 0) {
+ if (block != null && block.isConstant(symbol)) {
+ return true;
+ }
+ Scope blockScope = blockScopes.get(block);
+ for (LexicalUnit unit : blocks) {
+ Scope unitScope = blockScopes.get(unit);
+ // follow through potential capture
+ if (blockScope != unitScope) {
+ int cs = blockScope.getCaptureDeclaration(symbol);
+ if (cs >= 0) {
+ symbol = cs;
+ }
+ if (unitScope != null) {
+ blockScope = unitScope;
+ }
+ }
+ if (unit.isConstant(symbol)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
/**
* Check fat vs thin arrow syntax feature.
* @param token the arrow token
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index 6a418f04..3976e889 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -1236,4 +1236,23 @@ public class Issues300Test {
Assert.assertEquals(6, ja.getCmpCalls());
}
+ @Test
+ public void test393() {
+ String src = "const total = 0;\n" +
+ "if (true) {\n" +
+ " total = 1;\n" +
+ "}\n" +
+ "total; ";
+ JexlEngine jexl = new JexlBuilder()
+ .safe(false)
+ .strict(true)
+ .create();
+ try {
+ JexlScript script = jexl.createScript(src);
+ Assert.fail("should fail on const total assignment");
+ } catch(JexlException.Parsing xparse) {
+ Assert.assertTrue(xparse.getMessage().contains("total"));
+ }
+
+ }
}
diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
index ec7fc179..be97c44d 100644
--- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
@@ -895,23 +895,34 @@ public class LexicalTest {
@Test
- public void testConstFail() {
- List<String> srcs = Arrays.asList(
+ public void testConstCaptures() {
+ List<String> srcsFalse = Arrays.asList(
"const x = 0; x = 1;",
"const x = 0; x *= 1;",
- "cont x = 0; var x = 1;",
- "cont x = 0; if (true) { var x = 1;}" ,
- "cont x = 0; if (true) { x = 1;}" ,
+ "const x = 0; var x = 1;",
+ "const x = 0; if (true) { var x = 1;}" ,
+ "const x = 0; if (true) { x = 1;}" ,
+ "const x = 0; if (true) { var f = y -> { x = y + 1; x } }" ,
+ "const x = 0; if (true) { var f = y -> { z -> { x = y + 1; x } } }" ,
+ "const x = 0; if (true) { if (false) { y -> { x = y + 1; x } } }" ,
+ "const x = 0; if (true) { if (false) { y -> { z -> { x = y + 1; x } } }" ,
""
);
- checkParse(srcs, false);
+ checkParse(srcsFalse, false);
+ List<String> srcsTrue = Arrays.asList(
+ "const x = 0; if (true) { var f = x -> x + 1;}" ,
+ "const x = 0; if (true) { var f = y -> { var x = y + 1; x } }" ,
+ "const x = 0; if (true) { var f = y -> { const x = y + 1; x } }" ,
+ "const x = 0; if (true) { var f = y -> { z -> { let x = y + 1; x } } }" ,
+ "");
+ checkParse(srcsTrue, true);
}
@Test
public void testConst0a() {
final JexlFeatures f = new JexlFeatures();
final JexlEngine jexl = new JexlBuilder().strict(true).create();
- final JexlScript script = jexl.createScript(
+ JexlScript script = jexl.createScript(
"{ const x = 10; x + 1 }; { let x = 20; x = 22}");
final JexlContext jc = new MapContext();
final Object result = script.execute(jc);
@@ -980,6 +991,20 @@ public class LexicalTest {
@Test
public void testConst2b() {
+ final JexlFeatures f = new JexlFeatures();
+ final JexlEngine jexl = new JexlBuilder().strict(true).create();
+ for(String op : Arrays.asList("=", "+=", "-=", "/=", "*=", "%=", "<<=", ">>=", ">>>=", "^=", "&=", "|=")) {
+ try {
+ final JexlScript script = jexl.createScript("const foo = 42; if (true) { foo "+op+" 1; }");
+ Assert.fail("should fail, const precludes assignment");
+ } catch (JexlException.Parsing xparse) {
+ Assert.assertTrue(xparse.getMessage().contains("foo"));
+ }
+ }
+ }
+
+ @Test
+ public void testConst2c() {
final JexlFeatures f = new JexlFeatures();
final JexlEngine jexl = new JexlBuilder().strict(true).create();
for(String op : Arrays.asList("=", "+=", "-=", "/=", "*=", "%=", "<<=", ">>=", ">>>=", "^=", "&=", "|=")) {