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 2020/04/22 16:10:06 UTC

[groovy] 01/01: GROOVY-9517: use inferred right expression type for array checks

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

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

commit d85548e2bfb43a0ae09cdeba4a7fa1e80c55ad20
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Apr 22 10:41:01 2020 -0500

    GROOVY-9517: use inferred right expression type for array checks
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 37 +++++++++---------
 .../stc/ArraysAndCollectionsSTCTest.groovy         | 45 +++++++++++++++-------
 2 files changed, 49 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 fec7971..60bc637 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1175,30 +1175,29 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return true;
     }
 
-    private void addPrecisionErrors(final ClassNode leftRedirect, final ClassNode lhsType, final ClassNode inferredrhsType, final Expression rightExpression) {
-        if (isNumberType(leftRedirect) && isNumberType(inferredrhsType)) {
-            if (checkPossibleLossOfPrecision(leftRedirect, inferredrhsType, rightExpression)) {
-                addStaticTypeError("Possible loss of precision from " + inferredrhsType + " to " + leftRedirect, rightExpression);
-                return;
+    private void addPrecisionErrors(final ClassNode leftRedirect, final ClassNode lhsType, final ClassNode rhsType, final Expression rightExpression) {
+        if (isNumberType(leftRedirect)) {
+            if (isNumberType(rhsType) && checkPossibleLossOfPrecision(leftRedirect, rhsType, rightExpression)) {
+                addStaticTypeError("Possible loss of precision from " + rhsType.toString(false) + " to " + lhsType.toString(false), rightExpression);
             }
+            return;
         }
-        // if left type is array, we should check the right component types
-        if (!lhsType.isArray()) return;
-        ClassNode leftComponentType = lhsType.getComponentType();
-        ClassNode rightRedirect = rightExpression.getType().redirect();
-        if (rightRedirect.isArray()) {
-            ClassNode rightComponentType = rightRedirect.getComponentType();
-            if (!checkCompatibleAssignmentTypes(leftComponentType, rightComponentType)) {
-                addStaticTypeError("Cannot assign value of type " + rightComponentType.toString(false) + " into array of type " + lhsType.toString(false), rightExpression);
-            }
-        } else if (rightExpression instanceof ListExpression) {
-            for (Expression element : ((ListExpression) rightExpression).getExpressions()) {
-                ClassNode rightComponentType = this.getType(element);
-                if (!checkCompatibleAssignmentTypes(leftComponentType, rightComponentType)
-                        && !(isNullConstant(element) && !isPrimitiveType(leftComponentType))) {
+        if (!leftRedirect.isArray()) return;
+        // left type is an array, check the right component types
+        if (rightExpression instanceof ListExpression) {
+            ClassNode leftComponentType = leftRedirect.getComponentType();
+            for (Expression expression : ((ListExpression) rightExpression).getExpressions()) {
+                ClassNode rightComponentType = getType(expression);
+                if (!checkCompatibleAssignmentTypes(leftComponentType, rightComponentType) && !(isNullConstant(expression) && !isPrimitiveType(leftComponentType))) {
                     addStaticTypeError("Cannot assign value of type " + rightComponentType.toString(false) + " into array of type " + lhsType.toString(false), rightExpression);
                 }
             }
+        } else if (rhsType.redirect().isArray()) {
+            ClassNode leftComponentType = leftRedirect.getComponentType();
+            ClassNode rightComponentType = rhsType.redirect().getComponentType();
+            if (!checkCompatibleAssignmentTypes(leftComponentType, rightComponentType)) {
+                addStaticTypeError("Cannot assign value of type " + rightComponentType.toString(false) + " into array of type " + lhsType.toString(false), rightExpression);
+            }
         }
     }
 
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index 9152b2d..d841ed5 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -91,6 +91,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             for (int i in arr) { }
         ''', 'Cannot loop with element of type int with collection of type java.lang.String[]'
     }
+
     void testJava5StyleForLoopWithArray() {
         assertScript '''
             String[] arr = ['1','2','3']
@@ -321,6 +322,20 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
         ''', 'Cannot assign value of type Foo[] to variable of type FooAnother'
     }
 
+    // GROOVY-9517
+    void testShouldAllowArrayAssignment() {
+        assertScript '''
+            void test(File directory) {
+                File[] files = directory.listFiles()
+                files = files?.sort { it.name }
+                for (file in files) {
+                    // ...
+                }
+            }
+            println 'works'
+        '''
+    }
+
     void testListPlusEquals() {
         assertScript '''
             List<String> list = ['a','b']
@@ -334,7 +349,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             assert list == ['a','b','c']
         '''
     }
-    
+
     void testObjectArrayGet() {
         assertScript '''
             Object[] arr = [new Object()]
@@ -478,18 +493,21 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
     // GROOVY-5793
     void testShouldNotForceAsTypeWhenListOfNullAssignedToArray() {
         assertScript '''
-        Integer[] m() {
-          Integer[] arr = [ null, null ]
-        }
-        assert m().length == 2
+            Integer[] m() {
+                Integer[] array = [ null, null ]
+                return array
+            }
+            assert m().length == 2
         '''
     }
+
     void testShouldNotForceAsTypeWhenListOfNullAssignedToArrayUnlessPrimitive() {
         shouldFailWithMessages '''
-        int[] m() {
-          int[] arr = [ null, null ]
-        }
-        ''', 'into array of type'
+            int[] m() {
+                int[] array = [ null, null ]
+                return array
+            }
+        ''', 'Cannot assign value of type java.lang.Object into array of type int[]'
     }
 
     // GROOVY-6131
@@ -512,7 +530,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             assert AR.'key'[0] == ['val1']
         """
     }
-    
+
     // GROOVY-6311
     void testSetSpread() {
         assertScript """
@@ -525,7 +543,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             assert res[0].contains('def')
         """
     }
-    
+
     // GROOVY-6241
     void testAsImmutable() {
         assertScript """
@@ -535,7 +553,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             Map<String, Integer> immutableMap = [foo: 123, bar: 456].asImmutable()
         """
     }
-    
+
     // GROOVY-6350
     void testListPlusList() {
         assertScript """
@@ -566,8 +584,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
                 Set<Foo> foos = [new Foo(name: 'pls'), new Foo(name: 'bar')].toSet()
                 foos*.name
             }
-            assert meth().toSet() == ['pls', 'bar'].toSet()            
+            assert meth().toSet() == ['pls', 'bar'].toSet()
         '''
     }
 }
-