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 2023/01/26 18:07:01 UTC

[groovy] branch GROOVY_3_0_X updated (ce91dbfc32 -> b8a6b01e57)

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

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


    from ce91dbfc32 bump copyright/notice year to 2023
     new 9e9843fcbc GROOVY-10002: STC: fix checks for list literal assignment
     new c7b0d43293 GROOVY-6912: STC: support "ArrayList a = [...]" and "HashSet b = [...]"
     new fcbd0a0d93 GROOVY-7128: STC: support "List<Number> list = [1,2,3]"
     new b8a6b01e57 GROOVY-8136: STC: map literal assignment to interface that extends `Map`

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../typehandling/DefaultTypeTransformation.java    |  18 +--
 .../transform/stc/StaticTypeCheckingSupport.java   |   6 +-
 .../transform/stc/StaticTypeCheckingVisitor.java   | 104 +++++++++----
 src/spec/test/typing/TypeCheckingTest.groovy       |   2 +-
 src/test/groovy/bugs/Groovy6086Bug.groovy          |  69 ---------
 .../stc/ArraysAndCollectionsSTCTest.groovy         | 172 ++++++++++++++++++++-
 .../transform/stc/ConstructorsSTCTest.groovy       |   2 +-
 .../stc/DefaultGroovyMethodsSTCTest.groovy         |  17 +-
 .../groovy/transform/stc/GenericsSTCTest.groovy    |  16 +-
 .../InheritConstructorsTransformTest.groovy        | 119 +++++++-------
 10 files changed, 333 insertions(+), 192 deletions(-)
 delete mode 100644 src/test/groovy/bugs/Groovy6086Bug.groovy


[groovy] 01/04: GROOVY-10002: STC: fix checks for list literal assignment

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 9e9843fcbc6b8519f1829b3619c5c70f704f8307
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Mar 27 00:29:54 2021 -0500

    GROOVY-10002: STC: fix checks for list literal assignment
    
    - empty and non-empty list literals can initialize Set variables
    - list literal generics checking skipped when constructor may be used
    - list literal as constructor arguments does not apply to abstract types
---
 .../transform/stc/StaticTypeCheckingVisitor.java   |  49 +++++----
 src/test/groovy/bugs/Groovy6086Bug.groovy          |  69 ------------
 .../stc/ArraysAndCollectionsSTCTest.groovy         |  61 ++++++++++-
 .../InheritConstructorsTransformTest.groovy        | 119 +++++++++++----------
 4 files changed, 144 insertions(+), 154 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 38ad511812..bfa0849d30 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -312,7 +312,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     private static final AtomicLong UNIQUE_LONG = new AtomicLong();
 
     protected static final Object ERROR_COLLECTOR = ErrorCollector.class;
-    protected static final ClassNode ITERABLE_TYPE = ClassHelper.make(Iterable.class);
     protected static final List<MethodNode> EMPTY_METHODNODE_LIST = Collections.emptyList();
     protected static final ClassNode TYPECHECKED_CLASSNODE = ClassHelper.make(TypeChecked.class);
     protected static final ClassNode[] TYPECHECKING_ANNOTATIONS = new ClassNode[]{TYPECHECKED_CLASSNODE};
@@ -329,8 +328,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     protected static final ClassNode CLOSUREPARAMS_CLASSNODE = ClassHelper.make(ClosureParams.class);
     protected static final ClassNode NAMED_PARAMS_CLASSNODE = ClassHelper.make(NamedParams.class);
     protected static final ClassNode NAMED_PARAM_CLASSNODE = ClassHelper.make(NamedParam.class);
-    protected static final ClassNode MAP_ENTRY_TYPE = ClassHelper.make(Map.Entry.class);
     protected static final ClassNode ENUMERATION_TYPE = ClassHelper.make(Enumeration.class);
+    protected static final ClassNode MAP_ENTRY_TYPE = ClassHelper.make(Map.Entry.class);
+    protected static final ClassNode ITERABLE_TYPE = ClassHelper.make(Iterable.class);
+    private   static final ClassNode SET_TYPE = ClassHelper.make(Set.class);
 
     public static final Statement GENERATED_EMPTY_STATEMENT = EmptyStatement.INSTANCE;
 
@@ -1272,11 +1273,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         // if left type is not a list but right type is a list, then we're in the case of a groovy
         // constructor type : Dimension d = [100,200]
         // In that case, more checks can be performed
-        if (rightExpression instanceof ListExpression
-                && !implementsInterfaceOrIsSubclassOf(LIST_TYPE, leftRedirect)) {
-            ArgumentListExpression argList = args(((ListExpression) rightExpression).getExpressions());
-            ClassNode[] args = getArgumentTypes(argList);
-            MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, args, assignmentExpression);
+        if (!implementsInterfaceOrIsSubclassOf(LIST_TYPE, leftRedirect)
+                && (!leftRedirect.isAbstract() || leftRedirect.isArray())) {
+            ClassNode[] types = getArgumentTypes(args(((ListExpression) rightExpression).getExpressions()));
+            MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, types, assignmentExpression);
             if (methodNode != null) {
                 rightExpression.putNodeMetaData(DIRECT_METHOD_CALL_TARGET, methodNode);
             }
