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/29 14:57:43 UTC

[groovy] branch GROOVY-7232a updated (6f75900 -> 7c23478)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch GROOVY-7232a
in repository https://gitbox.apache.org/repos/asf/groovy.git.


 discard 6f75900  GROOVY-7232: prefer missingMethod over declared method on next candidate
     new 7c23478  GROOVY-7232: prefer methodMissing over declared method on next candidate

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   (6f75900)
            \
             N -- N -- N   refs/heads/GROOVY-7232a (7c23478)

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/groovy/lang/ClosureResolvingTest.groovy | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)


[groovy] 01/01: GROOVY-7232: prefer methodMissing over declared method on next candidate

Posted by em...@apache.org.
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 <er...@thomsonreuters.com>
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 {