This is an automated email from the ASF dual-hosted git repository.
paulk 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 22955a5 Revert "GROOVY-10294: STC: track assign `null` to variable
within `if` or `else`"
22955a5 is described below
commit 22955a57b4972f32408824d20fc554c9d40e3291
Author: Paul King <[email protected]>
AuthorDate: Sun Oct 31 21:06:57 2021 +1000
Revert "GROOVY-10294: STC: track assign `null` to variable within `if` or
`else`"
This reverts commit c59a02ad15b9b9fd7548860d51fc7f7e643bb12f.
---
.../transform/stc/StaticTypeCheckingVisitor.java | 38 +++++++++++++---------
.../groovy/transform/stc/STCAssignmentTest.groovy | 37 +++------------------
.../transform/stc/TypeInferenceSTCTest.groovy | 30 +++++++++++++++++
3 files changed, 57 insertions(+), 48 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 555d0fe..a63feca 100644
---
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -890,7 +890,8 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
}
// track conditional assignment
- if (leftExpression instanceof VariableExpression
+ if (!isNullConstant(rightExpression)
+ && leftExpression instanceof VariableExpression
&& typeCheckingContext.ifElseForWhileAssignmentTracker
!= null) {
Variable accessedVariable = ((VariableExpression)
leftExpression).getAccessedVariable();
if (accessedVariable instanceof Parameter) {
@@ -1066,10 +1067,14 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
protected ClassNode getOriginalDeclarationType(final Expression lhs) {
if (lhs instanceof VariableExpression) {
Variable var = findTargetVariable((VariableExpression) lhs);
- if (!(var instanceof DynamicVariable || var instanceof
PropertyNode)) {
- return var.getOriginType();
+ if (var instanceof PropertyNode) {
+ // Do NOT trust the type of the property node!
+ return getType(lhs);
}
- } else if (lhs instanceof FieldExpression) {
+ if (var instanceof DynamicVariable) return getType(lhs);
+ return var.getOriginType();
+ }
+ if (lhs instanceof FieldExpression) {
return ((FieldExpression) lhs).getField().getOriginType();
}
return getType(lhs);
@@ -4049,17 +4054,18 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
private void restoreTypeBeforeConditional() {
typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var,
types) -> {
- var.putNodeMetaData(INFERRED_TYPE, types.get(0));
+ ClassNode originType = types.get(0);
+ storeType(var, originType);
});
}
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(t -> t != null && t !=
UNKNOWN_PARAMETER_TYPE) // GROOVY-6099, GROOVY-10294
+ ClassNode type = types.stream().filter(Objects::nonNull) //
GROOVY-6099
.reduce(WideningCategories::lowestUpperBound).get();
- var.putNodeMetaData(INFERRED_TYPE, type);
assignments.put(var, type);
+ storeType(var, type);
});
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
return assignments;
@@ -4309,15 +4315,15 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
if (exp instanceof VariableExpression) {
VariableExpression var = (VariableExpression) exp;
Variable accessedVariable = var.getAccessedVariable();
- 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()) {
+ 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) {
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 de133f5..7b8a01f 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -1038,41 +1038,14 @@ class STCAssignmentTest extends
StaticTypeCheckingTestCase {
// GROOVY-5535
void testAssignToNullInsideIf() {
assertScript '''
- Date test() {
- Date x = new Date()
+ Date foo() {
+ Date result = new Date()
if (true) {
- x = null
+ result = null
}
- x
+ return result
}
- 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
+ assert foo() == null
'''
}
diff --git a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
index 032fe5a..0c596f6 100644
--- a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
@@ -18,6 +18,7 @@
*/
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
@@ -780,6 +781,35 @@ 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