This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch JEXL-369 in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit 75f9fe97b336f6061d0b1c2d9c7affc2cc817783 Author: henrib <hen...@apache.org> AuthorDate: Fri May 6 18:14:23 2022 +0200 JEXL-369: initial feature drop, lacks serious tests; --- .../apache/commons/jexl3/internal/Debugger.java | 8 ++- .../apache/commons/jexl3/internal/Engine32.java | 2 +- .../org/apache/commons/jexl3/internal/Frame.java | 7 +++ .../apache/commons/jexl3/internal/Interpreter.java | 10 ++-- .../commons/jexl3/internal/InterpreterBase.java | 8 ++- .../org/apache/commons/jexl3/internal/Scope.java | 51 ++++++++++++++++++- .../apache/commons/jexl3/parser/ASTIdentifier.java | 20 ++++++++ .../apache/commons/jexl3/parser/JexlParser.java | 31 +++++++++--- .../org/apache/commons/jexl3/parser/Parser.jjt | 15 +++--- .../org/apache/commons/jexl3/JexlTestCase.java | 13 +++++ .../java/org/apache/commons/jexl3/LexicalTest.java | 57 ++++++++++++++++++++-- 11 files changed, 193 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java index dcd6ea4b..106a060f 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java @@ -962,7 +962,13 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail { @Override protected Object visit(final ASTVar node, final Object data) { - builder.append("var "); + if (node.isConstant()) { + builder.append("const "); + } else if (node.isLexical()) { + builder.append("let "); + } else { + builder.append("var "); + } check(node, node.getName(), data); return data; } diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine32.java b/src/main/java/org/apache/commons/jexl3/internal/Engine32.java index 66235b4d..f478c8ec 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Engine32.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Engine32.java @@ -76,7 +76,7 @@ public class Engine32 extends Engine { static Object getVariable(Interpreter ii, Frame frame, LexicalScope block, ASTIdentifier identifier) { int symbol = identifier.getSymbol(); // if we have a symbol, we have a scope thus a frame - if (ii.options.isLexicalShade() && identifier.isShaded()) { + if ((ii.options.isLexicalShade() || identifier.isLexical()) && identifier.isShaded()) { return ii.undefinedVariable(identifier, identifier.getName()); } if (symbol >= 0) { diff --git a/src/main/java/org/apache/commons/jexl3/internal/Frame.java b/src/main/java/org/apache/commons/jexl3/internal/Frame.java index 6fc58d97..fe2b1718 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Frame.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Frame.java @@ -58,6 +58,13 @@ public final class Frame { return scope; } + boolean isLexical(int symbol) { + return scope != null && scope.isLexical(symbol); + } + boolean isConstant(int symbol) { + return scope != null && scope.isConstant(symbol); + } + @Override public int hashCode() { return Arrays.deepHashCode(this.stack); 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 b3c1ed84..19358524 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -564,7 +564,7 @@ public class Interpreter extends InterpreterBase { protected Object visit(final ASTVar node, final Object data) { final int symbol = node.getSymbol(); // if we have a var, we have a scope thus a frame - if (!options.isLexical()) { + if (!options.isLexical() && !node.isLexical()) { if (frame.has(symbol)) { return frame.get(symbol); } @@ -578,7 +578,7 @@ public class Interpreter extends InterpreterBase { @Override protected Object visit(final ASTBlock node, final Object data) { final int cnt = node.getSymbolCount(); - if (!options.isLexical() || cnt <= 0) { + if (cnt <= 0) { return visitBlock(node, data); } try { @@ -629,7 +629,7 @@ public class Interpreter extends InterpreterBase { final ASTReference loopReference = (ASTReference) node.jjtGetChild(0); final ASTIdentifier loopVariable = (ASTIdentifier) loopReference.jjtGetChild(0); final int symbol = loopVariable.getSymbol(); - final boolean lexical = options.isLexical();// && node.getSymbolCount() > 0; + final boolean lexical = loopVariable.isLexical() || options.isLexical() ;// && node.getSymbolCount() > 0; final LexicalFrame locals = lexical? new LexicalFrame(frame, block) : null; final boolean loopSymbol = symbol >= 0 && loopVariable instanceof ASTVar; if (lexical) { @@ -1274,12 +1274,12 @@ public class Interpreter extends InterpreterBase { if (left instanceof ASTIdentifier) { var = (ASTIdentifier) left; symbol = var.getSymbol(); - if (symbol >= 0 && options.isLexical()) { + if (symbol >= 0 && (var.isLexical() || options.isLexical())) { if (var instanceof ASTVar) { if (!defineVariable((ASTVar) var, block)) { return redefinedVariable(var, var.getName()); } - } else if (options.isLexicalShade() && var.isShaded()) { + } else if (var.isShaded() && (var.isLexical() || options.isLexicalShade())) { return undefinedVariable(var, var.getName()); } } 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 f09ca21f..aa5f0e3b 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java +++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java @@ -328,7 +328,7 @@ public abstract class InterpreterBase extends ParserVisitor { final int symbol = identifier.getSymbol(); final String name = identifier.getName(); // if we have a symbol, we have a scope thus a frame - if (options.isLexicalShade() && identifier.isShaded()) { + if ((options.isLexicalShade() || identifier.isLexical()) && identifier.isShaded()) { return undefinedVariable(identifier, name); } // a local var ? @@ -371,7 +371,11 @@ public abstract class InterpreterBase extends ParserVisitor { * @param value the variable value */ protected void setContextVariable(final JexlNode node, final String name, final Object value) { - if (options.isLexicalShade() && !context.has(name)) { + boolean lexical = options.isLexicalShade(); + if (!lexical && node instanceof ASTIdentifier) { + lexical = ((ASTIdentifier) node).isLexical(); + } + if (lexical && !context.has(name)) { throw new JexlException.Variable(node, name, true); } try { 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 733c251a..59dddb9f 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java @@ -18,6 +18,7 @@ package org.apache.commons.jexl3.internal; import java.util.ArrayList; import java.util.Arrays; +import java.util.BitSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -66,6 +67,32 @@ public final class Scope { * The map of local captured variables to parent scope variables, ie closure. */ private Map<Integer, Integer> capturedVariables = null; + + private LexicalScope constVariables = null; + private LexicalScope lexicalVariables = null; + + void addLexical(int s) { + if (lexicalVariables == null) { + lexicalVariables = new LexicalScope(); + } + lexicalVariables.addSymbol(s); + } + + public boolean isLexical(int s) { + return lexicalVariables != null && s >= 0 && lexicalVariables.hasSymbol(s); + } + + void addConstant(int s) { + if (constVariables == null) { + constVariables = new LexicalScope(); + } + constVariables.addSymbol(s); + } + + public boolean isConstant(int s) { + return constVariables != null && s >= 0 && constVariables.hasSymbol(s); + } + /** * The empty string array. */ @@ -143,6 +170,12 @@ public final class Scope { register = namedVariables.size(); namedVariables.put(name, register); capturedVariables.put(register, pr); + if (parent.isLexical(pr)) { + this.addLexical(register); + if (parent.isConstant(pr)) { + this.addConstant(register); + } + } } } return register; @@ -188,7 +221,7 @@ public final class Scope { * @param name the variable name * @return the register index storing this variable */ - public int declareVariable(final String name) { + public int declareVariable(final String name, boolean lexical, boolean constant) { if (namedVariables == null) { namedVariables = new LinkedHashMap<String, Integer>(); } @@ -207,6 +240,22 @@ public final class Scope { capturedVariables.put(register, pr); } } + if (lexical) { + addLexical(register); + if (constant) { + addConstant(register); + } + } + } else { + // belt and suspenders + if (lexical) { + if (!isLexical(register)) { + throw new IllegalStateException("cant redefine lexical variable"); + } + if (constant && !isConstant(register)) { + throw new IllegalStateException("cant redefine const variable"); + } + } } return register; } 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 427078f2..944b5915 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifier.java +++ b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifier.java @@ -30,6 +30,10 @@ public class ASTIdentifier extends JexlNode { private static final int SHADED = 1; /** The captured variable flag. */ private static final int CAPTURED = 2; + /** The lexical variable flag. */ + private static final int LEXICAL = 3; + /** The const variable flag. */ + private static final int CONST = 4; ASTIdentifier(final int id) { super(id); @@ -105,6 +109,22 @@ public class ASTIdentifier extends JexlNode { return isSet(CAPTURED, flags); } + public boolean isLexical() { + return isSet(LEXICAL, flags); + } + + public void setLexical(final boolean f) { + flags = set(LEXICAL, flags, f); + } + + public boolean isConstant() { + return isSet(CONST, flags); + } + + public void setConstant(final boolean f) { + flags = set(CONST, flags, f); + } + public String getName() { return name; } 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 5151cbac..274fbbf6 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java +++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java @@ -309,6 +309,8 @@ public abstract class JexlParser extends StringParser { if (frame != null) { final Integer symbol = frame.getSymbol(name); if (symbol != null) { + identifier.setLexical(frame.isLexical(symbol)); + identifier.setConstant(frame.isConstant(symbol)); boolean declared = true; if (frame.isCapturedSymbol(symbol)) { // captured are declared in all cases @@ -331,7 +333,7 @@ public abstract class JexlParser extends StringParser { identifier.setSymbol(symbol, name); if (!declared) { identifier.setShaded(true); - if (getFeatures().isLexicalShade()) { + if (identifier.isLexical() || getFeatures().isLexicalShade()) { // can not reuse a local as a global throw new JexlException(identifier, name + ": variable is not defined"); } @@ -382,7 +384,7 @@ public abstract class JexlParser extends StringParser { * @param variable the identifier used to declare * @param token the variable name toekn */ - protected void declareVariable(final ASTVar variable, final Token token, boolean lexical) { + protected void declareVariable(final ASTVar variable, final Token token, boolean lexical, boolean constant) { final String name = token.image; if (!allowVariable(name)) { throwFeatureException(JexlFeatures.LOCAL_VAR, token); @@ -390,7 +392,7 @@ public abstract class JexlParser extends StringParser { if (frame == null) { frame = new Scope(null, (String[]) null); } - final int symbol = frame.declareVariable(name); + final int symbol = frame.declareVariable(name, lexical, constant); variable.setSymbol(symbol, name); if (frame.isCapturedSymbol(symbol)) { variable.setCaptured(true); @@ -402,6 +404,8 @@ public abstract class JexlParser extends StringParser { } variable.setRedefined(true); } + variable.setLexical(lexical); + variable.setConstant(constant); } /** @@ -513,12 +517,16 @@ public abstract class JexlParser extends StringParser { Arrays.asList( ASTAssignment.class, ASTSetAddNode.class, + ASTSetSubNode.class, ASTSetMultNode.class, ASTSetDivNode.class, + ASTSetModNode.class, ASTSetAndNode.class, ASTSetOrNode.class, ASTSetXorNode.class, - ASTSetSubNode.class + ASTSetShiftLeftNode.class, + ASTSetShiftRightNode.class, + ASTSetShiftRightUnsignedNode.class ) ); @@ -556,6 +564,13 @@ public abstract class JexlParser extends StringParser { if (!lv.isLeftValue()) { throwParsingException(JexlException.Assignment.class, null); } + if (lv instanceof ASTIdentifier) { + ASTIdentifier var = (ASTIdentifier) lv; + int symbol = var.getSymbol(); + if (symbol >= 0 && frame.isConstant(symbol)) { + throwParsingException(JexlException.Assignment.class, null); + } + } } // heavy check featureController.controlNode(node); @@ -572,7 +587,7 @@ public abstract class JexlParser extends StringParser { final Token t = getToken(0); final JexlInfo end = info.at(t.beginLine, t.endColumn); final String msg = readSourceLine(source, end.getLine()); - throw new JexlException.Ambiguous(begin, end, msg); + throw new JexlException.Ambiguous(begin, end, msg).clean(); } /** @@ -583,7 +598,7 @@ public abstract class JexlParser extends StringParser { */ protected void throwFeatureException(final int feature, final JexlInfo info) { final String msg = info != null? readSourceLine(source, info.getLine()) : null; - throw new JexlException.Feature(info, feature, msg); + throw new JexlException.Feature(info, feature, msg).clean(); } /** @@ -626,14 +641,14 @@ public abstract class JexlParser extends StringParser { if (xclazz != null) { try { final Constructor<T> ctor = xclazz.getConstructor(JexlInfo.class, String.class); - xparse = ctor.newInstance(xinfo, msg); + xparse = (JexlException.Parsing) ctor.newInstance(xinfo, msg).clean(); } catch (final Exception xany) { // ignore, very unlikely but then again.. } } } // unlikely but safe - throw xparse != null ? xparse : new JexlException.Parsing(xinfo, msg); + throw xparse != null ? xparse : new JexlException.Parsing(xinfo, msg).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 94efc9d1..baf31d30 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt +++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt @@ -128,6 +128,7 @@ TOKEN_MGR_DECLS : { | < NEW : "new" > { popDot(); } | < VAR : "var" > { popDot(); } | < LET : "let" > { popDot(); } + | < CONST : "const" > { popDot(); } | < EMPTY : "empty" > { popDot(); } /* Revert state to default if was DOT_ID. */ | < SIZE : "size" > { popDot(); } /* Revert state to default if was DOT_ID. */ | < NULL : "null" > { popDot(); } @@ -422,26 +423,28 @@ void ForeachStatement() : {} void ForEachVar() #Reference : {} { - <VAR> DeclareVar(false) + <VAR> DeclareVar(false, false) | - <LET> DeclareVar(true) + <LET> DeclareVar(true, false) | Identifier(true) } void Var() #void : {} { - <VAR> DeclareVar(false) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))? + <VAR> DeclareVar(false, false) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))? | - <LET> DeclareVar(true) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))? + <LET> DeclareVar(true, false) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))? + | + <CONST> DeclareVar(true, true) (LOOKAHEAD(1) <assign> Expression() #Assignment(2))? } -void DeclareVar(boolean lexical) #Var : +void DeclareVar(boolean lexical, boolean constant) #Var : { Token t; } { - t=<IDENTIFIER> { declareVariable(jjtThis, t, lexical); } + t=<IDENTIFIER> { declareVariable(jjtThis, t, lexical, constant); } } void Pragma() #void : diff --git a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java index 8a04c975..5c63c58d 100644 --- a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java +++ b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java @@ -107,6 +107,19 @@ public class JexlTestCase { } } + /** + * Compare strings ignoring white space differences. + * <p>This replaces any sequence of whitespaces (ie \\s) by one space (ie ASCII 32) in both + * arguments then compares them.</p> + * @param lhs left hand side + * @param rhs right hand side + * @return true if strings are equal besides whitespace + */ + public static boolean equalsIgnoreWhiteSpace(String lhs, String rhs) { + String lhsw = lhs.trim().replaceAll("\\s+", ""); + String rhsw = rhs.trim().replaceAll("\\s+", ""); + return lhsw.equals(rhsw); + } /** * A very secure singleton. diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java index deff63c2..5f589b00 100644 --- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java +++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java @@ -506,13 +506,35 @@ public class LexicalTest { } @Test - public void testInnerAccess1() throws Exception { + public void testInnerAccess1a() throws Exception { final JexlEngine jexl = new JexlBuilder().strict(true).lexical(true).create(); final JexlScript script = jexl.createScript("var x = 32; (()->{ for(var x : null) { var c = 0; {return x; }} })();"); + Assert.assertNotNull(script); } @Test - public void testForVariable0() throws Exception { + public void testInnerAccess1b() throws Exception { + final JexlEngine jexl = new JexlBuilder().strict(true).create(); + final JexlScript script = jexl.createScript("let x = 32; (()->{ for(let x : null) { let c = 0; { return x; } } } )(); "); + Assert.assertNotNull(script); + String dbg = script.getParsedText(); + String src = script.getSourceText(); + Assert.assertTrue(JexlTestCase.equalsIgnoreWhiteSpace(src, dbg)); + } + + @Test + public void testForVariable0a() throws Exception { + final JexlEngine jexl = new JexlBuilder().strict(true).create(); + try { + final JexlScript script = jexl.createScript("for(let x : 1..3) { let c = 0}; return x"); + Assert.fail("Should not have been parsed"); + } catch (final JexlException ex) { + // OK + } + } + + @Test + public void testForVariable0b() throws Exception { final JexlFeatures f = new JexlFeatures(); f.lexical(true); f.lexicalShade(true); @@ -521,12 +543,12 @@ public class LexicalTest { final JexlScript script = jexl.createScript("for(var x : 1..3) { var c = 0}; return x"); Assert.fail("Should not have been parsed"); } catch (final JexlException ex) { - // OK + // OK } } @Test - public void testForVariable1() throws Exception { + public void testForVariable1a() throws Exception { final JexlFeatures f = new JexlFeatures(); f.lexical(true); f.lexicalShade(true); @@ -540,6 +562,18 @@ public class LexicalTest { } } + @Test + public void testForVariable1b() throws Exception { + final JexlEngine jexl = new JexlBuilder().strict(true).create(); + try { + final JexlScript script = jexl.createScript("for(let x : 1..3) { let c = 0} for(let x : 1..3) { var c = 0}; return x"); + Assert.fail("Should not have been parsed"); + } catch (final JexlException ex) { + // OK + Assert.assertTrue(ex instanceof JexlException); + } + } + @Test public void testUndeclaredVariable() throws Exception { final JexlFeatures f = new JexlFeatures(); @@ -762,10 +796,23 @@ public class LexicalTest { f.lexical(true); final JexlEngine jexl = new JexlBuilder().strict(true).features(f).create(); final JexlScript script = jexl.createScript( - "{var x = 10; } (b->{ x + b })(32)"); + "{ var x = 10; } (b->{ x + b })(32)"); + final JexlContext jc = new MapContext(); + jc.set("x", 11); + final Object result = script.execute(jc); + Assert.assertEquals(43, result); + } + + @Test + public void testLet0() { + final JexlFeatures f = new JexlFeatures(); + final JexlEngine jexl = new JexlBuilder().strict(true).create(); + final JexlScript script = jexl.createScript( + "{ let x = 10; } (b->{ x + b })(32)"); final JexlContext jc = new MapContext(); jc.set("x", 11); final Object result = script.execute(jc); Assert.assertEquals(43, result); + } }