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

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new b6bfd49  GROOVY-9852: closure/lambda match @FunctionalInterface vs 
Object methods
b6bfd49 is described below

commit b6bfd49f841c6ed4f2f6f9a56247b7241673f108
Author: Eric Milles <[email protected]>
AuthorDate: Thu Dec 10 11:16:36 2020 -0600

    GROOVY-9852: closure/lambda match @FunctionalInterface vs Object methods
    
    given:
      def m(AutoCloseable ac) // functional interface
      def m(InputStream is) // SAM type
      def m(Object o)
    expect:
      "m { }" should choose the AutoCloseable method
    
    given:
      def m(InputStream x)
      def m(Object o)
    expect:
      "m { }" should choose the Object method
    
    given:
      def m(AutoCloseable x)
      def m(GroovyObject o)
    expect:
      "m { }" should choose the GroovyObject method
---
 .../codehaus/groovy/runtime/MetaClassHelper.java   | 55 ++++++++++++----------
 .../transform/stc/StaticTypeCheckingSupport.java   | 33 ++++++-------
 src/test/gls/invocation/MethodSelectionTest.groovy | 28 +++++++----
 .../groovy/transform/stc/ClosuresSTCTest.groovy    | 27 +++++++----
 4 files changed, 82 insertions(+), 61 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java 
