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 8bdde09 JEXL-271: reporting the unbound parameters (curry()), tls interpreter cleanup 8bdde09 is described below commit 8bdde09be9a2e9abe2ee08bbd87f54d4871e602e Author: Henri Biestro <hen...@apache.org> AuthorDate: Wed Sep 12 08:31:59 2018 +0200 JEXL-271: reporting the unbound parameters (curry()), tls interpreter cleanup --- .../java/org/apache/commons/jexl3/JexlScript.java | 42 ++++++----- .../org/apache/commons/jexl3/internal/Closure.java | 7 +- .../apache/commons/jexl3/internal/Interpreter.java | 8 +-- .../org/apache/commons/jexl3/internal/Scope.java | 29 ++++++-- .../org/apache/commons/jexl3/internal/Script.java | 18 ++--- .../org/apache/commons/jexl3/Issues200Test.java | 83 ++++------------------ .../java/org/apache/commons/jexl3/LambdaTest.java | 67 ++++++++++++++++- 7 files changed, 146 insertions(+), 108 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/JexlScript.java b/src/main/java/org/apache/commons/jexl3/JexlScript.java index e7c041d..67b3e5d 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlScript.java +++ b/src/main/java/org/apache/commons/jexl3/JexlScript.java @@ -24,15 +24,15 @@ import java.util.concurrent.Callable; /** * A JEXL Script. - * + * * <p>A script is some valid JEXL syntax to be executed with a given set of {@link JexlContext} variables.</p> - * + * * <p>A script is a group of statements, separated by semicolons.</p> - * + * * <p>The statements can be <code>blocks</code> (curly braces containing code), * Control statements such as <code>if</code> and <code>while</code> * as well as expressions and assignment statements.</p> - * + * * <p>Do <em>not</em> create classes that implement this interface; delegate or compose instead.</p> * * @since 1.1 @@ -41,21 +41,21 @@ public interface JexlScript { /** * Returns the source text of this expression. - * + * * @return the source text */ String getSourceText(); /** * Recreates the source text of this expression from the internal syntactic tree. - * + * * @return the source text */ String getParsedText(); /** * Recreates the source text of this expression from the internal syntactic tree. - * + * * @param indent the number of spaces for indentation, 0 meaning no indentation * @return the source text */ @@ -86,15 +86,23 @@ public interface JexlScript { /** * Gets this script parameters. - * + * * @return the parameters or null * @since 2.1 */ String[] getParameters(); /** + * Gets this script unbound parameters. + * <p>Parameters that haven't been bound by a previous call to curry(). + * @return the parameters or null + * @since 3.2 + */ + String[] getUnboundParameters(); + + /** * Gets this script local variables. - * + * * @return the local variables or null * @since 2.1 */ @@ -104,7 +112,7 @@ public interface JexlScript { * Gets this script variables. * <p>Note that since variables can be in an ant-ish form (ie foo.bar.quux), each variable is returned as * a list of strings where each entry is a fragment of the variable ({"foo", "bar", "quux"} in the example.</p> - * + * * @return the variables or null * @since 2.1 */ @@ -112,17 +120,17 @@ public interface JexlScript { /** * Gets this script pragmas. - * + * * @return the (non null, may be empty) pragmas map */ Map<String, Object> getPragmas(); /** * Creates a Callable from this script. - * + * * <p>This allows to submit it to an executor pool and provides support for asynchronous calls.</p> * <p>The interpreter will handle interruption/cancellation gracefully if needed.</p> - * + * * @param context the context * @return the callable * @since 2.1 @@ -131,10 +139,10 @@ public interface JexlScript { /** * Creates a Callable from this script. - * + * * <p>This allows to submit it to an executor pool and provides support for asynchronous calls.</p> * <p>The interpreter will handle interruption/cancellation gracefully if needed.</p> - * + * * @param context the context * @param args the script arguments * @return the callable @@ -144,10 +152,10 @@ public interface JexlScript { /** * Curries this script, returning a script with bound arguments. - * + * * <p>If this script does not declare parameters or if all of them are already bound, * no error is generated and this script is returned.</p> - * + * * @param args the arguments to bind * @return the curried script or this script if no binding can occur */ 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 c297db0..009c962 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Closure.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Closure.java @@ -37,7 +37,7 @@ public class Closure extends Script { super(theCaller.jexl, null, lambda); frame = lambda.createFrame(theCaller.frame); } - + /** * Creates a curried version of a script. * @param base the base script @@ -83,6 +83,11 @@ public class Closure extends Script { return true; } + @Override + public String[] getUnboundParameters() { + return frame.getUnboundParameters(); + } + /** * Sets the hoisted index of a given symbol, ie the target index of a parent hoisted symbol in this closure's frame. * <p>This is meant to allow a locally defined function to "see" and call itself as a local (hoisted) variable; 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 09c229f..51b25ee 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -180,11 +180,11 @@ public class Interpreter extends InterpreterBase { JexlContext.ThreadLocal tcontext = null; JexlEngine tjexl = null; try { - cancelCheck(node); if (context instanceof JexlContext.ThreadLocal) { tcontext = jexl.putThreadLocal((JexlContext.ThreadLocal) context); } tjexl = jexl.putThreadEngine(jexl); + cancelCheck(node); return node.jjtAccept(this, null); } catch (JexlException.Return xreturn) { return xreturn.getValue(); @@ -1413,7 +1413,7 @@ public class Interpreter extends InterpreterBase { } else if (context.has(methodName)) { functor = context.get(methodName); isavar = functor != null; - } + } // name is a variable, cant be cached cacheable &= !isavar; } @@ -1431,7 +1431,7 @@ public class Interpreter extends InterpreterBase { } else { return unsolvableMethod(node, "?"); } - + // solving the call site CallDispatcher call = new CallDispatcher(node, cacheable); try { @@ -1439,7 +1439,7 @@ public class Interpreter extends InterpreterBase { Object eval = call.tryEval(target, methodName, argv); if (JexlEngine.TRY_FAILED != eval) { return eval; - } + } boolean functorp = false; boolean narrow = false; // pseudo loop to try acquiring methods without and with argument narrowing diff --git a/src/main/java/org/apache/commons/jexl3/internal/Scope.java b/src/main/java/org/apache/commons/jexl3/internal/Scope.java index 0625daf..245f38c 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java @@ -47,6 +47,10 @@ public final class Scope { * The map of local hoisted variables to parent scope variables, ie closure. */ private Map<Integer, Integer> hoistedVariables = null; + /** + * The empty string array. + */ + private static final String[] EMPTY_STRS = new String[0]; /** * Creates a new scope with a list of parameters. @@ -229,7 +233,7 @@ public final class Scope { * @return the symbol names */ public String[] getSymbols() { - return namedVariables != null ? namedVariables.keySet().toArray(new String[0]) : new String[0]; + return namedVariables != null ? namedVariables.keySet().toArray(new String[0]) : EMPTY_STRS; } /** @@ -237,17 +241,22 @@ public final class Scope { * @return the parameter names */ public String[] getParameters() { - if (namedVariables != null && parms > 0) { - String[] pa = new String[parms]; + return getParameters(0); + } + protected String[] getParameters(int bound) { + int unbound = parms - bound; + if (namedVariables != null && unbound > 0) { + String[] pa = new String[unbound]; int p = 0; for (Map.Entry<String, Integer> entry : namedVariables.entrySet()) { - if (entry.getValue().intValue() < parms) { + int argn = entry.getValue(); + if (argn >= bound && argn < parms) { pa[p++] = entry.getKey(); } } return pa; } else { - return null; + return EMPTY_STRS; } } @@ -267,7 +276,7 @@ public final class Scope { } return pa; } else { - return null; + return EMPTY_STRS; } } @@ -296,6 +305,14 @@ public final class Scope { } /** + * Gets this script unbound parameters, i.e. parameters not bound through curry(). + * @return the parameter names + */ + public String[] getUnboundParameters() { + return scope.getParameters(curried); + } + + /** * Gets the scope. * @return this frame scope */ diff --git a/src/main/java/org/apache/commons/jexl3/internal/Script.java b/src/main/java/org/apache/commons/jexl3/internal/Script.java index f7450cf..2cd6642 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Script.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Script.java @@ -123,7 +123,7 @@ public class Script implements JexlScript, JexlExpression { public String getParsedText() { return getParsedText(2); } - + @Override public String getParsedText(int indent) { Debugger debug = new Debugger(); @@ -191,7 +191,7 @@ public class Script implements JexlScript, JexlExpression { Interpreter interpreter = createInterpreter(context, frame); return interpreter.interpret(script); } - + @Override public JexlScript curry(Object... args) { String[] parms = script.getParameters(); @@ -201,20 +201,16 @@ public class Script implements JexlScript, JexlExpression { return new Closure(this, args); } - /** - * Gets this script parameters. - * @return the parameters or null - * @since 3.0 - */ @Override public String[] getParameters() { return script.getParameters(); } - /** - * Gets this script local variables. - * @return the local variables or null - */ + @Override + public String[] getUnboundParameters() { + return getParameters(); + } + @Override public String[] getLocalVariables() { return script.getLocalVariables(); diff --git a/src/test/java/org/apache/commons/jexl3/Issues200Test.java b/src/test/java/org/apache/commons/jexl3/Issues200Test.java index 1102c07..184b371 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues200Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues200Test.java @@ -469,7 +469,7 @@ public class Issues200Test extends JexlTestCase { result = script.execute(ctx); Assert.assertEquals(10, result); } - + @Test public void test230() throws Exception { JexlEngine jexl = new JexlBuilder().cache(4).create(); @@ -488,7 +488,7 @@ public class Issues200Test extends JexlTestCase { Assert.assertEquals(42, value); } } - + @Test public void test265() throws Exception { JexlEngine jexl = new JexlBuilder().cache(4).create(); @@ -502,17 +502,17 @@ public class Issues200Test extends JexlTestCase { // ambiguous, parsing fails } script = jexl.createScript("(true) ? (x) : abs(2)"); - result = script.execute(ctxt); + result = script.execute(ctxt); Assert.assertEquals(42, result); script = jexl.createScript("(true) ? x : (abs(3))"); - result = script.execute(ctxt); + result = script.execute(ctxt); Assert.assertEquals(42, result); script = jexl.createScript("(!true) ? abs(4) : x"); - result = script.execute(ctxt); + result = script.execute(ctxt); Assert.assertEquals(42, result); } - - + + /** * An iterator that implements Closeable (at least implements a close method). */ @@ -574,25 +574,25 @@ public class Issues200Test extends JexlTestCase { public Arithmetic266(boolean strict) { super(strict); } - + static void closeIterator(Iterator266 i266) { Deque<Iterator266> queue = TLS_FOREACH.get(); if (queue != null) { queue.remove(i266); } } - + public Iterator<?> forEach(Iterable<?> collection) { Iterator266 it266 = new Iterator266((Iterator<Object>) collection.iterator()); Deque<Iterator266> queue = TLS_FOREACH.get(); queue.addFirst(it266); return it266; } - + public Iterator<?> forEach(Map<?,?> collection) { return forEach(collection.values()); } - + public void remove() { Deque<Iterator266> queue = TLS_FOREACH.get(); Iterator266 i266 = queue.getFirst(); @@ -604,21 +604,21 @@ public class Issues200Test extends JexlTestCase { } } } - + @Test public void test266() throws Exception { Object result; JexlScript script; JexlEngine jexl = new JexlBuilder().arithmetic(new Arithmetic266(true)).create(); JexlContext ctxt = new MapContext(); - + List<Integer> li = new ArrayList<Integer>(Arrays.asList(1, 2, 3, 4, 5 ,6)); ctxt.set("list", li); script = jexl.createScript("for (var item : list) { if (item <= 3) remove(); } return size(list)"); result = script.execute(ctxt); Assert.assertEquals(3, result); Assert.assertEquals(3, li.size()); - + Map<String, Integer> msi = new HashMap<String, Integer>(); msi.put("a", 1); msi.put("b", 2); @@ -632,7 +632,7 @@ public class Issues200Test extends JexlTestCase { Assert.assertEquals(4, result); Assert.assertEquals(4, msi.size()); } - + @Test public void test267() throws Exception { Object result; @@ -652,57 +652,4 @@ public class Issues200Test extends JexlTestCase { result = script.execute(ctxt); Assert.assertTrue(result instanceof JexlScript); } - - @Test - public void test270() throws Exception { - JexlEngine jexl = new JexlBuilder().create(); - JexlScript base = jexl.createScript("(x, y, z)->{ x + y + z }"); - String text = base.toString(); - JexlScript script = base.curry(5, 15); - Assert.assertEquals(text, script.toString()); - - JexlEvalContext ctxt = new JexlEvalContext(); - ctxt.set("s", base); - script = jexl.createScript("return s"); - Object result = script.execute(ctxt); - Assert.assertEquals(text, result.toString()); - - script = jexl.createScript("return s.curry(1)"); - result = script.execute(ctxt); - Assert.assertEquals(text, result.toString()); - } - - @Test - public void test271a() throws Exception { - JexlEngine jexl = new JexlBuilder().strict(false).create(); - JexlScript base = jexl.createScript("var base = 1; var x = (a)->{ var y = (b) -> {base + b}; return base + y(a)}; x(40)"); - Object result = base.execute(null); - Assert.assertEquals(42, result); - } - - @Test - public void test271b() throws Exception { - JexlEngine jexl = new JexlBuilder().strict(false).create(); - JexlScript base = jexl.createScript("var base = 2; var sum = (x, y, z)->{ base + x + y + z }; var y = sum.curry(1); y(2,3)"); - Object result = base.execute(null); - Assert.assertEquals(8, result); - } - - @Test - public void test271c() throws Exception { - JexlEngine jexl = new JexlBuilder().strict(false).create(); - JexlScript base = jexl.createScript("(x, y, z)->{ 2 + x + y + z };"); - JexlScript y = base.curry(1); - Object result = y.execute((JexlContext) null, 2, 3); - Assert.assertEquals(8, result); - } - - @Test - public void test271d() throws Exception { - JexlEngine jexl = new JexlBuilder().strict(false).create(); - JexlScript base = jexl.createScript("var base = 2; return (x, y, z)->{ base + x + y + z };"); - JexlScript y = ((JexlScript) base.execute(null)).curry(1); - Object result = y.execute((JexlContext) null, 2, 3); - Assert.assertEquals(8, result); - } } diff --git a/src/test/java/org/apache/commons/jexl3/LambdaTest.java b/src/test/java/org/apache/commons/jexl3/LambdaTest.java index a4f18e9..353cbc2 100644 --- a/src/test/java/org/apache/commons/jexl3/LambdaTest.java +++ b/src/test/java/org/apache/commons/jexl3/LambdaTest.java @@ -164,7 +164,7 @@ public class LambdaTest extends JexlTestCase { Assert.assertTrue(result instanceof JexlScript); s15 = (JexlScript) result; localv = s15.getLocalVariables(); - Assert.assertNull(localv); + Assert.assertEquals(0, localv.length); hvars = s15.getVariables(); Assert.assertEquals(1, hvars.size()); @@ -246,11 +246,20 @@ public class LambdaTest extends JexlTestCase { JexlEngine jexl = new Engine(); JexlScript script; Object result; + String[] parms; JexlScript base = jexl.createScript("(x, y, z)->{ x + y + z }"); + parms = base.getUnboundParameters(); + Assert.assertEquals(3, parms.length); script = base.curry(5); + parms = script.getUnboundParameters(); + Assert.assertEquals(2, parms.length); script = script.curry(15); + parms = script.getUnboundParameters(); + Assert.assertEquals(1, parms.length); script = script.curry(22); + parms = script.getUnboundParameters(); + Assert.assertEquals(0, parms.length); result = script.execute(null); Assert.assertEquals(42, result); } @@ -260,9 +269,12 @@ public class LambdaTest extends JexlTestCase { JexlEngine jexl = new Engine(); JexlScript script; Object result; + String[] parms; JexlScript base = jexl.createScript("(x, y, z)->{ x + y + z }"); script = base.curry(5, 15); + parms = script.getUnboundParameters(); + Assert.assertEquals(1, parms.length); script = script.curry(22); result = script.execute(null); Assert.assertEquals(42, result); @@ -279,4 +291,57 @@ public class LambdaTest extends JexlTestCase { result = script.execute(null, 22); Assert.assertEquals(42, result); } + + @Test + public void test270() throws Exception { + JexlEngine jexl = new Engine(); + JexlScript base = jexl.createScript("(x, y, z)->{ x + y + z }"); + String text = base.toString(); + JexlScript script = base.curry(5, 15); + Assert.assertEquals(text, script.toString()); + + JexlEvalContext ctxt = new JexlEvalContext(); + ctxt.set("s", base); + script = jexl.createScript("return s"); + Object result = script.execute(ctxt); + Assert.assertEquals(text, result.toString()); + + script = jexl.createScript("return s.curry(1)"); + result = script.execute(ctxt); + Assert.assertEquals(text, result.toString()); + } + + @Test + public void test271a() throws Exception { + JexlEngine jexl = new Engine(); + JexlScript base = jexl.createScript("var base = 1; var x = (a)->{ var y = (b) -> {base + b}; return base + y(a)}; x(40)"); + Object result = base.execute(null); + Assert.assertEquals(42, result); + } + + @Test + public void test271b() throws Exception { + JexlEngine jexl = new Engine(); + JexlScript base = jexl.createScript("var base = 2; var sum = (x, y, z)->{ base + x + y + z }; var y = sum.curry(1); y(2,3)"); + Object result = base.execute(null); + Assert.assertEquals(8, result); + } + + @Test + public void test271c() throws Exception { + JexlEngine jexl = new Engine(); + JexlScript base = jexl.createScript("(x, y, z)->{ 2 + x + y + z };"); + JexlScript y = base.curry(1); + Object result = y.execute(null, 2, 3); + Assert.assertEquals(8, result); + } + + @Test + public void test271d() throws Exception { + JexlEngine jexl = new Engine(); + JexlScript base = jexl.createScript("var base = 2; return (x, y, z)->{ base + x + y + z };"); + JexlScript y = ((JexlScript) base.execute(null)).curry(1); + Object result = y.execute(null, 2, 3); + Assert.assertEquals(8, result); + } }