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 <[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 | 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