b/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
index 0545ca1..73b81dc 100644
--- a/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
@@ -43,6 +43,8 @@ import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static 
org.codehaus.groovy.reflection.stdclasses.CachedSAMClass.getSAMMethod;
+
 public class MetaClassHelper {
 
     public static final Object[] EMPTY_ARRAY = {};
@@ -290,56 +292,57 @@ public class MetaClassHelper {
         return Math.max(max, superClassMax);
     }
 
-    private static long calculateParameterDistance(Class argument, CachedClass 
parameter) {
-        /**
+    private static long calculateParameterDistance(final Class<?> argument, 
final CachedClass parameter) {
+        /*
          * note: when shifting with 32 bit, you should only shift on a long. 
If you do
          *       that with an int, then i==(i<<32), which means you loose the 
shift
          *       information
          */
 
-        if (parameter.getTheClass() == argument) return 0;
+        Class<?> parameterClass = parameter.getTheClass();
+        if (parameterClass == argument) return 0;
 
         if (parameter.isInterface()) {
-            int dist = getMaximumInterfaceDistance(argument, 
parameter.getTheClass()) << INTERFACE_SHIFT;
-            if (dist>-1 || !(argument!=null && 
Closure.class.isAssignableFrom(argument))) {
-                return dist;
-            } // else go to object case
+            long dist = getMaximumInterfaceDistance(argument, parameterClass);
+            if (dist >= 0 || argument == null || 
!Closure.class.isAssignableFrom(argument)) {
+                return dist << INTERFACE_SHIFT;
+            }
         }
 
         long objectDistance = 0;
         if (argument != null) {
-            long pd = getPrimitiveDistance(parameter.getTheClass(), argument);
-            if (pd != -1) return pd << PRIMITIVE_SHIFT;
+            long dist = getPrimitiveDistance(parameterClass, argument);
+            if (dist >= 0) {
+                return dist << PRIMITIVE_SHIFT;
+            }
 
             // add one to dist to be sure interfaces are preferred
-            objectDistance += PRIMITIVES.length + 1;
+            objectDistance += (PRIMITIVES.length + 1);
 
-            // GROOVY-5114 : if we have to choose between two methods
-            // foo(Object[]) and foo(Object) and that the argument is an array 
type
-            // then the array version should be preferred
+            // GROOVY-5114: if choosing between foo(Object[]) and foo(Object)
+            // and the argument is an array, then array version is preferable
             if (argument.isArray() && !parameter.isArray) {
-                objectDistance+=4;
+                objectDistance += 4;
             }
-            Class clazz = ReflectionCache.autoboxType(argument);
-            while (clazz != null) {
-                if (clazz == parameter.getTheClass()) break;
-                if (clazz == GString.class && parameter.getTheClass() == 
String.class) {
+
+            for (Class<?> c = ReflectionCache.autoboxType(argument); c != null 
&& c != parameterClass; c = c.getSuperclass()) {
+                if (c == Closure.class && parameterClass.isInterface() && 
getSAMMethod(parameterClass) != null) {
+                    objectDistance += 5; // ahead of Object but behind 
GroovyObjectSupport
+                    break;
+                }
+                if (c == GString.class && parameterClass == String.class) {
                     objectDistance += 2;
                     break;
                 }
-                clazz = clazz.getSuperclass();
                 objectDistance += 3;
             }
         } else {
-            // choose the distance to Object if a parameter is null
-            // this will mean that Object is preferred over a more
-            // specific type
-            Class clazz = parameter.getTheClass();
-            if (clazz.isPrimitive()) {
+            // choose the distance to Object if an argument is null, which 
means
+            // that Object is preferred over a more specific type
+            if (parameterClass.isPrimitive()) {
                 objectDistance += 2;
             } else {
-                while (clazz != Object.class && clazz != null) {
-                    clazz = clazz.getSuperclass();
+                for (Class<?> c = parameterClass; c != null && c != 
Object.class; c = c.getSuperclass()) {
                     objectDistance += 2;
                 }
             }
diff --git 
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
 
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index da96fe2..16008b7 100644
--- 
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ 
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -903,28 +903,25 @@ public abstract class StaticTypeCheckingSupport {
         if (isPrimitiveType(receiver) ^ isPrimitiveType(compare)) {
             dist = (dist + 1) << 1;
         }
-        if (unwrapCompare.equals(unwrapReceiver)) return dist;
-        if (receiver.isArray() && !compare.isArray()) {
-            // Object[] vs Object
-            dist += 256;
-        }
-
-        if (receiver == UNKNOWN_PARAMETER_TYPE) {
+        if (unwrapCompare.equals(unwrapReceiver)
+                || receiver == UNKNOWN_PARAMETER_TYPE) {
             return dist;
         }
-
-        ClassNode ref = isPrimitiveType(receiver) && !isPrimitiveType(compare) 
? ClassHelper.getWrapper(receiver) : receiver;
-        while (ref != null) {
-            if (compare.equals(ref)) {
-                break;
-            }
-            if (compare.isInterface() && ref.implementsInterface(compare)) {
-                dist += getMaximumInterfaceDistance(ref, compare);
-                break;
+        if (receiver.isArray()) {
+            dist += 256; // GROOVY-5114: Object[] vs Object
+        }
+        if (compare.isInterface()) {
+            if (receiver.implementsInterface(compare)) {
+                return dist + getMaximumInterfaceDistance(receiver, compare);
+            } else if (receiver.equals(CLOSURE_TYPE) && isSAMType(compare)) {
+                return dist + 13; // GROOVY-9852: @FunctionalInterface vs 
Object
             }
-            ref = ref.getSuperClass();
+        }
+        ClassNode cn = isPrimitiveType(receiver) && !isPrimitiveType(compare) 
? ClassHelper.getWrapper(receiver) : receiver;
+        while (cn != null && !cn.equals(compare)) {
+            cn = cn.getSuperClass();
             dist += 1;
-            if (OBJECT_TYPE.equals(ref))
+            if (OBJECT_TYPE.equals(cn))
                 dist += 1;
             dist = (dist + 1) << 1;
         }
diff --git a/src/test/gls/invocation/MethodSelectionTest.groovy 
b/src/test/gls/invocation/MethodSelectionTest.groovy
index a30902c..361cdf3 100644
--- a/src/test/gls/invocation/MethodSelectionTest.groovy
+++ b/src/test/gls/invocation/MethodSelectionTest.groovy
@@ -384,27 +384,37 @@ class MethodSelectionTest extends CompilableTestSupport {
       """
   }
   
-  //GROOVY-6189
-  void testSAMs(){
+  // GROOVY-6189, GROOVY-9852
+  void testSAMs() {
       // simple direct case
-      assertScript """
+      assertScript '''
           interface MySAM {
               def someMethod()
           }
           def foo(MySAM sam) {sam.someMethod()}
           assert foo {1} == 1
-      """
+      '''
 
       // overloads with classes implemented by Closure
-      ["java.util.concurrent.Callable", "Object", "Closure", 
"GroovyObjectSupport", "Cloneable", "Runnable", "GroovyCallable", 
"Serializable", "GroovyObject"].each {
-          className ->
+      [
+          'groovy.lang.Closure'            : 'not',
+          'groovy.lang.GroovyCallable'     : 'not',
+          'groovy.lang.GroovyObject'       : 'not',
+          'groovy.lang.GroovyObjectSupport': 'not',
+
+          'java.lang.Object'               : 'sam',
+          'java.lang.Runnable'             : 'not',
+          'java.lang.Cloneable'            : 'not',
+          'java.io.Serializable'           : 'not',
+          'java.util.concurrent.Callable'  : 'not',
+      ].each { type, which ->
           assertScript """
               interface MySAM {
                   def someMethod()
               }
-              def foo(MySAM sam) {sam.someMethod()}
-              def foo($className x) {2}
-              assert foo {1} == 2
+              def foo($type ref) { 'not' }
+              def foo(MySAM sam) { sam.someMethod() }
+              assert foo { 'sam' } == '$which' : '$type'
           """
       }
   }
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy 
b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index 56ab82e..3fa8cd9 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -352,27 +352,38 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
         ''', 'Cannot find matching method'
     }
 
-    // GROOVY-6189
+    // GROOVY-6189, GROOVY-9852
     void testSAMsInMethodSelection() {
         // simple direct case
-        assertScript """
+        assertScript '''
             interface MySAM {
                 def someMethod()
             }
             def foo(MySAM sam) {sam.someMethod()}
             assert foo {1} == 1
-        """
+        '''
 
         // overloads with classes implemented by Closure
-        ["java.util.concurrent.Callable", "Object", "Closure", 
"GroovyObjectSupport", "Cloneable", "Runnable", "GroovyCallable", 
"Serializable", "GroovyObject"].each {
-            className ->
+        [
+            'groovy.lang.Closure'            : 'not',
+            'groovy.lang.GroovyCallable'     : 'not',
+            'groovy.lang.GroovyObject'       : 'not',
+            'groovy.lang.GroovyObjectSupport': 'not',
+
+            'java.lang.Object'               : 'sam',
+            'java.lang.Runnable'             : 'not',
+            'java.lang.Cloneable'            : 'not',
+            'java.io.Serializable'           : 'not',
+            'java.util.concurrent.Callable'  : 'not',
+        ].each { type, which ->
             assertScript """
                 interface MySAM {
                     def someMethod()
                 }
-                def foo(MySAM sam) {sam.someMethod()}
-                def foo($className x) {2}
-                assert foo {1} == 2
+                def foo($type ref) { 'not' }
+                def foo(MySAM sam) { sam.someMethod() }
+                assert foo { 'sam' } == '$which' : '$type'
+                assert foo(() -> 'sam') == '$which' : '$type'
             """
         }
     }

Reply via email to