-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Usage of `SuggestedFix.Builder#add{,Static}Import` does not always yield
valid code, so this check suggests alternatives instead.- Loading branch information
1 parent
dbd4853
commit c6ecb82
Showing
7 changed files
with
146 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ImportSuggestion.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package tech.picnic.errorprone.bugpatterns; | ||
|
|
||
| import static com.google.errorprone.BugPattern.LinkType.CUSTOM; | ||
| import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; | ||
| import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE; | ||
| import static com.google.errorprone.matchers.Matchers.instanceMethod; | ||
| import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; | ||
|
|
||
| import com.google.auto.service.AutoService; | ||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; | ||
| import com.google.errorprone.fixes.SuggestedFix; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.google.errorprone.matchers.Matcher; | ||
| import com.google.errorprone.util.ASTHelpers; | ||
| import com.sun.source.tree.ExpressionTree; | ||
| import com.sun.source.tree.MethodInvocationTree; | ||
|
|
||
| /** | ||
| * A {@link BugChecker} that flags suggested fixes that involve unconditional imports. | ||
| * | ||
| * <p>Such unconditional imports may clash with other imports of the same identifier. | ||
| */ | ||
| @AutoService(BugChecker.class) | ||
| @BugPattern( | ||
| summary = "Avoid direct invocation of `SuggestedFix#add{,Static}Import`", | ||
| link = BUG_PATTERNS_BASE_URL + "ImportSuggestion", | ||
| linkType = CUSTOM, | ||
| severity = WARNING, | ||
| tags = FRAGILE_CODE) | ||
| public final class ImportSuggestion extends BugChecker implements MethodInvocationTreeMatcher { | ||
| private static final long serialVersionUID = 1L; | ||
| private static final Matcher<ExpressionTree> FIX_BUILDER_METHOD = | ||
| instanceMethod().onDescendantOf(SuggestedFix.Builder.class.getCanonicalName()); | ||
|
|
||
| /** Instantiates a new {@link ImportSuggestion} instance. */ | ||
| public ImportSuggestion() {} | ||
|
|
||
| @Override | ||
| public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { | ||
| if (!FIX_BUILDER_METHOD.matches(tree, state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| switch (ASTHelpers.getSymbol(tree).name.toString()) { | ||
| case "addImport": | ||
| return createDescription( | ||
| tree, "SuggestedFix.Builder#addImport", "SuggestedFixes#qualifyType"); | ||
| case "addStaticImport": | ||
| return createDescription( | ||
| tree, "SuggestedFix.Builder#addStaticImport", "SuggestedFixes#qualifyStaticImport"); | ||
| default: | ||
| return Description.NO_MATCH; | ||
| } | ||
| } | ||
|
|
||
| private Description createDescription( | ||
| MethodInvocationTree tree, String method, String alternative) { | ||
| return buildDescription(tree) | ||
| .setMessage( | ||
| String.format("Prefer `%s` over direct invocation of `%s`", alternative, method)) | ||
| .build(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
...-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImportSuggestionTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package tech.picnic.errorprone.bugpatterns; | ||
|
|
||
| import com.google.errorprone.CompilationTestHelper; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| final class ImportSuggestionTest { | ||
| @Test | ||
| void identification() { | ||
| CompilationTestHelper.newInstance(ImportSuggestion.class, getClass()) | ||
| .expectErrorMessage( | ||
| "IMPORT", | ||
| m -> | ||
| m.contains( | ||
| "Prefer `SuggestedFixes#qualifyType` over direct invocation of `SuggestedFix.Builder#addImport`")) | ||
| .expectErrorMessage( | ||
| "STATIC_IMPORT", | ||
| m -> | ||
| m.contains( | ||
| "Prefer `SuggestedFixes#qualifyStaticImport` over direct invocation of `SuggestedFix.Builder#addStaticImport`")) | ||
| .addSourceLines( | ||
| "A.java", | ||
| "import com.google.errorprone.fixes.SuggestedFix;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " System.out.println(\"foo\");", | ||
| " addImport(\"bar\");", | ||
| " addStaticImport(\"baz\");", | ||
| "", | ||
| " SuggestedFix.Builder builder = SuggestedFix.builder();", | ||
| " // BUG: Diagnostic matches: IMPORT", | ||
| " builder.addImport(\"java.lang.String\");", | ||
| " // BUG: Diagnostic matches: STATIC_IMPORT", | ||
| " builder.addStaticImport(\"java.lang.String.toString\");", | ||
| " builder.build();", | ||
| " }", | ||
| "", | ||
| " private void addImport(String s) {}", | ||
| "", | ||
| " private void addStaticImport(String s) {}", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
| } |