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 2022/08/22 00:36:42 UTC
[groovy] 01/01: GROOVY-8788: STC: prefer closer parameter match over receiver-type match
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-8788-take2
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f9ef470f321c0770aab97e918f28123a810327d6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Aug 21 19:36:17 2022 -0500
GROOVY-8788: STC: prefer closer parameter match over receiver-type match
---
src/main/groovy/groovy/grape/GrapeIvy.groovy | 2 +-
.../transform/stc/StaticTypeCheckingSupport.java | 96 ++++++++++------------
src/test/groovy/bugs/Groovy8629Bug.groovy | 5 +-
.../groovy/transform/stc/STCAssignmentTest.groovy | 4 +-
.../classgen/asm/sc/bugs/Groovy7325Bug.groovy | 29 +++----
.../groovy/transform/DelegateTransformTest.groovy | 6 +-
6 files changed, 67 insertions(+), 75 deletions(-)
diff --git a/src/main/groovy/groovy/grape/GrapeIvy.groovy b/src/main/groovy/groovy/grape/GrapeIvy.groovy
index bfc00258f1..8a192b35b4 100644
--- a/src/main/groovy/groovy/grape/GrapeIvy.groovy
+++ b/src/main/groovy/groovy/grape/GrapeIvy.groovy
@@ -596,7 +596,7 @@ class GrapeIvy implements GrapeEngine {
// check for mutually exclusive arguments
Set<String> keys = (Set<String>) args.keySet()
keys.each { key ->
- Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS[key]
+ Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS.get(key)
if (badArgs && !badArgs.disjoint(keys)) {
throw new RuntimeException("Mutually exclusive arguments passed into grab: ${keys.intersect(badArgs) + key}")
}
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 4d4090fc82..6024b92e61 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1031,47 +1031,28 @@ public abstract class StaticTypeCheckingSupport {
*
* @return zero or more results
*/
- public static List<MethodNode> chooseBestMethod(final ClassNode receiver, final Collection<MethodNode> methods, final ClassNode... argumentTypes) {
+ public static List<MethodNode> chooseBestMethod(final ClassNode receiver, Collection<MethodNode> methods, final ClassNode... argumentTypes) {
if (!asBoolean(methods)) {
return Collections.emptyList();
}
int bestDist = Integer.MAX_VALUE;
- List<MethodNode> bestChoices = new LinkedList<>();
- boolean duckType = receiver instanceof UnionTypeClassNode; // GROOVY-8965: type disjunction
- boolean noCulling = methods.size() <= 1 || "<init>".equals(methods.iterator().next().getName());
- Iterable<MethodNode> candidates = noCulling ? methods : removeCovariantsAndInterfaceEquivalents(methods, duckType);
-
- for (MethodNode candidate : candidates) {
- MethodNode safeNode = candidate;
- ClassNode[] safeArgs = argumentTypes;
- boolean isExtensionMethod = candidate instanceof ExtensionMethodNode;
- if (isExtensionMethod) {
- int nArgs = argumentTypes.length;
- safeArgs = new ClassNode[nArgs + 1];
- System.arraycopy(argumentTypes, 0, safeArgs, 1, nArgs);
- safeArgs[0] = receiver; // prepend self-type as first argument
- safeNode = ((ExtensionMethodNode) candidate).getExtensionMethodNode();
- }
-
- /* TODO: corner case
- class B extends A {}
- Animal foo(A a) {}
- Person foo(B b) {}
-
- B b = new B()
- Person p = foo(b)
- */
-
- ClassNode declaringClass = candidate.getDeclaringClass();
+ List<MethodNode> bestMethods = new LinkedList<>();
+ boolean duckType = receiver instanceof UnionTypeClassNode;
+ if (methods.size() > 1 && !methods.iterator().next().isConstructor())
+ methods = removeCovariantsAndInterfaceEquivalents(methods, duckType);
+
+ // phase 1: argument-parameter distance classifier
+ for (MethodNode method : methods) {
+ ClassNode declaringClass = method.getDeclaringClass();
ClassNode actualReceiver = receiver != null ? receiver : declaringClass;
Map<GenericsType, GenericsType> spec;
- if (candidate.isStatic()) {
+ if (method.isStatic()) {
spec = Collections.emptyMap(); // none visible
} else {
spec = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClass, actualReceiver);
- GenericsType[] methodGenerics = candidate.getGenericsTypes();
+ GenericsType[] methodGenerics = method.getGenericsTypes();
if (methodGenerics != null) { // GROOVY-10322: remove hidden type parameters
for (int i = 0, n = methodGenerics.length; i < n && !spec.isEmpty(); i += 1) {
for (Iterator<GenericsType> it = spec.keySet().iterator(); it.hasNext(); ) {
@@ -1080,34 +1061,49 @@ public abstract class StaticTypeCheckingSupport {
}
}
}
+ Parameter[] parameters = makeRawTypes(method.getParameters(), spec);
- Parameter[] params = makeRawTypes(safeNode.getParameters(), spec);
- int dist = measureParametersAndArgumentsDistance(params, safeArgs);
- if (dist >= 0) {
- dist += getClassDistance(declaringClass, actualReceiver);
- dist += getExtensionDistance(isExtensionMethod);
+ int dist = measureParametersAndArgumentsDistance(parameters, argumentTypes);
+ if (dist >= 0 && dist <= bestDist) {
if (dist < bestDist) {
bestDist = dist;
- bestChoices.clear();
- bestChoices.add(candidate);
- } else if (dist == bestDist) {
- bestChoices.add(candidate);
+ bestMethods.clear();
+ }
+ bestMethods.add(method);
+ }
+ }
+
+ // phase 2: receiver-provider distance classifier
+ if (bestMethods.size() > 1 && receiver != null) {
+ methods = bestMethods;
+ bestDist = Integer.MAX_VALUE;
+ bestMethods = new LinkedList<>();
+ for (MethodNode method : methods) {
+ int dist = getClassDistance(method.getDeclaringClass(), receiver);
+ if (dist <= bestDist) {
+ if (dist < bestDist) {
+ bestDist = dist;
+ bestMethods.clear();
+ }
+ bestMethods.add(method);
}
}
}
- if (bestChoices.size() > 1 && !duckType) {
- // GROOVY-6849: prefer extension method in case of ambiguity
- List<MethodNode> onlyExtensionMethods = new LinkedList<>();
- for (MethodNode choice : bestChoices) {
- if (choice instanceof ExtensionMethodNode) {
- onlyExtensionMethods.add(choice);
+
+ // phase 3: prefer extension method in case of tie
+ if (bestMethods.size() > 1 && !duckType) {
+ List<MethodNode> extensionMethods = new LinkedList<>();
+ for (MethodNode method : bestMethods) {
+ if (method instanceof ExtensionMethodNode) {
+ extensionMethods.add(method);
}
}
- if (onlyExtensionMethods.size() == 1) {
- return onlyExtensionMethods;
+ if (extensionMethods.size() == 1) {
+ bestMethods = extensionMethods;
}
}
- return bestChoices;
+
+ return bestMethods;
}
private static int measureParametersAndArgumentsDistance(final Parameter[] parameters, final ClassNode[] argumentTypes) {
@@ -1169,10 +1165,6 @@ public abstract class StaticTypeCheckingSupport {
return getDistance(actualReceiverForDistance, declaringClassForDistance);
}
- private static int getExtensionDistance(final boolean isExtensionMethodNode) {
- return isExtensionMethodNode ? 0 : 1;
- }
-
private static Parameter[] makeRawTypes(final Parameter[] parameters, final Map<GenericsType, GenericsType> genericsPlaceholderAndTypeMap) {
return Arrays.stream(parameters).map(param -> {
String name = param.getType().getUnresolvedName();
diff --git a/src/test/groovy/bugs/Groovy8629Bug.groovy b/src/test/groovy/bugs/Groovy8629Bug.groovy
index 3b56d81bc1..643cca4219 100644
--- a/src/test/groovy/bugs/Groovy8629Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8629Bug.groovy
@@ -62,8 +62,8 @@ public class Groovy8629Bug extends GroovyTestCase {
@Override
IntegerPair next() {
String key = keyIterator.next()
- IntegerPair comp = new IntegerPair(m1[key], m2[key])
- return comp
+ IntegerPair pair = new IntegerPair(m1.get(key), m2.get(key))
+ return pair
}
@Override
@@ -87,5 +87,4 @@ public class Groovy8629Bug extends GroovyTestCase {
'''
}
-
}
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index c573582bcc..e537f2fd1c 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -1251,8 +1251,8 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
else if (value instanceof GString) {
value = value.toString()
}
- if (mvm[name]) {
- mvm[name].add value
+ if (mvm.containsKey(name)) {
+ mvm.get(name).add(value)
} else {
mvm.put(name, [value])
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
index 49ea6ed365..a50ae36ece 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
@@ -27,22 +27,22 @@ class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCompilat
void testGenericIdentityWithClosure() {
assertScript '''
-public static <T> T identity(T self) { self }
-
-@ASTTest(phase=INSTRUCTION_SELECTION,value={
- assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
-})
-Integer i = identity(2)
-
-@ASTTest(phase=INSTRUCTION_SELECTION,value={
- assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
-})
-Closure c = identity {'foo'}
-'''
+ static <T> T itself(T self) { self }
+
+ @ASTTest(phase=INSTRUCTION_SELECTION,value={
+ assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
+ })
+ Integer i = itself(2)
+
+ @ASTTest(phase=INSTRUCTION_SELECTION,value={
+ assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
+ })
+ Closure c = itself {'foo'}
+ '''
}
void testShouldNotThrowIllegalAccessToProtectedData() {
- shouldFailWithMessages('''
+ shouldFailWithMessages '''
class Test {
final Set<String> HISTORY = [] as HashSet
@@ -53,6 +53,7 @@ Closure c = identity {'foo'}
Test test = new Test()
println test.history
- ''', 'Method clone is protected in java.lang.Object')
+ ''',
+ 'Method clone is protected in java.lang.Object'
}
}
diff --git a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
index 7ad281e6ce..1e20c733f9 100644
--- a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
@@ -112,12 +112,12 @@ final class DelegateTransformTest {
def ms = new MapSet()
assert ms.size() == 1
- assert ms.toString() == '{a=1} [2, 3, 4]'
+ assert ms.toString() == '[a:1] [2, 3, 4]'
ms.remove(3)
assert ms.size() == 1
- assert ms.toString() == '{a=1} [2, 4]'
+ assert ms.toString() == '[a:1] [2, 4]'
ms.clear()
- assert ms.toString() == '{a=1} []'
+ assert ms.toString() == '[a:1] []'
'''
}