@@ -1289,9 +1289,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final Expression rightExpression) {
-        if ((!(rightExpression instanceof MapExpression)
-                || leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
-                || (isWildcardLeftHandSide(leftRedirect) && !CLASS_Type.equals(leftRedirect)) // GROOVY-6802, GROOVY-6803
+        if ((leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
+                || (isWildcardLeftHandSide(leftRedirect) && !leftRedirect.equals(CLASS_Type)) // GROOVY-6802, GROOVY-6803
                 || implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE)) {
             return;
         }
@@ -1321,6 +1320,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return false;
     }
 
+    private static boolean isConstructorAbbreviation(final ClassNode leftType, final Expression rightExpression) {
+        return (rightExpression instanceof ListExpression && !(leftType.equals(LIST_TYPE) || leftType.equals(SET_TYPE)
+                || leftType.equals(ITERABLE_TYPE) || leftType.equals(Collection_TYPE)));
+    }
+
     protected void typeCheckAssignment(final BinaryExpression assignmentExpression, final Expression leftExpression, final ClassNode leftExpressionType, final Expression rightExpression, final ClassNode rightExpressionType) {
         if (!typeCheckMultipleAssignmentAndContinue(leftExpression, rightExpression)) return;
 
@@ -1342,9 +1346,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         } else {
             ClassNode lTypeRedirect = leftExpressionType.redirect();
             addPrecisionErrors(lTypeRedirect, leftExpressionType, rTypeAdjusted, rightExpression);
-            addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
-            addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
-            if (!hasGStringStringError(leftExpressionType, rTypeAdjusted, rightExpression)) {
+            if (rightExpression instanceof ListExpression) {
+                addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
+            } else if (rightExpression instanceof MapExpression) {
+                addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
+            }
+            if (!hasGStringStringError(leftExpressionType, rTypeAdjusted, rightExpression)
+                    && !isConstructorAbbreviation(leftExpressionType, rightExpression)) {
                 checkTypeGenerics(leftExpressionType, rTypeAdjusted, rightExpression);
             }
         }
@@ -4501,22 +4509,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 }
 
                 // as anything can be assigned to a String, Class or [Bb]oolean, return the left type instead
-                if (STRING_TYPE.equals(initialType)
-                        || CLASS_Type.equals(initialType)
-                        || Boolean_TYPE.equals(initialType)
-                        || boolean_TYPE.equals(initialType)) {
+                if (isWildcardLeftHandSide(initialType) && !initialType.equals(OBJECT_TYPE)) {
                     return initialType;
                 }
             }
 
-            if (isOrImplements(rightRedirect, Collection_TYPE)) {
-                if (leftRedirect.isArray()) {
-                    return leftRedirect;
-                }
-                if (isOrImplements(leftRedirect, Collection_TYPE) &&
-                        rightExpression instanceof ListExpression && isEmptyCollection(rightExpression)) {
-                    return left;
-                }
+            if (rightExpression instanceof ListExpression && leftRedirect.equals(SET_TYPE)) {
+                return GenericsUtils.parameterizeType(right, leftRedirect.getPlainNodeReference());
             }
 
             return right;
diff --git a/src/test/groovy/bugs/Groovy6086Bug.groovy b/src/test/groovy/bugs/Groovy6086Bug.groovy
deleted file mode 100644
index 49eb6f4692..0000000000
--- a/src/test/groovy/bugs/Groovy6086Bug.groovy
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- *  Licensed to the Apache Software Foundation (ASF) under one
- *  or more contributor license agreements.  See the NOTICE file
- *  distributed with this work for additional information
- *  regarding copyright ownership.  The ASF licenses this file
- *  to you under the Apache License, Version 2.0 (the
- *  "License"); you may not use this file except in compliance
- *  with the License.  You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- *  Unless required by applicable law or agreed to in writing,
- *  software distributed under the License is distributed on an
- *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- *  KIND, either express or implied.  See the License for the
- *  specific language governing permissions and limitations
- *  under the License.
- */
-package groovy.bugs
-
-import groovy.test.GroovyTestCase
-import org.codehaus.groovy.control.CompilePhase
-import org.codehaus.groovy.control.CompilerConfiguration
-import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit
-
-class Groovy6086Bug extends GroovyTestCase {
-
-    // Note that this unit test reproduces the code that we can
-    // see on the Grails build. However, it never managed to reproduce
-    // the issue so the latter has been fixed independently.
-    void testGroovy6086() {
-
-        def config = new CompilerConfiguration()
-        config.with {
-            targetDirectory = File.createTempDir()
-            jointCompilationOptions = [stubDir: File.createTempDir()]
-        }
-
-        try {
-            def unit = new JavaAwareCompilationUnit(config, null, new GroovyClassLoader(getClass().classLoader))
-
-            unit.addSource('Boo.java', 'interface Boo {}')
-            unit.addSource('Wrapper.groovy', '''
-            import groovy.transform.CompileStatic
-
-            @CompileStatic
-            class Wrapper {
-                private Map cache
-                Boo[] boos() {
-                    Boo[] locations = (Boo[]) cache.a
-                    if (locations == null) {
-                        if (true) {
-                            locations = [].collect { it }
-                        }
-                        else {
-                            locations = [] as Boo[]
-                        }
-                    }
-                    return locations
-                }
-            }
-        ''')
-            unit.compile(CompilePhase.INSTRUCTION_SELECTION.phaseNumber)
-        } finally {
-            config.targetDirectory.deleteDir()
-            config.jointCompilationOptions.stubDir.deleteDir()
-        }
-    }
-}
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index 8cd4a3bbb2..3d7381db7d 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -247,7 +247,6 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
     }
 
     void testInlineMap() {
-
         assertScript '''
             Map map = [a:1, b:2]
         '''
@@ -397,7 +396,7 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
         shouldFailWithMessages '''
             class Foo {
                 def say() {
-                    FooAnother foo1 = new Foo[13] // but FooAnother foo1 = new Foo() reports a STC                        Error
+                    FooAnother foo1 = new Foo[13] // but FooAnother foo1 = new Foo() reports a STC error
                 }
             }
             class FooAnother {
@@ -749,4 +748,62 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             assert meth().toSet() == ['pls', 'bar'].toSet()
         '''
     }
+
+    void testAbstractTypeInitializedByListLiteral() {
+        shouldFailWithMessages '''
+            abstract class A {
+                A(int n) {}
+            }
+            A a = [1]
+        ''', 'Cannot assign value of type java.util.List <java.lang.Integer> to variable of type A'
+    }
+
+    void testCollectionTypesInitializedByListLiteral1() {
+        assertScript '''
+            Set<String> set = []
+            set << 'foo'
+            set << 'bar'
+            set << 'foo'
+            assert set.size() == 2
+        '''
+    }
+
+    // GROOVY-10002
+    void testCollectionTypesInitializedByListLiteral2() {
+        assertScript '''
+            Set<String> set = ['foo', 'bar', 'foo']
+            assert set.size() == 2
+        '''
+
+        assertScript '''
+            ArrayDeque<String> deque = [123] // ArrayDeque(int numElements)
+        '''
+    }
+
+    // GROOVY-10002
+    void testCollectionTypesInitializedByListLiteral3() {
+        shouldFailWithMessages '''
+            Set<String> set = [1,2,3]
+        ''', 'Cannot assign java.util.Set <java.lang.Integer> to: java.util.Set <String>'
+
+        shouldFailWithMessages '''
+            Iterable<String> iter = [1,2,3]
+        ''', 'Cannot assign java.util.List <java.lang.Integer> to: java.lang.Iterable <String>'
+
+        shouldFailWithMessages '''
+            Collection<String> coll = [1,2,3]
+        ''', 'Cannot assign java.util.List <java.lang.Integer> to: java.util.Collection <String>'
+
+        shouldFailWithMessages '''
+            Deque<String> deque = [""]
+        ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
+
+        shouldFailWithMessages '''
+            Deque<String> deque = []
+        ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
+
+        shouldFailWithMessages '''
+            Queue<String> queue = []
+        ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Queue <String>'
+    }
 }
diff --git a/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy b/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy
index c3f1aea32e..91f0caa547 100644
--- a/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/InheritConstructorsTransformTest.groovy
@@ -23,16 +23,16 @@ import groovy.test.GroovyShellTestCase
 class InheritConstructorsTransformTest extends GroovyShellTestCase {
 
     void testStandardCase() {
-        assertScript """
+        assertScript '''
             import groovy.transform.InheritConstructors
             @InheritConstructors class CustomException extends RuntimeException { }
             def ce = new CustomException('foo')
             assert ce.message == 'foo'
-        """
+        '''
     }
 
     void testOverrideCase() {
-        assertScript """
+        assertScript '''
             import groovy.transform.InheritConstructors
             @InheritConstructors
             class CustomException2 extends RuntimeException {
@@ -42,11 +42,11 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
             assert ce.message == 'bar'
             ce = new CustomException2('foo')
             assert ce.message == 'foo'
-        """
+        '''
     }
 
     void testChainedCase() {
-        assertScript """
+        assertScript '''
             import groovy.transform.InheritConstructors
             @InheritConstructors
             class CustomException5 extends CustomException4 {}
@@ -56,11 +56,12 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
             class CustomException4 extends CustomException3 {}
             def ce = new CustomException5('baz')
             assert ce.message == 'baz'
-        """
+        '''
     }
 
-    void testCopyAnnotations_groovy7059() {
-        assertScript """
+    // GROOVY-7059
+    void testCopyAnnotations() {
+        assertScript '''
             import java.lang.annotation.*
             import groovy.transform.InheritConstructors
 
@@ -109,11 +110,11 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
                         break
                 }
             }
-        """
+        '''
     }
 
     void testInnerClassUsage() {
-        assertScript """
+        assertScript '''
             import groovy.transform.InheritConstructors
             @InheritConstructors
             class Outer extends RuntimeException {
@@ -138,88 +139,90 @@ class InheritConstructorsTransformTest extends GroovyShellTestCase {
             assert o.message == 'baz'
             o.test()
             new Outer2().test()
-        """
+        '''
     }
 
