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 2021/05/01 19:34:20 UTC
[groovy] branch master updated: GROOVY-10067: use bounded
placeholder from context in method return type
This is an automated email from the ASF dual-hosted git repository.
emilles 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 7119f9b GROOVY-10067: use bounded placeholder from context in method return type
7119f9b is described below
commit 7119f9b4fca3013951d7c9e4da1d5f834cc3dace
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat May 1 11:24:34 2021 -0500
GROOVY-10067: use bounded placeholder from context in method return type
---
.../java/org/codehaus/groovy/ast/GenericsType.java | 29 ++++----
.../codehaus/groovy/ast/tools/GenericsUtils.java | 84 ++++++++++++----------
.../transform/stc/StaticTypeCheckingSupport.java | 82 ++++++++++-----------
src/test/groovy/bugs/Groovy7985.groovy | 10 +--
.../groovy/transform/stc/GenericsSTCTest.groovy | 29 ++++++++
5 files changed, 136 insertions(+), 98 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index 5db7a38..5ae6766 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -206,26 +206,29 @@ public class GenericsType extends ASTNode {
return true; // diamond always matches
}
if (classNode.isGenericsPlaceHolder()) {
- // if the compare type is a generics placeholder (like <E>) then we
- // only need to check that the names are equal
if (genericsTypes == null) {
return true;
}
- if (isWildcard()) {
- if (getLowerBound() != null) {
- ClassNode lowerBound = getLowerBound();
- return genericsTypes[0].name.equals(lowerBound.getUnresolvedName());
+ String name = genericsTypes[0].name;
+ if (!isWildcard()) {
+ return this.name.equals(name);
+ }
+ if (getLowerBound() != null) {
+ // check for "? super T" vs "T"
+ ClassNode lowerBound = getLowerBound();
+ if (lowerBound.getUnresolvedName().equals(name)) {
+ return true;
}
- if (getUpperBounds() != null) {
- for (ClassNode upperBound : getUpperBounds()) {
- if (genericsTypes[0].name.equals(upperBound.getUnresolvedName())) {
- return true;
- }
+ } else if (getUpperBounds() != null) {
+ // check for "? extends T & I" vs "T" or "I"
+ for (ClassNode upperBound : getUpperBounds()) {
+ if (upperBound.getUnresolvedName().equals(name)) {
+ return true;
}
- return false;
}
}
- return genericsTypes[0].name.equals(name);
+ // check for "? extends/super X" vs "T extends/super X"
+ return checkGenerics(classNode);
}
if (isWildcard() || isPlaceholder()) {
// if the generics spec is a wildcard or a placeholder then check the bounds
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
index 5521f6a..1d66547 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
@@ -49,6 +49,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -140,50 +141,68 @@ public class GenericsUtils {
return gt;
}
- public static Map<GenericsType.GenericsTypeName, GenericsType> extractPlaceholders(ClassNode cn) {
- Map<GenericsType.GenericsTypeName, GenericsType> ret = new HashMap<>();
- extractPlaceholders(cn, ret);
- return ret;
+ public static Map<GenericsType.GenericsTypeName, GenericsType> extractPlaceholders(final ClassNode type) {
+ Map<GenericsType.GenericsTypeName, GenericsType> placeholders = new HashMap<>();
+ extractPlaceholders(type, placeholders);
+ return placeholders;
}
/**
* For a given classnode, fills in the supplied map with the parameterized
* types it defines.
*
- * @param node the class node to check
- * @param map the generics type information collector
+ * @param type the class node to check
+ * @param placeholders the generics type information collector
*/
- public static void extractPlaceholders(ClassNode node, Map<GenericsType.GenericsTypeName, GenericsType> map) {
- if (node == null) return;
+ public static void extractPlaceholders(final ClassNode type, final Map<GenericsType.GenericsTypeName, GenericsType> placeholders) {
+ if (type == null) return;
- if (node.isArray()) {
- extractPlaceholders(node.getComponentType(), map);
+ if (type.isArray()) {
+ extractPlaceholders(type.getComponentType(), placeholders);
return;
}
- if (!node.isUsingGenerics() || !node.isRedirectNode()) return;
- GenericsType[] parameterized = node.getGenericsTypes();
+ if (!type.isUsingGenerics() || !type.isRedirectNode()) return;
+ GenericsType[] parameterized = type.getGenericsTypes();
if (parameterized == null || parameterized.length == 0) return;
- GenericsType[] redirectGenericsTypes = node.redirect().getGenericsTypes();
- if (redirectGenericsTypes == null ||
- (node.isGenericsPlaceHolder() && redirectGenericsTypes.length != parameterized.length) /* GROOVY-8609 */ ) {
- redirectGenericsTypes = parameterized;
+
+ Consumer<GenericsType> extractor = (GenericsType gt) -> {
+ ClassNode lowerBound = gt.getLowerBound();
+ if (lowerBound != null) {
+ extractPlaceholders(lowerBound, placeholders);
+ }
+ ClassNode[] upperBounds = gt.getUpperBounds();
+ if (upperBounds != null) {
+ for (ClassNode upperBound : upperBounds) {
+ extractPlaceholders(upperBound, placeholders);
+ }
+ }
+ };
+
+ // GROOVY-8609, GROOVY-10067
+ if (type.isGenericsPlaceHolder()) {
+ GenericsType gt = parameterized[0];
+ placeholders.put(new GenericsType.GenericsTypeName(gt.getName()), gt);
+ extractor.accept(gt);
+ return;
}
- if (redirectGenericsTypes.length != parameterized.length) {
+
+ GenericsType[] redirectGenericsTypes = type.redirect().getGenericsTypes();
+ if (redirectGenericsTypes == null) redirectGenericsTypes = parameterized;
+ else if (redirectGenericsTypes.length != parameterized.length) {
throw new GroovyBugError("Expected earlier checking to detect generics parameter arity mismatch" +
- "\nExpected: " + node.getName() + toGenericTypesString(redirectGenericsTypes) +
- "\nSupplied: " + node.getName() + toGenericTypesString(parameterized));
+ "\nExpected: " + type.getName() + toGenericTypesString(redirectGenericsTypes) +
+ "\nSupplied: " + type.getName() + toGenericTypesString(parameterized));
}
List<GenericsType> valueList = new LinkedList<>();
- for (int i = 0; i < redirectGenericsTypes.length; i++) {
- GenericsType redirectType = redirectGenericsTypes[i];
- if (redirectType.isPlaceholder()) {
- GenericsType.GenericsTypeName name = new GenericsType.GenericsTypeName(redirectType.getName());
- if (!map.containsKey(name)) {
+ for (int i = 0, n = redirectGenericsTypes.length; i < n; i += 1) {
+ GenericsType rgt = redirectGenericsTypes[i];
+ if (rgt.isPlaceholder()) {
+ GenericsType.GenericsTypeName name = new GenericsType.GenericsTypeName(rgt.getName());
+ if (!placeholders.containsKey(name)) {
GenericsType value = parameterized[i];
- map.put(name, value);
-
+ placeholders.put(name, value);
valueList.add(value);
}
}
@@ -191,18 +210,9 @@ public class GenericsUtils {
for (GenericsType value : valueList) {
if (value.isWildcard()) {
- ClassNode lowerBound = value.getLowerBound();
- if (lowerBound != null) {
- extractPlaceholders(lowerBound, map);
- }
- ClassNode[] upperBounds = value.getUpperBounds();
- if (upperBounds != null) {
- for (ClassNode upperBound : upperBounds) {
- extractPlaceholders(upperBound, map);
- }
- }
+ extractor.accept(value);
} else if (!value.isPlaceholder()) {
- extractPlaceholders(value.getType(), map);
+ extractPlaceholders(value.getType(), placeholders);
}
}
}
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 20965df..287b72c 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -464,10 +464,8 @@ public abstract class StaticTypeCheckingSupport {
return true;
}
if (implementsInterfaceOrIsSubclassOf(type, toBeAssignedTo)) {
- if (toBeAssignedTo.getGenericsTypes() != null) {
- // perform additional check on generics
- // ? extends toBeAssignedTo
- GenericsType gt = GenericsUtils.buildWildcardType(toBeAssignedTo);
+ if (toBeAssignedTo.getGenericsTypes() != null) { // perform additional check on generics
+ GenericsType gt = toBeAssignedTo.isGenericsPlaceHolder() ? toBeAssignedTo.getGenericsTypes()[0] : GenericsUtils.buildWildcardType(toBeAssignedTo);
return gt.isCompatibleWith(type);
}
return true;
@@ -1423,28 +1421,20 @@ public abstract class StaticTypeCheckingSupport {
}
private static boolean typeCheckMethodsWithGenerics(final ClassNode receiver, final ClassNode[] argumentTypes, final MethodNode candidateMethod, final boolean isExtensionMethod) {
- boolean failure = false;
-
- // correct receiver for inner class
- // we assume the receiver is an instance of the declaring class of the
- // candidate method, but findMethod returns also outer class methods
- // for that receiver. For now we skip receiver based checks in that case
- // TODO: correct generics for when receiver is to be skipped
- boolean skipBecauseOfInnerClassNotReceiver = !implementsInterfaceOrIsSubclassOf(receiver, candidateMethod.getDeclaringClass());
-
Parameter[] parameters = candidateMethod.getParameters();
- Map<GenericsTypeName, GenericsType> classGTs;
- if (skipBecauseOfInnerClassNotReceiver) {
- classGTs = Collections.emptyMap();
- } else {
- classGTs = GenericsUtils.extractPlaceholders(receiver);
- }
if (parameters.length > argumentTypes.length || parameters.length == 0) {
// this is a limitation that must be removed in a future version
// we cannot check generic type arguments if there are default parameters!
return true;
}
+ boolean failure = false;
+ // correct receiver for inner class
+ // we assume the receiver is an instance of the declaring class of the
+ // candidate method, but findMethod returns also outer class methods
+ // for that receiver. For now we skip receiver based checks in that case
+ // TODO: correct generics for when receiver is to be skipped
+ boolean skipBecauseOfInnerClassNotReceiver = !implementsInterfaceOrIsSubclassOf(receiver, candidateMethod.getDeclaringClass());
// we have here different generics contexts we have to deal with.
// There is firstly the context given through the class, and the method.
// The method context may hide generics given through the class, but use
@@ -1452,15 +1442,14 @@ public abstract class StaticTypeCheckingSupport {
Map<GenericsTypeName, GenericsType> resolvedMethodGenerics = new HashMap<>();
if (!skipBecauseOfInnerClassNotReceiver) {
addMethodLevelDeclaredGenerics(candidateMethod, resolvedMethodGenerics);
- }
- // first remove hidden generics
- for (GenericsTypeName key : resolvedMethodGenerics.keySet()) {
- classGTs.remove(key);
- }
- // then use the remaining information to refine the given generics
- applyGenericsConnections(classGTs, resolvedMethodGenerics);
- // and then start our checks with the receiver
- if (!failure && !skipBecauseOfInnerClassNotReceiver) {
+ if (!resolvedMethodGenerics.isEmpty()) {
+ // first remove hidden generics
+ Map<GenericsTypeName, GenericsType> receiverGenerics = GenericsUtils.extractPlaceholders(receiver);
+ receiverGenerics.keySet().removeAll(resolvedMethodGenerics.keySet());
+ // then use the remaining information to refine the method generics
+ applyGenericsConnections(receiverGenerics, resolvedMethodGenerics);
+ }
+ // and then start our checks with the receiver
failure = inferenceCheck(Collections.emptySet(), resolvedMethodGenerics, candidateMethod.getDeclaringClass(), receiver, false);
}
// the outside context parts till now define placeholder we are not allowed to
@@ -1724,6 +1713,10 @@ public abstract class StaticTypeCheckingSupport {
if (target.isGenericsPlaceHolder()) {
connections.put(new GenericsTypeName(target.getUnresolvedName()), new GenericsType(type));
+ } else if (type.isGenericsPlaceHolder()) {
+ // "T extends java.util.List<X> -> java.util.List<E>" vs "java.util.List<E>"
+ extractGenericsConnections(connections, extractType(new GenericsType(type)), target);
+
} else if (type.isArray() && target.isArray()) {
extractGenericsConnections(connections, type.getComponentType(), target.getComponentType());
@@ -1897,27 +1890,30 @@ public abstract class StaticTypeCheckingSupport {
if (type.isArray()) {
return applyGenericsContext(spec, type.getComponentType()).makeArray();
}
- ClassNode newType = type.getPlainNodeReference();
+
GenericsType[] gt = type.getGenericsTypes();
if (asBoolean(spec)) {
gt = applyGenericsContext(spec, gt);
}
- newType.setGenericsTypes(gt);
- if (type.isGenericsPlaceHolder()) {
- boolean nonTrivial = hasNonTrivialBounds(gt[0]);
- if (nonTrivial || !gt[0].isPlaceholder()) {
- return getCombinedBoundType(gt[0]);
- }
- String placeholderName = gt[0].getName();
- if (!placeholderName.equals(newType.getUnresolvedName())) {
- ClassNode clean = make(placeholderName);
- clean.setGenericsTypes(gt);
- clean.setRedirect(newType);
- newType = clean;
+ if (!type.isGenericsPlaceHolder()) { // convert Type<T> to Type<...>
+ ClassNode cn = type.getPlainNodeReference();
+ cn.setGenericsTypes(gt);
+ return cn;
+ }
+
+ if (gt[0].isPlaceholder()) { // convert T to X
+ if (type.getGenericsTypes()[0] == gt[0]) {
+ return type; // nothing to do
+ } else if (!hasNonTrivialBounds(gt[0])) {
+ ClassNode cn = make(gt[0].getName());
+ cn.setRedirect(type);
+ cn.setGenericsTypes(gt);
+ cn.setGenericsPlaceHolder(true);
+ return cn;
}
- newType.setGenericsPlaceHolder(true);
}
- return newType;
+
+ return getCombinedBoundType(gt[0]); // convert T to Type
}
static ClassNode getCombinedBoundType(final GenericsType genericsType) {
diff --git a/src/test/groovy/bugs/Groovy7985.groovy b/src/test/groovy/bugs/Groovy7985.groovy
index c1cb35d..7bed318 100644
--- a/src/test/groovy/bugs/Groovy7985.groovy
+++ b/src/test/groovy/bugs/Groovy7985.groovy
@@ -46,16 +46,16 @@ final class Groovy7985 {
}
@groovy.transform.CompileStatic
- Pair<Pair<String, Integer>, Pair<String, Integer>> doSmething() {
+ Pair<Pair<String, Integer>, Pair<String, Integer>> doSomething() {
def one = (Pair<String, Integer>) Pair.of('a', 1)
def two = (Pair<String, Integer>) Pair.of('b', 2)
return Pair.of(one, two)
}
- assert doSmething().left.left == 'a'
- assert doSmething().left.right == 1
- assert doSmething().right.left == 'b'
- assert doSmething().right.right == 2
+ assert doSomething().left.left == 'a'
+ assert doSomething().left.right == 1
+ assert doSomething().right.left == 'b'
+ assert doSomething().right.right == 2
'''
}
}
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 607cfa6..11c1c7f 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -367,6 +367,35 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-10067
+ void testReturnTypeInferenceWithMethodGenerics11() {
+ assertScript '''
+ def <N extends Number> N getNumber() {
+ return (N) 42
+ }
+ def f(Integer i) {
+ }
+ def g(int i) {
+ }
+
+ Integer i = this.<Integer>getNumber()
+ f(this.<Integer>getNumber())
+ g(this.<Integer>getNumber())
+
+ i = (Integer) getNumber()
+ f((Integer) getNumber())
+ g((Integer) getNumber())
+
+ i = getNumber()
+ //f(getNumber())
+ //g(getNumber())
+
+ i = number
+ //f(number)
+ //g(number)
+ '''
+ }
+
void testDiamondInferrenceFromConstructor1() {
assertScript '''
class Foo<U> {