You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/07/18 06:04:52 UTC

[groovy] branch master updated: GROOVY-7848: retain generics of list or map elements (closes #1311)

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

paulk 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 1b6e64e  GROOVY-7848: retain generics of list or map elements (closes #1311)
1b6e64e is described below

commit 1b6e64e82654026c30a706c6ba4241431c939e2d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jul 13 09:52:56 2020 -0500

    GROOVY-7848: retain generics of list or map elements (closes #1311)
    
        [[1],[2]] should infer as List<List<Integer>>
        [1:['a'],2:['b']] should infer as Map<Integer,List<String>>
---
 .../java/org/codehaus/groovy/ast/GenericsType.java |  2 +-
 .../transform/stc/StaticTypeCheckingVisitor.java   | 38 ++++++++++------------
 .../groovy/transform/stc/GenericsSTCTest.groovy    | 16 +++++++--
 .../antlr4/util/ASTComparatorCategory.groovy       | 18 +++++-----
 4 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index 61cf867..98f7b83 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -58,7 +58,7 @@ public class GenericsType extends ASTNode {
     }
 
     public void setType(final ClassNode type) {
-        this.type = Objects.requireNonNull(type);
+        this.type = Objects.requireNonNull(type); // TODO: ensure type is not primitive
     }
 
     public String toString() {
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 0dc82fc..cc8cfc1 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4979,18 +4979,17 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
     protected ClassNode inferListExpressionType(final ListExpression list) {
         List<Expression> expressions = list.getExpressions();
-        if (expressions.isEmpty()) {
-            // cannot infer, return list type
+        int nExpressions = expressions.size();
+        if (nExpressions == 0) {
             return list.getType();
         }
         ClassNode listType = list.getType();
         GenericsType[] genericsTypes = listType.getGenericsTypes();
         if ((genericsTypes == null
                 || genericsTypes.length == 0
-                || (genericsTypes.length == 1 && OBJECT_TYPE.equals(genericsTypes[0].getType())))
-                && (!expressions.isEmpty())) {
+                || (genericsTypes.length == 1 && OBJECT_TYPE.equals(genericsTypes[0].getType())))) {
             // maybe we can infer the component type
-            List<ClassNode> nodes = new LinkedList<>();
+            List<ClassNode> nodes = new ArrayList<>(nExpressions);
             for (Expression expression : expressions) {
                 if (isNullConstant(expression)) {
                     // a null element is found in the list, skip it because we'll use the other elements from the list
@@ -4998,14 +4997,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                     nodes.add(getType(expression));
                 }
             }
-            if (nodes.isEmpty()) {
-                // every element was the null constant
-                return listType;
+            if (!nodes.isEmpty()) {
+                ClassNode itemType = lowestUpperBound(nodes);
+
+                listType = listType.getPlainNodeReference();
+                listType.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(itemType))});
             }
-            ClassNode superType = getWrapper(lowestUpperBound(nodes)); // to be used in generics, type must be boxed
-            ClassNode inferred = listType.getPlainNodeReference();
-            inferred.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(superType))});
-            return inferred;
         }
         return listType;
     }
@@ -5025,23 +5022,24 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     protected ClassNode inferMapExpressionType(final MapExpression map) {
         ClassNode mapType = LINKEDHASHMAP_CLASSNODE.getPlainNodeReference();
         List<MapEntryExpression> entryExpressions = map.getMapEntryExpressions();
-        if (entryExpressions.isEmpty()) return mapType;
+        int nExpressions = entryExpressions.size();
+        if (nExpressions == 0) return mapType;
+
         GenericsType[] genericsTypes = mapType.getGenericsTypes();
         if (genericsTypes == null
                 || genericsTypes.length < 2
                 || (genericsTypes.length == 2 && OBJECT_TYPE.equals(genericsTypes[0].getType()) && OBJECT_TYPE.equals(genericsTypes[1].getType()))) {
-            List<ClassNode> keyTypes = new LinkedList<>();
-            List<ClassNode> valueTypes = new LinkedList<>();
+            List<ClassNode> keyTypes = new ArrayList<>(nExpressions);
+            List<ClassNode> valueTypes = new ArrayList<>(nExpressions);
             for (MapEntryExpression entryExpression : entryExpressions) {
                 keyTypes.add(getType(entryExpression.getKeyExpression()));
                 valueTypes.add(getType(entryExpression.getValueExpression()));
             }
-            ClassNode keyType = getWrapper(lowestUpperBound(keyTypes));  // to be used in generics, type must be boxed
-            ClassNode valueType = getWrapper(lowestUpperBound(valueTypes));  // to be used in generics, type must be boxed
+            ClassNode keyType = lowestUpperBound(keyTypes);
+            ClassNode valueType = lowestUpperBound(valueTypes);
             if (!OBJECT_TYPE.equals(keyType) || !OBJECT_TYPE.equals(valueType)) {
-                ClassNode inferred = mapType.getPlainNodeReference();
-                inferred.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(keyType)), new GenericsType(wrapTypeIfNecessary(valueType))});
-                return inferred;
+                mapType = mapType.getPlainNodeReference();
+                mapType.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(keyType)), new GenericsType(wrapTypeIfNecessary(valueType))});
             }
         }
         return mapType;
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index a924ab8..1fb352c 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -1311,7 +1311,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         ''', '[Static type checking] - Cannot find matching method Foo#<init>(java.lang.String, int)'
     }
 
-    // Groovy-5742
+    // GROOVY-5742
     void testNestedGenerics() {
         assertScript '''
             import static Next.*
