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