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 2019/11/05 21:42:45 UTC
[commons-jexl] branch master updated: JEXL-307: clean up lexical
frames after usage,
fixed template & for-loops lexical handling Task #JEXL-307 - Variable
redeclaration option
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 0a9ddb9 JEXL-307: clean up lexical frames after usage, fixed template & for-loops lexical handling Task #JEXL-307 - Variable redeclaration option
0a9ddb9 is described below
commit 0a9ddb9065a1c25a80b99c05bbad126845c4d16f
Author: henrib <he...@apache.org>
AuthorDate: Tue Nov 5 22:41:57 2019 +0100
JEXL-307: clean up lexical frames after usage, fixed template & for-loops lexical handling
Task #JEXL-307 - Variable redeclaration option
---
.../apache/commons/jexl3/internal/Interpreter.java | 23 +++----
.../commons/jexl3/internal/InterpreterBase.java | 5 +-
.../commons/jexl3/internal/LexicalFrame.java | 25 +++++--
.../commons/jexl3/internal/LexicalScope.java | 10 ++-
.../jexl3/internal/TemplateInterpreter.java | 22 +++----
.../java/org/apache/commons/jexl3/JXLTTest.java | 77 ++++++++++++++++++++--
.../java/org/apache/commons/jexl3/LexicalTest.java | 48 ++++++++++++++
7 files changed, 172 insertions(+), 38 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
index a673f56..0dbbcfa 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -169,9 +169,6 @@ public class Interpreter extends InterpreterBase {
* @throws JexlException if any error occurs during interpretation.
*/
public Object interpret(JexlNode node) {
- return interpret(node, false);
- }
- public Object interpret(JexlNode node, boolean rethrow) {
JexlContext.ThreadLocal tcontext = null;
JexlEngine tjexl = null;
Interpreter tinter = null;
@@ -205,7 +202,7 @@ public class Interpreter extends InterpreterBase {
throw xcancel.clean();
}
} catch (JexlException xjexl) {
- if (rethrow || !isSilent()) {
+ if (!isSilent()) {
throw xjexl.clean();
}
if (logger.isWarnEnabled()) {
@@ -674,14 +671,14 @@ public class Interpreter extends InterpreterBase {
ASTReference loopReference = (ASTReference) node.jjtGetChild(0);
ASTIdentifier loopVariable = (ASTIdentifier) loopReference.jjtGetChild(0);
final int symbol = loopVariable.getSymbol();
- final LexicalFrame lexical = block;
- if (options.isLexical()) {
- // the iteration variable can not be declared in parent block
- if (symbol >= 0 && block.hasSymbol(symbol)) {
+ final boolean lexical = options.isLexical() && symbol >= 0;
+ if (lexical) {
+ // create lexical frame
+ LexicalFrame locals = new LexicalFrame(frame, block);
+ if (!locals.declareSymbol(symbol)) {
return redefinedVariable(node, loopVariable.getName());
}
- // create lexical frame
- block = new LexicalFrame(frame, lexical);
+ block = locals;
}
Object forEach = null;
try {
@@ -723,8 +720,8 @@ public class Interpreter extends InterpreterBase {
// closeable iterator handling
closeIfSupported(forEach);
// restore lexical frame
- if (lexical != null && block != null) {
- block = lexical;
+ if (lexical) {
+ block = block.pop();
}
}
return result;
@@ -987,7 +984,7 @@ public class Interpreter extends InterpreterBase {
block = new LexicalFrame(frame, block).declareArgs();
try {
JexlNode body = script.jjtGetChild(script.jjtGetNumChildren() - 1);
- return interpret(body, true);
+ return interpret(body);
} finally {
block = block.pop();
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
index 7727d55..a41192f 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
@@ -252,7 +252,10 @@ public abstract class InterpreterBase extends ParserVisitor {
return undefinedVariable(identifier, identifier.getName());
}
}
- return frame.get(symbol);
+ Object value = frame.get(symbol);
+ if (value != Scope.UNDEFINED) {
+ return value;
+ }
}
}
String name = identifier.getName();
diff --git a/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java b/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java
index 1deb14a..d2b341e 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalFrame.java
@@ -30,19 +30,23 @@ public class LexicalFrame extends LexicalScope {
private Deque<Object> stack = null;
/**
* Lexical frame ctor.
- * @param frame the script frame
+ * @param scriptf the script frame
* @param previous the previous lexical frame
*/
- public LexicalFrame(Frame frame, LexicalFrame previous) {
+ public LexicalFrame(Frame scriptf, LexicalFrame previous) {
super(previous);
- this.frame = frame;
+ this.frame = scriptf;
}
+ /**
+ * Declare the arguments.
+ * @return the number of arguments
+ */
public LexicalFrame declareArgs() {
if (frame != null) {
int argc = frame.getScope().getArgCount();
for(int a = 0; a < argc; ++a) {
- super.registerSymbol(a);
+ super.addSymbol(a);
}
}
return this;
@@ -70,6 +74,19 @@ public class LexicalFrame extends LexicalScope {
* @return the previous frame
*/
public LexicalFrame pop() {
+ long clean = symbols;
+ // undefine symbols getting out of scope
+ while (clean != 0L) {
+ int s = Long.numberOfTrailingZeros(clean);
+ clean &= ~(1L << s);
+ frame.set(s, Scope.UNDEFINED);
+ }
+ if (moreSymbols != null) {
+ for (int s = moreSymbols.nextSetBit(0); s != -1; s = moreSymbols.nextSetBit(s + 1)) {
+ frame.set(s, Scope.UNDEFINED);
+ }
+ }
+ // restore values of hoisted symbols that were overwritten
if (stack != null) {
while(!stack.isEmpty()) {
Object value = stack.pop();
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 a6e6fff..3ab83ad 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
@@ -35,7 +35,6 @@ public class LexicalScope {
/**
* Create a scope.
- * @param frame the current execution frame
* @param scope the previous scope
*/
public LexicalScope(LexicalScope scope) {
@@ -80,10 +79,15 @@ public class LexicalScope {
}
walk = walk.previous;
}
- return registerSymbol(symbol);
+ return addSymbol(symbol);
}
- protected final boolean registerSymbol(int symbol) {
+ /**
+ * Adds a symbol in this scope.
+ * @param symbol the symbol
+ * @return true if registered, false if symbol was already registered
+ */
+ protected final boolean addSymbol(int symbol) {
if (symbol < LONGBITS) {
if ((symbols & (1L << symbol)) != 0L) {
return false;
diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
index 3ae8017..17b0ff9 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
@@ -51,6 +51,7 @@ public class TemplateInterpreter extends Interpreter {
super(jexl, jcontext, jframe);
exprs = expressions;
writer = out;
+ block = new LexicalFrame(frame, null);
}
/**
@@ -153,20 +154,15 @@ public class TemplateInterpreter extends Interpreter {
}
};
}
- block = new LexicalFrame(frame, block).declareArgs();
- try {
- // otherwise...
- final int numChildren = node.jjtGetNumChildren();
- Object result = null;
- for (int i = 0; i < numChildren; i++) {
- JexlNode child = node.jjtGetChild(i);
- result = child.jjtAccept(this, data);
- cancelCheck(child);
- }
- return result;
- } finally {
- block = block.pop();
+ // otherwise...
+ final int numChildren = node.jjtGetNumChildren();
+ Object result = null;
+ for (int i = 0; i < numChildren; i++) {
+ JexlNode child = node.jjtGetChild(i);
+ result = child.jjtAccept(this, data);
+ cancelCheck(child);
}
+ return result;
}
}
diff --git a/src/test/java/org/apache/commons/jexl3/JXLTTest.java b/src/test/java/org/apache/commons/jexl3/JXLTTest.java
index ca085fc..c59193d 100644
--- a/src/test/java/org/apache/commons/jexl3/JXLTTest.java
+++ b/src/test/java/org/apache/commons/jexl3/JXLTTest.java
@@ -19,6 +19,7 @@ package org.apache.commons.jexl3;
import org.apache.commons.jexl3.internal.TemplateDebugger;
import org.apache.commons.jexl3.internal.TemplateScript;
import org.apache.commons.jexl3.internal.Debugger;
+import org.apache.commons.jexl3.internal.Options;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -787,10 +788,27 @@ public class JXLTTest extends JexlTestCase {
}
}
- public static class Context311 extends MapContext {
+ public static class Context311 extends MapContext
+ implements JexlContext.OptionsHandle, JexlContext.ThreadLocal {
+ private JexlOptions options = null;
+
+ public void setOptions(JexlOptions o) {
+ options = o;
+ }
+
public Executor311 exec(String name) {
return new Executor311(name);
}
+
+ @Override
+ public JexlOptions getEngineOptions() {
+ return options;
+ }
+
+ JexlOptions newOptions() {
+ options = new Options();
+ return options;
+ }
}
@Test
@@ -823,7 +841,8 @@ public class JXLTTest extends JexlTestCase {
@Test
public void test311c() throws Exception {
- JexlContext ctx311 = new Context311();
+ Context311 ctx311 = new Context311();
+ ctx311.newOptions().setLexical(true);
String rpt
= "$$ exec('42').execute((a)->{"
+ "\n<p>Universe ${a}</p>"
@@ -837,7 +856,8 @@ public class JXLTTest extends JexlTestCase {
@Test
public void test311d() throws Exception {
- JexlContext ctx311 = new Context311();
+ Context311 ctx311 = new Context311();
+ ctx311.newOptions().setLexical(true);
String rpt
= "$$ exec('4').execute((a, b)->{"
+ "\n<p>Universe ${a}${b}</p>"
@@ -848,9 +868,58 @@ public class JXLTTest extends JexlTestCase {
String output = strw.toString();
Assert.assertEquals("<p>Universe 42</p>\n", output);
}
-
+
@Test
public void test311e() throws Exception {
+ Context311 ctx311 = new Context311();
+ ctx311.newOptions().setLexical(true);
+ String rpt
+ = "exec('4').execute((a, b)->{"
+ + " '<p>Universe ' + a + b + '</p>'"
+ + "}, '2')";
+ JexlScript script = JEXL.createScript(rpt);
+ String output = script.execute(ctx311, 42).toString();
+ Assert.assertEquals("<p>Universe 42</p>", output);
+ }
+
+ @Test
+ public void test311f() throws Exception {
+ Context311 ctx311 = new Context311();
+ ctx311.newOptions().setLexical(true);
+ String rpt
+ = "exec('4').execute((a, b)->{"
+ + " `<p>Universe ${a}${b}</p>`"
+ + "}, '2')";
+ JexlScript script = JEXL.createScript(rpt);
+ String output = script.execute(ctx311, 42).toString();
+ Assert.assertEquals("<p>Universe 42</p>", output);
+ }
+
+ @Test
+ public void test311g() throws Exception {
+ Context311 ctx311 = new Context311();
+ ctx311.newOptions().setLexical(true);
+ String rpt
+ = "(a, b)->{"
+ + " `<p>Universe ${a}${b}</p>`"
+ + "}";
+ JexlScript script = JEXL.createScript(rpt);
+ String output = script.execute(ctx311, "4", "2").toString();
+ Assert.assertEquals("<p>Universe 42</p>", output);
+ }
+
+ @Test
+ public void test311h() throws Exception {
+ Context311 ctx311 = new Context311();
+ ctx311.newOptions().setLexical(true);
+ String rpt= " `<p>Universe ${a}${b}</p>`";
+ JexlScript script = JEXL.createScript(rpt, "a", "b");
+ String output = script.execute(ctx311, "4", "2").toString();
+ Assert.assertEquals("<p>Universe 42</p>", output);
+ }
+
+ @Test
+ public void test311i() throws Exception {
JexlContext ctx311 = new Context311();
String rpt
= "$$var u = 'Universe'; exec('4').execute((a, b)->{"
diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
index fff27bd..43c59bd 100644
--- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
@@ -296,4 +296,52 @@ public class LexicalTest {
Assert.fail(ww);
}
}
+
+ @Test
+ public void testLexical6a() throws Exception {
+ String str = "i = 0; { var i = 32; }; i";
+ JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).create();
+ JexlScript e = jexl.createScript(str);
+ JexlContext ctxt = new MapContext();
+ Object o = e.execute(ctxt);
+ Assert.assertEquals(0, o);
+ }
+
+ @Test
+ public void testLexical6b() throws Exception {
+ String str = "i = 0; { var i = 32; }; i";
+ JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).lexicalShade(true).create();
+ JexlScript e = jexl.createScript(str);
+ JexlContext ctxt = new MapContext();
+ try {
+ Object o = e.execute(ctxt);
+ Assert.fail("i should be shaded");
+ } catch (JexlException xany) {
+ Assert.assertNotNull(xany);
+ }
+ }
+
+ @Test
+ public void testLexical6c() throws Exception {
+ String str = "i = 0; for (var i : [42]) i; i";
+ JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).create();
+ JexlScript e = jexl.createScript(str);
+ JexlContext ctxt = new MapContext();
+ Object o = e.execute(ctxt);
+ Assert.assertEquals(0, o);
+ }
+
+ @Test
+ public void testLexical6d() throws Exception {
+ String str = "i = 0; for (var i : [42]) i;; i";
+ JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).lexicalShade(true).create();
+ JexlScript e = jexl.createScript(str);
+ JexlContext ctxt = new MapContext();
+ try {
+ Object o = e.execute(ctxt);
+ Assert.fail("i should be shaded");
+ } catch (JexlException xany) {
+ Assert.assertNotNull(xany);
+ }
+ }
}