@@ -1328,6 +1328,18 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-7848
+    void testNestedGenerics2() {
+        assertScript '''
+            List<Integer> test() {
+              def listOfLists = [[1,2], [3,4]]
+              listOfLists.collect { pair -> pair[0] + pair[1] }
+            }
+            def result = test()
+            assert result == [3,7]
+        '''
+    }
+
     void testMethodLevelGenericsFromInterface() {
         assertScript '''
             interface A {
@@ -1344,7 +1356,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
-    // Groovy-5610
+    // GROOVY-5610
     void testMethodWithDefaultArgument() {
         assertScript '''
             class A{}
diff --git a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy
index 9b85163..25671ea 100644
--- a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy
+++ b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy
@@ -108,22 +108,22 @@ class ASTComparatorCategory {
     private static final List<String> EXPRESSION_IGNORE_LIST = (['text'] + LOCATION_IGNORE_LIST).asUnmodifiable()
 
     public static final Map<Class, List<String>> DEFAULT_CONFIGURATION = [
-            (ClassNode)                   : ['module', 'declaredMethodsMap', 'plainNodeReference', 'typeClass', 'allInterfaces', 'orAddStaticConstructorNode', 'allDeclaredMethods', 'unresolvedSuperClass', 'innerClasses' ] + LOCATION_IGNORE_LIST,
+            (ClassNode)                   : ['module', 'declaredMethodsMap', 'plainNodeReference', 'typeClass', 'allInterfaces', 'orAddStaticConstructorNode', 'allDeclaredMethods', 'unresolvedSuperClass', 'innerClasses'] + LOCATION_IGNORE_LIST,
             (ConstructorNode)             : ['declaringClass'],
-            (DynamicVariable)             : [],
+            (DynamicVariable)             : [] as List<String>,
             (EnumConstantClassNode)       : ['typeClass'],
             (FieldNode)                   : ['owner', 'declaringClass', 'initialValueExpression', 'assignToken'],
-            (GenericsType)                : [],
+            (GenericsType)                : [] as List<String>,
             (ImportNode)                  : LOCATION_IGNORE_LIST,
-            (InnerClassNode)              : ['module', 'declaredMethodsMap', 'plainNodeReference', 'typeClass', 'allInterfaces', 'orAddStaticConstructorNode', 'allDeclaredMethods', 'unresolvedSuperClass', 'innerClasses' ] + LOCATION_IGNORE_LIST,
-            (InterfaceHelperClassNode)    : [],
+            (InnerClassNode)              : ['module', 'declaredMethodsMap', 'plainNodeReference', 'typeClass', 'allInterfaces', 'orAddStaticConstructorNode', 'allDeclaredMethods', 'unresolvedSuperClass', 'innerClasses'] + LOCATION_IGNORE_LIST,
+            (InterfaceHelperClassNode)    : [] as List<String>,
             (MethodNode)                  : ['text', 'declaringClass'],
-            (MixinNode)                   : [],
+            (MixinNode)                   : [] as List<String>,
             (ModuleNode)                  : ['context'],
-            (PackageNode)                 : [],
-            (Parameter)                   : [],
+            (PackageNode)                 : [] as List<String>,
+            (Parameter)                   : [] as List<String>,
             (PropertyNode)                : ['declaringClass', 'initialValueExpression', 'assignToken'],
-            (Variable)                    : [],
+            (Variable)                    : [] as List<String>,
             (VariableScope)               : ['clazzScope', 'parent', 'declaredVariablesIterator'],
             (Token)                       : ['root', 'startColumn'],
             (AnnotationNode)              : (['text'] + LOCATION_IGNORE_LIST),