You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/10/31 20:37:20 UTC
[groovy] 01/02: GROOVY-10294: STC: track assign `null` to variable
within `if` or `else`
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
commit e7e6356ac2045dc5100c6a03e87d8d9947774dcf
Author: Eric Milles <er...@thomsonreuters.com>
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 | 43 ++++++++++------------
.../groovy/transform/stc/STCAssignmentTest.groovy | 37 ++++++++++++++++---
.../transform/stc/TypeInferenceSTCTest.groovy | 30 ---------------
3 files changed, 51 insertions(+), 59 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..e132713 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,18 @@ 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
- .reduce(WideningCategories::lowestUpperBound).get();
- assignments.put(var, type);
- storeType(var, type);
+ types.stream().filter(t -> t != null && t != UNKNOWN_PARAMETER_TYPE) // GROOVY-6099, GROOVY-10294
+ .reduce(WideningCategories::lowestUpperBound).ifPresent(type -> {
+ assignments.put(var, type);
+ storeType(var, type);
+ });
});
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
return assignments;
@@ -4315,15 +4310,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