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'