diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MonoZipOfMonoVoidUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MonoZipOfMonoVoidUsage.java new file mode 100644 index 00000000000..e9995a980a4 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MonoZipOfMonoVoidUsage.java @@ -0,0 +1,132 @@ +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.allOf; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.staticMethod; +import static com.google.errorprone.matchers.Matchers.toType; +import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; +import static java.util.stream.Collectors.joining; +import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; + +import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Streams; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +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.suppliers.Supplier; +import com.google.errorprone.suppliers.Suppliers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberReferenceTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.code.Type; +import java.util.stream.Stream; + +/** Bla. */ +@AutoService(BugChecker.class) +@BugPattern( + summary = + "`Flux#flatMap` and `Flux#flatMapSequential` have subtle semantics; " + + "please use `Flux#concatMap` or explicitly specify the desired amount of concurrency", + link = BUG_PATTERNS_BASE_URL + "MonoZipOfMonoVoidUsage", + linkType = CUSTOM, + severity = ERROR, + tags = LIKELY_ERROR) +public final class MonoZipOfMonoVoidUsage extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher, BugChecker.MemberReferenceTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Supplier MONO = + Suppliers.typeFromString("reactor.core.publisher.Mono"); + private static final Supplier EMPTY_MONO = + Suppliers.typeFromString("reactor.core.publisher.MonoEmpty"); + + // On Mono.zip, at least one element should match empty in order to proceed. + private static final Matcher STATIC_MONO_ZIP = + anyOf( + staticMethod().onClass(MONO).named("zip"), + toType(MethodInvocationTree.class, hasArgumentOfType(EMPTY_MONO))); + // On mono.zipWith, argument should match empty in order to proceed. + private static final Matcher DYNAMIC_MONO_ZIP = + allOf( + instanceMethod().onDescendantOf(MONO).named("zipWith"), + toType(MethodInvocationTree.class, hasArgumentOfType(EMPTY_MONO))); + + // On emptyMono.zipWith, argument should match non-empty in order to proceed. + private static final Matcher DYNAMIC_EMPTY_MONO_WITH_NON_EMPTY_PARAM_ZIP = + allOf( + instanceMethod().onDescendantOf(EMPTY_MONO).named("zipWith"), + toType(MethodInvocationTree.class, hasArgumentOfType(MONO))); + + // On emptyMono.zipWith, argument should match empty in order to proceed. + private static final Matcher DYNAMIC_EMPTY_MONO_WITH_EMPTY_PARAM_ZIP = + allOf( + instanceMethod().onDescendantOf(EMPTY_MONO).named("zipWith"), + toType(MethodInvocationTree.class, hasArgumentOfType(EMPTY_MONO))); + + private static final Matcher MONO_ZIP = + anyOf( + DYNAMIC_MONO_ZIP, + DYNAMIC_EMPTY_MONO_WITH_NON_EMPTY_PARAM_ZIP, + DYNAMIC_EMPTY_MONO_WITH_EMPTY_PARAM_ZIP, + STATIC_MONO_ZIP + ); + + /** Instantiates a new {@link MonoZipOfMonoVoidUsage} instance. */ + public MonoZipOfMonoVoidUsage() {} + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!MONO_ZIP.matches(tree, state)) { + return Description.NO_MATCH; + } + + Description.Builder description = buildDescription(tree); + + if (STATIC_MONO_ZIP.matches(tree, state)) { + ImmutableList arguments = ImmutableList.copyOf(tree.getArguments()); + + String replacement = + Streams.concat( + Stream.of( + arguments.stream().findFirst().map(ExpressionTree::toString).orElseThrow()), + arguments.stream() + .skip(1) + .map(ExpressionTree::toString) + .map(arg -> String.format("(%s)", arg))) + .collect(joining(".then")); + SuggestedFix staticZipUsage = SuggestedFix.replace(tree, replacement); + description.addFix(staticZipUsage); + } else { + SuggestedFix instanceZipUsage = + SuggestedFixes.renameMethodInvocation(tree, "concatWith", state); + description.addFix(instanceZipUsage); + } + + return description.build(); + } + + @Override + public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) { + if (!MONO_ZIP.matches(tree, state)) { + return Description.NO_MATCH; + } + + // Method references are expected to occur very infrequently; generating both variants of + // suggested fixes is not worth the trouble. + return describeMatch(tree); + } + + private static Matcher hasArgumentOfType(Supplier type) { + return (tree, state) -> + tree.getArguments().stream() + .anyMatch(arg -> ASTHelpers.isSubtype(ASTHelpers.getType(arg), type.get(state), state)); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MonoZipOfMonoVoidUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MonoZipOfMonoVoidUsageTest.java new file mode 100644 index 00000000000..08f23f91f05 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MonoZipOfMonoVoidUsageTest.java @@ -0,0 +1,84 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; + +final class MonoZipOfMonoVoidUsageTest { + @Test + void identification() { + CompilationTestHelper.newInstance(MonoZipOfMonoVoidUsage.class, getClass()) + .addSourceLines( + "A.java", + "import reactor.core.publisher.Mono;", + "", + "class A {", + " void m() {", + " Mono a = Mono.empty();", + " Mono b = Mono.empty();", + " Mono c = Mono.just(1);", + " // BUG: Diagnostic contains:", + " Mono.zip(a, b);", + " // BUG: Diagnostic contains:", + " b.zipWith(c);", + " // BUG: Diagnostic contains:", + " Mono.zip(a, Mono.empty());", + " // BUG: Diagnostic contains:", + " Mono.zip(Mono.empty(), Mono.empty());", + " // BUG: Diagnostic contains:", + " Mono.zip(a, c, b);", + " }", + "}") + .doTest(); + } + + @Test + void replacementFirstSuggestedFix() { + BugCheckerRefactoringTestHelper.newInstance(MonoZipOfMonoVoidUsage.class, getClass()) + .addInputLines( + "A.java", + "import reactor.core.publisher.Mono;", + "", + "class A {", + " void m() {", + " Mono a = Mono.empty();", + " Mono b = Mono.empty();", + " Mono c = Mono.just(1);", + " Mono.zip(a, a);", + " Mono.zip(Mono.empty(), a);", + " Mono.zip(Mono.empty(), Mono.empty());", + "", + " Mono.empty().zipWith(a);", + " Mono.empty().zipWith(Mono.empty());", + " b.zipWith(b).zipWith(c).map(entry -> entry);", + " c.zipWith(Mono.empty());", + " c.zipWith(b);", + " c.zipWith(c);", + " }", + "}") + .addOutputLines( + "A.java", + "import reactor.core.publisher.Mono;", + "", + "class A {", + " void m() {", + " Mono a = Mono.empty();", + " Mono b = Mono.empty();", + " Mono c = Mono.just(1);", + " a.then(a);", + " Mono.empty().then(a);", + " Mono.empty().then(Mono.empty());", + "", + " Mono.empty().concatWith(a);", + " Mono.empty().concatWith(Mono.empty());", + " b.concatWith(b).concatWith(c).map(entry -> entry);", + " c.concatWith(Mono.empty());", + " c.concatWith(b);", + " c.concatWith(c);", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +}