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

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


The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
     new 0b40f8a  GROOVY-9786: STC: propagate assignments to outer if-else for 
if-else-if
0b40f8a is described below

commit 0b40f8ade841e727da505285df42ed51e57a61d5
Author: Eric Milles <[email protected]>
AuthorDate: Thu Oct 22 11:32:04 2020 -0500

    GROOVY-9786: STC: propagate assignments to outer if-else for if-else-if
    
    Conflicts:
        
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 55 +++++++++++++---------
 .../groovy/transform/stc/STCAssignmentTest.groovy  | 44 +++++++++++++++--
 2 files changed, 73 insertions(+), 26 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 0d9d4dc..87b7af9 100644
--- 
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ 
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -896,28 +896,22 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
                     resultType = originType;
                 }
 
-                // if we are in an if/else branch, keep track of assignment
-                if (typeCheckingContext.ifElseForWhileAssignmentTracker != 
null && leftExpression instanceof VariableExpression
-                        && !isNullConstant(rightExpression)) {
+                // track conditional assignment
+                if (!isNullConstant(rightExpression)
+                        && leftExpression instanceof VariableExpression
+                        && typeCheckingContext.ifElseForWhileAssignmentTracker 
!= null) {
                     Variable accessedVariable = ((VariableExpression) 
leftExpression).getAccessedVariable();
                     if (accessedVariable instanceof Parameter) {
                         accessedVariable = new 
ParameterVariableExpression((Parameter) accessedVariable);
                     }
                     if (accessedVariable instanceof VariableExpression) {
-                        VariableExpression var = (VariableExpression) 
accessedVariable;
-                        List<ClassNode> types = 
typeCheckingContext.ifElseForWhileAssignmentTracker.get(var);
-                        if (types == null) {
-                            types = new LinkedList<ClassNode>();
-                            ClassNode type = 
var.getNodeMetaData(INFERRED_TYPE);
-                            types.add(type);
-                            
typeCheckingContext.ifElseForWhileAssignmentTracker.put(var, types);
-                        }
-                        types.add(resultType);
+                        recordAssignment((VariableExpression) 
accessedVariable, resultType);
                     }
                 }
+
                 storeType(leftExpression, resultType);
 
-                // if right expression is a ClosureExpression, store parameter 
type information
+                // propagate closure parameter type information
                 if (leftExpression instanceof VariableExpression) {
                     if (rightExpression instanceof ClosureExpression) {
                         Parameter[] parameters = ((ClosureExpression) 
rightExpression).getParameters();
@@ -3684,7 +3678,6 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
     @Override
     public void visitIfElse(final IfStatement ifElse) {
         Map<VariableExpression, List<ClassNode>> oldTracker = 
pushAssignmentTracking();
-
         try {
             // create a new temporary element in the if-then-else type info
             typeCheckingContext.pushTemporaryTypeInfo();
@@ -3706,19 +3699,29 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
             } else {
                 elseBlock.visit(this);
             }
+
+            // GROOVY-9786: if chaining: "if (...) x=?; else if (...) x=?;"
+            Map<VariableExpression, ClassNode> updates = 
elseBlock.getNodeMetaData("assignments");
+            if (updates != null) {
+                for (Map.Entry<VariableExpression, ClassNode> entry : 
updates.entrySet()) {
+                    recordAssignment(entry.getKey(), entry.getValue());
+                }
+            }
         } finally {
-            popAssignmentTracking(oldTracker);
+            ifElse.putNodeMetaData("assignments", 
popAssignmentTracking(oldTracker));
         }
-        BinaryExpression instanceOfExpression = 
findInstanceOfNotReturnExpression(ifElse);
-        if (instanceOfExpression == null) {
-        } else {
-            if (typeCheckingContext.enclosingBlocks.size() > 0) {
+
+        if (!typeCheckingContext.enclosingBlocks.isEmpty()) {
+            BinaryExpression instanceOfExpression = 
findInstanceOfNotReturnExpression(ifElse);
+            if (instanceOfExpression == null) {
+                instanceOfExpression = 
findInstanceOfNotReturnExpression(ifElse);
+            }
+            if (instanceOfExpression != null) {
                 visitInstanceofNot(instanceOfExpression);
             }
         }
     }
 
-
     public void visitInstanceofNot(BinaryExpression be) {
         final BlockStatement currentBlock = 
typeCheckingContext.enclosingBlocks.getFirst();
         assert currentBlock != null;
@@ -3733,7 +3736,6 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
         pushInstanceOfTypeInfo(be.getLeftExpression(), 
be.getRightExpression());
     }
 
-
     @Override
     public void visitBlockStatement(BlockStatement block) {
         if (block != null) {
@@ -3816,6 +3818,17 @@ public class StaticTypeCheckingVisitor extends 
ClassCodeVisitorSupport {
         restoreTypeBeforeConditional();
     }
 
+    private void recordAssignment(final VariableExpression lhsExpr, final 
ClassNode rhsType) {
+        List<ClassNode> types = 
typeCheckingContext.ifElseForWhileAssignmentTracker.get(lhsExpr);
+        if (types == null) {
+            types = new LinkedList<>();
+            ClassNode lhsType = lhsExpr.getNodeMetaData(INFERRED_TYPE);
+            types.add(lhsType);
+            typeCheckingContext.ifElseForWhileAssignmentTracker.put(lhsExpr, 
types);
+        }
+        types.add(rhsType);
+    }
+
     private void restoreTypeBeforeConditional() {
         Set<Map.Entry<VariableExpression, List<ClassNode>>> entries = 
typeCheckingContext.ifElseForWhileAssignmentTracker.entrySet();
         for (Map.Entry<VariableExpression, List<ClassNode>> entry : entries) {
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy 
b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index 645a6de..7d63cb6 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -486,18 +486,52 @@ class STCAssignmentTest extends 
StaticTypeCheckingTestCase {
 
     void testIfWithCommonInterface() {
         assertScript '''
-            interface Foo { void foo() }
-            class A implements Foo { void foo() { println 'A' } }
-            class B implements Foo { void foo() { println 'B' } }
+            interface I {
+                def foo()
+            }
+            class A implements I {
+                def foo() { 'A' }
+            }
+            class B implements I {
+                def foo() { 'B' }
+            }
+
             def x = new A()
-            def y = 'foo'
+            def y = true
             if (y) {
                 x = new B()
             }
-            x.foo()
+            assert x.foo() == 'B'
         '''
     }
 
+    // GROOVY-9786
+    void testIfElseIfWithCommonInterface() {
+        ['I', 'def', 'Object'].each {
+            assertScript """
+                interface I {
+                    def foo()
+                }
+                class A implements I {
+                    def foo() { 'A' }
+                }
+                class B implements I {
+                    def foo() { 'B' }
+                }
+
+                $it x
+                def y = false
+                def z = true
+                if (y) {
+                    x = new A()
+                } else if (z) {
+                    x = new B()
+                }
+                assert x.foo() == 'B'
+            """
+        }
+    }
+
     void testForLoopWithNewAssignment() {
         shouldFailWithMessages '''
             def x = '123'

Reply via email to