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
index 9dfc8e9e53a..a9fafcaf6cb 100644
--- 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
@@ -5,9 +5,8 @@
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyMethod;
-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 static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
@@ -26,18 +25,35 @@
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. */
+import org.reactivestreams.Publisher;
+import reactor.core.publisher.Mono;
+
+/**
+ * A {@link BugChecker} that flags usages of {@link Mono#zip(Mono, Mono)}} and {@link
+ * Mono#zipWith(Mono)}} with {@link Mono#empty()} parameters.
+ *
+ * {@link Mono#zip(Mono, Mono)} and {@link Mono#zipWith(Mono)} perform incorrectly upon retrieval
+ * of the empty publisher and prematurely terminates the reactive chain from the execution. In most
+ * cases this is not the desired behaviour and {@link Mono#concatWith(Publisher)} or {@link
+ * Mono#then(Mono)} should be preferred, as it produces consistent results and performs in the
+ * predictable manner.
+ *
+ *
NB: Mono<?>#zipWith(Mono<Void>) is allowed be the Reactor API, but it is an incorrect
+ * usage of the API. It will be replaced with Mono<?>#concatWith(Mono<Void>) but it will lead
+ * to the compilation errors, indicating that the logic under the hood should be revisited and
+ * fixed.
+ */
@AutoService(BugChecker.class)
@BugPattern(
- summary = "XXX: Write this.",
+ summary =
+ "`Mono#zip` and `Mono#zipWith` should not be executed against `Mono#empty` parameter; "
+ + "please use `Mono#then` or `Mono#concatWith` instead",
link = BUG_PATTERNS_BASE_URL + "MonoZipOfMonoVoidUsage",
linkType = CUSTOM,
severity = ERROR,
@@ -45,42 +61,21 @@
public final class MonoZipOfMonoVoidUsage extends BugChecker
implements MethodInvocationTreeMatcher, 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");
- private static final Supplier MONO_VOID =
- VisitorState.memoize(generic(type("reactor.core.publisher.Mono"), type("java.lang.Void")));
+ private static final String MONO = "reactor.core.publisher.Mono";
+ private static final Supplier MONO_VOID_TYPE =
+ VisitorState.memoize(generic(type(MONO), type("java.lang.Void")));
// On Mono.zip, at least one element should match empty in order to proceed.
private static final Matcher MONO_ZIP_AND_WITH =
- anyOf(
- anyMethod().onClass("reactor.core.publisher.Mono").namedAnyOf("zip", "zipWith"),
- toType(MethodInvocationTree.class, hasArgumentOfType(MONO_VOID)));
- // 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)));
+ anyMethod().onDescendantOf(MONO).namedAnyOf("zip", "zipWith"),
+ toType(MethodInvocationTree.class, hasArgumentOfType(MONO_VOID_TYPE)));
- // On emptyMono.zipWith, argument should match empty in order to proceed.
- private static final Matcher DYNAMIC_EMPTY_MONO_WITH_EMPTY_PARAM_ZIP =
+ // On Mono.zip, at least one element should match empty in order to proceed.
+ private static final Matcher STATIC_MONO_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,
- // MONO_ZIP_AND_WITH);
+ staticMethod().onClass(MONO).named("zip"),
+ toType(MethodInvocationTree.class, hasArgumentOfType(MONO_VOID_TYPE)));
/** Instantiates a new {@link MonoZipOfMonoVoidUsage} instance. */
public MonoZipOfMonoVoidUsage() {}
@@ -93,10 +88,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
Description.Builder description = buildDescription(tree);
- // ASTHelpers.getType(tree.getArguments().get(0))
- // MoreTypes.generic(MoreTypes.type("reactor.core.publisher.Mono"),
- // MoreTypes.type("java.lang.Void")).get(state)
- if (MONO_ZIP_AND_WITH.matches(tree, state)) {
+ if (STATIC_MONO_ZIP.matches(tree, state)) {
ImmutableList arguments = ImmutableList.copyOf(tree.getArguments());
String replacement =
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
index 40a238baae5..91c4a2d571e 100644
--- 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
@@ -16,27 +16,95 @@ void identification() {
"class A {",
" void m() {",
" Mono a = Mono.empty();",
- " Mono.empty().zipWith(a);",
- " Mono b = Mono.empty();",
+ " Mono b = Mono.empty();",
" Mono c = Mono.just(1);",
+ " Mono d = this.publisher();",
" // BUG: Diagnostic contains:",
- " Mono.zip(a, b);",
- " // BUG: Diagnostic contains:",
- " b.zipWith(c);",
+ " Mono.zip(a, a);",
" // BUG: Diagnostic contains:",
- " Mono.zip(a, Mono.empty());",
+ " Mono.zip(Mono.empty(), a);",
" // BUG: Diagnostic contains:",
" Mono.zip(Mono.empty(), Mono.empty());",
" // BUG: Diagnostic contains:",
- " Mono.zip(a, c, b);",
- "",
+ " Mono.empty().zipWith(a);",
+ " // BUG: Diagnostic contains:",
+ " Mono.empty().zipWith(Mono.empty());",
+ " // BUG: Diagnostic contains:",
+ " b.zipWith(b).zipWith(c).map(entry -> entry);",
+ " // BUG: Diagnostic contains:",
+ " c.zipWith(Mono.empty());",
+ " // BUG: Diagnostic contains:",
+ " c.zipWith(b);",
+ " c.zipWith(d);",
+ " Mono.just(1).zipWith(Mono.just(1));",
+ " c.zipWith(c);",
" }",
+ "",
+ " private Mono publisher() {return Mono.empty();}",
"}")
.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 d = this.publisher();",
+ // " 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(d);",
+ " Mono.just(1).zipWith(Mono.just(1));",
+ // " c.zipWith(c);",
+ " }",
+ "",
+ " private Mono publisher() {return Mono.empty();}",
+ "}")
+ .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);",
+ // " Mono d = this.publisher();",
+ // " 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.zipWith(d);",
+ " Mono.just(1).zipWith(Mono.just(1));",
+ // " c.zipWith(c);",
+ " }",
+ "",
+ " private Mono publisher() {return Mono.empty();}",
+ "}")
+ .doTest(TestMode.TEXT_MATCH);
+ }
+
+ @Test
+ void replacementFirstSuggestedFixWithCompilationErrors() {
BugCheckerRefactoringTestHelper.newInstance(MonoZipOfMonoVoidUsage.class, getClass())
.addInputLines(
"A.java",
@@ -46,17 +114,8 @@ void replacementFirstSuggestedFix() {
" 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);",
+ " Mono.zip(a, b);",
+ " a.zipWith(b);",
" }",
"}")
.addOutputLines(
@@ -67,19 +126,11 @@ void replacementFirstSuggestedFix() {
" 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);",
+ " a.then(b);",
+ " a.concatWith(b);",
" }",
"}")
+ .allowBreakingChanges()
.doTest(TestMode.TEXT_MATCH);
}
}