GROOVY-10308: STC: variable flow type: do not use INFERRED_RETURN_TYPE

3_0_X backport

Author: eric-milles

src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

@@ -337,7 +337,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     protected static final ClassNode ITERABLE_TYPE = ClassHelper.make(Iterable.class);
     private   static final ClassNode SET_TYPE = ClassHelper.make(Set.class);
 
-    private static final List<ClassNode> TUPLE_TYPES = Arrays.stream(ClassHelper.TUPLE_CLASSES).map(ClassHelper::makeWithoutCaching).collect(Collectors.toList());
+    private static final List<ClassNode> TUPLE_TYPES = Arrays.stream(TUPLE_CLASSES).map(ClassHelper::makeWithoutCaching).collect(Collectors.toList());
 
     public static final Statement GENERATED_EMPTY_STATEMENT = EmptyStatement.INSTANCE;
 
@@ -655,7 +655,7 @@ public void visitVariableExpression(final VariableExpression vexp) {
             inferredType = getInferredTypeFromTempInfo(localVariable, inferredType);
             if (inferredType != null && !inferredType.equals(OBJECT_TYPE)
                     && !inferredType.equals(accessedVariable.getOriginType())) {
-                vexp.putNodeMetaData(INFERRED_RETURN_TYPE, inferredType);
+                vexp.putNodeMetaData(INFERRED_TYPE, inferredType);
             }
         }
     }
