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/10 23:44:36 UTC

[groovy] branch master updated: GROOVY-8983: STC: support "x=collectionOfTypeOrSubtype" via setX(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 f8403f2  GROOVY-8983: STC: support "x=collectionOfTypeOrSubtype" via setX(Type[])
f8403f2 is described below

commit f8403f2610cdcee912a2c717798262748eacaaee
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Apr 10 18:32:17 2021 -0500

    GROOVY-8983: STC: support "x=collectionOfTypeOrSubtype" via setX(Type[])
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 50 ++++++++++++----------
 .../stc/ArraysAndCollectionsSTCTest.groovy         | 36 +++++++++++++++-
 .../stc/FieldsAndPropertiesSTCTest.groovy          |  2 +-
 .../groovy/transform/stc/GenericsSTCTest.groovy    | 16 +++----
 4 files changed, 71 insertions(+), 33 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 9a35798..3d7d122 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -134,6 +134,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiPredicate;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.IntStream;
 
@@ -958,37 +959,40 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         // we know that the RHS type is a closure
         // but we must check if the binary expression is an assignment
         // because we need to check if a setter uses @DelegatesTo
-        VariableExpression ve = varX("%", setterInfo.receiverType);
-        // for compound assignment "x op= y" find type as if it was "x = (x op y)"
+        VariableExpression receiver = varX("%", setterInfo.receiverType);
+        // for "x op= y" expression, find type as if it was "x = x op y"
         Expression newRightExpression = isCompoundAssignment(expression)
                 ? binX(leftExpression, getOpWithoutEqual(expression), rightExpression)
                 : rightExpression;
-        MethodCallExpression call = callX(ve, setterInfo.name, newRightExpression);
-        call.setImplicitThis(false);
-        visitMethodCallExpression(call);
-        MethodNode directSetterCandidate = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
-        if (directSetterCandidate == null) {
-            // this may happen if there's a setter of type boolean/String/Class, and that we are using the property
-            // notation AND that the RHS is not a boolean/String/Class
+
+        Function<Expression, MethodNode> setterCall = right -> {
+            MethodCallExpression call = callX(receiver, setterInfo.name, right);
+            call.setImplicitThis(false);
+            visitMethodCallExpression(call);
+            return call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
+        };
+
+        MethodNode methodTarget = setterCall.apply(newRightExpression);
+        if (methodTarget == null && !isCompoundAssignment(expression)) {
+            // if no direct match, try implicit conversion
             for (MethodNode setter : setterInfo.setters) {
-                ClassNode type = getWrapper(setter.getParameters()[0].getOriginType());
-                if (Boolean_TYPE.equals(type) || STRING_TYPE.equals(type) || CLASS_Type.equals(type)) {
-                    call = callX(ve, setterInfo.name, castX(type, newRightExpression));
-                    call.setImplicitThis(false);
-                    visitMethodCallExpression(call);
-                    directSetterCandidate = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
-                    if (directSetterCandidate != null) {
+                ClassNode lType = setter.getParameters()[0].getOriginType();
+                ClassNode rType = getDeclaredOrInferredType(newRightExpression);
+                if (checkCompatibleAssignmentTypes(lType, rType, newRightExpression, false)) {
+                    methodTarget = setterCall.apply(castX(lType, newRightExpression));
+                    if (methodTarget != null) {
                         break;
                     }
                 }
             }
         }
-        if (directSetterCandidate != null) {
+
+        if (methodTarget != null) {
             for (MethodNode setter : setterInfo.setters) {
-                if (setter == directSetterCandidate) {
-                    leftExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, directSetterCandidate);
-                    leftExpression.removeNodeMetaData(INFERRED_TYPE); // clear assumption
-                    storeType(leftExpression, getType(newRightExpression));
+                if (setter == methodTarget) {
+                    leftExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, methodTarget);
+                    leftExpression.removeNodeMetaData(INFERRED_TYPE); // clear the assumption
+                    storeType(leftExpression, methodTarget.getParameters()[0].getOriginType());
                     break;
                 }
             }
@@ -1004,13 +1008,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return type.equals(CLOSURE_TYPE) && Optional.ofNullable(type.getGenericsTypes()).filter(gts -> gts != null && gts.length == 1).isPresent();
     }
 
-    private boolean isCompoundAssignment(final Expression exp) {
+    private static boolean isCompoundAssignment(final Expression exp) {
         if (!(exp instanceof BinaryExpression)) return false;
         int type = ((BinaryExpression) exp).getOperation().getType();
         return isAssignment(type) && type != ASSIGN;
     }
 
-    private Token getOpWithoutEqual(final Expression exp) {
+    private static Token getOpWithoutEqual(final Expression exp) {
         if (!(exp instanceof BinaryExpression)) return null; // should never happen
         Token op = ((BinaryExpression) exp).getOperation();
         int typeWithoutEqual = TokenUtil.removeAssignment(op.getType());
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index c9f659b..8e745b4 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -414,9 +414,23 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
-    // GROOVY-9517
+    // GROOVY-8983
     void testShouldAllowArrayAssignment2() {
         assertScript '''
+            List<String> m() { ['foo'] }
+            void test(Set<String> set) {
+                String[] one, two
+                one = m()
+                two = set
+                assert one + two == ['foo','bar']
+            }
+            test(['bar'].toSet())
+        '''
+    }
+
+    // GROOVY-9517
+    void testShouldAllowArrayAssignment3() {
+        assertScript '''
             void test(File directory) {
                 File[] files = directory.listFiles()
                 files = files?.sort { it.name }
@@ -428,6 +442,26 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-8983
+    void testShouldAllowArrayAssignment4() {
+        assertScript '''
+            class C {
+                List<String> list = []
+                void setX(String[] array) {
+                    Collections.addAll(list, array)
+                }
+            }
+            List<String> m() { ['foo'] }
+            void test(Set<String> set) {
+                def c = new C()
+                c.x = m()
+                c.x = set
+                assert c.list == ['foo','bar']
+            }
+            test(['bar'].toSet())
+        '''
+    }
+
     void testListPlusEquals() {
         assertScript '''
             List<String> list = ['a','b']
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 4ff9ccf..c34ed6b 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -857,7 +857,7 @@ new FooWorker().doSomething()'''
 
             new FooWorker().doSomething()
         ''',
-        'Cannot assign value of type java.util.ArrayList <Integer> to variable of type java.util.List <String>'
+        'Incompatible generic argument types. Cannot assign java.util.ArrayList <Integer> to: java.util.List <String>'
     }
 
     void testAICAsStaticProperty() {
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index b6ecffb..e7c8acb 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -985,7 +985,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void setM(List<String> strings) {
                 }
                 void test() {
-                  m = Collections.emptyList() // Cannot assign value of type List<T> to variable of List<String>
+                    m = Collections.emptyList() // Cannot assign value of type List<T> to variable of List<String>
                 }
                 test()
             """
@@ -993,7 +993,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void setM(Collection<String> strings) {
                 }
                 void test() {
-                  m = Collections.emptyList()
+                    m = Collections.emptyList()
                 }
                 test()
             """
@@ -1001,7 +1001,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void setM(Iterable<String> strings) {
                 }
                 void test() {
-                  m = Collections.emptyList()
+                    m = Collections.emptyList()
                 }
                 test()
             """
@@ -1010,9 +1010,9 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void setM(List<String> strings) {
                 }
                 void test() {
-                  m = Collections.<Integer>emptyList()
+                    m = Collections.<Integer>emptyList()
                 }
-            """, '[Static type checking] - Cannot assign value of type java.util.List <Integer> to variable of type java.util.List <String>'
+            """, 'Incompatible generic argument types. Cannot assign java.util.List <Integer> to: java.util.List <String>'
         }
     }
 
@@ -1023,7 +1023,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void m(List<String> strings) {
                 }
                 void test() {
-                  m(Collections.emptyList()) // Cannot call m(List<String>) with arguments [List<T>]
+                    m(Collections.emptyList()) // Cannot call m(List<String>) with arguments [List<T>]
                 }
                 test()
             """
@@ -1031,7 +1031,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void m(Collection<String> strings) {
                 }
                 void test() {
-                  m(Collections.emptyList())
+                    m(Collections.emptyList())
                 }
                 test()
             """
@@ -1039,7 +1039,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
                 $mods void m(Iterable<String> strings) {
                 }
                 void test() {
-                  m(Collections.emptyList())
+                    m(Collections.emptyList())
                 }
                 test()
             """