forked from PicnicSupermarket/error-prone-support
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Introduce
ImplicitBlockingFluxOperation check (PicnicSupermarket#468)
- Loading branch information
Showing
2 changed files
with
256 additions
and
0 deletions.
There are no files selected for viewing
115 changes: 115 additions & 0 deletions
115
...ntrib/src/main/java/tech/picnic/errorprone/bugpatterns/ImplicitBlockingFluxOperation.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,115 @@ | ||
| package tech.picnic.errorprone.bugpatterns; | ||
|
|
||
| import static com.google.errorprone.BugPattern.LinkType.CUSTOM; | ||
| import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; | ||
| import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR; | ||
| import static com.google.errorprone.matchers.Matchers.anyOf; | ||
| import static com.google.errorprone.matchers.method.MethodMatchers.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.fixes.SuggestedFixes; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.google.errorprone.matchers.Matcher; | ||
| import com.google.errorprone.matchers.method.MethodMatchers; | ||
| import com.google.errorprone.suppliers.Supplier; | ||
| import com.google.errorprone.suppliers.Suppliers; | ||
| import com.sun.source.tree.ExpressionTree; | ||
| import com.sun.source.tree.MethodInvocationTree; | ||
| import com.sun.tools.javac.code.Type; | ||
| import tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary; | ||
|
|
||
| @AutoService(BugChecker.class) | ||
| @BugPattern( | ||
| summary = | ||
| "`Flux#toStream` and `Flux#toIterable` are documented to block, but this is not apparent from the method signature; please make sure that they are used with this in mind.", | ||
| link = BUG_PATTERNS_BASE_URL + "ImplicitBlockingFluxOperation", | ||
| linkType = CUSTOM, | ||
| severity = ERROR, | ||
| tags = LIKELY_ERROR) | ||
| public class ImplicitBlockingFluxOperation extends BugChecker | ||
| implements MethodInvocationTreeMatcher { | ||
| private static final long serialVersionUID = 1L; | ||
| private static final Supplier<Type> FLUX = | ||
| Suppliers.typeFromString("reactor.core.publisher.Flux"); | ||
| public static final MethodMatchers.MethodClassMatcher FLUX_METHOD = | ||
| instanceMethod().onDescendantOf(FLUX); | ||
| private static final Matcher<ExpressionTree> FLUX_TO_ITERABLE = | ||
| FLUX_METHOD.named("toIterable").withNoParameters(); | ||
| private static final Matcher<ExpressionTree> FLUX_TO_STREAM = | ||
| FLUX_METHOD.named("toStream").withNoParameters(); | ||
| private static final Matcher<ExpressionTree> FLUX_IMPLICIT_BLOCKING_METHOD = | ||
| anyOf(FLUX_TO_ITERABLE, FLUX_TO_STREAM); | ||
|
|
||
| @Override | ||
| public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { | ||
| if (!FLUX_IMPLICIT_BLOCKING_METHOD.matches(tree, state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| Description.Builder description = buildDescription(tree); | ||
|
|
||
| description.addFix(getSuppressWarningsFix(state)); | ||
| if (ThirdPartyLibrary.GUAVA.isIntroductionAllowed(state)) { | ||
| description.addFix(getGuavaFix(tree, state)); | ||
| } | ||
| description.addFix(getUnmodifiableListFix(tree, state)); | ||
|
|
||
| return description.build(); | ||
| } | ||
|
|
||
| private static SuggestedFix getSuppressWarningsFix(VisitorState state) { | ||
| return SuggestedFixes.addSuppressWarnings(state, "ImplicitBlockingFluxOperation"); | ||
| } | ||
|
|
||
| private static SuggestedFix getGuavaFix(MethodInvocationTree tree, VisitorState state) { | ||
| SuggestedFix.Builder guavaFix = SuggestedFix.builder(); | ||
| String toImmutableList = | ||
| SuggestedFixes.qualifyStaticImport( | ||
| "com.google.common.collect.ImmutableList.toImmutableList", guavaFix, state); | ||
| return getCollectAndBlockFix(tree, state, guavaFix, toImmutableList + "()").build(); | ||
| } | ||
|
|
||
| private static SuggestedFix getUnmodifiableListFix( | ||
| MethodInvocationTree tree, VisitorState state) { | ||
| SuggestedFix.Builder unmodifiableListFix = SuggestedFix.builder(); | ||
| String toUnmodifiableList = | ||
| SuggestedFixes.qualifyStaticImport( | ||
| "java.util.stream.Collectors.toUnmodifiableList", unmodifiableListFix, state); | ||
| return getCollectAndBlockFix(tree, state, unmodifiableListFix, toUnmodifiableList + "()") | ||
| .build(); | ||
| } | ||
|
|
||
| /** | ||
| * @param collector expression | ||
| * @return `collect(...).block()...` fix with specified collector and postfix to match the | ||
| * original expression tree. | ||
| */ | ||
| private static SuggestedFix.Builder getCollectAndBlockFix( | ||
| MethodInvocationTree tree, VisitorState state, SuggestedFix.Builder fix, String collector) { | ||
| String postfix = getCollectAndBlockFixPostfix(tree, state); | ||
| // XXX: replace DIY string replace fix with something more resilient | ||
| String flux = | ||
| state.getSourceForNode(tree).substring(0, state.getSourceForNode(tree).indexOf(".")); | ||
| String replacement = String.format("%s.collect(%s).block()%s", flux, collector, postfix); | ||
| // fix.merge(SuggestedFix.replace(startPos, endPos, replacement)); | ||
| fix.merge(SuggestedFix.replace(tree, replacement)); | ||
| return fix; | ||
| } | ||
|
|
||
| /** | ||
| * @return postfix for `Flux.collect(...).block()` fix to match the original expression tree. | ||
| */ | ||
| private static String getCollectAndBlockFixPostfix( | ||
| MethodInvocationTree tree, VisitorState state) { | ||
| if (FLUX_TO_STREAM.matches(tree, state)) { | ||
| return ".stream()"; | ||
| } | ||
| return ""; | ||
| } | ||
| } |
141 changes: 141 additions & 0 deletions
141
...b/src/test/java/tech/picnic/errorprone/bugpatterns/ImplicitBlockingFluxOperationTest.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,141 @@ | ||
| package tech.picnic.errorprone.bugpatterns; | ||
|
|
||
| import com.google.errorprone.BugCheckerRefactoringTestHelper; | ||
| import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; | ||
| import com.google.errorprone.CompilationTestHelper; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| final class ImplicitBlockingFluxOperationTest { | ||
|
|
||
| @Test | ||
| void identification() { | ||
| CompilationTestHelper.newInstance(ImplicitBlockingFluxOperation.class, getClass()) | ||
| .addSourceLines( | ||
| "A.java", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " // BUG: Diagnostic contains:", | ||
| " flux().toIterable();", | ||
| " // BUG: Diagnostic contains:", | ||
| " flux().toStream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| @Test | ||
| void replacementFirstSuggestedFix() { | ||
| BugCheckerRefactoringTestHelper.newInstance(ImplicitBlockingFluxOperation.class, getClass()) | ||
| .setFixChooser(FixChoosers.FIRST) | ||
| .addInputLines( | ||
| "A.java", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " flux().toIterable();", | ||
| " flux().toStream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .addOutputLines( | ||
| "A.java", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " @SuppressWarnings(\"ImplicitBlockingFluxOperation\")", | ||
| " void m() {", | ||
| " flux().toIterable();", | ||
| " flux().toStream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); | ||
| } | ||
|
|
||
| @Test | ||
| void replacementSecondSuggestedFix() { | ||
| BugCheckerRefactoringTestHelper.newInstance(ImplicitBlockingFluxOperation.class, getClass()) | ||
| .setFixChooser(FixChoosers.SECOND) | ||
| .addInputLines( | ||
| "A.java", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " flux().toIterable();", | ||
| " flux().toStream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .addOutputLines( | ||
| "A.java", | ||
| "import static com.google.common.collect.ImmutableList.toImmutableList;", | ||
| "", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " flux().collect(toImmutableList()).block();", | ||
| " flux().collect(toImmutableList()).block().stream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); | ||
| } | ||
|
|
||
| @Test | ||
| void replacementThirdSuggestedFix() { | ||
| BugCheckerRefactoringTestHelper.newInstance(ImplicitBlockingFluxOperation.class, getClass()) | ||
| .setFixChooser(FixChoosers.THIRD) | ||
| .addInputLines( | ||
| "A.java", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " flux().toIterable();", | ||
| " flux().toStream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .addOutputLines( | ||
| "A.java", | ||
| "import static java.util.stream.Collectors.toUnmodifiableList;", | ||
| "", | ||
| "import reactor.core.publisher.Flux;", | ||
| "", | ||
| "class A {", | ||
| " void m() {", | ||
| " flux().collect(toUnmodifiableList()).block();", | ||
| " flux().collect(toUnmodifiableList()).block().stream();", | ||
| " }", | ||
| "", | ||
| " Flux<Integer> flux() {", | ||
| " return Flux.just(1, 2, 3);", | ||
| " }", | ||
| "}") | ||
| .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); | ||
| } | ||
| } |