@@ -764,11 +764,7 @@ public void visitBinaryExpression(final BinaryExpression expression) {
                 lType = getType(leftExpression);
             } else {
                 if (op != EQUAL && op != ELVIS_EQUAL) {
-                    if (leftExpression instanceof VariableExpression && hasInferredReturnType(leftExpression)) {
-                        lType = getInferredReturnType(leftExpression); // GROOVY-10217
-                    } else {
-                        lType = getType(leftExpression);
-                    }
+                    lType = getType(leftExpression);
                 } else {
                     lType = getOriginalDeclarationType(leftExpression);
 
@@ -1335,23 +1331,17 @@ protected void typeCheckAssignment(final BinaryExpression assignmentExpression,
         // TODO: need errors for write-only too!
         if (addedReadOnlyPropertyError(leftExpression)) return;
 
-        ClassNode rTypeInferred; // see if any instanceof exists
-        if (rightExpression instanceof VariableExpression && hasInferredReturnType(rightExpression) && assignmentExpression.getOperation().getType() == EQUAL) {
-            rTypeInferred = getInferredReturnType(rightExpression);
-        } else {
-            rTypeInferred = rightExpressionType;
-        }
-        ClassNode rTypeAdjusted = adjustTypeForSpreading(rTypeInferred, leftExpression);
+        ClassNode rTypeAdjusted = adjustTypeForSpreading(rightExpressionType, leftExpression);
 
         if (!checkCompatibleAssignmentTypes(leftExpressionType, rTypeAdjusted, rightExpression)) {
             if (!extension.handleIncompatibleAssignment(leftExpressionType, rTypeAdjusted, assignmentExpression)) {
-                addAssignmentError(leftExpressionType, rTypeInferred, rightExpression);
+                addAssignmentError(leftExpressionType, rightExpressionType, rightExpression);
             }
         } else {
             ClassNode lTypeRedirect = leftExpressionType.redirect();
             addPrecisionErrors(lTypeRedirect, leftExpressionType, rTypeAdjusted, rightExpression);
             if (rightExpression instanceof ListExpression) {
-                addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rTypeInferred, rightExpression, assignmentExpression);
+                addListAssignmentConstructorErrors(lTypeRedirect, leftExpressionType, rightExpressionType, rightExpression, assignmentExpression);
             } else if (rightExpression instanceof MapExpression) {
                 addMapAssignmentConstructorErrors(lTypeRedirect, leftExpression, (MapExpression)rightExpression);
             }
@@ -1999,12 +1989,7 @@ public void visitForLoop(final ForStatement forLoop) {
         } else {
             visitStatement(forLoop);
             collectionExpression.visit(this);
-            ClassNode collectionType;
-            if (collectionExpression instanceof VariableExpression && hasInferredReturnType(collectionExpression)) {
-                collectionType = getInferredReturnType(collectionExpression);
-            } else {
-                collectionType = getType(collectionExpression);
-            }
+            ClassNode collectionType = getType(collectionExpression);
             ClassNode forLoopVariableType = forLoop.getVariableType();
             ClassNode componentType;
             if (Character_TYPE.equals(getWrapper(forLoopVariableType)) && STRING_TYPE.equals(collectionType)) {
@@ -2271,18 +2256,12 @@ private ClassNode infer(final ClassNode target, final ClassNode source) {
 
     protected ClassNode checkReturnType(final ReturnStatement statement) {
         Expression expression = statement.getExpression();
-        ClassNode type;
-        if (expression instanceof VariableExpression && hasInferredReturnType(expression)) {
-            type = getInferredReturnType(expression);
-        } else {
-            type = getType(expression);
-        }
+        ClassNode type = getType(expression);
         if (typeCheckingContext.getEnclosingClosure() != null) {
             ClassNode inferredReturnType = getInferredReturnType(typeCheckingContext.getEnclosingClosure().getClosureExpression());
             // GROOVY-9995: return ctor call with diamond operator
             if (expression instanceof ConstructorCallExpression) {
-                ClassNode inferredClosureReturnType = getInferredReturnType(typeCheckingContext.getEnclosingClosure().getClosureExpression());
-                if (inferredClosureReturnType != null) inferDiamondType((ConstructorCallExpression) expression, inferredClosureReturnType);
+                inferDiamondType((ConstructorCallExpression) expression, inferredReturnType != null ? inferredReturnType : DYNAMIC_TYPE); // GROOVY-10080
             }
             if (inferredReturnType != null && inferredReturnType.equals(STRING_TYPE) && isGStringOrGStringStringLUB(type)) {
                 type = STRING_TYPE; // GROOVY-9971: convert GString to String at point of return
@@ -2419,7 +2398,7 @@ protected MethodNode typeCheckMapConstructor(final ConstructorCallExpression cal
 
     protected ClassNode[] getArgumentTypes(final ArgumentListExpression args) {
         return args.getExpressions().stream().map(exp ->
-            isNullConstant(exp) ? UNKNOWN_PARAMETER_TYPE : getInferredTypeFromTempInfo(exp, getType(exp))
+            isNullConstant(exp) ? UNKNOWN_PARAMETER_TYPE : getType(exp)
         ).toArray(ClassNode[]::new);
     }
 
@@ -3848,11 +3827,7 @@ protected ClassNode getInferredReturnTypeFromWithClosureArgument(final Expressio
 
         visitClosureExpression(closure);
 
-        if (getInferredReturnType(closure) != null) {
-            return getInferredReturnType(closure);
-        }
-
-        return null;
+        return getInferredReturnType(closure);
     }
 
     /**
@@ -4328,7 +4303,6 @@ && isOrImplements(typeOfFalse, typeOfTrue))) { // List/Collection/Iterable : []
      * @param type the inferred type of {@code expr}
      */
     private ClassNode checkForTargetType(final Expression expr, final ClassNode type) {
-        ClassNode sourceType = type;
         ClassNode targetType = null;
         MethodNode enclosingMethod = typeCheckingContext.getEnclosingMethod();
         MethodCall enclosingMethodCall = (MethodCall)typeCheckingContext.getEnclosingMethodCall();
@@ -4348,27 +4322,15 @@ && isTypeSource(expr, enclosingMethod)) {
              targetType = enclosingMethod.getReturnType();
         }
 
-        if (expr instanceof VariableExpression && hasInferredReturnType(expr)) {
-            sourceType = getInferredReturnType(expr);
-        }
+        if (targetType == null)
+            targetType = type.getPlainNodeReference();
+        if (type == UNKNOWN_PARAMETER_TYPE) return targetType;
 
-        if (expr instanceof ConstructorCallExpression) { // GROOVY-9972, GROOVY-9983
-            // GROOVY-10114: type parameter(s) could be inferred from call arguments
-            if (targetType == null) targetType = sourceType.getPlainNodeReference();
+        if (expr instanceof ConstructorCallExpression) { // GROOVY-9972, GROOVY-9983, GROOVY-10114
             inferDiamondType((ConstructorCallExpression) expr, targetType);
-            return sourceType;
-        }
-
-        if (targetType == null) return sourceType;
-
-        if (!isPrimitiveType(getUnwrapper(targetType)) && !OBJECT_TYPE.equals(targetType)
-                && !sourceType.isGenericsPlaceHolder() && missesGenericsTypes(sourceType)) {
-            // unchecked assignment with ternary/elvis, like "List<T> list = listOfT ?: []"
-            // the inferred type is the RHS type "completed" with generics information from LHS
-            return GenericsUtils.parameterizeType(targetType, sourceType.getPlainNodeReference());
         }
 
-        return sourceType != UNKNOWN_PARAMETER_TYPE ? sourceType : targetType;
+        return adjustForTargetType(type, targetType); // GROOVY-10688
     }
 
     private static ClassNode adjustForTargetType(final ClassNode resultType, final ClassNode targetType) {
@@ -4454,11 +4416,6 @@ private static boolean isEmptyCollection(final Expression expr) {
                 || (expr instanceof MapExpression && ((MapExpression) expr).getMapEntryExpressions().isEmpty());
     }
 
-    private static boolean hasInferredReturnType(final Expression expression) {
-        ClassNode type = expression.getNodeMetaData(INFERRED_RETURN_TYPE);
-        return type != null && !type.getName().equals(ClassHelper.OBJECT);
-    }
-
     @Override
     public void visitTryCatchFinally(final TryCatchStatement statement) {
         List<CatchStatement> catchStatements = statement.getCatchStatements();
@@ -4778,8 +4735,7 @@ protected static ClassNode getGroupOperationResultType(final ClassNode a, final
     protected ClassNode inferComponentType(final ClassNode containerType, final ClassNode indexType) {
         ClassNode componentType = containerType.getComponentType();
         if (componentType == null) {
-            // GROOVY-5521
-            // try to identify a getAt method
+            // GROOVY-5521: check for "getAt" method
             typeCheckingContext.pushErrorCollector();
             MethodCallExpression vcall = callX(localVarX("_hash_", containerType), "getAt", varX("_index_", indexType));
             vcall.setImplicitThis(false); // GROOVY-8943
@@ -4788,10 +4744,9 @@ protected ClassNode inferComponentType(final ClassNode containerType, final Clas
             } finally {
                 typeCheckingContext.popErrorCollector();
             }
-            return getType(vcall);
-        } else {
-            return componentType;
+            componentType = getType(vcall);
         }
+        return componentType;
     }
 
     protected MethodNode findMethodOrFail(final Expression expr, final ClassNode receiver, final String name, final ClassNode... args) {
@@ -5312,14 +5267,14 @@ protected ClassNode storeInferredReturnType(final ASTNode node, final ClassNode
     }
 
     /**
-     * Returns the inferred return type of a closure or a method, if stored on the AST node. This method
-     * doesn't perform any type inference by itself.
+     * Returns the inferred return type of a closure or method, if stored on the
+     * AST node. This method doesn't perform any type inference by itself.
      *
-     * @param exp a {@link ClosureExpression} or {@link MethodNode}
-     * @return the inferred type, as stored on node metadata.
+     * @param node a {@link ClosureExpression} or {@link MethodNode}
+     * @return the expected return type
      */
-    protected ClassNode getInferredReturnType(final ASTNode exp) {
-        return exp.getNodeMetaData(INFERRED_RETURN_TYPE);
+    protected ClassNode getInferredReturnType(final ASTNode node) {
+        return node.getNodeMetaData(INFERRED_RETURN_TYPE);
     }
 
     protected ClassNode inferListExpressionType(final ListExpression list) {

src/test/groovy/transform/stc/GenericsSTCTest.groovy

@@ -1229,7 +1229,6 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
     }
 
     // GROOVY-10080
-    @NotYetImplemented
     void testDiamondInferenceFromConstructor11() {
         assertScript '''
             @groovy.transform.TupleConstructor(defaults=false)

src/test/groovy/transform/stc/STCAssignmentTest.groovy

@@ -1037,6 +1037,19 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-10308
+    void testAssignToNullAfterCall() {
+        assertScript '''
+            class C<T> {
+                T p
+            }
+            def x = { -> new C<String>() }
+            def y = x()
+            def z = y.p // false positive: field access error
+            y = null
+        '''
+    }
+
     // GROOVY-5798
     void testShouldNotThrowConversionError() {
         assertScript '''

src/test/groovy/transform/stc/TernaryOperatorSTCTest.groovy

@@ -257,7 +257,6 @@ class TernaryOperatorSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
-    @NotYetImplemented
     void testCommonInterface3() {
         assertScript '''import static java.util.concurrent.ConcurrentHashMap.*
             Set<Integer> integers = false ? new HashSet<>() : newKeySet()

src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7333Bug.groovy src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7333.groovy

@@ -21,7 +21,7 @@ package org.codehaus.groovy.classgen.asm.sc.bugs
 import groovy.transform.stc.StaticTypeCheckingTestCase
 import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
 
-final class Groovy7333Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+final class Groovy7333 extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
 
     // GROOVY-7333
     void testIncorrectInstanceOfInference1() {
@@ -49,7 +49,7 @@ final class Groovy7333Bug extends StaticTypeCheckingTestCase implements StaticCo
             void test(A a) {
                 if (a instanceof B) {
                     @ASTTest(phase=INSTRUCTION_SELECTION, value={
-                        def type = node.rightExpression.getNodeMetaData(INFERRED_RETURN_TYPE)
+                        def type = node.rightExpression.getNodeMetaData(INFERRED_TYPE)
                         assert type.text == 'B' // not '<UnionType:A+B>'
                     })
                     def x = a