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/07 23:48:31 UTC
[groovy] 01/01: GROOVY-7232: check resolve strategy of each closure
during method search
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 7757186fa8ec76f98d099680c0bc4af60b16a2a6
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 | 27 ++++----
src/test/groovy/lang/ClosureResolvingTest.groovy | 41 ++++++++++++
3 files changed, 93 insertions(+), 51 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..cbb6fd8 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
}
}
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 {