From c1ec13b41469f36a95d9ecac100ccf17beb56ce7 Mon Sep 17 00:00:00 2001 From: henrib Date: Sat, 21 May 2022 12:02:25 +0200 Subject: [PATCH] JEXL-369: named function declarations are statements, not expressions; made debugger properly recode let/const parameters; tests shuffling; --- .../commons/jexl3/internal/Debugger.java | 20 +++-- .../apache/commons/jexl3/parser/Parser.jjt | 9 ++- .../org/apache/commons/jexl3/LambdaTest.java | 76 +++++-------------- .../org/apache/commons/jexl3/LexicalTest.java | 63 +++++++++++++++ 4 files changed, 102 insertions(+), 66 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 6617c3db..ca3dacdb 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java @@ -731,11 +731,21 @@ protected Object visit(final ASTJexlScript node, Object arg) { } builder.append('('); final String[] params = lambda.getParameters(); - if (params != null && params.length > 0) { - builder.append(visitParameter(params[0], data)); - for (int p = 1; p < params.length; ++p) { - builder.append(", "); - builder.append(visitParameter(params[p], data)); + if (params != null ) { + Scope scope = lambda.getScope(); + LexicalScope lexicalScope = lambda.getLexicalScope(); + for (int p = 0; p < params.length; ++p) { + if (p > 0) { + builder.append(", "); + } + String param = params[p]; + int symbol = scope.getSymbol(param); + if (lexicalScope.isConstant(symbol)) { + builder.append("const "); + } else if (scope.isLexical(symbol)) { + builder.append("let "); + } + builder.append(visitParameter(param, data)); } } builder.append(')'); 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 753bad00..865ccc8c 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt +++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt @@ -352,6 +352,8 @@ void Statement() #void : {} { LOOKAHEAD(||) Var() | + LOOKAHEAD( ) FunctionStatement() + | StatementNoVar() } @@ -367,7 +369,6 @@ void StatementNoVar() #void : {} | LOOKAHEAD() ReturnStatement() | LOOKAHEAD() Continue() | LOOKAHEAD() Break() - | LOOKAHEAD( ) Lambda() | LOOKAHEAD(Expression()) ExpressionStatement() | Block() | LOOKAHEAD(, {!getFeatures().isLexical()} ) Var() @@ -378,6 +379,10 @@ void Block() #Block : {} { pushUnit(jjtThis); } ( Statement() )* { popUnit(jjtThis); } } +void FunctionStatement() #JexlLambda : {} +{ + DeclareFunction() { pushScope(); pushUnit(jjtThis); } Parameters() ( LOOKAHEAD(3) Block() | Expression()) { popUnit(jjtThis); popScope(); } +} void ExpressionStatement() #void : {} { @@ -887,7 +892,7 @@ void ParametersLookahead() #void : {} void LambdaLookahead() #void : {} { - ()? ParametersLookahead() + ParametersLookahead() | ParametersLookahead() ( | ) | diff --git a/src/test/java/org/apache/commons/jexl3/LambdaTest.java b/src/test/java/org/apache/commons/jexl3/LambdaTest.java index 31f27401..fb9a5dfb 100644 --- a/src/test/java/org/apache/commons/jexl3/LambdaTest.java +++ b/src/test/java/org/apache/commons/jexl3/LambdaTest.java @@ -428,7 +428,7 @@ public void test271e() { } @Test public void testNamedFunc() { - String src = "(a)->{ function fact(x) { x <= 1? 1 : x * fact(x - 1); } fact(a); }"; + String src = "(let a)->{ function fact(const x) { x <= 1? 1 : x * fact(x - 1); } fact(a); }"; JexlEngine jexl = createEngine(); JexlScript script = jexl.createScript(src); Object result = script.execute(null, 6); @@ -437,77 +437,35 @@ public void test271e() { Assert.assertEquals(simpleWhitespace(src), parsed); } - @Test - public void testConst0a() { - final JexlFeatures f = new JexlFeatures(); - final JexlEngine jexl = new JexlBuilder().strict(true).create(); - final 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); - 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(); + @Test public void testNamedFuncIsConst() { + String src = "function foo(x) { x + x }; var foo ='nonononon'"; + JexlEngine jexl = createEngine(); try { - final JexlScript script = jexl.createScript( - "const foo; foo"); - Assert.fail("should fail, const foo must be followed by assign."); + JexlScript script = jexl.createScript(src); + Assert.fail("should fail, foo is already defined"); } catch(JexlException.Parsing xparse) { - Assert.assertTrue(xparse.getMessage().contains("const")); + Assert.assertTrue(xparse.getMessage().contains("foo")); } } - @Test - public void testConst2a() { - 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; foo "+op+" 1;"); - Assert.fail("should fail, const precludes assignment"); - } catch (JexlException.Parsing xparse) { - Assert.assertTrue(xparse.getMessage().contains("foo")); - } + public void testFailParseFunc0() { + String src = "if (false) function foo(x) { x + x }; var foo = 1"; + JexlEngine jexl = createEngine(); + try { + JexlScript script = jexl.createScript(src); + } catch(JexlException.Parsing xparse) { + Assert.assertTrue(xparse.getMessage().contains("function")); } } @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; } { let foo = 0; foo " + op + " 1; }"); - Assert.assertNotNull(script); - } catch(JexlException.Parsing xparse) { - Assert.fail(xparse.toString()); - } - } - } - - @Test public void testNamedFuncIsConst() { - String src = "function foo(x) { x + x }; var foo ='nonononon'"; + public void testFailParseFunc1() { + String src = "if (false) let foo = (x) { x + x }; var foo = 1"; JexlEngine jexl = createEngine(); try { JexlScript script = jexl.createScript(src); - Assert.fail("should fail, foo is already defined"); } catch(JexlException.Parsing xparse) { - Assert.assertTrue(xparse.getMessage().contains("foo")); + Assert.assertTrue(xparse.getMessage().contains("let")); } } diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java index 9271c554..90f7a9b8 100644 --- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java +++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java @@ -884,6 +884,69 @@ public void testConstFail() { checkParse(srcs, false); } + @Test + public void testConst0a() { + final JexlFeatures f = new JexlFeatures(); + final JexlEngine jexl = new JexlBuilder().strict(true).create(); + final 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); + 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 foo; foo"); + Assert.fail("should fail, const foo must be followed by assign."); + } catch(JexlException.Parsing xparse) { + Assert.assertTrue(xparse.getMessage().contains("const")); + } + } + + @Test + public void testConst2a() { + 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; foo "+op+" 1;"); + Assert.fail("should fail, const precludes assignment"); + } catch (JexlException.Parsing xparse) { + Assert.assertTrue(xparse.getMessage().contains("foo")); + } + } + } + + @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; } { let foo = 0; foo " + op + " 1; }"); + Assert.assertNotNull(script); + } catch(JexlException.Parsing xparse) { + Assert.fail(xparse.toString()); + } + } + } + @Test public void testSingleStatementDeclFail() { List srcs = Arrays.asList(