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
commit 53e5e77cc9ec42c96840c8f4ae04dc500e0baf63 Author: henrib <hen...@apache.org> AuthorDate: Mon Dec 2 17:09:50 2019 +0100 JEXL-307: fixed lexical feature handling of variable scope in for/lambda units Task #JEXL-307 - Variable redeclaration option --- .../commons/jexl3/parser/ASTForeachStatement.java | 62 ---------------------- .../apache/commons/jexl3/parser/JexlParser.java | 44 +++++++-------- .../org/apache/commons/jexl3/parser/Parser.jjt | 15 +++--- .../java/org/apache/commons/jexl3/LexicalTest.java | 14 +++++ 4 files changed, 42 insertions(+), 93 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java b/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java deleted file mode 100644 index 1ad61c0..0000000 --- a/src/main/java/org/apache/commons/jexl3/parser/ASTForeachStatement.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.commons.jexl3.parser; - -import org.apache.commons.jexl3.internal.LexicalScope; - -/** - * Declares a local variable. - */ -public class ASTForeachStatement extends JexlNode implements JexlParser.LexicalUnit { - private LexicalScope locals = null; - - public ASTForeachStatement(int id) { - super(id); - } - - public ASTForeachStatement(Parser p, int id) { - super(p, id); - } - - @Override - public Object jjtAccept(ParserVisitor visitor, Object data) { - return visitor.visit(this, data); - } - - @Override - public boolean declareSymbol(int symbol) { - if (locals == null) { - locals = new LexicalScope(null); - } - return locals.declareSymbol(symbol); - } - - @Override - public int getSymbolCount() { - return locals == null? 0 : locals.getSymbolCount(); - } - - @Override - public boolean hasSymbol(int symbol) { - return locals == null? false : locals.hasSymbol(symbol); - } - - @Override - public void clearUnit() { - locals = null; - } -} \ No newline at end of file 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 4aa2c05..d32feb5 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java +++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java @@ -61,7 +61,7 @@ public abstract class JexlParser extends StringParser { /** * When parsing inner functions/lambda, need to stack the scope (sic). */ - protected Deque<Scope> frames = new ArrayDeque<Scope>(); + protected final Deque<Scope> frames = new ArrayDeque<Scope>(); /** * The list of pragma declarations. */ @@ -73,7 +73,7 @@ public abstract class JexlParser extends StringParser { /** * Stack of parsing loop counts. */ - protected Deque<Integer> loopCounts = new ArrayDeque<Integer>(); + protected final Deque<Integer> loopCounts = new ArrayDeque<Integer>(); /** * Lexical unit merge, next block push is swallowed. */ @@ -85,7 +85,7 @@ public abstract class JexlParser extends StringParser { /** * Stack of lexical blocks. */ - protected Deque<LexicalUnit> blocks = new ArrayDeque<LexicalUnit>(); + protected final Deque<LexicalUnit> blocks = new ArrayDeque<LexicalUnit>(); /** * A lexical unit is the container defining local symbols and their @@ -242,19 +242,9 @@ public abstract class JexlParser extends StringParser { /** * Pushes a new lexical unit. - * <p>The merge flag allows the for(...) and lamba(...) constructs to - * merge in the next block since their loop-variable/parameter spill in the - * same lexical unit as their first block. * @param unit the new lexical unit - * @param merge whether the next unit merges in this one - */ - protected void pushUnit(LexicalUnit unit, boolean merge) { - if (merge) { - mergeBlock = true; - } else if (mergeBlock) { - mergeBlock = false; - return; - } + */ + protected void pushUnit(LexicalUnit unit) { if (block != null) { blocks.push(block); } @@ -262,14 +252,6 @@ public abstract class JexlParser extends StringParser { } /** - * Pushes a block as new lexical unit. - * @param unit the lexical unit - */ - protected void pushUnit(LexicalUnit unit) { - pushUnit(unit, false); - } - - /** * Restores the previous lexical unit. * @param unit restores the previous lexical scope */ @@ -295,8 +277,20 @@ public abstract class JexlParser extends StringParser { Integer symbol = frame.getSymbol(name); if (symbol != null) { // can not reuse a local as a global - if (!block.hasSymbol(symbol) && getFeatures().isLexical()) { - throw new JexlException(identifier, name + ": variable is not defined"); + if (getFeatures().isLexical()) { + // one of the lexical blocks above must declare it + if (!block.hasSymbol(symbol)) { + boolean declared = false; + for (LexicalUnit u : blocks) { + if (u.hasSymbol(symbol)) { + declared = true; + break; + } + } + if (!declared) { + throw new JexlException(identifier, name + ": variable is not defined"); + } + } } identifier.setSymbol(symbol, name); } 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 cdb19ca..a750261 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt +++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt @@ -296,7 +296,7 @@ ASTJexlScript JexlScript(Scope frame) : { } { { - pushUnit(jjtThis, frame != null && frame.getArgCount() > 0); + pushUnit(jjtThis); } ( ( Statement() )*) <EOF> { @@ -310,7 +310,7 @@ ASTJexlScript JexlExpression(Scope frame) #JexlScript : { } { { - pushUnit(jjtThis, true); + pushUnit(jjtThis); } ( Expression() )? <EOF> { @@ -399,7 +399,10 @@ void Break() #Break : { void ForeachStatement() : {} { - { pushUnit(jjtThis, true); } <FOR> <LPAREN> ForEachVar() <COLON> Expression() <RPAREN> { loopCount += 1; } (LOOKAHEAD(1) Block() | Statement()) { loopCount -= 1; popUnit(jjtThis); } + <FOR> <LPAREN> ForEachVar() <COLON> Expression() <RPAREN> + { loopCount += 1; } + (LOOKAHEAD(1) Block() | Statement()) + { loopCount -= 1; } } void ForEachVar() #Reference : {} @@ -834,11 +837,11 @@ void Lambda() #JexlLambda() : pushFrame(); } { - { pushUnit(jjtThis, true); } <FUNCTION> Parameters() Block() { popUnit(jjtThis); } + { pushUnit(jjtThis); } <FUNCTION> Parameters() Block() { popUnit(jjtThis); } | - { pushUnit(jjtThis, true); } Parameters() <LAMBDA> Block() { popUnit(jjtThis); } + { pushUnit(jjtThis); } Parameters() <LAMBDA> Block() { popUnit(jjtThis); } | - { pushUnit(jjtThis, true); } Parameter() <LAMBDA> Block() { popUnit(jjtThis); } + { pushUnit(jjtThis); } Parameter() <LAMBDA> Block() { popUnit(jjtThis); } } diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java index 8a6273f..64cfe6b 100644 --- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java +++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java @@ -480,4 +480,18 @@ public class LexicalTest { Object result = s42.execute(jc); Assert.assertEquals(42, result); } + + @Test + public void testInnerAccess0() throws Exception { + JexlFeatures f = new JexlFeatures(); + f.lexical(true); + JexlEngine jexl = new JexlBuilder().strict(true).features(f).create(); + JexlScript script = jexl.createScript("var x = 32; (()->{ for(var x : null) { var c = 0; {return x; }} })();"); + } + + @Test + public void testInnerAccess1() throws Exception { + JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).create(); + JexlScript script = jexl.createScript("var x = 32; (()->{ for(var x : null) { var c = 0; {return x; }} })();"); + } }