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

emilles 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 c59a02a  GROOVY-10294: STC: track assign `null` to variable within 
`if` or `else`
c59a02a is described below

commit c59a02ad15b9b9fd7548860d51fc7f7e643bb12f
Author: Eric Milles <[email protected]>
AuthorDate: Sat Oct 30 18:49:03 2021 -0500

    GROOVY-10294: STC: track assign `null` to variable within `if` or `else`
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 38 +++++++++-------------
 .../groovy/transform/stc/STCAssignmentTest.groovy  | 37 ++++++++++++++++++---
 .../transform/stc/TypeInferenceSTCTest.groovy      | 30 -----------------
 3 files changed, 48 insertions(+), 57 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 a63feca..555d0fe 100644
--- 
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ 
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -890,8 +890,7 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
                 }
 
                 // track conditional assignment
-                if (!isNullConstant(rightExpression)
-                        && leftExpression instanceof VariableExpression
+                if (leftExpression instanceof VariableExpression
                         && typeCheckingContext.ifElseForWhileAssignmentTracker 
!= null) {
                     Variable accessedVariable = ((VariableExpression) 
leftExpression).getAccessedVariable();
                     if (accessedVariable instanceof Parameter) {
@@ -1067,14 +1066,10 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
     protected ClassNode getOriginalDeclarationType(final Expression lhs) {
         if (lhs instanceof VariableExpression) {
             Variable var = findTargetVariable((VariableExpression) lhs);
-            if (var instanceof PropertyNode) {
-                // Do NOT trust the type of the property node!
-                return getType(lhs);
+            if (!(var instanceof DynamicVariable || var instanceof 
PropertyNode)) {
+                return var.getOriginType();
             }
-            if (var instanceof DynamicVariable) return getType(lhs);
-            return var.getOriginType();
-        }
-        if (lhs instanceof FieldExpression) {
+        } else if (lhs instanceof FieldExpression) {
             return ((FieldExpression) lhs).getField().getOriginType();
         }
         return getType(lhs);
@@ -4054,18 +4049,17 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
 
     private void restoreTypeBeforeConditional() {
         typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, 
types) -> {
-            ClassNode originType = types.get(0);
-            storeType(var, originType);
+            var.putNodeMetaData(INFERRED_TYPE, types.get(0));
         });
     }
 
     protected Map<VariableExpression, ClassNode> popAssignmentTracking(final 
Map<VariableExpression, List<ClassNode>> oldTracker) {
         Map<VariableExpression, ClassNode> assignments = new HashMap<>();
         typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, 
types) -> {
-            ClassNode type = types.stream().filter(Objects::nonNull) // 
GROOVY-6099
+            ClassNode type = types.stream().filter(t -> t != null && t != 
UNKNOWN_PARAMETER_TYPE) // GROOVY-6099, GROOVY-10294
                 .reduce(WideningCategories::lowestUpperBound).get();
+            var.putNodeMetaData(INFERRED_TYPE, type);
             assignments.put(var, type);
-            storeType(var, type);
         });
         typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
         return assignments;
@@ -4315,15 +4309,15 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
         if (exp instanceof VariableExpression) {
             VariableExpression var = (VariableExpression) exp;
             Variable accessedVariable = var.getAccessedVariable();
-            if (accessedVariable != exp && accessedVariable instanceof 
VariableExpression) {
-                storeType((VariableExpression) accessedVariable, cn);
-            }
-            if (accessedVariable instanceof Parameter
-                    || (accessedVariable instanceof PropertyNode
-                        && ((PropertyNode) 
accessedVariable).getField().isSynthetic())) {
-                ((ASTNode) accessedVariable).putNodeMetaData(INFERRED_TYPE, 
cn);
-            }
-            if (var.isClosureSharedVariable() && cn != null) {
+            if (accessedVariable instanceof VariableExpression) {
+                if (accessedVariable != exp)
+                    storeType((VariableExpression) accessedVariable, cn);
+            } else if (accessedVariable instanceof Parameter
+                    || accessedVariable instanceof PropertyNode
+                    && ((PropertyNode) 
accessedVariable).getField().isSynthetic()) {
+                ((AnnotatedNode) 
accessedVariable).putNodeMetaData(INFERRED_TYPE, cn);
+            }
+            if (cn != null && var.isClosureSharedVariable()) {
                 List<ClassNode> assignedTypes = 
typeCheckingContext.closureSharedVariablesAssignmentTypes.computeIfAbsent(var, 
k -> new LinkedList<ClassNode>());
                 assignedTypes.add(cn);
             }
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy 
b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index 7b8a01f..de133f5 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -1038,14 +1038,41 @@ class STCAssignmentTest extends 
StaticTypeCheckingTestCase {
     // GROOVY-5535
     void testAssignToNullInsideIf() {
         assertScript '''
-            Date foo() {
-                Date result = new Date()
+            Date test() {
+                Date x = new Date()
                 if (true) {
-                    result = null
+                    x = null
                 }
-                return result
+                x
             }
-            assert foo() == null
+            assert test() == null
+        '''
+    }
+
+    // GROOVY-10294
+    void testAssignToNullInsideIf2() {
+        assertScript '''
+            CharSequence test() {
+                def x = 'works'
+                if (false) {
+                    x = null
+                }
+                x
+            }
+            assert test() == 'works'
+        '''
+    }
+
+    // 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
         '''
     }
 
diff --git a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy 
b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
index 0c596f6..032fe5a 100644
--- a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
@@ -18,7 +18,6 @@
  */
 package groovy.transform.stc
 
-import groovy.test.NotYetImplemented
 import org.codehaus.groovy.ast.ClassHelper
 import org.codehaus.groovy.ast.ClassNode
 import org.codehaus.groovy.ast.MethodNode
@@ -781,35 +780,6 @@ class TypeInferenceSTCTest extends 
StaticTypeCheckingTestCase {
         '''
     }
 
-    @NotYetImplemented // GROOVY-10294
-    void testFlowTypingWithNullAssignment() {
-        assertScript '''
-            class C {
-            }
-            C test() {
-                def x = new C()
-                if (false) {
-                    x = null
-                }
-                x
-            }
-            assert test() != null
-        '''
-    }
-
-    // GROOVY-10308
-    void testFlowTypingWithNullAssignment2() {
-        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
-        '''
-    }
-
     void testDefTypeAfterLongThenIntAssignments() {
         assertScript '''
             def o

Reply via email to