From 679f59745379b73d8cd06d6b056f6492a83f5a7b Mon Sep 17 00:00:00 2001 From: ccernat Date: Fri, 6 Jan 2023 14:25:14 +0100 Subject: [PATCH] Avoid flagging variables and same file classes --- .../bugpatterns/BadStaticImport.java | 29 ++++++++++++++++--- .../bugpatterns/BadStaticImportTest.java | 13 +++++++-- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/BadStaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/BadStaticImport.java index 53d0cf69357..eea559f3db4 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/BadStaticImport.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/BadStaticImport.java @@ -3,6 +3,7 @@ import static com.google.errorprone.BugPattern.LinkType.CUSTOM; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static com.sun.tools.javac.code.Kinds.Kind.TYP; import static tech.picnic.errorprone.bugpatterns.StaticImport.STATIC_IMPORT_CANDIDATE_MEMBERS; import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; @@ -18,8 +19,11 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.ImportTree; import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol; +import tech.picnic.errorprone.bugpatterns.util.SourceCode; /** A {@link BugChecker} that flags methods and constants that should not be statically imported. */ // XXX: Also introduce checks that disallows the following candidates: @@ -119,7 +123,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState @Override public Description matchIdentifier(IdentifierTree tree, VisitorState state) { - if (isMatch(tree)) { + if (isMatch(tree, state)) { return getDescription(tree, state); } @@ -142,19 +146,23 @@ private static String getImportToRemove(Symbol symbol) { ".", symbol.getEnclosingElement().getQualifiedName(), symbol.getSimpleName()); } - private static boolean isMatch(IdentifierTree tree) { + private static boolean isMatch(IdentifierTree tree, VisitorState state) { Symbol symbol = ASTHelpers.getSymbol(tree); if (symbol == null) { return false; } Symbol enclosingSymbol = symbol.getEnclosingElement(); - if (enclosingSymbol == null) { + if (enclosingSymbol == null || enclosingSymbol.kind != TYP) { return false; } - String qualifiedTypeName = enclosingSymbol.getQualifiedName().toString(); String identifierName = symbol.getSimpleName().toString(); + if (!isIdentifierStaticallyImported(identifierName, state)) { + return false; + } + + String qualifiedTypeName = enclosingSymbol.getQualifiedName().toString(); return !isExempted(qualifiedTypeName, identifierName) && isCandidate(qualifiedTypeName, identifierName); } @@ -168,4 +176,17 @@ private static boolean isCandidate(String qualifiedTypeName, String identifierNa private static boolean isExempted(String qualifiedTypeName, String identifierName) { return STATIC_IMPORT_CANDIDATE_MEMBERS.containsEntry(qualifiedTypeName, identifierName); } + + private static boolean isIdentifierStaticallyImported(String identifierName, VisitorState state) { + return state.getPath().getCompilationUnit().getImports().stream() + .filter(ImportTree::isStatic) + .map(ImportTree::getQualifiedIdentifier) + .map(tree -> getStaticImportIdentifier(tree, state)) + .anyMatch(identifierName::contentEquals); + } + + private static CharSequence getStaticImportIdentifier(Tree tree, VisitorState state) { + String source = SourceCode.treeToString(tree, state); + return source.subSequence(source.lastIndexOf('.') + 1, source.length()); + } } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/BadStaticImportTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/BadStaticImportTest.java index 9ebee082754..45693ece71d 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/BadStaticImportTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/BadStaticImportTest.java @@ -40,7 +40,7 @@ void badIdentifiersDontClashWithStaticImportCandidates() { } @Test - void identifySimpleMethodInvocation() { + void identification() { compilationTestHelper .addSourceLines( "A.java", @@ -81,13 +81,22 @@ void identifySimpleMethodInvocation() { "", " // BUG: Diagnostic contains:", " ImmutableSet.of(min(ImmutableSet.of()));", + "", + " Object builder = null;", + " // Not flagged because identifier is variable name", + " Object lBuilder = ImmutableList.of(builder);", + "", + " // Not flagged because method is not statically imported", + " create();", " }", + "", + " void create() {}", "}") .doTest(); } @Test - void replaceSimpleMethodInvocation() { + void replacement() { refactoringTestHelper .addInputLines( "A.java",