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 2023/07/10 15:14:17 UTC
[commons-jexl] branch master updated: JEXL-401: added constCapture feature flag to control captured variables constness (sic); - added test; - refined debugger output when depth <= 1 to allow for better feature error reporting; - updated tests accordingly; - changes, release-notes
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 3276e2af JEXL-401: added constCapture feature flag to control captured variables constness (sic); - added test; - refined debugger output when depth <= 1 to allow for better feature error reporting; - updated tests accordingly; - changes, release-notes
3276e2af is described below
commit 3276e2afe7496b4217b5ad62bf60bfe901749ebf
Author: Henri Biestro <hb...@cloudera.com>
AuthorDate: Mon Jul 10 17:10:04 2023 +0200
JEXL-401: added constCapture feature flag to control captured variables constness (sic);
- added test;
- refined debugger output when depth <= 1 to allow for better feature error reporting;
- updated tests accordingly;
- changes, release-notes
---
RELEASE-NOTES.txt | 1 +
src/changes/changes.xml | 5 ++-
.../org/apache/commons/jexl3/JexlFeatures.java | 26 +++++++++++-
.../apache/commons/jexl3/internal/Debugger.java | 48 +++++++++++++++-------
.../commons/jexl3/parser/FeatureController.java | 8 +++-
.../org/apache/commons/jexl3/parser/Parser.jjt | 2 +-
.../org/apache/commons/jexl3/FeaturesTest.java | 12 ++++++
.../jexl3/parser/FeatureControllerTest.java | 32 ++++++++-------
8 files changed, 101 insertions(+), 33 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index a75a5b81..454820c7 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -30,6 +30,7 @@ Version 3.3.1 is source and binary compatible with 3.3.
New Features in 3.3.1:
====================
+* JEXL-401: Captured variables should be read-only
* JEXL-398: Allow 'trailing commas' or ellipsis while defining array, map and set literals
Bugs Fixed in 3.3.1:
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index c891b8ff..b6f897f3 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -29,7 +29,10 @@
<body>
<release version="3.3.1" date="20YY-MM-DD">
<!-- ADD -->
- <action dev="henrib" type="add" due-to="Xu Pengcheng">
+ <action dev="henrib" type="add" issue="JEXL-401">
+ Captured variables should be read-only
+ </action>
+ <action dev="henrib" type="add" issue="JEXL-398" due-to="Xu Pengcheng">
Allow 'trailing commas' or ellipsis while defining array, map and set literals
</action>
<!-- FIX -->
diff --git a/src/main/java/org/apache/commons/jexl3/JexlFeatures.java b/src/main/java/org/apache/commons/jexl3/JexlFeatures.java
index b8d798c4..b7bd6a4b 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlFeatures.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlFeatures.java
@@ -51,6 +51,7 @@ import java.util.function.Predicate;
* <code>lt</code> for <, ...)</li>
* <li>Pragma anywhere: whether pragma, that are <em>not</em> statements and handled before execution begins,
* can appear anywhere in the source or before any statements - ie at the beginning of a script.</li>
+ * <li>Const Capture: whether variables captured by lambdas are read-only (aka const, same as Java) or read-write.</li>
* </ul>
* @since 3.2
*/
@@ -68,7 +69,8 @@ public final class JexlFeatures {
"register", "reserved variable", "local variable", "assign/modify",
"global assign/modify", "array reference", "create instance", "loop", "function",
"method call", "set/map/array literal", "pragma", "annotation", "script", "lexical", "lexicalShade",
- "thin-arrow", "fat-arrow", "namespace pragma", "import pragma", "comparator names", "pragma anywhere"
+ "thin-arrow", "fat-arrow", "namespace pragma", "import pragma", "comparator names", "pragma anywhere",
+ "const capture"
};
/** Registers feature ordinal. */
private static final int REGISTER = 0;
@@ -114,6 +116,8 @@ public final class JexlFeatures {
public static final int COMPARATOR_NAMES = 20;
/** The pragma anywhere feature ordinal. */
public static final int PRAGMA_ANYWHERE = 21;
+ /** Captured variables are const. */
+ public static final int CONST_CAPTURE = 22;
/**
* The default features flag mask.
*/
@@ -464,6 +468,26 @@ public final class JexlFeatures {
return getFeature(LAMBDA);
}
+ /**
+ * Sets whether lambda captured-variables are const or not.
+ * <p>
+ * When disabled, lambda-captured variables are implicitly converted to read-write local variable (let),
+ * when enabled, those are implicitly converted to read-only local variables (const).
+ * @param flag true to enable, false to disable
+ * @return this features instance
+ */
+ public JexlFeatures constCapture(final boolean flag) {
+ setFeature(CONST_CAPTURE, flag);
+ return this;
+ }
+
+ /**
+ * @return true if lambda captured-variables are const, false otherwise
+ */
+ public boolean supportsConstCapture() {
+ return getFeature(CONST_CAPTURE);
+ }
+
/**
* Sets whether thin-arrow lambda syntax is enabled.
* <p>
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 80b56699..ad65b2ba 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
@@ -268,7 +268,7 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail {
* @return visitor pattern value
*/
protected Object accept(final JexlNode node, final Object data) {
- if (depth <= 0) {
+ if (depth <= 0 && builder.length() > 0) {
builder.append("...");
return data;
}
@@ -501,10 +501,14 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail {
final int num = node.jjtGetNumChildren();
builder.append("[ ");
if (num > 0) {
- accept(node.jjtGetChild(0), data);
- for (int i = 1; i < num; ++i) {
- builder.append(", ");
- accept(node.jjtGetChild(i), data);
+ if (depth <= 0) {
+ builder.append("...");
+ } else {
+ accept(node.jjtGetChild(0), data);
+ for (int i = 1; i < num; ++i) {
+ builder.append(", ");
+ accept(node.jjtGetChild(i), data);
+ }
}
}
builder.append(" ]");
@@ -513,6 +517,10 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail {
@Override
protected Object visit(final ASTRangeNode node, final Object data) {
+ if (depth <= 0) {
+ builder.append("( .. )");
+ return data;
+ }
return infixChildren(node, " .. ", false, data);
}
@@ -901,10 +909,14 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail {
final int num = node.jjtGetNumChildren();
builder.append("{ ");
if (num > 0) {
- accept(node.jjtGetChild(0), data);
- for (int i = 1; i < num; ++i) {
- builder.append(",");
- accept(node.jjtGetChild(i), data);
+ if (depth <= 0) {
+ builder.append("...");
+ } else {
+ accept(node.jjtGetChild(0), data);
+ for (int i = 1; i < num; ++i) {
+ builder.append(",");
+ accept(node.jjtGetChild(i), data);
+ }
}
}
builder.append(" }");
@@ -916,10 +928,14 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail {
final int num = node.jjtGetNumChildren();
builder.append("{ ");
if (num > 0) {
- accept(node.jjtGetChild(0), data);
- for (int i = 1; i < num; ++i) {
- builder.append(",");
- accept(node.jjtGetChild(i), data);
+ if (depth <= 0) {
+ builder.append("...");
+ } else {
+ accept(node.jjtGetChild(0), data);
+ for (int i = 1; i < num; ++i) {
+ builder.append(",");
+ accept(node.jjtGetChild(i), data);
+ }
}
} else {
builder.append(':');
@@ -977,7 +993,11 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail {
final int num = node.jjtGetNumChildren();
if (num == 2) {
accept(node.jjtGetChild(0), data);
- accept(node.jjtGetChild(1), data);
+ if (depth <= 0) {
+ builder.append("(...)");
+ } else {
+ accept(node.jjtGetChild(1), data);
+ }
}
return data;
}
diff --git a/src/main/java/org/apache/commons/jexl3/parser/FeatureController.java b/src/main/java/org/apache/commons/jexl3/parser/FeatureController.java
index 3a9d81b0..2e0b14e2 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/FeatureController.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/FeatureController.java
@@ -19,6 +19,7 @@ package org.apache.commons.jexl3.parser;
import org.apache.commons.jexl3.JexlException;
import org.apache.commons.jexl3.JexlFeatures;
import org.apache.commons.jexl3.JexlInfo;
+import org.apache.commons.jexl3.internal.Debugger;
import org.apache.commons.jexl3.internal.ScriptVisitor;
/**
* Controls that a script only uses enabled features.
@@ -73,7 +74,9 @@ public class FeatureController extends ScriptVisitor {
*/
public void throwFeatureException(final int feature, final JexlNode node) {
final JexlInfo dbgInfo = node.jexlInfo();
- throw new JexlException.Feature(dbgInfo, feature, "");
+ Debugger dbg = new Debugger().depth(1);
+ String msg = dbg.data(node);
+ throw new JexlException.Feature(dbgInfo, feature, msg);
}
/**
@@ -189,6 +192,9 @@ public class FeatureController extends ScriptVisitor {
if (!features.supportsSideEffectGlobal() && lv.isGlobalVar()) {
throwFeatureException(JexlFeatures.SIDE_EFFECT_GLOBAL, lv);
}
+ if (features.supportsConstCapture() && lv instanceof ASTIdentifier && ((ASTIdentifier) lv).isCaptured()) {
+ throwFeatureException(JexlFeatures.CONST_CAPTURE, lv);
+ }
if (!features.supportsSideEffect()) {
throwFeatureException(JexlFeatures.SIDE_EFFECT, lv);
}
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 655a0612..cfa7a97e 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
+++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
@@ -350,7 +350,7 @@ void Annotation() #Annotation :
Token t;
}
{
- t=<ANNOTATION> (LOOKAHEAD(<LPAREN>) Arguments() )? { jjtThis.setName(t.image); }
+ t=<ANNOTATION> { jjtThis.setName(t.image); } (LOOKAHEAD(<LPAREN>) Arguments() )?
}
void AnnotatedStatement() #AnnotatedStatement : {}
diff --git a/src/test/java/org/apache/commons/jexl3/FeaturesTest.java b/src/test/java/org/apache/commons/jexl3/FeaturesTest.java
index 300a4fad..a197238a 100644
--- a/src/test/java/org/apache/commons/jexl3/FeaturesTest.java
+++ b/src/test/java/org/apache/commons/jexl3/FeaturesTest.java
@@ -311,4 +311,16 @@ public class FeaturesTest extends JexlTestCase {
checkFeature(f, scripts);
}
+ @Test
+ public void testConstCapture() throws Exception {
+ final JexlFeatures f = new JexlFeatures().constCapture(true);
+ final String[] scripts = new String[]{
+ "let x = 0; const f = y -> x += y; f(42)",
+ "let x = 0; function f(y) { z -> x *= y }; f(42)"
+ };
+ checkFeature(f, scripts);
+ final JexlFeatures nof = new JexlFeatures().constCapture(true);
+ assertOk(nof, scripts);
+ }
+
}
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 6cef4cfc..5b4463c9 100644
--- a/src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
+++ b/src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
@@ -43,22 +43,24 @@ public class FeatureControllerTest extends JexlTestCase {
Asserter offAsserter = new Asserter(createEngine(new JexlFeatures().loops(false)));
offAsserter.setVariable("cond", true);
offAsserter.setVariable("i", 0);
- String matchException = "@1:1 loop error in ''";
+ String matchException = "@1:1 loop error in 'while (...) ...'";
String whileExpr = "while(cond) { i++; cond = false; }; i;";
onAsserter.assertExpression(whileExpr, 1);
- offAsserter.failExpression(whileExpr, matchException);
+ offAsserter.failExpression(whileExpr, matchException, String::equals);
+ matchException = "@1:1 loop error in 'do ... while (...)'";
onAsserter.setVariable("i", 0);
offAsserter.setVariable("i", 0);
String doWhileExpr = "do { i++; } while(false); i;";
onAsserter.assertExpression(doWhileExpr, 1);
- offAsserter.failExpression(doWhileExpr, matchException);
+ offAsserter.failExpression(doWhileExpr, matchException, String::equals);
+ 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;";
onAsserter.assertExpression(forExpr, 3);
- offAsserter.failExpression(forExpr, matchException);
+ offAsserter.failExpression(forExpr, matchException, String::equals);
int[] a = new int[]{1, 2};
onAsserter.setVariable("a", a);
@@ -67,7 +69,7 @@ public class FeatureControllerTest extends JexlTestCase {
offAsserter.setVariable("i", 0);
forExpr = "for(let j = 0; j < 2; ++j) { i = i + a[j]; } i;";
onAsserter.assertExpression(forExpr, 3);
- offAsserter.failExpression(forExpr, matchException);
+ offAsserter.failExpression(forExpr, matchException, String::equals);
}
@Test
@@ -76,10 +78,10 @@ public class FeatureControllerTest extends JexlTestCase {
Asserter offAsserter = new Asserter(createEngine(new JexlFeatures().newInstance(false)));
String expr = "new('java.lang.String', 'JEXL')";
onAsserter.assertExpression(expr, "JEXL");
- offAsserter.failExpression(expr, "@1:1 create instance error in ''");
+ offAsserter.failExpression(expr, "@1:1 create instance error in 'new(..., ...)'", String::equals);
expr = "new String('JEXL')";
onAsserter.assertExpression(expr, "JEXL");
- offAsserter.failExpression(expr, "@1:1 create instance error in ''");
+ offAsserter.failExpression(expr, "@1:1 create instance error in 'new ...(...)'", String::equals);
}
@Test
@@ -88,7 +90,7 @@ public class FeatureControllerTest extends JexlTestCase {
Asserter offAsserter = new Asserter(createEngine(new JexlFeatures().methodCall(false)));
String expr = "'jexl'.toUpperCase()";
onAsserter.assertExpression(expr, "JEXL");
- offAsserter.failExpression(expr, "@1:7 method call error in ''");
+ offAsserter.failExpression(expr, "@1:7 method call error in '.toUpperCase(...)'", String::equals);
}
@Test
@@ -97,7 +99,7 @@ public class FeatureControllerTest extends JexlTestCase {
Asserter offAsserter = new Asserter(createEngine(new JexlFeatures().methodCall(true).annotation(false)));
String expr = "@silent ''.toString()";
onAsserter.assertExpression(expr, "");
- offAsserter.failExpression(expr, "@1:1 annotation error in ''");
+ offAsserter.failExpression(expr, "@1:1 annotation error in '@silent'");
}
@Test
@@ -106,19 +108,19 @@ public class FeatureControllerTest extends JexlTestCase {
Asserter offAsserter = new Asserter(createEngine(new JexlFeatures().structuredLiteral(false)));
String arrayLitExpr = "[1, 2, 3, 4][3]";
onAsserter.assertExpression(arrayLitExpr, 4);
- offAsserter.failExpression(arrayLitExpr, "@1:1 set/map/array literal error in ''");
+ offAsserter.failExpression(arrayLitExpr, "@1:1 set/map/array literal error in '[ ... ]'", String::equals);
String mapLitExpr = "{'A' : 1, 'B' : 2}['B']";
onAsserter.assertExpression(mapLitExpr, 2);
- offAsserter.failExpression(mapLitExpr, "@1:1 set/map/array literal error in ''");
+ offAsserter.failExpression(mapLitExpr, "@1:1 set/map/array literal error in '{ ... }'", String::equals);
String setLitExpr = "{'A', 'B'}.size()";
onAsserter.assertExpression(setLitExpr, 2);
- offAsserter.failExpression(setLitExpr, "@1:1 set/map/array literal error in ''");
+ offAsserter.failExpression(setLitExpr, "@1:1 set/map/array literal error in '{ ... }'", String::equals);
String rangeLitExpr = "(0..3).size()";
onAsserter.assertExpression(rangeLitExpr, 4);
- offAsserter.failExpression(rangeLitExpr, "@1:5 set/map/array literal error in ''");
+ offAsserter.failExpression(rangeLitExpr, "@1:5 set/map/array literal error in '( .. )'", String::equals);
}
@Test
@@ -159,7 +161,7 @@ public class FeatureControllerTest extends JexlTestCase {
public void testSideEffectDisabled() throws Exception {
Asserter asserter = new Asserter(createEngine(new JexlFeatures().sideEffect(false)));
asserter.setVariable("i", 1);
- String matchException = "@1:1 assign/modify error in ''";
+ String matchException = "@1:1 assign/modify error in 'i'";
asserter.failExpression("i = 1", matchException);
asserter.failExpression("i = i + 1", matchException);
asserter.failExpression("i = i - 1", matchException);
@@ -183,7 +185,7 @@ public class FeatureControllerTest extends JexlTestCase {
asserter.failExpression("i++", matchException);
asserter.failExpression("i--", matchException);
- matchException = "@1:3 assign/modify error in ''";
+ matchException = "@1:3 assign/modify error in 'i'";
asserter.failExpression("++i", matchException);
asserter.failExpression("--i", matchException);
}