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/03/01 17:15:00 UTC

[groovy] 01/03: GROOVY-9902: STC: allow method checking with partial generic information

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

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

commit 0d54bbc5c7f344fb1cef02a10dd848914169970a
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
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/ast/GenericsType.java
    	src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
---
 .../java/org/codehaus/groovy/ast/GenericsType.java | 16 ++++-----
 .../transform/stc/StaticTypeCheckingSupport.java   | 11 +++---
 src/test/groovy/bugs/Groovy6786Bug.groovy          | 39 +++++++++------------
 .../groovy/transform/stc/GenericsSTCTest.groovy    | 40 ++++++++++++++++++++++
 4 files changed, 68 insertions(+), 38 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index c67db97..271fb1f 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -384,15 +384,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) { // multiple bounds not allowed for ?
+                                        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 2e9d221..d7b7f72 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1449,19 +1449,16 @@ public abstract class StaticTypeCheckingSupport {
     }
 
     protected static boolean typeCheckMethodsWithGenerics(ClassNode receiver, ClassNode[] arguments, MethodNode candidateMethod) {
-        if (isUsingUncheckedGenerics(receiver)) {
-            return true;
-        }
-        if (CLASS_Type.equals(receiver)
+        boolean isExtensionMethod = candidateMethod instanceof ExtensionMethodNode;
+        if (!isExtensionMethod
                 && receiver.isUsingGenerics()
-                && !candidateMethod.getDeclaringClass().equals(receiver)
-                && !(candidateMethod instanceof ExtensionMethodNode)) {
+                && receiver.equals(CLASS_Type)
+                && !candidateMethod.getDeclaringClass().equals(CLASS_Type)) {
             return typeCheckMethodsWithGenerics(receiver.getGenericsTypes()[0].getType(), arguments, candidateMethod);
         }
         // both candidate method and receiver have generic information so a check is possible
         GenericsType[] genericsTypes = candidateMethod.getGenericsTypes();
         boolean methodUsesGenerics = (genericsTypes != null && genericsTypes.length > 0);
-        boolean isExtensionMethod = candidateMethod instanceof ExtensionMethodNode;
         if (isExtensionMethod && methodUsesGenerics) {
             ClassNode[] dgmArgs = new ClassNode[arguments.length + 1];
             dgmArgs[0] = receiver;
diff --git a/src/test/groovy/bugs/Groovy6786Bug.groovy b/src/test/groovy/bugs/Groovy6786Bug.groovy
index d8611e8..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.transform.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 dabe166..6ac3534 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -1042,6 +1042,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> {