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/08/31 15:32:02 UTC
[commons-jexl] branch master updated: JEXL-405: simplified and narrowed logic around recursive lambdas; - moved tests to LambdaTest; - various nitpicks;
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 53eb3dbd JEXL-405: simplified and narrowed logic around recursive lambdas; - moved tests to LambdaTest; - various nitpicks;
53eb3dbd is described below
commit 53eb3dbd42d27ba5411197f024ce9ddf2c1bcca6
Author: Henri Biestro <hb...@cloudera.com>
AuthorDate: Thu Aug 31 17:31:53 2023 +0200
JEXL-405: simplified and narrowed logic around recursive lambdas;
- moved tests to LambdaTest;
- various nitpicks;
---
.../org/apache/commons/jexl3/internal/Closure.java | 31 +++++++--------
.../apache/commons/jexl3/internal/Interpreter.java | 6 +--
.../org/apache/commons/jexl3/Issues400Test.java | 44 ++++++----------------
.../java/org/apache/commons/jexl3/LambdaTest.java | 31 ++++++++++++++-
.../apache/commons/jexl3/PropertyAccessTest.java | 10 +++++
5 files changed, 67 insertions(+), 55 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Closure.java b/src/main/java/org/apache/commons/jexl3/internal/Closure.java
index 733ef85a..ef70ad7f 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Closure.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Closure.java
@@ -102,22 +102,23 @@ public class Closure extends Script {
}
/**
- * Sets the captured index of a given symbol, ie the target index of a parent
- * captured symbol in this closure's frame.
- * <p>This is meant to allow a locally defined function to "see" and call
- * itself as a local (captured) variable;
- * in other words, this allows recursive call of a function.
+ * Enable lambda recursion.
+ * <p>Assign this lambda in its own frame if the symbol it is assigned to in its definition scope
+ * is captured in its body.</p>
+ * <p>This done allow a locally defined function to "see" and call itself as a local (captured) variable.</p>
+ * Typical case is: <code>const f = (x)->x <= 0? 1 : x*f(x-1)</code>. Since assignment of f occurs after
+ * the lambda creation, we need to patch the lambda frame to expose itself through the captured symbol.
+ * @param parentFrame the parent calling frame
* @param symbol the symbol index (in the caller of this closure)
- * @param value the value to set in the local frame
*/
- public void setCaptured(final int symbol, final Object value) {
+ void captureSelfIfRecursive(final Frame parentFrame, final int symbol) {
if (script instanceof ASTJexlLambda) {
- final ASTJexlLambda lambda = (ASTJexlLambda) script;
- final Scope scope = lambda.getScope();
- if (scope != null) {
- final Integer reg = scope.getCaptured(symbol);
+ Scope parentScope = parentFrame != null ? parentFrame.getScope() : null;
+ Scope localScope = frame != null ? frame.getScope() : null;
+ if (parentScope != null && localScope != null && parentScope == localScope.getParent()) {
+ final Integer reg = localScope.getCaptured(symbol);
if (reg != null) {
- frame.set(reg, value);
+ frame.set(reg, this);
}
}
}
@@ -151,10 +152,4 @@ public class Closure extends Script {
};
}
- boolean hasParent(Scope scope) {
- Scope s = frame != null? frame.getScope() : null;
- return scope != null && s != null
- ? scope == s.getParent()
- : false;
- }
}
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 80397f5e..dffae1ea 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -1070,7 +1070,7 @@ public class Interpreter extends InterpreterBase {
final int symbol = variable.getSymbol();
frame.set(symbol, closure);
// make the closure accessible to itself, ie capture the 'function' variable after frame creation
- closure.setCaptured(symbol, closure);
+ closure.captureSelfIfRecursive(frame, symbol);
}
return closure;
}
@@ -1430,9 +1430,7 @@ public class Interpreter extends InterpreterBase {
if (right instanceof Closure) {
Closure closure = (Closure) right;
// the variable scope must be the parent of the lambdas
- if (closure.hasParent(frame.getScope())) {
- closure.setCaptured(symbol, right);
- }
+ closure.captureSelfIfRecursive(frame, symbol);
}
frame.set(symbol, right);
} else {
diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
index b988ff02..70be9f19 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
@@ -135,7 +135,8 @@ public class Issues400Test {
.strict(true)
.safe(false)
.create();
- Map<String, Object> a = Collections.singletonMap("b", Collections.singletonMap("c", 42));
+ Map<String, Object> b = Collections.singletonMap("c", 42);
+ Map<String, Object> a = Collections.singletonMap("b", b);
JexlScript script;
Object result = -42;
script = jexl.createScript("a?['B']?['C']", "a");
@@ -154,37 +155,16 @@ public class Issues400Test {
Assert.assertEquals(script.getSourceText(), script.getParsedText());
result = script.execute(null, a);
Assert.assertEquals(1042, result);
+ // can still do ternary, note the space between ? and [
+ script = jexl.createScript("a? ['B']:['C']", "a");
+ result = script.execute(null, a);
+ Assert.assertArrayEquals(new String[]{"B"}, (String[]) result);
+ script = jexl.createScript("a?['b'] ?: ['C']", "a");
+ result = script.execute(null, a);
+ Assert.assertEquals(b, result);
+ script = jexl.createScript("a?['B'] ?: ['C']", "a");
+ result = script.execute(null, a);
+ Assert.assertArrayEquals(new String[]{"C"}, (String[]) result);
}
- @Test
- public void test405a() {
- final JexlEngine jexl = new JexlBuilder()
- .cache(64)
- .strict(true)
- .safe(false)
- .create();
- String libSrc = "var tfn = pfn -> { var fn = pfn; fn() }; { 'theFunction' : tfn }";
- String src1 = "var v0 = 42; var v1 = -42; lib.theFunction(()->{ v1 + v0 }) ";
- JexlScript libMap = jexl.createScript(libSrc);
- Object theLib = libMap.execute(null);
- JexlScript f1 = jexl.createScript(src1, "lib");
- Object result = f1.execute(null, theLib);
- Assert.assertEquals(0, result);
- }
-
- @Test
- public void test405b() {
- final JexlEngine jexl = new JexlBuilder()
- .cache(64)
- .strict(true)
- .safe(false)
- .create();
- String libSrc = "function tfn(pfn) { var fn = pfn; fn() }; { 'theFunction' : tfn }";
- String src1 = "var v0 = 42; var v1 = -42; lib.theFunction(()->{ v1 + v0 }) ";
- JexlScript libMap = jexl.createScript(libSrc);
- Object theLib = libMap.execute(null);
- JexlScript f1 = jexl.createScript(src1, "lib");
- Object result = f1.execute(null, theLib);
- Assert.assertEquals(0, result);
- }
}
diff --git a/src/test/java/org/apache/commons/jexl3/LambdaTest.java b/src/test/java/org/apache/commons/jexl3/LambdaTest.java
index 756dfb91..4a040e70 100644
--- a/src/test/java/org/apache/commons/jexl3/LambdaTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LambdaTest.java
@@ -422,7 +422,7 @@ public class LambdaTest extends JexlTestCase {
@Test public void testFatFact0() {
JexlFeatures features = new JexlFeatures();
features.fatArrow(true);
- String src = "function (a) { const fact = (x)=>{ x <= 1? 1 : x * fact(x - 1) }; fact(a) }";
+ String src = "function (a) { const fact = x =>{ x <= 1? 1 : x * fact(x - 1) }; fact(a) }";
JexlEngine jexl = createEngine(features);
JexlScript script = jexl.createScript(src);
Object result = script.execute(null, 6);
@@ -533,4 +533,33 @@ public class LambdaTest extends JexlTestCase {
Assert.assertEquals(1, set.size());
Assert.assertTrue(set.contains(42));
}
+
+ @Test
+ public void test405a() {
+ final JexlEngine jexl = new JexlBuilder()
+ .cache(4).strict(true).safe(false)
+ .create();
+ String libSrc = "var theFunction = argFn -> { var fn = argFn; fn() }; { 'theFunction' : theFunction }";
+ String src1 = "var v0 = 42; var v1 = -42; lib.theFunction(()->{ v1 + v0 }) ";
+ JexlScript libMap = jexl.createScript(libSrc);
+ Object theLib = libMap.execute(null);
+ JexlScript f1 = jexl.createScript(src1, "lib");
+ Object result = f1.execute(null, theLib);
+ Assert.assertEquals(0, result);
+ }
+
+ @Test
+ public void test405b() {
+ final JexlEngine jexl = new JexlBuilder()
+ .cache(4).strict(true).safe(false)
+ .create();
+ String libSrc = "function theFunction(argFn) { var fn = argFn; fn() }; { 'theFunction' : theFunction }";
+ String src1 = "var v0 = 42; var v1 = -42; lib.theFunction(()->{ v1 + v0 }) ";
+ JexlScript libMap = jexl.createScript(libSrc);
+ Object theLib = libMap.execute(null);
+ JexlScript f1 = jexl.createScript(src1, "lib");
+ Object result = f1.execute(null, theLib);
+ Assert.assertEquals(0, result);
+ }
+
}
diff --git a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
index 77c9e2c0..ba7c3366 100644
--- a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
+++ b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
@@ -89,6 +89,16 @@ public class PropertyAccessTest extends JexlTestCase {
asserter.assertExpression("foo.0?.`${one}`", i43);
asserter.assertExpression("foo.0.`${one}` = 42", i42);
asserter.assertExpression("foo?.0?.`${one}`", i42);
+ //
+ asserter.assertExpression("foo?[0].'1'", i42);
+ asserter.assertExpression("foo?[0]", foo);
+ asserter.assertExpression("foo?[0].'0'", foo);
+ asserter.assertExpression("foo?[1]", foo[1]);
+ asserter.assertExpression("foo[0]?.'1'", foo[1]);
+ asserter.assertExpression("foo?[0].'1' = 43", i43);
+ asserter.assertExpression("foo?[0]?.'1'", i43);
+ asserter.assertExpression("foo?[0].'1' = 42", i42);
+ asserter.assertExpression("foo?[0].'1'", i42);
}
}