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'
"""
}
}