You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2020/07/08 00:00:57 UTC
[groovy] branch GROOVY-7232 updated (7757186 -> 7b02712)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-7232
in repository https://gitbox.apache.org/repos/asf/groovy.git.
discard 7757186 GROOVY-7232: check resolve strategy of each closure during method search
new 7b02712 GROOVY-7232: check resolve strategy of each closure during method search
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (7757186)
\
N -- N -- N refs/heads/GROOVY-7232 (7b02712)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/test/gls/closures/ResolveStrategyTest.groovy | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
[groovy] 01/01: GROOVY-7232: check resolve strategy of each closure
during method search
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-7232
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 7b02712a72e1db737277adf434c7ec066a06753e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jul 7 18:47:48 2020 -0500
GROOVY-7232: check resolve strategy of each closure during method search
---
.../groovy/runtime/metaclass/ClosureMetaClass.java | 76 +++++++++++-----------
src/test/gls/closures/ResolveStrategyTest.groovy | 38 +++++------
src/test/groovy/lang/ClosureResolvingTest.groovy | 41 ++++++++++++
3 files changed, 98 insertions(+), 57 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
index af0bf2b..1b97ef9 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
@@ -47,7 +47,6 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.HashMap;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -244,6 +243,7 @@ public final class ClosureMetaClass extends MetaClassImpl {
MetaMethod method = null;
final Closure closure = (Closure) object;
+ final int resolveStrategy = closure.getResolveStrategy();
if (CLOSURE_DO_CALL_METHOD.equals(methodName) || CLOSURE_CALL_METHOD.equals(methodName)) {
method = pickClosureMethod(argClasses);
@@ -255,7 +255,7 @@ public final class ClosureMetaClass extends MetaClassImpl {
if (method == null) throw new MissingMethodException(methodName, theClass, arguments, false);
}
- boolean shouldDefer = closure.getResolveStrategy() == Closure.DELEGATE_ONLY && isInternalMethod(methodName);
+ boolean shouldDefer = resolveStrategy == Closure.DELEGATE_ONLY && isInternalMethod(methodName);
if (method == null && !shouldDefer) {
method = CLOSURE_METACLASS.pickMethod(methodName, argClasses);
}
@@ -267,7 +267,6 @@ public final class ClosureMetaClass extends MetaClassImpl {
final Object owner = closure.getOwner();
final Object delegate = closure.getDelegate();
final Object thisObject = closure.getThisObject();
- final int resolveStrategy = closure.getResolveStrategy();
boolean invokeOnDelegate = false;
boolean invokeOnOwner = false;
boolean ownerFirst = true;
@@ -288,48 +287,21 @@ public final class ClosureMetaClass extends MetaClassImpl {
if (method == null) {
invokeOnOwner = owner != closure && (owner instanceof GroovyObject);
}
-
break;
- case Closure.DELEGATE_FIRST:
- method = getDelegateMethod(closure, delegate, methodName, argClasses);
- callObject = delegate;
- if (method == null) {
- method = getDelegateMethod(closure, owner, methodName, argClasses);
- callObject = owner;
- }
- if (method == null) {
- invokeOnDelegate = delegate != closure && (delegate instanceof GroovyObject);
- invokeOnOwner = owner != closure && (owner instanceof GroovyObject);
- ownerFirst = false;
- }
- break;
- default: // owner first
- // owner first means we start with the outer most owner that is not a generated closure
- // this owner is equal to the this object, so we check that one first.
- method = getDelegateMethod(closure, thisObject, methodName, argClasses);
- callObject = thisObject;
- if (method == null) {
- // try finding a delegate that has that method... we start from
- // outside building a stack and try each delegate
- LinkedList list = new LinkedList();
- for (Object current = closure; current != thisObject;) {
- if (!(current instanceof Closure)) break;
- Closure currentClosure = (Closure) current;
- if (currentClosure.getDelegate() != null) list.add(current);
- current = currentClosure.getOwner();
- }
-
- while (!list.isEmpty() && method == null) {
- Closure closureWithDelegate = (Closure) list.removeLast();
- Object currentDelegate = closureWithDelegate.getDelegate();
- method = getDelegateMethod(closureWithDelegate, currentDelegate, methodName, argClasses);
- callObject = currentDelegate;
+ default: // Closure.*_FIRST:
+ for (Object candidate : ownersAndDelegatesOf(closure, new ArrayList<>())) {
+ method = getDelegateMethod(closure, candidate, methodName, argClasses);
+ callObject = candidate;
+ if (method != null) {
+ break;
}
}
if (method == null) {
invokeOnDelegate = delegate != closure && (delegate instanceof GroovyObject);
invokeOnOwner = owner != closure && (owner instanceof GroovyObject);
+ ownerFirst = resolveStrategy != Closure.DELEGATE_FIRST;
}
+ break;
}
if (method == null && (invokeOnOwner || invokeOnDelegate)) {
try {
@@ -379,6 +351,34 @@ public final class ClosureMetaClass extends MetaClassImpl {
return arguments;
}
+ private static List ownersAndDelegatesOf(Closure closure, List ownersAndDelegates) {
+ switch (closure.getResolveStrategy()) {
+ case Closure.TO_SELF:
+ ownersAndDelegates.add(closure);
+ break;
+ case Closure.OWNER_ONLY:
+ ownersAndDelegates.add(closure.getOwner());
+ break;
+ case Closure.DELEGATE_ONLY:
+ ownersAndDelegates.add(closure.getDelegate());
+ break;
+ case Closure.DELEGATE_FIRST:
+ ownersAndDelegates.add(closure.getDelegate());
+ ownersAndDelegates.add(closure.getOwner());
+ if (closure.getOwner() instanceof Closure) {
+ ownersAndDelegatesOf((Closure) closure.getOwner(), ownersAndDelegates);
+ }
+ break;
+ default: // Closure.OWNER_FIRST:
+ ownersAndDelegates.add(closure.getOwner());
+ if (closure.getOwner() instanceof Closure) {
+ ownersAndDelegatesOf((Closure) closure.getOwner(), ownersAndDelegates);
+ }
+ ownersAndDelegates.add(closure.getDelegate());
+ }
+ return ownersAndDelegates;
+ }
+
private static Throwable unwrap(GroovyRuntimeException gre) {
Throwable th = gre;
if (th.getCause() != null && th.getCause() != gre) th = th.getCause();
diff --git a/src/test/gls/closures/ResolveStrategyTest.groovy b/src/test/gls/closures/ResolveStrategyTest.groovy
index e9cb637..1a8111d 100644
--- a/src/test/gls/closures/ResolveStrategyTest.groovy
+++ b/src/test/gls/closures/ResolveStrategyTest.groovy
@@ -20,30 +20,31 @@ package gls.closures
import groovy.test.GroovyTestCase
import groovy.transform.CompileStatic
+
import static groovy.lang.Closure.*
class ResolveStrategyTest extends GroovyTestCase {
void testDynamicSettingOfResolveStrategy() {
new MyClass().with {
- assert run(DELEGATE_ONLY) == 12340000 // (*)
- assert run(DELEGATE_FIRST) == 12040030
assert run(OWNER_ONLY) == 1234 // (*)
- assert run(OWNER_FIRST) == 41230
-
- assert runOwnerOnly { m1() + m2() + m3() } == 1230
assert runOwnerOnly { m1() + m2() + m3() + m4() } == 1234 // (*)
- assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230
+
+ assert run(DELEGATE_ONLY) == 12340000 // (*)
assert runDelegateOnly { m1() + m2() + m3() + m4() } == 12340000 // (*)
- assert runDelegateOnly { m1() + m2() + m4() } == 12040000
- assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12340000 // (**)
+
+ // (*) involves methodMissing as forced by ONLY strategy (no equivalent CS case below)
+
+ assert run(OWNER_FIRST) == 41230
+ assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230
+
+ assert run(DELEGATE_FIRST) == 12040030
+ assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12040030
// nested cases
- assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 41230
- assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12340000 // (**)
assert runOwnerFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 41230
- assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12340000 // (**)
- // (*) involves methodMissing as forced by ONLY strategy (no equivalent CS case below)
- // (**) involves methodMissing since delegate methodMissing has precedence over explicit owner method
+ assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030
+ assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 12040030
+ assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030
}
}
@@ -53,14 +54,13 @@ class ResolveStrategyTest extends GroovyTestCase {
assert runOwnerOnly { m1() + m2() + m3() } == 1230
assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230
assert runDelegateOnly { m1() + m2() + m4() } == 12040000
- assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12040030 // (*)
+ assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12040030
- // nested cases (GROOVY-9086)
- assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 12040030 // (*)
- assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030 // (*)
+ // GROOVY-9086: nested cases
assert runOwnerFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 41230
- assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030 // (*)
- // (*) different to dynamic since static assumes no methodMissing
+ assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 12040030
+ assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030
+ assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12040030
}
}
}
diff --git a/src/test/groovy/lang/ClosureResolvingTest.groovy b/src/test/groovy/lang/ClosureResolvingTest.groovy
index 69d2932..e266647 100644
--- a/src/test/groovy/lang/ClosureResolvingTest.groovy
+++ b/src/test/groovy/lang/ClosureResolvingTest.groovy
@@ -203,6 +203,47 @@ class ClosureResolvingTest extends GroovyTestCase {
cout()
}
+ // GROOVY-7232
+ void testOwnerDelegateChain2() {
+ assertScript '''
+ def outer = { ->
+ def inner = { ->
+ [x, keySet()]
+ }
+ inner.resolveStrategy = Closure.DELEGATE_ONLY
+ inner.delegate = [x: 1, f: 0]
+ inner.call()
+ }
+ //outer.resolveStrategy = Closure.OWNER_FIRST
+ outer.delegate = [x: 0, g: 0]
+ def result = outer.call()
+
+ assert result.flatten() == [1, 'x', 'f']
+ '''
+ }
+
+ // GROOVY-7232
+ void testOwnerDelegateChain3() {
+ assertScript '''
+ def outer = { ->
+ def inner = { ->
+ def inner_inner = { ->
+ [x, keySet()]
+ }
+ //inner_inner.resolveStrategy = Closure.OWNER_FIRST
+ return inner_inner.call()
+ }
+ inner.resolveStrategy = Closure.DELEGATE_ONLY
+ inner.delegate = [x: 1, f: 0]
+ inner()
+ }
+ //outer.resolveStrategy = Closure.OWNER_FIRST
+ outer.delegate = [x: 0, g: 0]
+ def result = outer.call()
+
+ assert result.flatten() == [1, 'x', 'f']
+ '''
+ }
}
class TestResolve1 {