This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new a66bc2f Revert "GROOVY-7232: check resolve strategy of each closure
during method search"
a66bc2f is described below
commit a66bc2facb0f018c807493379aeca50455b77f52
Author: Daniel Sun <[email protected]>
AuthorDate: Sat Aug 1 19:24:53 2020 +0800
Revert "GROOVY-7232: check resolve strategy of each closure during method
search"
This is a breaking change and should not appear in a maintenance branch
This reverts commit 4d04c10b
---
.../groovy/runtime/metaclass/ClosureMetaClass.java | 76 +++++++++++-----------
src/test/gls/closures/ResolveStrategyTest.groovy | 38 +++++------
src/test/groovy/lang/ClosureResolvingTest.groovy | 41 ------------
3 files changed, 57 insertions(+), 98 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 1b97ef9..af0bf2b 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
@@ -47,6 +47,7 @@ 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;
@@ -243,7 +244,6 @@ 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 = resolveStrategy == Closure.DELEGATE_ONLY &&
isInternalMethod(methodName);
+ boolean shouldDefer = closure.getResolveStrategy() ==
Closure.DELEGATE_ONLY && isInternalMethod(methodName);
if (method == null && !shouldDefer) {
method = CLOSURE_METACLASS.pickMethod(methodName, argClasses);
}
@@ -267,6 +267,7 @@ 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;
@@ -287,21 +288,48 @@ public final class ClosureMetaClass extends MetaClassImpl
{
if (method == null) {
invokeOnOwner = owner != closure && (owner instanceof
GroovyObject);
}
+
break;
- default: // Closure.*_FIRST:
- for (Object candidate : ownersAndDelegatesOf(closure, new
ArrayList<>())) {
- method = getDelegateMethod(closure, candidate, methodName,
argClasses);
- callObject = candidate;
- if (method != null) {
- 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 = resolveStrategy != Closure.DELEGATE_FIRST;
+ 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;
+ }
+ }
+ if (method == null) {
+ invokeOnDelegate = delegate != closure && (delegate
instanceof GroovyObject);
+ invokeOnOwner = owner != closure && (owner instanceof
GroovyObject);
+ }
}
if (method == null && (invokeOnOwner || invokeOnDelegate)) {
try {
@@ -351,34 +379,6 @@ 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 1a8111d..e9cb637 100644
--- a/src/test/gls/closures/ResolveStrategyTest.groovy
+++ b/src/test/gls/closures/ResolveStrategyTest.groovy
@@ -20,31 +20,30 @@ 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(OWNER_ONLY) == 1234 // (*)
- assert runOwnerOnly { m1() + m2() + m3() + m4() } == 1234 // (*)
-
assert run(DELEGATE_ONLY) == 12340000 // (*)
- assert runDelegateOnly { m1() + m2() + m3() + m4() } == 12340000
// (*)
-
- // (*) involves methodMissing as forced by ONLY strategy (no
equivalent CS case below)
-
+ assert run(DELEGATE_FIRST) == 12040030
+ assert run(OWNER_ONLY) == 1234 // (*)
assert run(OWNER_FIRST) == 41230
- assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230
- assert run(DELEGATE_FIRST) == 12040030
- assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12040030
+ assert runOwnerOnly { m1() + m2() + m3() } == 1230
+ assert runOwnerOnly { m1() + m2() + m3() + m4() } == 1234 // (*)
+ assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230
+ assert runDelegateOnly { m1() + m2() + m3() + m4() } == 12340000
// (*)
+ assert runDelegateOnly { m1() + m2() + m4() } == 12040000
+ assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12340000
// (**)
// 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 runOwnerFirst { runDelegateFirst { m1() + m2() + m3() +
m4() } } == 12040030
- assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() +
m4() } } == 12040030
- assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() +
m4() } } == 12040030
+ 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
}
}
@@ -54,13 +53,14 @@ 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
// (*)
- // GROOVY-9086: nested cases
+ // nested cases (GROOVY-9086)
+ assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() +
m4() } } == 12040030 // (*)
+ assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() +
m4() } } == 12040030 // (*)
assert runOwnerFirst { runOwnerFirst { m1() + m2() + m3() + m4() }
} == 41230
- assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() +
m4() } } == 12040030
- assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() +
m4() } } == 12040030
- assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() +
m4() } } == 12040030
+ assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() +
m4() } } == 12040030 // (*)
+ // (*) different to dynamic since static assumes no methodMissing
}
}
}
diff --git a/src/test/groovy/lang/ClosureResolvingTest.groovy
b/src/test/groovy/lang/ClosureResolvingTest.groovy
index e266647..69d2932 100644
--- a/src/test/groovy/lang/ClosureResolvingTest.groovy
+++ b/src/test/groovy/lang/ClosureResolvingTest.groovy
@@ -203,47 +203,6 @@ 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 {
