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/04/03 22:02:52 UTC

[groovy] 01/01: GROOVY-8983: STC: support "Type[] array = collectionOfTypeOrSubtype"

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

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

commit 62338ad24bbef0217eea7d89d50bede1d575db45
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Apr 3 17:01:56 2021 -0500

    GROOVY-8983: STC: support "Type[] array = collectionOfTypeOrSubtype"
---
 .../codehaus/groovy/ast/tools/GenericsUtils.java   | 10 +++-
 .../transform/stc/StaticTypeCheckingSupport.java   | 28 ++++++++---
 .../stc/ArraysAndCollectionsSTCTest.groovy         | 57 +++++++++++++++++++++-
 3 files changed, 83 insertions(+), 12 deletions(-)

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 8cb3f2c..5521f6a 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
@@ -253,6 +253,13 @@ public class GenericsUtils {
             }
             return target;
         }
+        if (hint.isGenericsPlaceHolder()) {
+            ClassNode bound = hint.redirect();
+            return parameterizeType(bound, target);
+        }
+        if (target.redirect().getGenericsTypes() == null) {
+            return target;
+        }
         if (!target.equals(hint) && implementsInterfaceOrIsSubclassOf(target, hint)) {
             ClassNode nextSuperClass = ClassHelper.getNextSuperClass(target, hint);
             if (!hint.equals(nextSuperClass)) {
@@ -267,7 +274,6 @@ public class GenericsUtils {
         genericsSpec = createGenericsSpec(targetRedirect, genericsSpec);
         extractSuperClassGenerics(hint, targetRedirect, genericsSpec);
         return correctToGenericsSpecRecurse(genericsSpec, targetRedirect);
-
     }
 
     public static ClassNode nonGeneric(ClassNode type) {
@@ -416,7 +422,7 @@ public class GenericsUtils {
             String name = type.getName();
             ret = genericsSpec.get(name);
         } else if (type.isWildcard()) { // GROOVY-9891
-            ret = type.getLowerBound(); // use lower or upper
+          //ret = type.getLowerBound(); // use lower or upper
             if (ret == null && type.getUpperBounds() != null) {
                 ret = type.getUpperBounds()[0]; // ? supports 1
             }
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 de491c4..c991cdb 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -37,6 +37,7 @@ import org.codehaus.groovy.ast.expr.ListExpression;
 import org.codehaus.groovy.ast.expr.MapExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
+import org.codehaus.groovy.ast.tools.GeneralUtils;
 import org.codehaus.groovy.ast.tools.GenericsUtils;
 import org.codehaus.groovy.ast.tools.ParameterUtils;
 import org.codehaus.groovy.ast.tools.WideningCategories;
@@ -645,6 +646,11 @@ public abstract class StaticTypeCheckingSupport {
         return checkCompatibleAssignmentTypes(left, right, rightExpression, true);
     }
 
+    /**
+     * Everything that can be done by {@code castToType} should be allowed for assignment.
+     *
+     * @see org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation#castToType(Object,Class)
+     */
     public static boolean checkCompatibleAssignmentTypes(final ClassNode left, final ClassNode right, final Expression rightExpression, final boolean allowConstructorCoercion) {
         // GROOVY-7307, GROOVY-9952, et al.
         if (left.isGenericsPlaceHolder()) {
@@ -655,14 +661,22 @@ public abstract class StaticTypeCheckingSupport {
             }
         }
 
+        if (left.isArray()) {
+            if (right.isArray()) {
+                return checkCompatibleAssignmentTypes(left.getComponentType(), right.getComponentType(), rightExpression, false);
+            }
+            if (GeneralUtils.isOrImplements(right, Collection_TYPE) && !(rightExpression instanceof ListExpression)) {
+                GenericsType elementType = GenericsUtils.parameterizeType(right, Collection_TYPE).getGenericsTypes()[0];
+                return OBJECT_TYPE.equals(left.getComponentType()) // Object[] can accept any collection element type(s)
+                    || (elementType.getLowerBound() == null && isCovariant(extractType(elementType), left.getComponentType()));
+                    //  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ GROOVY-8984: "? super T" is only compatible with an Object[] target
+            }
+        }
+
         ClassNode leftRedirect = left.redirect();
         ClassNode rightRedirect = right.redirect();
         if (leftRedirect == rightRedirect) return true;
 
-        if (leftRedirect.isArray() && rightRedirect.isArray()) {
-            return checkCompatibleAssignmentTypes(leftRedirect.getComponentType(), rightRedirect.getComponentType(), rightExpression, false);
-        }
-
         if (rightRedirect == void_WRAPPER_TYPE) return leftRedirect == VOID_TYPE;
         if (rightRedirect == VOID_TYPE) return leftRedirect == void_WRAPPER_TYPE;
 
@@ -682,8 +696,6 @@ public abstract class StaticTypeCheckingSupport {
             return true;
         }
 
-        // on an assignment everything that can be done by a GroovyCast is allowed
-
         // anything can be assigned to an Object, String, Boolean or Class typed variable
         if (isWildcardLeftHandSide(left) && !(leftRedirect == boolean_TYPE && rightExpressionIsNull)) return true;
 
@@ -824,7 +836,7 @@ public abstract class StaticTypeCheckingSupport {
                     return !Double.valueOf(val).equals(number);
                 }
                 default: // double
-                    return false; // no possible loose here
+                    return false; // no possible loss here
             }
         }
         return true; // possible loss of precision
@@ -1625,7 +1637,7 @@ public abstract class StaticTypeCheckingSupport {
 
     private static ClassNode extractType(final GenericsType gt) {
         if (!gt.isPlaceholder()) {
-            return gt.getType();
+            return getCombinedBoundType(gt);
         }
         // For a placeholder, a type based on the generics type is used for the compatibility check, to match on
         // the actual bounds and not the name of the placeholder.
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index dcfe3ff..c9f659b 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -361,8 +361,61 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
         ''', 'Cannot assign value of type java.util.List <? super java.lang.Runnable> to variable of type java.lang.Runnable[]'
     }
 
+    // GROOVY-8983
+    void testShouldAllowArrayAssignment1() {
+        assertScript '''
+            List<String> m() { ['foo'] }
+            void test(Set<String> set) {
+                String[] one = m()
+                String[] two = set
+                assert one + two == ['foo','bar']
+            }
+            test(['bar'].toSet())
+        '''
+
+        assertScript '''
+            List<String> m() { ['foo'] }
+            void test(Set<String> set) {
+                CharSequence[] one = m()
+                CharSequence[] two = set
+                assert one + two == ['foo','bar']
+            }
+            test(['bar'].toSet())
+        '''
+
+        assertScript '''
+            List<String> m() { ['foo'] }
+            void test(Set<String> set) {
+                Object[] one = m()
+                Object[] two = set
+                assert one + two == ['foo','bar']
+            }
+            test(['bar'].toSet())
+        '''
+
+        assertScript '''
+            List<? extends CharSequence> m() { ['foo'] }
+            void test(Set<? extends CharSequence> set) {
+                CharSequence[] one = m()
+                CharSequence[] two = set
+                assert one + two == ['foo','bar']
+            }
+            test(['bar'].toSet())
+        '''
+
+        assertScript '''
+            List<? super CharSequence> m() { [null] }
+            void test(Set<? super CharSequence> set) {
+                Object[] one = m()
+                Object[] two = set
+                assert one + two == [null,null]
+            }
+            test([null].toSet())
+        '''
+    }
+
     // GROOVY-9517
-    void testShouldAllowArrayAssignment() {
+    void testShouldAllowArrayAssignment2() {
         assertScript '''
             void test(File directory) {
                 File[] files = directory.listFiles()
@@ -371,7 +424,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
                     // ...
                 }
             }
-            println 'works'
+            assert 'no error'
         '''
     }