-    void testParametersWithGenericsAndCompileStatic_GROOVY6874() {
-        assertScript """
+    // GROOVY-6874
+    void testParametersWithGenericsAndCompileStatic() {
+        assertScript '''
             import groovy.transform.*
             import java.math.RoundingMode
 
             @CompileStatic
             abstract class BasePublisher<T, U> {
-               final Deque<T> items
-               private U mode
-               BasePublisher(Deque<T> items) { this.items = items }
-               BasePublisher(U mode) {
-                  this.mode = mode
-                  this.items = []
-               }
-               BasePublisher(Set<U> modes) { this(modes[0]) }
-               void publish(T item) { items.addFirst(item) }
-               void init(U mode) { this.mode = mode }
-               String toString() { items.join('|') + "|" + mode.toString() }
+                final Deque<T> items
+                private U mode
+                BasePublisher(Deque<T> items) { this.items = items }
+                BasePublisher(U mode) {
+                    this.mode = mode
+                    this.items = new ArrayDeque<>()
+                }
+                BasePublisher(Set<U> modes) { this(modes[0]) }
+                void publish(T item) { items.addFirst(item) }
+                void init(U mode) { this.mode = mode }
+                String toString() { items.join('|') + "|" + mode.toString() }
             }
 
             @CompileStatic @InheritConstructors
             class OrderPublisher<V> extends BasePublisher<Integer, V> {
-              static OrderPublisher make() {
-                new OrderPublisher<RoundingMode>(new LinkedList<Integer>())
-              }
-              void foo() { publish(3) }
-              void bar(V mode) { init(mode) }
-              void baz() {
-                new OrderPublisher<RoundingMode>(RoundingMode.UP)
-                new OrderPublisher<RoundingMode>(new HashSet<RoundingMode>())
-              }
+                static OrderPublisher make() {
+                    new OrderPublisher<RoundingMode>(new LinkedList<Integer>())
+                }
+                void foo() { publish(3) }
+                void bar(V mode) { init(mode) }
+                void baz() {
+                    new OrderPublisher<RoundingMode>(RoundingMode.UP)
+                    new OrderPublisher<RoundingMode>(new HashSet<RoundingMode>())
+                }
             }
 
             def op = OrderPublisher.make()
             op.foo()
             op.bar(RoundingMode.DOWN)
+            op.baz()
             assert op.toString() == '3|DOWN'
-        """
+        '''
     }
 
-    void testParametersWithGenericsAndCompileStatic_errors_GROOVY6874() {
-        def message = shouldFail """
+    // GROOVY-6874
+    void testParametersWithGenericsAndCompileStatic_errors() {
+        def message = shouldFail '''
             import groovy.transform.*
             import java.math.RoundingMode
 
             @CompileStatic
             abstract class BasePublisher<T, U> {
-               final Deque<T> items
-               private U mode
-               BasePublisher(Deque<T> items) { this.items = items }
-               BasePublisher(U mode) {
-                  this.mode = mode
-                  this.items = []
-               }
-               BasePublisher(Set<U> modes) { this(modes[0]) }
-               void publish(T item) { items.addFirst(item) }
-               void init(U mode) { this.mode = mode }
-               String toString() { items.join('|') + "|" + mode.toString() }
+                final Deque<T> items
+                private U mode
+                BasePublisher(Deque<T> items) { this.items = items }
+                BasePublisher(U mode) {
+                    this.mode = mode
+                    this.items = new ArrayDeque<>()
+                }
+                BasePublisher(Set<U> modes) { this(modes[0]) }
+                void publish(T item) { items.addFirst(item) }
+                void init(U mode) { this.mode = mode }
+                String toString() { items.join('|') + "|" + mode.toString() }
             }
 
             @CompileStatic @InheritConstructors
             class OrderPublisher<V> extends BasePublisher<Integer, V> {
-              static OrderPublisher make() {
-                new OrderPublisher<RoundingMode>(new LinkedList<String>())
-              }
-              void foo() { publish(3) }
-              void bar(V mode) { init(mode) }
-              void baz() {
-                new OrderPublisher<RoundingMode>(new Date())
-                new OrderPublisher<RoundingMode>(new HashSet<Date>())
-              }
+                static OrderPublisher make() {
+                    new OrderPublisher<RoundingMode>(new LinkedList<String>())
+                }
+                void foo() { publish(3) }
+                void bar(V mode) { init(mode) }
+                void baz() {
+                    new OrderPublisher<RoundingMode>(new Date())
+                    new OrderPublisher<RoundingMode>(new HashSet<Date>())
+                }
             }
 
             def op = OrderPublisher.make()
             op.foo()
             op.bar(RoundingMode.DOWN)
             assert op.toString() == '3|DOWN'
-        """
-        assert message.contains('Cannot call OrderPublisher <RoundingMode>#<init>(java.util.Deque <java.lang.Integer>) with arguments [java.util.LinkedList <String>]')
+        '''
         assert message.contains('Cannot find matching method OrderPublisher#<init>(java.util.Date)')
         assert message.contains('Cannot call OrderPublisher <RoundingMode>#<init>(java.util.Set <RoundingMode>) with arguments [java.util.HashSet <Date>]')
     }


[groovy] 03/04: GROOVY-7128: STC: support "List list = [1,2,3]"

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit fcbd0a0d93453c46dadd101a5ca548d06346158a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Mar 27 17:41:39 2021 -0500

    GROOVY-7128: STC: support "List<Number> list = [1,2,3]"
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 25 ++++++++++++++++++++--
 .../stc/ArraysAndCollectionsSTCTest.groovy         | 21 ++++++++++++++++++
 .../stc/DefaultGroovyMethodsSTCTest.groovy         |  2 +-
 3 files changed, 45 insertions(+), 3 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 3affb67462..e4945662e5 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4524,11 +4524,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                         || ITERABLE_TYPE.equals(leftRedirect)
                         || Collection_TYPE.equals(leftRedirect)
                         || ArrayList_TYPE.isDerivedFrom(leftRedirect)) { // GROOVY-6912
-                    return GenericsUtils.parameterizeType(right, ArrayList_TYPE.getPlainNodeReference());
+                    return getLiteralResultType(left, right, ArrayList_TYPE); // GROOVY-7128
                 }
                 if (SET_TYPE.equals(leftRedirect)
                         || LinkedHashSet_TYPE.isDerivedFrom(leftRedirect)) { // GROOVY-6912
-                    return GenericsUtils.parameterizeType(right, LinkedHashSet_TYPE.getPlainNodeReference());
+                    return getLiteralResultType(left, right, LinkedHashSet_TYPE); // GROOVY-7128
                 }
             }
 
@@ -4587,6 +4587,27 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return null;
     }
 
+    /**
+     * For "{@code List<Type> x = [...]}" or "{@code Set<Type> y = [...]}", etc.
+     * the literal may be composed of sub-types of {@code Type}. In these cases,
+     * {@code ArrayList<Type>} is an appropriate result type for the expression.
+     */
+    private ClassNode getLiteralResultType(final ClassNode targetType, final ClassNode sourceType, final ClassNode baseType) {
+        ClassNode resultType = sourceType.equals(baseType) ? sourceType
+                : GenericsUtils.parameterizeType(sourceType, baseType.getPlainNodeReference());
+
+        if (targetType.getGenericsTypes() != null
+                && !GenericsUtils.buildWildcardType(targetType).isCompatibleWith(resultType)) {
+            GenericsType[] lgt = targetType.getGenericsTypes(), rgt = resultType.getGenericsTypes();
+            if (!lgt[0].isPlaceholder() && !lgt[0].isWildcard() && GenericsUtils.buildWildcardType(
+                    getCombinedBoundType(lgt[0])).isCompatibleWith(getCombinedBoundType(rgt[0]))) {
+                resultType = GenericsUtils.parameterizeType(targetType, baseType.getPlainNodeReference());
+            }
+        }
+
+        return resultType;
+    }
+
     private ClassNode getMathResultType(final int op, final ClassNode leftRedirect, final ClassNode rightRedirect, final String operationName) {
         if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
             if (isOperationInGroup(op)) {
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index c7a467a19c..970d5d63ec 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -887,4 +887,25 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
         ''',
         'Cannot assign value of type java.util.List', ' to variable of type java.util.Queue <String>'
     }
+
+    // GROOVY-7128
+    void testCollectionTypesInitializedByListLiteral4() {
+        assertScript '''
+            Collection<Number> collection = [1,2,3]
+            assert collection.size() == 3
+            assert collection.last() == 3
+        '''
+
+        assertScript '''
+            List<Number> list = [1,2,3]
+            assert list.size() == 3
+            assert list.last() == 3
+        '''
+
+        assertScript '''
+            Set<Number> set = [1,2,3,3]
+            assert set.size() == 3
+            assert set.last() == 3
+        '''
+    }
 }
diff --git a/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy b/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
index f9401f7957..b481e43e62 100644
--- a/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
@@ -108,7 +108,7 @@ class DefaultGroovyMethodsSTCTest extends StaticTypeCheckingTestCase {
         '''
 
         shouldFailWithMessages '''
-            List<Number> numbers = [(Number)1, 2, 3]
+            List<Number> numbers = [1, 2, 3]
             numbers.getSequence()
             numbers.getString()
             numbers.sequence


[groovy] 02/04: GROOVY-6912: STC: support "ArrayList a = [...]" and "HashSet b = [...]"

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit c7b0d43293e2b36267195a47c323ae1f35b71ddc
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Mar 27 10:41:21 2021 -0500

    GROOVY-6912: STC: support "ArrayList a = [...]" and "HashSet b = [...]"
    
    - ArrayList and super types are the natural types of a list literal
    - LinkedHashSet and super types via castToType/continueCastOnCollection
---
 .../typehandling/DefaultTypeTransformation.java    | 18 ++--
 .../transform/stc/StaticTypeCheckingSupport.java   |  4 +-
 .../transform/stc/StaticTypeCheckingVisitor.java   | 24 ++++--
 src/spec/test/typing/TypeCheckingTest.groovy       |  2 +-
 .../stc/ArraysAndCollectionsSTCTest.groovy         | 95 ++++++++++++++++++++--
 .../transform/stc/ConstructorsSTCTest.groovy       |  2 +-
 .../stc/DefaultGroovyMethodsSTCTest.groovy         | 15 ++--
 .../groovy/transform/stc/GenericsSTCTest.groovy    | 16 ++--
 8 files changed, 136 insertions(+), 40 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java b/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
index aa65f334da..5601d98f3c 100644
--- a/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
+++ b/src/main/java/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
@@ -244,20 +244,17 @@ public class DefaultTypeTransformation {
     }
 
     private static Object continueCastOnCollection(Object object, Class type) {
-        int modifiers = type.getModifiers();
-        Collection answer;
-        if (object instanceof Collection && type.isAssignableFrom(LinkedHashSet.class) &&
-                (type == LinkedHashSet.class || Modifier.isAbstract(modifiers) || Modifier.isInterface(modifiers))) {
+        if (object instanceof Collection && type.isAssignableFrom(LinkedHashSet.class)) {
             return new LinkedHashSet((Collection) object);
         }
+
         if (object.getClass().isArray()) {
-            if (type.isAssignableFrom(ArrayList.class) && (Modifier.isAbstract(modifiers) || Modifier.isInterface(modifiers))) {
+            Collection answer;
+            if (type.isAssignableFrom(ArrayList.class) && Modifier.isAbstract(type.getModifiers())) {
                 answer = new ArrayList();
-            } else if (type.isAssignableFrom(LinkedHashSet.class) && (Modifier.isAbstract(modifiers) || Modifier.isInterface(modifiers))) {
+            } else if (type.isAssignableFrom(LinkedHashSet.class) && Modifier.isAbstract(type.getModifiers())) {
                 answer = new LinkedHashSet();
             } else {
-                // let's call the collections constructor
-                // passing in the list wrapper
                 try {
                     answer = (Collection) type.getDeclaredConstructor().newInstance();
                 } catch (Exception e) {
@@ -267,9 +264,8 @@ public class DefaultTypeTransformation {
 
             // we cannot just wrap in a List as we support primitive type arrays
             int length = Array.getLength(object);
-            for (int i = 0; i < length; i++) {
-                Object element = Array.get(object, i);
-                answer.add(element);
+            for (int i = 0; i < length; i += 1) {
+                answer.add(Array.get(object, i));
             }
             return answer;
         }
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 a09d9ede3e..795bf4e5c4 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -163,7 +163,7 @@ public abstract class StaticTypeCheckingSupport {
     protected static final ClassNode ArrayList_TYPE = makeWithoutCaching(ArrayList.class);
     protected static final ClassNode Collection_TYPE = makeWithoutCaching(Collection.class);
     protected static final ClassNode Deprecated_TYPE = makeWithoutCaching(Deprecated.class);
-    protected static final ExtensionMethodCache EXTENSION_METHOD_CACHE = ExtensionMethodCache.INSTANCE;
+    protected static final ClassNode LinkedHashSet_TYPE = makeWithoutCaching(LinkedHashSet.class);
 
     protected static final Map<ClassNode, Integer> NUMBER_TYPES = Maps.of(
             byte_TYPE,    0,
@@ -238,6 +238,8 @@ public abstract class StaticTypeCheckingSupport {
         }
     };
 
+    protected static final ExtensionMethodCache EXTENSION_METHOD_CACHE = ExtensionMethodCache.INSTANCE;
+
     public static void clearExtensionMethodCache() {
         EXTENSION_METHOD_CACHE.cache.clearAll();
     }
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 bfa0849d30..3affb67462 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -236,6 +236,7 @@ import static org.codehaus.groovy.syntax.Types.MOD_EQUAL;
 import static org.codehaus.groovy.syntax.Types.PLUS_PLUS;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.ArrayList_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.Collection_TYPE;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.LinkedHashSet_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.Matcher_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.NUMBER_OPS;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.UNKNOWN_PARAMETER_TYPE;
@@ -1274,7 +1275,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         // constructor type : Dimension d = [100,200]
         // In that case, more checks can be performed
         if (!implementsInterfaceOrIsSubclassOf(LIST_TYPE, leftRedirect)
-                && (!leftRedirect.isAbstract() || leftRedirect.isArray())) {
+                && (!leftRedirect.isAbstract() || leftRedirect.isArray())
+                && !ArrayList_TYPE.isDerivedFrom(leftRedirect) && !LinkedHashSet_TYPE.isDerivedFrom(leftRedirect)) {
             ClassNode[] types = getArgumentTypes(args(((ListExpression) rightExpression).getExpressions()));
             MethodNode methodNode = checkGroovyStyleConstructor(leftRedirect, types, assignmentExpression);
             if (methodNode != null) {
@@ -1321,8 +1323,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     private static boolean isConstructorAbbreviation(final ClassNode leftType, final Expression rightExpression) {
-        return (rightExpression instanceof ListExpression && !(leftType.equals(LIST_TYPE) || leftType.equals(SET_TYPE)
-                || leftType.equals(ITERABLE_TYPE) || leftType.equals(Collection_TYPE)));
+        if (rightExpression instanceof ListExpression) {
+            return !(ArrayList_TYPE.isDerivedFrom(leftType) || ArrayList_TYPE.implementsInterface(leftType)
+                    || LinkedHashSet_TYPE.isDerivedFrom(leftType) || LinkedHashSet_TYPE.implementsInterface(leftType));
+        }
+        return false;
     }
 
     protected void typeCheckAssignment(final BinaryExpression assignmentExpression, final Expression leftExpression, final ClassNode leftExpressionType, final Expression rightExpression, final ClassNode rightExpressionType) {
@@ -4514,8 +4519,17 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 }
             }
 
-            if (rightExpression instanceof ListExpression && leftRedirect.equals(SET_TYPE)) {
-                return GenericsUtils.parameterizeType(right, leftRedirect.getPlainNodeReference());
+            if (rightExpression instanceof ListExpression && !leftRedirect.equals(OBJECT_TYPE)) {
+                if (LIST_TYPE.equals(leftRedirect)
+                        || ITERABLE_TYPE.equals(leftRedirect)
+                        || Collection_TYPE.equals(leftRedirect)
+                        || ArrayList_TYPE.isDerivedFrom(leftRedirect)) { // GROOVY-6912
+                    return GenericsUtils.parameterizeType(right, ArrayList_TYPE.getPlainNodeReference());
+                }
+                if (SET_TYPE.equals(leftRedirect)
+                        || LinkedHashSet_TYPE.isDerivedFrom(leftRedirect)) { // GROOVY-6912
+                    return GenericsUtils.parameterizeType(right, LinkedHashSet_TYPE.getPlainNodeReference());
+                }
             }
 
             return right;
diff --git a/src/spec/test/typing/TypeCheckingTest.groovy b/src/spec/test/typing/TypeCheckingTest.groovy
index 6d2018bca3..1dc45860a6 100644
--- a/src/spec/test/typing/TypeCheckingTest.groovy
+++ b/src/spec/test/typing/TypeCheckingTest.groovy
@@ -686,7 +686,7 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound
             }
             // end::flowtyping_typeconstraints_failure[]
             flowTypingWithExplicitType()
-        ''', '[Static type checking] - Cannot find matching method java.util.List#add(int)'
+        ''', '[Static type checking] - Cannot find matching method java.util.ArrayList#add(int)'
 
         assertScript '''
             // tag::flowtyping_typeconstraints_fixed[]
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index 3d7381db7d..c7a467a19c 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -755,7 +755,64 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
                 A(int n) {}
             }
             A a = [1]
-        ''', 'Cannot assign value of type java.util.List <java.lang.Integer> to variable of type A'
+        ''',
+        'Cannot assign value of type java.util.List <java.lang.Integer> to variable of type A'
+    }
+
+    // GROOVY-6912
+    void testArrayListTypeInitializedByListLiteral() {
+        assertScript '''
+            ArrayList list = [1,2,3]
+            assert list.size() == 3
+            assert list.last() == 3
+        '''
+
+        assertScript '''
+            ArrayList list = [[1,2,3]]
+            assert list.size() == 1
+        '''
+
+        assertScript '''
+            ArrayList<Integer> list = [1,2,3]
+            assert list.size() == 3
+            assert list.last() == 3
+        '''
+
+        shouldFailWithMessages '''
+            ArrayList<String> strings = [1,2,3]
+        ''',
+        'Incompatible generic argument types. Cannot assign java.util.ArrayList <java.lang.Integer> to: java.util.ArrayList <String>'
+    }
+
+    // GROOVY-6912
+    void testSetDerivativesInitializedByListLiteral() {
+        assertScript '''
+            LinkedHashSet set = [1,2,3]
+            assert set.size() == 3
+            assert set.contains(3)
+        '''
+
+        assertScript '''
+            HashSet set = [1,2,3]
+            assert set.size() == 3
+            assert set.contains(3)
+        '''
+
+        assertScript '''
+            LinkedHashSet set = [[1,2,3]]
+            assert set.size() == 1
+        '''
+
+        assertScript '''
+            LinkedHashSet<Integer> set = [1,2,3]
+            assert set.size() == 3
+            assert set.contains(3)
+        '''
+
+        shouldFailWithMessages '''
+            LinkedHashSet<String> strings = [1,2,3]
+        ''',
+        'Incompatible generic argument types. Cannot assign java.util.LinkedHashSet <java.lang.Integer> to: java.util.LinkedHashSet <String>'
     }
 
     void testCollectionTypesInitializedByListLiteral1() {
@@ -766,6 +823,14 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             set << 'foo'
             assert set.size() == 2
         '''
+
+        assertScript '''
+            AbstractSet<String> set = []
+            set << 'foo'
+            set << 'bar'
+            set << 'foo'
+            assert set.size() == 2
+        '''
     }
 
     // GROOVY-10002
@@ -775,6 +840,11 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             assert set.size() == 2
         '''
 
+        assertScript '''
+            AbstractList<String> list = ['foo', 'bar', 'foo']
+            assert list.size() == 3
+        '''
+
         assertScript '''
             ArrayDeque<String> deque = [123] // ArrayDeque(int numElements)
         '''
@@ -784,26 +854,37 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
     void testCollectionTypesInitializedByListLiteral3() {
         shouldFailWithMessages '''
             Set<String> set = [1,2,3]
-        ''', 'Cannot assign java.util.Set <java.lang.Integer> to: java.util.Set <String>'
+        ''',
+        'Cannot assign java.util.LinkedHashSet <java.lang.Integer> to: java.util.Set <String>'
+
+        shouldFailWithMessages '''
+            List<String> list = ['a','b',3]
+        ''',
+        'Cannot assign java.util.ArrayList <java.io.Serializable> to: java.util.List <String>'
 
         shouldFailWithMessages '''
             Iterable<String> iter = [1,2,3]
-        ''', 'Cannot assign java.util.List <java.lang.Integer> to: java.lang.Iterable <String>'
+        ''',
+        'Cannot assign java.util.ArrayList <java.lang.Integer> to: java.lang.Iterable <String>'
 
         shouldFailWithMessages '''
             Collection<String> coll = [1,2,3]
-        ''', 'Cannot assign java.util.List <java.lang.Integer> to: java.util.Collection <String>'
+        ''',
+        'Cannot assign java.util.ArrayList <java.lang.Integer> to: java.util.Collection <String>'
 
         shouldFailWithMessages '''
             Deque<String> deque = [""]
-        ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
+        ''',
+        'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
 
         shouldFailWithMessages '''
             Deque<String> deque = []
-        ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
+        ''',
+        'Cannot assign value of type java.util.List', ' to variable of type java.util.Deque <String>'
 
         shouldFailWithMessages '''
             Queue<String> queue = []
-        ''', 'Cannot assign value of type java.util.List', ' to variable of type java.util.Queue <String>'
+        ''',
+        'Cannot assign value of type java.util.List', ' to variable of type java.util.Queue <String>'
     }
 }
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index bc13920cb0..1669a475d8 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -91,7 +91,7 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
             import java.awt.Dimension
             List args = [100,200]
             Dimension d = args // not supported
-        ''', 'Cannot assign value of type java.util.List <java.lang.Integer> to variable of type java.awt.Dimension'
+        ''', 'Cannot assign value of type java.util.ArrayList <java.lang.Integer> to variable of type java.awt.Dimension'
     }
 
     void testConstructFromMap() {
diff --git a/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy b/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
index 23e3aef9e0..f9401f7957 100644
--- a/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/DefaultGroovyMethodsSTCTest.groovy
@@ -114,10 +114,10 @@ class DefaultGroovyMethodsSTCTest extends StaticTypeCheckingTestCase {
             numbers.sequence
             numbers.string
         ''',
-        'Cannot call <CS extends java.lang.CharSequence> java.util.List <java.lang.Number>#getSequence() with arguments []',
-        'Cannot call java.util.List <java.lang.Number>#getString() with arguments []',
-        'No such property: sequence for class: java.util.List <java.lang.Number>',
-        'No such property: string for class: java.util.List <java.lang.Number>'
+        'Cannot call <CS extends java.lang.CharSequence> java.util.ArrayList <java.lang.Number>#getSequence() with arguments []',
+        'Cannot call java.util.ArrayList <java.lang.Number>#getString() with arguments []',
+        'No such property: sequence for class: java.util.ArrayList <java.lang.Number>',
+        'No such property: string for class: java.util.ArrayList <java.lang.Number>'
     }
 
     // GROOVY-5584
@@ -172,11 +172,14 @@ class DefaultGroovyMethodsSTCTest extends StaticTypeCheckingTestCase {
     }
 
     // GROOVY-7283
-    void testListWithDefaultInfersInt() {
+    void testListWithDefault() {
         assertScript '''
             def list = [].withDefault{ it.longValue() }
+            //                         ^^ int parameter
             list[0] = list[3]
-            assert list[0] == 3 && list[0].class == Long
+
+            assert list[0].class == Long
+            assert list[0] === 3L
         '''
     }
 
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 458fd9e5c0..fa1e949a10 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -59,7 +59,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             List<String> list = []
             list.add(1)
         ''',
-        'Cannot find matching method java.util.List#add(int)'
+        'Cannot find matching method java.util.ArrayList#add(int)'
     }
 
     void testAddOnList2() {
@@ -86,7 +86,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             List<String> list = []
             list << 1
         ''',
-        'Cannot call <T> java.util.List <String>#leftShift(T) with arguments [int]'
+        'Cannot call <T> java.util.ArrayList <String>#leftShift(T) with arguments [int]'
     }
 
     void testAddOnList2UsingLeftShift() {
@@ -1745,7 +1745,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         shouldFailWithMessages '''
             List<String> elements = ['a','b', 1]
         ''',
-        'Cannot assign java.util.List <java.io.Serializable> to: java.util.List <String>'
+        'Cannot assign java.util.ArrayList <java.io.Serializable> to: java.util.List <String>'
     }
 
     void testGenericAssignmentWithSubClass() {
@@ -1775,7 +1775,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         shouldFailWithMessages '''
             List<? super Number> list = ['string']
         ''',
-        'Cannot assign java.util.List <java.lang.String> to: java.util.List <? super java.lang.Number>'
+        'Cannot assign java.util.ArrayList <java.lang.String> to: java.util.List <? super java.lang.Number>'
     }
 
     // GROOVY-9914, GROOVY-10036
@@ -2662,8 +2662,8 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             Collection<String> strings = list.findAll { it }
             @ASTTest(phase=INSTRUCTION_SELECTION, value={
                 def dmt = node.rightExpression.getNodeMetaData(DIRECT_METHOD_CALL_TARGET)
+                assert dmt.declaringClass.implementsInterface(LIST_TYPE)
                 assert dmt !instanceof ExtensionMethodNode
-                assert dmt.declaringClass == LIST_TYPE
                 assert dmt.name == 'addAll'
             })
             boolean r = list.addAll(strings)
@@ -2676,7 +2676,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             Collection<Integer> numbers = (Collection<Integer>) [1,2,3]
             boolean r = list.addAll(numbers)
         ''',
-        'Cannot call java.util.List <java.lang.String>#addAll(java.util.Collection <? extends java.lang.String>) with arguments [java.util.Collection <Integer>]'
+        'Cannot call java.util.ArrayList <java.lang.String>#addAll(java.util.Collection <? extends java.lang.String>) with arguments [java.util.Collection <Integer>]'
     }
 
     // GROOVY-5528
@@ -3059,7 +3059,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             def test() {
                 @ASTTest(phase=INSTRUCTION_SELECTION, value={
                     def type = node.getNodeMetaData(INFERRED_TYPE)
-                    assert type.equals(LIST_TYPE)
+                    assert type.implementsInterface(LIST_TYPE)
                     assert type.genericsTypes.length == 1
                     assert type.genericsTypes[0].type == GSTRING_TYPE
                 })
@@ -3068,7 +3068,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
 
                 @ASTTest(phase=INSTRUCTION_SELECTION, value={
                     def type = node.getNodeMetaData(INFERRED_TYPE)
-                    assert type.equals(LIST_TYPE)
+                    assert type.implementsInterface(LIST_TYPE)
                     assert type.genericsTypes.length == 1
                     assert type.genericsTypes[0].type == GSTRING_TYPE
                 })


[groovy] 04/04: GROOVY-8136: STC: map literal assignment to interface that extends `Map`

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit b8a6b01e575433dafcb5563caa1b84f447499d83
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Sep 7 15:17:55 2022 -0500

    GROOVY-8136: STC: map literal assignment to interface that extends `Map`
    
    3_0_X backport
---
 .../transform/stc/StaticTypeCheckingSupport.java   |  2 ++
 .../transform/stc/StaticTypeCheckingVisitor.java   | 32 ++++++++++++----------
 .../stc/ArraysAndCollectionsSTCTest.groovy         |  9 ++++++
 3 files changed, 28 insertions(+), 15 deletions(-)

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 795bf4e5c4..5a9c842fe7 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -59,6 +59,7 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -163,6 +164,7 @@ public abstract class StaticTypeCheckingSupport {
     protected static final ClassNode ArrayList_TYPE = makeWithoutCaching(ArrayList.class);
     protected static final ClassNode Collection_TYPE = makeWithoutCaching(Collection.class);
     protected static final ClassNode Deprecated_TYPE = makeWithoutCaching(Deprecated.class);
+    protected static final ClassNode LinkedHashMap_TYPE = makeWithoutCaching(LinkedHashMap.class);
     protected static final ClassNode LinkedHashSet_TYPE = makeWithoutCaching(LinkedHashSet.class);
 
     protected static final Map<ClassNode, Integer> NUMBER_TYPES = Maps.of(
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 e4945662e5..598eff6c5b 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -236,6 +236,7 @@ import static org.codehaus.groovy.syntax.Types.MOD_EQUAL;
 import static org.codehaus.groovy.syntax.Types.PLUS_PLUS;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.ArrayList_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.Collection_TYPE;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.LinkedHashMap_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.LinkedHashSet_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.Matcher_TYPE;
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.NUMBER_OPS;
@@ -325,7 +326,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     protected static final MethodNode GET_THISOBJECT = CLOSURE_TYPE.getGetterMethod("getThisObject");
     protected static final ClassNode DELEGATES_TO = ClassHelper.make(DelegatesTo.class);
     protected static final ClassNode DELEGATES_TO_TARGET = ClassHelper.make(DelegatesTo.Target.class);
-    protected static final ClassNode LINKEDHASHMAP_CLASSNODE = ClassHelper.make(LinkedHashMap.class);
+    protected static final ClassNode LINKEDHASHMAP_CLASSNODE = LinkedHashMap_TYPE; //TODO @Deprecated
     protected static final ClassNode CLOSUREPARAMS_CLASSNODE = ClassHelper.make(ClosureParams.class);
     protected static final ClassNode NAMED_PARAMS_CLASSNODE = ClassHelper.make(NamedParams.class);
     protected static final ClassNode NAMED_PARAM_CLASSNODE = ClassHelper.make(NamedParam.class);
@@ -1290,19 +1291,18 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
     }
 
-    private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final Expression rightExpression) {
-        if ((leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
-                || (isWildcardLeftHandSide(leftRedirect) && !leftRedirect.equals(CLASS_Type)) // GROOVY-6802, GROOVY-6803
-                || implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE)) {
+    private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final MapExpression rightExpression) {
+        if (!isConstructorAbbreviation(leftRedirect, rightExpression)
+                // GROOVY-6802, GROOVY-6803: Object, String or [Bb]oolean target
+                || (isWildcardLeftHandSide(leftRedirect) && !leftRedirect.equals(CLASS_Type))) {
             return;
         }
 
         // groovy constructor shorthand: A a = [x:2, y:3]
-        ClassNode[] argTypes = getArgumentTypes(args(rightExpression));
+        ClassNode[] argTypes = {getType(rightExpression)};
         checkGroovyStyleConstructor(leftRedirect, argTypes, rightExpression);
         // perform additional type checking on arguments
-        MapExpression mapExpression = (MapExpression) rightExpression;
-        checkGroovyConstructorMap(leftExpression, leftRedirect, mapExpression);
+        checkGroovyConstructorMap(leftExpression, leftRedirect, rightExpression);
     }
 
     private void checkTypeGenerics(final ClassNode leftExpressionType, final ClassNode rightExpressionType, final Expression rightExpression) {
@@ -1327,6 +1327,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             return !(ArrayList_TYPE.isDerivedFrom(leftType) || ArrayList_TYPE.implementsInterface(leftType)
                     || LinkedHashSet_TYPE.isDerivedFrom(leftType) || LinkedHashSet_TYPE.implementsInterface(leftType));
         }
+        if (rightExpression instanceof MapExpression) {
+            return !(LinkedHashMap_TYPE.isDerivedFrom(leftType) || LinkedHashMap_TYPE.implementsInterface(leftType));
+        }
         return false;
     }
 
@@ -1354,7 +1357,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             if (rightExpression instanceof ListExpression) {
                 addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
             } else if (rightExpression instanceof MapExpression) {
-                addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, rightExpression);
+                addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, (MapExpression)rightExpression);
             }
             if (!hasGStringStringError(leftExpressionType, rTypeAdjusted, rightExpression)
                     && !isConstructorAbbreviation(leftExpressionType, rightExpression)) {
@@ -1438,10 +1441,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
         constructors = findMethod(node, "<init>", arguments);
         if (constructors.isEmpty()) {
-            if (isBeingCompiled(node) && !node.isInterface() && arguments.length == 1 && arguments[0].equals(LINKEDHASHMAP_CLASSNODE)) {
+            if (isBeingCompiled(node) && !node.isInterface() && arguments.length == 1 && arguments[0].equals(LinkedHashMap_TYPE)) {
                 // there will be a default hash map constructor added later
-                ConstructorNode cn = new ConstructorNode(Opcodes.ACC_PUBLIC, new Parameter[]{new Parameter(LINKEDHASHMAP_CLASSNODE, "args")}, ClassNode.EMPTY_ARRAY, EmptyStatement.INSTANCE);
-                return cn;
+                return new ConstructorNode(Opcodes.ACC_PUBLIC, new Parameter[]{new Parameter(LinkedHashMap_TYPE, "args")}, ClassNode.EMPTY_ARRAY, EmptyStatement.INSTANCE);
             } else {
                 addStaticTypeError("No matching constructor found: " + prettyPrintTypeName(node) + toMethodParametersString("", arguments), source);
                 return null;
@@ -4592,7 +4594,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
      * the literal may be composed of sub-types of {@code Type}. In these cases,
      * {@code ArrayList<Type>} is an appropriate result type for the expression.
      */
-    private ClassNode getLiteralResultType(final ClassNode targetType, final ClassNode sourceType, final ClassNode baseType) {
+    private static ClassNode getLiteralResultType(final ClassNode targetType, final ClassNode sourceType, final ClassNode baseType) {
         ClassNode resultType = sourceType.equals(baseType) ? sourceType
                 : GenericsUtils.parameterizeType(sourceType, baseType.getPlainNodeReference());
 
@@ -4608,7 +4610,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return resultType;
     }
 
-    private ClassNode getMathResultType(final int op, final ClassNode leftRedirect, final ClassNode rightRedirect, final String operationName) {
+    private static ClassNode getMathResultType(final int op, final ClassNode leftRedirect, final ClassNode rightRedirect, final String operationName) {
         if (isNumberType(leftRedirect) && isNumberType(rightRedirect)) {
             if (isOperationInGroup(op)) {
                 if (isIntCategory(leftRedirect) && isIntCategory(rightRedirect)) return int_TYPE;
@@ -5311,7 +5313,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     protected ClassNode inferMapExpressionType(final MapExpression map) {
-        ClassNode mapType = LINKEDHASHMAP_CLASSNODE.getPlainNodeReference();
+        ClassNode mapType = LinkedHashMap_TYPE.getPlainNodeReference();
         List<MapEntryExpression> entryExpressions = map.getMapEntryExpressions();
         int nExpressions = entryExpressions.size();
         if (nExpressions == 0) return mapType;
diff --git a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
index 970d5d63ec..f4e0ab4f4b 100644
--- a/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ArraysAndCollectionsSTCTest.groovy
@@ -908,4 +908,13 @@ class ArraysAndCollectionsSTCTest extends StaticTypeCheckingTestCase {
             assert set.last() == 3
         '''
     }
+
+    // GROOVY-8136
+    void testInterfaceThatExtendsMapInitializedByMapLiteral() {
+        shouldFailWithMessages '''
+            interface MVM<K, V> extends Map<K, List<V>> { }
+            MVM map = [:] // no STC error; fails at runtime
+        ''',
+        'No matching constructor found'
+    }
 }