You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2021/03/15 07:18:21 UTC
[groovy] 01/02: GROOVY-9902: STC: allow method checking with
partial generic information
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 39c289ac7a07dc0fc2754ae5d84f7d0aaed06298
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jan 20 20:46:54 2021 -0600
GROOVY-9902: STC: allow method checking with partial generic information
---
.../java/org/codehaus/groovy/ast/GenericsType.java | 16 ++++-----
.../transform/stc/StaticTypeCheckingSupport.java | 5 +--
src/test/groovy/bugs/Groovy6786Bug.groovy | 39 +++++++++------------
.../groovy/transform/stc/GenericsSTCTest.groovy | 40 ++++++++++++++++++++++
4 files changed, 65 insertions(+), 35 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index 84f28f7..a1da7a6 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -374,15 +374,15 @@ public class GenericsType extends ASTNode {
GenericsTypeName gtn = new GenericsTypeName(redirectBoundType.getName());
match = name.equals(gtn);
if (!match) {
- GenericsType genericsType = boundPlaceHolders.get(gtn);
- if (genericsType != null) {
- if (genericsType.isPlaceholder()) {
+ GenericsType boundGenericsType = boundPlaceHolders.get(gtn);
+ if (boundGenericsType != null) {
+ if (boundGenericsType.isPlaceholder()) {
match = true;
- } else if (genericsType.isWildcard()) {
- if (genericsType.getUpperBounds() != null) { // multiple bounds not allowed for ?
- match = redirectBoundType.isCompatibleWith(genericsType.getUpperBounds()[0]);
- } else if (genericsType.getLowerBound() != null) {
- match = redirectBoundType.isCompatibleWith(genericsType.getLowerBound());
+ } else if (boundGenericsType.isWildcard()) {
+ if (boundGenericsType.getUpperBounds() != null) { // ? supports single bound only
+ match = classNodeType.isCompatibleWith(boundGenericsType.getUpperBounds()[0]);
+ } else if (boundGenericsType.getLowerBound() != null) {
+ match = classNodeType.isCompatibleWith(boundGenericsType.getLowerBound());
} else {
match = true;
}
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 5f48a92..2c3db53 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1372,14 +1372,11 @@ public abstract class StaticTypeCheckingSupport {
}
protected static boolean typeCheckMethodsWithGenerics(final ClassNode receiver, final ClassNode[] argumentTypes, final MethodNode candidateMethod) {
- if (isUsingUncheckedGenerics(receiver)) {
- return true;
- }
boolean isExtensionMethod = candidateMethod instanceof ExtensionMethodNode;
if (!isExtensionMethod
&& receiver.isUsingGenerics()
&& receiver.equals(CLASS_Type)
- && !candidateMethod.getDeclaringClass().equals(receiver)) {
+ && !candidateMethod.getDeclaringClass().equals(CLASS_Type)) {
return typeCheckMethodsWithGenerics(receiver.getGenericsTypes()[0].getType(), argumentTypes, candidateMethod);
}
// both candidate method and receiver have generic information so a check is possible
diff --git a/src/test/groovy/bugs/Groovy6786Bug.groovy b/src/test/groovy/bugs/Groovy6786Bug.groovy
index 44aeaa4..f125a79 100644
--- a/src/test/groovy/bugs/Groovy6786Bug.groovy
+++ b/src/test/groovy/bugs/Groovy6786Bug.groovy
@@ -18,24 +18,22 @@
*/
package groovy.bugs
-import groovy.test.NotYetImplemented
import groovy.transform.stc.StaticTypeCheckingTestCase
class Groovy6786Bug extends StaticTypeCheckingTestCase {
-
+
void testGenericAddAll() {
assertScript '''
-
public class Class1<ENTITY> {
-
+
Container<ENTITY> container;
-
+
void refresh() {
def items = findAllItems();
-
+
container.addAll(items);
}
-
+
Collection<ENTITY> findAllItems() {
return null;
}
@@ -44,32 +42,30 @@ class Groovy6786Bug extends StaticTypeCheckingTestCase {
void addAll(Collection<? extends ENTITY> collection);
}
new Class1()
-
'''
}
void testGuavaCacheBuilderLikeGenerics() {
assertScript '''
class Class1 {
-
protected LoadingCache<String, Integer> cache;
-
+
Class1(CacheLoader<String, Integer> cacheLoader) {
this.cache = CacheBuilder.newBuilder().build(cacheLoader);
}
}
-
+
class CacheLoader<K, V> {}
-
+
class LoadingCache<K, V> {}
-
+
class CacheBuilder<K, V> {
public static CacheBuilder<Object, Object> newBuilder() {
return new CacheBuilder<Object, Object>();
}
-
+
public <K1 extends K, V1 extends V> LoadingCache<K1, V1> build(CacheLoader<? super K1, V1> loader) {
- return new LoadingCache<K1, V1>();
+ return new LoadingCache<K1, V1>();
}
}
new Class1(null)
@@ -83,9 +79,9 @@ class Groovy6786Bug extends StaticTypeCheckingTestCase {
return false;
}
}
-
+
abstract class AbstractExtendedManager<T> extends AbstractManager<T> {}
-
+
class ConcreteManager extends AbstractExtendedManager<String> {
@Override
protected boolean update(String item){
@@ -99,20 +95,18 @@ class Groovy6786Bug extends StaticTypeCheckingTestCase {
void testIfWithInstanceOfAndAnotherConditionAfter() {
assertScript '''
class Class1 {
-
-
Class1() {
Object obj = null;
-
+
if(obj instanceof String && someCondition() && testMethod(obj)) {
println "Hello!"
}
}
-
+
boolean someCondition() {
return true;
}
-
+
boolean testMethod(String arg) {
return true;
}
@@ -120,5 +114,4 @@ class Groovy6786Bug extends StaticTypeCheckingTestCase {
new Class1();
'''
}
-
}
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 5008fbf..9667da0 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -1113,6 +1113,46 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
''', '#printEqual(T, java.util.List <T>) with arguments [int, java.util.List <java.lang.String>]'
}
+ // GROOVY-9902
+ void testIncompatibleArgumentsForGenericArgument_IncludingDelegation() {
+ shouldFailWithMessages '''
+ class Holder<Unknown> {
+ TypedProperty<Number, Unknown> numberProperty = prop(Number)
+ TypedProperty<String, Unknown> stringProperty = prop(String)
+
+ def <T> TypedProperty<T, Unknown> prop(Class<T> clazz) {
+ return new TypedProperty<T, Unknown>(clazz: clazz)
+ }
+
+ // Note: type argument of Holder cannot be supplied to value attribute of @DelegatesTo
+ def <T> T of(@DelegatesTo(value=Holder, strategy=Closure.DELEGATE_FIRST) Closure<T> c) {
+ this.with(c)
+ }
+ }
+
+ class TypedProperty<X, Y> {
+ Class<X> clazz
+
+ void eq(X x) {
+ assert x.class == clazz : "x.class is ${x.class} not ${clazz}"
+ }
+ }
+
+ void test(Holder<Object> h) {
+ h.stringProperty.eq("${0}") // STC error
+ h.of { // <-- 2nd type parameter discarded
+ stringProperty.eq(1234) // expect STC error
+ numberProperty.eq("xx") // expect STC error
+ }
+ }
+
+ test(new Holder<Object>())
+ ''',
+ 'Cannot call TypedProperty <String, Object>#eq(java.lang.String) with arguments [groovy.lang.GString]',
+ 'Cannot call TypedProperty <String, Unknown>#eq(java.lang.String) with arguments [int]',
+ 'Cannot call TypedProperty <Number, Unknown>#eq(java.lang.Number) with arguments [java.lang.String]'
+ }
+
void testGroovy5748() {
assertScript '''
interface IStack<T> {