This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-7232a in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 7c2347835226954140e901550a8970fbd3f96137 Author: Eric Milles <[email protected]> AuthorDate: Tue Jul 28 20:28:43 2020 -0500 GROOVY-7232: prefer methodMissing over declared method on next candidate --- .../groovy/runtime/metaclass/ClosureMetaClass.java | 95 ++++++++-------------- src/test/gls/closures/ResolveStrategyTest.groovy | 33 ++++---- src/test/groovy/lang/ClosureResolvingTest.groovy | 26 +++++- 3 files changed, 76 insertions(+), 78 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..9335613 100644 --- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java +++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java @@ -231,6 +231,7 @@ public final class ClosureMetaClass extends MetaClassImpl { } } + @Override public Object invokeMethod(Class sender, Object object, String methodName, Object[] originalArguments, boolean isCallToSuper, boolean fromInsideClass) { checkInitalised(); if (object == null) { @@ -248,7 +249,7 @@ public final class ClosureMetaClass extends MetaClassImpl { if (CLOSURE_DO_CALL_METHOD.equals(methodName) || CLOSURE_CALL_METHOD.equals(methodName)) { method = pickClosureMethod(argClasses); if (method == null && arguments.length == 1 && arguments[0] instanceof List) { - Object[] newArguments = ((List) arguments[0]).toArray(); + Object[] newArguments = ((List<?>) arguments[0]).toArray(); Class[] newArgClasses = MetaClassHelper.convertToTypeArray(newArguments); method = createTransformMetaMethod(pickClosureMethod(newArgClasses)); } @@ -266,7 +267,6 @@ public final class ClosureMetaClass extends MetaClassImpl { Object callObject = object; final Object owner = closure.getOwner(); final Object delegate = closure.getDelegate(); - final Object thisObject = closure.getThisObject(); boolean invokeOnDelegate = false; boolean invokeOnOwner = false; boolean ownerFirst = true; @@ -288,18 +288,22 @@ public final class ClosureMetaClass extends MetaClassImpl { 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) { + invokeOnDelegate = delegate != closure; + invokeOnOwner = owner != closure; + ownerFirst = false; } + break; + default: // Closure.OWNER_FIRST: + 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; + invokeOnDelegate = delegate != closure; + invokeOnOwner = owner != closure; + ownerFirst = true; } break; } @@ -337,13 +341,20 @@ public final class ClosureMetaClass extends MetaClassImpl { } } - if (last != null) throw last; - throw new MissingMethodException(methodName, theClass, arguments, false); + throw last != null ? last : new MissingMethodException(methodName, theClass, arguments, false); } private static boolean isInternalMethod(String methodName) { - return methodName.equals("curry") || methodName.equals("ncurry") || methodName.equals("rcurry") || - methodName.equals("leftShift") || methodName.equals("rightShift"); + switch (methodName) { + case "curry": + case "ncurry": + case "rcurry": + case "leftShift": + case "rightShift": + return true; + default: + return false; + } } private static Object[] makeArguments(Object[] arguments, String methodName) { @@ -351,34 +362,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(); @@ -386,38 +369,32 @@ public final class ClosureMetaClass extends MetaClassImpl { return th; } - private static Object invokeOnDelegationObjects( - boolean invoke1, Object o1, - boolean invoke2, Object o2, - String methodName, Object[] args) { + private static Object invokeOnDelegationObjects(boolean invoke1, Object o1, boolean invoke2, Object o2, String methodName, Object[] args) { MissingMethodException first = null; if (invoke1) { - GroovyObject go = (GroovyObject) o1; try { - return go.invokeMethod(methodName, args); + return InvokerHelper.invokeMethod(o1, methodName, args); } catch (MissingMethodException mme) { first = mme; } catch (GroovyRuntimeException gre) { - Throwable th = unwrap(gre); - if ((th instanceof MissingMethodException) - && (methodName.equals(((MissingMethodException) th).getMethod()))) { - first = (MissingMethodException) th; + Throwable t = unwrap(gre); + if (t instanceof MissingMethodException && methodName.equals(((MissingMethodException) t).getMethod())) { + first = (MissingMethodException) t; } else { throw gre; } } } if (invoke2 && (!invoke1 || o1 != o2)) { - GroovyObject go = (GroovyObject) o2; try { - return go.invokeMethod(methodName, args); + return InvokerHelper.invokeMethod(o2, methodName, args); } catch (MissingMethodException mme) { // patch needed here too, but we need a test case to trip it first if (first == null) first = mme; } catch (GroovyRuntimeException gre) { - Throwable th = unwrap(gre); - if (th instanceof MissingMethodException) { - first = (MissingMethodException) th; + Throwable t = unwrap(gre); + if (t instanceof MissingMethodException) { + first = (MissingMethodException) t; } else { throw gre; } diff --git a/src/test/gls/closures/ResolveStrategyTest.groovy b/src/test/gls/closures/ResolveStrategyTest.groovy index 1a8111d..2124f2b 100644 --- a/src/test/gls/closures/ResolveStrategyTest.groovy +++ b/src/test/gls/closures/ResolveStrategyTest.groovy @@ -23,33 +23,32 @@ 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 // (*) +final class ResolveStrategyTest extends GroovyTestCase { - assert run(DELEGATE_ONLY) == 12340000 // (*) - assert runDelegateOnly { m1() + m2() + m3() + m4() } == 12340000 // (*) + void testDynamicResolveStrategy() { + new MyClass().with { + assert run(OWNER_ONLY) == 1234 + assert runOwnerOnly { m1() + m2() + m3() + m4() } == 1234 - // (*) involves methodMissing as forced by ONLY strategy (no equivalent CS case below) + assert run(DELEGATE_ONLY) == 12340000 + assert runDelegateOnly { m1() + m2() + m3() + m4() } == 12340000 - assert run(OWNER_FIRST) == 41230 - assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230 + assert run(OWNER_FIRST) == 1234 + assert runOwnerFirst { m1() + m2() + m3() + m4() } == 1234 - assert run(DELEGATE_FIRST) == 12040030 - assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12040030 + assert run(DELEGATE_FIRST) == 12340000 + assert runDelegateFirst { m1() + m2() + m3() + m4() } == 12340000 // nested cases - 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 runOwnerFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 1234 + assert runOwnerFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12340000 + assert runDelegateFirst { runOwnerFirst { m1() + m2() + m3() + m4() } } == 12340000 + assert runDelegateFirst { runDelegateFirst { m1() + m2() + m3() + m4() } } == 12340000 } } @CompileStatic - void testStaticCases() { + void testStaticResolveStrategy() { new MyClass().with { assert runOwnerOnly { m1() + m2() + m3() } == 1230 assert runOwnerFirst { m1() + m2() + m3() + m4() } == 41230 diff --git a/src/test/groovy/lang/ClosureResolvingTest.groovy b/src/test/groovy/lang/ClosureResolvingTest.groovy index e266647..686f573 100644 --- a/src/test/groovy/lang/ClosureResolvingTest.groovy +++ b/src/test/groovy/lang/ClosureResolvingTest.groovy @@ -25,8 +25,7 @@ import groovy.test.GroovyTestCase * * @since 1.5 */ - -class ClosureResolvingTest extends GroovyTestCase { +final class ClosureResolvingTest extends GroovyTestCase { def foo = "bar" def bar = "foo" @@ -244,6 +243,29 @@ class ClosureResolvingTest extends GroovyTestCase { assert result.flatten() == [1, 'x', 'f'] ''' } + + // GROOVY-7232 + void testOwnerDelegateChain4() { + assertScript ''' + @GrabResolver(name='grails', root='https://repo.grails.org/grails/core') + @Grab('org.grails:grails-web-url-mappings:4.0.1') + @GrabExclude('org.codehaus.groovy:*') + import grails.web.mapping.* + + def linkGenerator = new LinkGeneratorFactory().create { -> + group('/g') { -> + '/bars'(resources: 'bar') { -> + owner.owner.collection { -> // TODO: remove qualifier + '/baz'(controller: 'bar', action: 'baz') + } + } + } + } + + def link = linkGenerator.link(controller: 'bar', action: 'baz', params: [barId: 1]) + assert link == 'http://localhost/g/bars/1/baz' + ''' + } } class TestResolve1 {
