From 855940b55c51f5eb17bb9d33b86fe73be56b3d9c Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Sun, 15 May 2022 13:27:39 -0500 Subject: [PATCH] GROOVY-10356, GROOVY-10623: STC: inferred variable type following `null` --- src/main/groovy/groovy/grape/GrapeIvy.groovy | 6 +-- .../stc/StaticTypeCheckingVisitor.java | 21 ++++---- .../transform/stc/ClosuresSTCTest.groovy | 6 +-- .../transform/stc/STCAssignmentTest.groovy | 35 +++++++++---- .../groovy/macro/matcher/ASTMatcher.groovy | 52 +++++++++---------- 5 files changed, 65 insertions(+), 55 deletions(-) diff --git a/src/main/groovy/groovy/grape/GrapeIvy.groovy b/src/main/groovy/groovy/grape/GrapeIvy.groovy index 4afe5254721..bfc00258f17 100644 --- a/src/main/groovy/groovy/grape/GrapeIvy.groovy +++ b/src/main/groovy/groovy/grape/GrapeIvy.groovy @@ -457,11 +457,11 @@ class GrapeIvy implements GrapeEngine { if (report.getDownloadSize() && reportDownloads) { System.err.println("Downloaded ${report.getDownloadSize() >> 10} Kbytes in ${report.getDownloadTime()}ms:\n ${report.getAllArtifactsReports()*.toString().join('\n ')}") } - md = report.getModuleDescriptor() if (!args.preserveFiles) { - cacheManager.getResolvedIvyFileInCache(md.getModuleRevisionId()).delete() - cacheManager.getResolvedIvyPropertiesInCache(md.getModuleRevisionId()).delete() + def revision = report.getModuleDescriptor().getModuleRevisionId() + cacheManager.getResolvedIvyPropertiesInCache(revision).delete() + cacheManager.getResolvedIvyFileInCache(revision).delete() } report 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 aa977727626..cec013a9fc2 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -1073,15 +1073,13 @@ private static boolean isCompoundAssignment(final Expression exp) { } 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(); - } - } else if (lhs instanceof FieldExpression) { - return ((FieldExpression) lhs).getField().getOriginType(); + Variable var = null; + if (lhs instanceof FieldExpression) { + var = ((FieldExpression) lhs).getField(); + } else if (lhs instanceof VariableExpression) { + var = findTargetVariable((VariableExpression) lhs); } - return getType(lhs); + return var != null && !(var instanceof DynamicVariable) ? var.getOriginType() : getType(lhs); } protected void inferDiamondType(final ConstructorCallExpression cce, final ClassNode lType) { @@ -4300,11 +4298,12 @@ public void visitTryCatchFinally(final TryCatchStatement statement) { } protected void storeType(final Expression exp, ClassNode cn) { - if (cn == UNKNOWN_PARAMETER_TYPE) { - // this can happen for example when "null" is used in an assignment or a method parameter. - // In that case, instead of storing the virtual type, we must "reset" type information + if (cn == UNKNOWN_PARAMETER_TYPE) { // null for assignment or parameter + // instead of storing an "unknown" type, reset the type information // by determining the declaration type of the expression cn = getOriginalDeclarationType(exp); + // GROOVY-10356, GROOVY-10623: "def" + if (isDynamicTyped(cn)) return; } if (cn != null && isPrimitiveType(cn)) { if (exp instanceof VariableExpression && ((VariableExpression) exp).isClosureSharedVariable()) { diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy index d922bb572ac..b233142aa3e 100644 --- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy +++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy @@ -18,8 +18,6 @@ */ package groovy.transform.stc -import groovy.test.NotYetImplemented - /** * Unit tests for static type checking : closures. */ @@ -375,7 +373,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase { 'Cannot find matching method A#m()' } - @NotYetImplemented // GROOVY-10356 + // GROOVY-10356 void testClosureSharedVariable4() { assertScript ''' interface A { @@ -385,7 +383,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase { def x = { -> a = null } - a?.m() // A closure shared variable [a] has been assigned with various types and ... + a?.m() ''' } diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy index 2af80307e82..9064e5e225a 100644 --- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy +++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy @@ -1062,18 +1062,22 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase { ''', 'Inconvertible types: cannot cast java.lang.String[] to java.util.Set[]' } - // GROOVY-5535 + // GROOVY-5535, GROOVY-10623 void testAssignToNullInsideIf() { - assertScript ''' - Date test() { - Date x = new Date() - if (true) { - x = null + ['Date', 'def', 'var'].each { + assertScript """ + Date test() { + $it x = new Date() + if (true) { + x = null + Date y = x + } + Date z = x + return x } - x - } - assert test() == null - ''' + assert test() == null + """ + } } // GROOVY-10294 @@ -1103,6 +1107,17 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase { ''' } + // GROOVY-10623 + void testAssignToNullAfterInit() { + assertScript ''' + class C { + } + def x = new C() + x = null + C c = x + ''' + } + // GROOVY-5798 void testShouldNotThrowConversionError() { assertScript ''' diff --git a/subprojects/groovy-macro/src/main/groovy/org/codehaus/groovy/macro/matcher/ASTMatcher.groovy b/subprojects/groovy-macro/src/main/groovy/org/codehaus/groovy/macro/matcher/ASTMatcher.groovy index 857c71212d6..c2bd6a94802 100644 --- a/subprojects/groovy-macro/src/main/groovy/org/codehaus/groovy/macro/matcher/ASTMatcher.groovy +++ b/subprojects/groovy-macro/src/main/groovy/org/codehaus/groovy/macro/matcher/ASTMatcher.groovy @@ -75,17 +75,16 @@ import org.codehaus.groovy.macro.matcher.internal.MatchingConstraintsBuilder @AutoFinal @CompileStatic class ASTMatcher extends ContextualClassCodeVisitor { - public static final String WILDCARD = "_" + public static final String WILDCARD = '_' - private Object current = null private boolean match = true + private Object current private ASTMatcher() { } @Override protected SourceUnit getSourceUnit() { - return null } /** @@ -230,7 +229,7 @@ class ASTMatcher extends ContextualClassCodeVisitor { def nodeProps = node.properties def curProps = cur.properties if (nodeProps.size() == curProps.size()) { - def iter = curProps.iterator() + Iterator iter = curProps.iterator() // now let's visit the contents of the class for (PropertyNode pn : nodeProps) { doWithNode(pn, iter.next()) { @@ -307,8 +306,7 @@ class ASTMatcher extends ContextualClassCodeVisitor { void visitImports(ModuleNode node) { if (node) { doWithNode(node, current) { - ModuleNode module = (ModuleNode) current - def imports = module.imports + def imports = ((ModuleNode) current).imports if (imports.size() == node.imports.size()) { def iter = imports.iterator() for (ImportNode importNode : node.imports) { @@ -321,38 +319,38 @@ class ASTMatcher extends ContextualClassCodeVisitor { failIfNot(false) return } - imports = module.starImports - if (imports.size() == node.starImports.size()) { - def iter = imports.iterator() - for (ImportNode importNode : node.starImports) { - doWithNode(importNode, iter.next()) { - visitAnnotations(importNode) - importNode.visit(this) + def starImports = ((ModuleNode) current).starImports + if (starImports.size() == node.starImports.size()) { + def iter = starImports.iterator() + for (starImport in node.starImports) { + doWithNode(starImport, iter.next()) { + visitAnnotations(starImport) + starImport.visit(this) } } } else { failIfNot(false) return } - imports = module.staticImports - if (imports.size() == node.staticImports.size()) { - def iter = imports.values().iterator() - for (ImportNode importNode : node.staticImports.values()) { - doWithNode(importNode, iter.next()) { - visitAnnotations(importNode) - importNode.visit(this) + def staticImports = ((ModuleNode) current).staticImports + if (staticImports.size() == node.staticImports.size()) { + def iter = staticImports.values().iterator() + for (staticImport in node.staticImports.values()) { + doWithNode(staticImport, iter.next()) { + visitAnnotations(staticImport) + staticImport.visit(this) } } } else { failIfNot(false) } - imports = module.staticStarImports - if (imports.size() == node.staticStarImports.size()) { - def iter = imports.values().iterator() - for (ImportNode importNode : node.staticStarImports.values()) { - doWithNode(importNode, iter.next()) { - visitAnnotations(importNode) - importNode.visit(this) + def staticStarImports = ((ModuleNode) current).staticStarImports + if (staticStarImports.size() == node.staticStarImports.size()) { + def iter = staticStarImports.values().iterator() + for (staticStarImport in node.staticStarImports.values()) { + doWithNode(staticStarImport, iter.next()) { + visitAnnotations(staticStarImport) + staticStarImport.visit(this) } } } else {