Skip to content

Commit

Permalink
JEXL-401: added constCapture feature flag to control captured variabl…
Browse files Browse the repository at this point in the history 
…es constness (sic);

- added test;
- refined debugger output when depth <= 1 to allow for better feature error reporting;
- updated tests accordingly;
- changes, release-notes
  • Loading branch information
Henri Biestro committed Jul 10, 2023
1 parent 0b048dd commit 3276e2a
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 33 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 4 additions & 1 deletion src/changes/changes.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down
26 changes: 25 additions & 1 deletion src/main/java/org/apache/commons/jexl3/JexlFeatures.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
* <code>lt</code> for &lt;, ...)</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
*/
Expand All @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -464,6 +468,26 @@ public boolean supportsLambda() {
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>
Expand Down
48 changes: 34 additions & 14 deletions src/main/java/org/apache/commons/jexl3/internal/Debugger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public Debugger lineFeed(final String lf) {
* @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;
}
Expand Down Expand Up @@ -501,10 +501,14 @@ protected Object visit(final ASTArrayLiteral node, final Object data) {
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(" ]");
Expand All @@ -513,6 +517,10 @@ protected Object visit(final ASTArrayLiteral node, final Object data) {

@Override
protected Object visit(final ASTRangeNode node, final Object data) {
if (depth <= 0) {
builder.append("( .. )");
return data;
}
return infixChildren(node, " .. ", false, data);
}

Expand Down Expand Up @@ -901,10 +909,14 @@ protected Object visit(final ASTSetLiteral node, final Object data) {
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(" }");
Expand All @@ -916,10 +928,14 @@ protected Object visit(final ASTMapLiteral node, final Object data) {
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(':');
Expand Down Expand Up @@ -977,7 +993,11 @@ protected Object visit(final ASTMethodNode node, final Object data) {
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;
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/org/apache/commons/jexl3/parser/FeatureController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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.
Expand Down Expand Up @@ -73,7 +74,9 @@ protected Object visitNode(final JexlNode node, final Object data) {
*/
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);
}

/**
Expand Down Expand Up @@ -189,6 +192,9 @@ private Object controlSideEffect(final JexlNode node, final Object data) {
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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 : {}
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/org/apache/commons/jexl3/FeaturesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,4 +311,16 @@ public void testNoComparatorNames() throws Exception {
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);
}

}
32 changes: 17 additions & 15 deletions src/test/java/org/apache/commons/jexl3/parser/FeatureControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,24 @@ public void testLoopFeatureSwitch() throws Exception {
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);
Expand All @@ -67,7 +69,7 @@ public void testLoopFeatureSwitch() throws Exception {
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
Expand All @@ -76,10 +78,10 @@ public void testNewInstanceFeatureSwitch() throws Exception {
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
Expand All @@ -88,7 +90,7 @@ public void testMethodCallFeatureSwitch() throws Exception {
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
Expand All @@ -97,7 +99,7 @@ public void testAnnotationFeatureSwitch() throws Exception {
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
Expand All @@ -106,19 +108,19 @@ public void testStructuredLiteralFeatureSwitch() throws Exception {
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
Expand Down Expand Up @@ -159,7 +161,7 @@ public void testSideEffectEnabled() throws Exception {
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);
Expand All @@ -183,7 +185,7 @@ public void testSideEffectDisabled() throws Exception {

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);
}
Expand Down

0 comments on commit 3276e2a

Please sign in to comment.