groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject groovy git commit: GROOVY-8255: Odd problems with flow typing and generics in Groovy 2.4.12+: add target typing for empty literals within Ternary/Elvis (closes #635)
Date Tue, 14 Nov 2017 12:15:33 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_6_X bdb058dd8 -> 891366edd


GROOVY-8255: Odd problems with flow typing and generics in Groovy 2.4.12+: add target typing
for empty literals within Ternary/Elvis (closes #635)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/891366ed
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/891366ed
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/891366ed

Branch: refs/heads/GROOVY_2_6_X
Commit: 891366eddb3e87d4102099594664f47760f92019
Parents: bdb058d
Author: paulk <paulk@asert.com.au>
Authored: Mon Nov 13 18:43:02 2017 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Tue Nov 14 22:15:18 2017 +1000

----------------------------------------------------------------------
 .../stc/StaticTypeCheckingVisitor.java          | 36 +++++++++++++++++++-
 .../groovy/transform/stc/BugsSTCTest.groovy     | 30 ++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/891366ed/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index bd2a44f..ce2f406 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3442,6 +3442,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         if (hasInferredReturnType(trueExpression)) {
             typeOfTrue = trueExpression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
         }
+        // TODO consider moving next two statements "up a level", i.e. have just one more
widely invoked
+        // check but determine no -ve consequences first
+        typeOfFalse = checkForTargetType(falseExpression, typeOfFalse);
+        typeOfTrue = checkForTargetType(trueExpression, typeOfTrue);
         if (isNullConstant(trueExpression) || isNullConstant(falseExpression)) {
             BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
             if (enclosingBinaryExpression != null && enclosingBinaryExpression.getRightExpression()==expression)
{
@@ -3461,7 +3465,37 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         popAssignmentTracking(oldTracker);
     }
 
-    private boolean hasInferredReturnType(Expression expression) {
+    // currently just for empty literals, not for e.g. Collections.emptyList() at present
+    /// it seems attractive to want to do this for more cases but perhaps not all cases
+    private ClassNode checkForTargetType(final Expression expr, final ClassNode type) {
+        if (typeCheckingContext.getEnclosingBinaryExpression() != null && isEmptyCollection(expr))
{
+            int op = typeCheckingContext.getEnclosingBinaryExpression().getOperation().getType();
+            if (isAssignment(op)) {
+                VariableExpression target = (VariableExpression) typeCheckingContext.getEnclosingBinaryExpression().getLeftExpression();
+                return adjustForTargetType(target.getType(), type);
+            }
+        }
+        return type;
+    }
+
+    private static ClassNode adjustForTargetType(final ClassNode targetType, final ClassNode
resultType) {
+        if (targetType.isUsingGenerics() && missesGenericsTypes(resultType)) {
+            // unchecked assignment within ternary/elvis
+            // examples:
+            // List<A> list = existingAs ?: []
+            // in that case, the inferred type of the RHS is the type of the RHS
+            // "completed" with generics type information available in the LHS
+            return GenericsUtils.parameterizeType(targetType, resultType.getPlainNodeReference());
+        }
+        return resultType;
+    }
+
+    private static boolean isEmptyCollection(Expression expr) {
+        return (expr instanceof ListExpression && ((ListExpression) expr).getExpressions().size()
== 0) ||
+        (expr instanceof MapExpression && ((MapExpression) expr).getMapEntryExpressions().size()
== 0);
+    }
+
+    private static boolean hasInferredReturnType(Expression expression) {
         ClassNode type = expression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
         return type != null && !type.getName().equals("java.lang.Object");
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/891366ed/src/test/groovy/transform/stc/BugsSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 07b3ba4..a6578d4 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -744,4 +744,34 @@ Printer
             assert X.makeX().makeY().makeZ().toString() == 'X$Y$Z(3)'
         '''
     }
+
+    // GROOVY-8255
+    void testTargetTypingEmptyCollectionLiterals() {
+        assertScript '''
+            class Foo {
+                List<List<String>> items = [['x']]
+                def bar() {
+                    List<String> result = []
+                    List<String> selections = items.size() ? (items.get(0) ?: []) :
items.size() > 1 ? items.get(1) : []
+                    for (String selection: selections) {
+                        result << selection
+                    }
+                    result
+                }
+            }
+            assert new Foo().bar() == ['x']
+        '''
+        assertScript '''
+            class Foo {
+                def bar() {
+                    def items = [x:1]
+                    Map<String, Integer> empty = [:]
+                    Map<String, Integer> first = items ?: [:]
+                    Map<String, Integer> second = first.isEmpty() ? [:] : [y:2]
+                    [first, second]
+                }
+            }
+            assert new Foo().bar() == [[x:1], [y:2]]
+        '''
+    }
 }


Mime
View raw message