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 1934f2bd JEXL-431: try statement must be a lexical statement (local
variable declaration in catach); - nitpicking on formatting
(if/while/for/try/catch + space); - try and improve precision for error
messages; - unrelated, use signum to invert sign in compare method
1934f2bd is described below
commit 1934f2bd3a392382ee525bd649da10eda0e9ef2d
Author: Henrib <hbies...@gmail.com>
AuthorDate: Sun Dec 8 18:49:45 2024 +0100
JEXL-431: try statement must be a lexical statement (local variable
declaration in catach);
- nitpicking on formatting (if/while/for/try/catch + space);
- try and improve precision for error messages;
- unrelated, use signum to invert sign in compare method
---
.../org/apache/commons/jexl3/JexlArithmetic.java | 2 +-
.../org/apache/commons/jexl3/JexlException.java | 2 +-
.../apache/commons/jexl3/internal/Debugger.java | 6 ++---
.../org/apache/commons/jexl3/internal/Engine.java | 7 ++----
.../apache/commons/jexl3/internal/Operator.java | 2 +-
.../commons/jexl3/parser/ASTTryStatement.java | 2 +-
.../org/apache/commons/jexl3/parser/JexlNode.java | 12 +++++++++-
.../apache/commons/jexl3/parser/JexlParser.java | 11 +++++----
.../org/apache/commons/jexl3/parser/Parser.jjt | 6 +++++
.../java/org/apache/commons/jexl3/ForEachTest.java | 26 +++++++++++-----------
.../org/apache/commons/jexl3/Issues400Test.java | 10 +++++++++
.../java/org/apache/commons/jexl3/JXLTTest.java | 2 +-
.../jexl3/parser/FeatureControllerTest.java | 6 ++---
13 files changed, 58 insertions(+), 36 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index 8f731996..f9c2472e 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -804,7 +804,7 @@ public class JexlArithmetic {
@SuppressWarnings("unchecked") // OK because of instanceof
check above
final Comparable<Object> comparable = (Comparable<Object>)
right;
try {
- return -comparable.compareTo(left);
+ return -Integer.signum(comparable.compareTo(left));
} catch (final ClassCastException castException) {
// ignore it, continue in sequence
}
diff --git a/src/main/java/org/apache/commons/jexl3/JexlException.java
b/src/main/java/org/apache/commons/jexl3/JexlException.java
index 086630e5..640f0ef3 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlException.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlException.java
@@ -658,7 +658,7 @@ public class JexlException extends RuntimeException {
private static final long serialVersionUID = 20210606123900L;
/** Maximum number of characters around exception location. */
- private static final int MAX_EXCHARLOC = 42;
+ private static final int MAX_EXCHARLOC = 128;
/** Used 3 times. */
private static final String VARQUOTE = "variable '";
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 508d61da..6d6e3bab 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
@@ -876,7 +876,7 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
@Override
protected Object visit(final ASTForeachStatement node, final Object data) {
final int form = node.getLoopForm();
- builder.append("for(");
+ builder.append("for (");
final JexlNode body;
if (form == 0) {
// for( .. : ...)
@@ -1411,7 +1411,7 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
@Override
protected Object visit(final ASTTryResources node, final Object data) {
final int tryBody = node.jjtGetNumChildren() - 1;
- builder.append('(');
+ builder.append(" (");
accept(node.jjtGetChild(0), data);
for(int c = 1; c < tryBody; ++c) {
builder.append("; ");
@@ -1430,7 +1430,7 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
accept(node.jjtGetChild(nc++), data);
// catch-body
if (node.hasCatchClause()) {
- builder.append("catch(");
+ builder.append("catch (");
accept(node.jjtGetChild(nc++), data);
builder.append(") ");
accept(node.jjtGetChild(nc++), data);
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java
b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
index 96d1b2c4..a9671363 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
@@ -796,11 +796,8 @@ public class Engine extends JexlEngine {
ASTJexlScript script;
if (source != null) {
script = cache.get(source);
- if (script != null) {
- final Scope f = script.getScope();
- if (f == null && scope == null || f != null &&
f.equals(scope)) {
- return script;
- }
+ if (script != null && (scope == null ||
scope.equals(script.getScope()))) {
+ return script;
}
}
final JexlInfo ninfo = info == null && debug ? createInfo() : info;
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Operator.java
b/src/main/java/org/apache/commons/jexl3/internal/Operator.java
index faa755f6..c31ccd1b 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Operator.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Operator.java
@@ -645,7 +645,7 @@ public final class Operator implements
JexlOperator.Uberspect {
@Override
public Object tryInvoke(String name, Object arithmetic, Object...
params) throws JexlException.TryFailed {
final Object cmp =
compare.tryInvoke(JexlOperator.COMPARE.getMethodName(), arithmetic, params[1],
params[0]);
- return cmp instanceof Integer? operate(-(int) cmp) :
JexlEngine.TRY_FAILED;
+ return cmp instanceof Integer? operate(-Integer.signum((Integer)
cmp)) : JexlEngine.TRY_FAILED;
}
}
}
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTTryStatement.java
b/src/main/java/org/apache/commons/jexl3/parser/ASTTryStatement.java
index aa8d0d13..f01de8fb 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTTryStatement.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTTryStatement.java
@@ -19,7 +19,7 @@ package org.apache.commons.jexl3.parser;
/**
* Declares a try/catch/finally statement.
*/
-public class ASTTryStatement extends JexlNode {
+public class ASTTryStatement extends JexlLexicalNode {
private static final long serialVersionUID = 1L;
/** catch() &= 1, finally &= 2. */
private int tryForm;
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
index 3154081f..5b30c413 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
@@ -313,6 +313,16 @@ public abstract class JexlNode extends SimpleNode
implements JexlCache.Reference
* @return the info
*/
public JexlInfo jexlInfo() {
+ return jexlInfo(null);
+ }
+
+ /**
+ * Gets the associated JexlInfo instance.
+ *
+ * @param name the source name
+ * @return the info
+ */
+ public JexlInfo jexlInfo(String name) {
JexlInfo info = null;
JexlNode node = this;
while (node != null) {
@@ -326,7 +336,7 @@ public abstract class JexlNode extends SimpleNode
implements JexlCache.Reference
final int c = lc & 0xfff;
final int l = lc >> 0xc;
// at least an info with line/column number
- return info != null ? info.at(info.getLine() + l - 1, c) : new
JexlInfo(null, l, c);
+ return info != null ? info.at(info.getLine() + l - 1, c) : new
JexlInfo(name, l, c);
}
// weird though; no jjSetFirstToken(...) ever called?
return info;
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 c360bf7e..340036d2 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
@@ -502,7 +502,8 @@ public abstract class JexlParser extends StringParser {
// if not the first time we declare this symbol...
if (!declareSymbol(symbol)) {
if (lexical || scope.isLexical(symbol) ||
getFeatures().isLexical()) {
- throw new JexlException.Parsing(variable.jexlInfo(), name + ":
variable is already declared").clean();
+ final JexlInfo location = info.at(token.beginLine,
token.beginColumn);
+ throw new JexlException.Parsing(location, name + ": variable
is already declared").clean();
}
// not lexical, redefined nevertheless
variable.setRedefined(true);
@@ -555,6 +556,7 @@ public abstract class JexlParser extends StringParser {
protected void Identifier(final boolean top) throws ParseException {
// Overridden by generated code
}
+
/**
* Checks whether a symbol has been declared as a const in the current
stack of lexical units.
* @param symbol the symbol
@@ -603,10 +605,7 @@ public abstract class JexlParser extends StringParser {
return true;
}
// declared through engine features ?
- if (getFeatures().namespaceTest().test(name)) {
- return true;
- }
- return false;
+ return getFeatures().namespaceTest().test(name);
}
/**
@@ -814,7 +813,7 @@ public abstract class JexlParser extends StringParser {
* @throws JexlException.Ambiguous in all cases
*/
protected void throwAmbiguousException(final JexlNode node) {
- final JexlInfo begin = node.jexlInfo();
+ final JexlInfo begin = node.jexlInfo(info.getName());
final Token t = getToken(0);
final JexlInfo end = info.at(t.beginLine, t.endColumn);
final String msg = readSourceLine(source, end.getLine());
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 4dd4fee6..9eeac794 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
+++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
@@ -444,9 +444,15 @@ void IfStatement() : {}
void TryStatement() : {}
{
+ {
+ pushUnit(jjtThis);
+ }
<TRY> (LOOKAHEAD(1) (TryResources() ) | Block())
(LOOKAHEAD(1) <CATCH> <LPAREN> InlineVar() <RPAREN> Block() {
jjtThis.catchClause(); })?
(LOOKAHEAD(1) <FINALLY> Block() { jjtThis.finallyClause(); })?
+ {
+ popUnit(jjtThis);
+ }
}
void TryResources() : {}
diff --git a/src/test/java/org/apache/commons/jexl3/ForEachTest.java
b/src/test/java/org/apache/commons/jexl3/ForEachTest.java
index 624507de..d3c65341 100644
--- a/src/test/java/org/apache/commons/jexl3/ForEachTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ForEachTest.java
@@ -53,7 +53,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachBreakMethod() throws Exception {
final JexlScript e = JEXL.createScript(
- "var rr = -1; for(var item : [1, 2, 3 ,4 ,5, 6]) { if (item ==
3) { rr = item; break; }} rr"
+ "var rr = -1; for (var item : [1, 2, 3 ,4 ,5, 6]) { if (item
== 3) { rr = item; break; }} rr"
);
final JexlContext jc = new MapContext();
jc.set("list", new Foo());
@@ -71,7 +71,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachContinueMethod() throws Exception {
final JexlScript e = JEXL.createScript(
- "var rr = 0; for(var item : [1, 2, 3 ,4 ,5, 6]) { if (item <=
3) continue; rr = rr + item;}"
+ "var rr = 0; for (var item : [1, 2, 3 ,4 ,5, 6]) { if (item <=
3) continue; rr = rr + item;}"
);
final JexlContext jc = new MapContext();
jc.set("list", new Foo());
@@ -81,7 +81,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithArray() throws Exception {
- final JexlScript e = JEXL.createScript("for(item : list) item");
+ final JexlScript e = JEXL.createScript("for (item : list) item");
final JexlContext jc = new MapContext();
jc.set("list", new Object[]{"Hello", "World"});
final Object o = e.execute(jc);
@@ -90,7 +90,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithBlock() throws Exception {
- final JexlScript exs0 = JEXL.createScript("for(var in : list) { x = x
+ in; }");
+ final JexlScript exs0 = JEXL.createScript("for (var in : list) { x = x
+ in; }");
final JexlContext jc = new MapContext();
jc.set("list", new Object[]{2, 3});
jc.set("x", Integer.valueOf(1));
@@ -101,7 +101,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithCollection() throws Exception {
- final JexlScript e = JEXL.createScript("for(var item : list) item");
+ final JexlScript e = JEXL.createScript("for (var item : list) item");
final JexlContext jc = new MapContext();
jc.set("list", Arrays.asList("Hello", "World"));
final Object o = e.execute(jc);
@@ -110,7 +110,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithEmptyList() throws Exception {
- final JexlScript e = JEXL.createScript("for(item : list) 1+1");
+ final JexlScript e = JEXL.createScript("for (item : list) 1+1");
final JexlContext jc = new MapContext();
jc.set("list", Collections.emptyList());
@@ -120,7 +120,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithEmptyStatement() throws Exception {
- final JexlScript e = JEXL.createScript("for(item : list) ;");
+ final JexlScript e = JEXL.createScript("for (item : list) ;");
final JexlContext jc = new MapContext();
jc.set("list", Collections.emptyList());
@@ -130,7 +130,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithEnumeration() throws Exception {
- final JexlScript e = JEXL.createScript("for(var item : list) item");
+ final JexlScript e = JEXL.createScript("for (var item : list) item");
final JexlContext jc = new MapContext();
jc.set("list", new StringTokenizer("Hello,World", ","));
final Object o = e.execute(jc);
@@ -139,7 +139,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithIterator() throws Exception {
- final JexlScript e = JEXL.createScript("for(var item : list) item");
+ final JexlScript e = JEXL.createScript("for (var item : list) item");
final JexlContext jc = new MapContext();
jc.set("list", Arrays.asList("Hello", "World").iterator());
final Object o = e.execute(jc);
@@ -148,7 +148,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithIteratorMethod() throws Exception {
- final JexlScript e = JEXL.createScript("for(var item : list.cheezy)
item");
+ final JexlScript e = JEXL.createScript("for (var item : list.cheezy)
item");
final JexlContext jc = new MapContext();
jc.set("list", new Foo());
final Object o = e.execute(jc);
@@ -157,7 +157,7 @@ public class ForEachTest extends JexlTestCase {
@Test
public void testForEachWithListExpression() throws Exception {
- final JexlScript e = JEXL.createScript("for(var item : list.keySet())
item");
+ final JexlScript e = JEXL.createScript("for (var item : list.keySet())
item");
final JexlContext jc = new MapContext();
final Map<?, ?> map = System.getProperties();
final String lastKey = (String) new
ArrayList<Object>(map.keySet()).get(System.getProperties().size() - 1);
@@ -187,7 +187,7 @@ public class ForEachTest extends JexlTestCase {
}
@Test public void testForLoop0a() {
- final String src = "(l)->{ for(let x = 0; x < 4; ++x) { l.add(x); } }";
+ final String src = "(l)->{ for (let x = 0; x < 4; ++x) { l.add(x); }
}";
final JexlEngine jexl = new JexlBuilder().safe(false).create();
final JexlScript script = jexl.createScript(src);
final List<Integer> l = new ArrayList<>();
@@ -199,7 +199,7 @@ public class ForEachTest extends JexlTestCase {
}
@Test public void testForLoop0b0() {
- final String src = "(l)->{ for(let x = 0, y = 0; x < 4; ++x) l.add(x)
}";
+ final String src = "(l)->{ for (let x = 0, y = 0; x < 4; ++x) l.add(x)
}";
final JexlEngine jexl = new JexlBuilder().safe(false).create();
final JexlScript script = jexl.createScript(src);
final List<Integer> l = new ArrayList<>();
diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
index 81e7cd55..f599fbc8 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
@@ -493,4 +493,14 @@ public class Issues400Test {
script = jexl.createScript(src);
assertEquals(20042, (int) script.execute(ctxt));
}
+
+ @Test
+ void test431() {
+ JexlEngine jexl = new JexlBuilder().create();
+ final String src = "let x = 0; try { x += 19 } catch (let error) {
return 169 } try { x += 23 } catch (let error) { return 169 }";
+ final JexlScript script = jexl.createScript(src);
+ assertNotNull(script);
+ final Object result = script.execute(null);
+ assertEquals(42, result);
+ }
}
diff --git a/src/test/java/org/apache/commons/jexl3/JXLTTest.java
b/src/test/java/org/apache/commons/jexl3/JXLTTest.java
index 0a30af3e..9869bb62 100644
--- a/src/test/java/org/apache/commons/jexl3/JXLTTest.java
+++ b/src/test/java/org/apache/commons/jexl3/JXLTTest.java
@@ -420,7 +420,7 @@ public class JXLTTest extends JexlTestCase {
init(builder);
// @formatter:off
final String test42
- = "$$ for(var x : list) {\n"
+ = "$$ for (var x : list) {\n"
+ "$$ if (x == 42) {\n"
+ "Life, the universe, and everything\n"
+ "$$ } else if (x > 42) {\n"
diff --git
a/src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
b/src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
index d804b93b..72ffd62a 100644
--- a/src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
+++ b/src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
@@ -53,7 +53,7 @@ public class FeatureControllerTest extends JexlTestCase {
offAsserter.setVariable("cond", true);
offAsserter.setVariable("i", 0);
String matchException = "@1:1 loop error in 'while (...) ...'";
- final String whileExpr = "while(cond) { i++; cond = false; }; i;";
+ final String whileExpr = "while (cond) { i++; cond = false; }; i;";
onAsserter.assertExpression(whileExpr, 1);
offAsserter.failExpression(whileExpr, matchException, String::equals);
@@ -64,7 +64,7 @@ public class FeatureControllerTest extends JexlTestCase {
onAsserter.assertExpression(doWhileExpr, 1);
offAsserter.failExpression(doWhileExpr, matchException,
String::equals);
- matchException = "@1:1 loop error in 'for(... : ...) ...'";
+ matchException = "@1:1 loop error in 'for (... : ...) ...'";
onAsserter.setVariable("i", 0);
offAsserter.setVariable("i", 0);
String forExpr = "for (let j : [1, 2]) { i = i + j; }; i;";
@@ -76,7 +76,7 @@ public class FeatureControllerTest extends JexlTestCase {
offAsserter.setVariable("a", a);
onAsserter.setVariable("i", 0);
offAsserter.setVariable("i", 0);
- forExpr = "for(let j = 0; j < 2; ++j) { i = i + a[j]; } i;";
+ forExpr = "for (let j = 0; j < 2; ++j) { i = i + a[j]; } i;";
onAsserter.assertExpression(forExpr, 3);
offAsserter.failExpression(forExpr, matchException, String::equals);
}
