From 264762a7272c856347ccb14e2abb3f22409369c7 Mon Sep 17 00:00:00 2001 From: Dima Legeza Date: Mon, 11 Dec 2023 15:19:50 +0100 Subject: [PATCH] PR comments - add proper compatibility with Mono.empty publisher --- .../bugpatterns/MonoZipOfMonoVoidUsage.java | 14 ++++++++++++-- .../bugpatterns/MonoZipOfMonoVoidUsageTest.java | 15 +++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) 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 d888aeba0dc..520e3d63aa3 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 @@ -3,8 +3,11 @@ 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.ChildMultiMatcher.MatchType.AT_LEAST_ONE; import static com.google.errorprone.matchers.Matchers.allOf; import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.argument; +import static com.google.errorprone.matchers.Matchers.hasArguments; import static com.google.errorprone.matchers.Matchers.instanceMethod; import static com.google.errorprone.matchers.Matchers.staticMethod; import static com.google.errorprone.matchers.Matchers.toType; @@ -73,11 +76,18 @@ public final class MonoZipOfMonoVoidUsage extends BugChecker toType(MethodInvocationTree.class, hasGenericArgumentOfExactType(MONO_VOID_TYPE))), allOf( instanceMethod().onDescendantOf(MONO).named("zipWith"), - toType(MethodInvocationTree.class, staticMethod().onClass(MONO).named("empty"))), + toType( + MethodInvocationTree.class, + argument(0, staticMethod().onClass(MONO).named("empty")))), onClassWithMethodName(MONO_VOID_TYPE, "zipWith"), allOf( staticMethod().onClass(MONO).named("zip"), - toType(MethodInvocationTree.class, hasGenericArgumentOfExactType(MONO_VOID_TYPE)))); + toType(MethodInvocationTree.class, hasGenericArgumentOfExactType(MONO_VOID_TYPE))), + allOf( + staticMethod().onClass(MONO).named("zip"), + toType( + MethodInvocationTree.class, + hasArguments(AT_LEAST_ONE, staticMethod().onClass(MONO).named("empty"))))); /** Instantiates a new {@link MonoZipOfMonoVoidUsage} instance. */ public MonoZipOfMonoVoidUsage() {} 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 d760262f18f..34916ecb53a 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 @@ -6,12 +6,8 @@ final class MonoZipOfMonoVoidUsageTest { /** - * Line 44 won't be reported as a bug. It's quite hard to catch this case as {@code Mono.empty()} - * yields {@code Mono}, so matcher will be too wide. Additionally, it's not expected to - * occur in the real production code. - * - *

Line 25 needed to simulate the unwanted intrinsic operations, which are not intended to be - * processed by the rule. + * Line 21 is needed to simulate the unwanted intrinsic operations, which are not intended to be + * processed by the rule but will be scanned anyway. */ @Test void identification() { @@ -41,6 +37,9 @@ void identification() { " Mono.zip(d, c, b, a);", " Mono.zip(d, c, b);", " b.zipWith(b).zipWith(c).map(entry -> entry);", + " // BUG: Diagnostic contains: `Mono#zip` and `Mono#zipWith` should not be executed against", + " // `Mono#empty` or `Mono` parameter; please revisit the parameters used and make sure to", + " // supply correct publishers instead", " Mono.zip(d, Mono.empty());", " c.zipWith(b);", " c.zipWith(d);", @@ -61,6 +60,10 @@ void identification() { " // `Mono#empty` or `Mono` parameter; please revisit the parameters used and make sure to", " // supply correct publishers instead", " a.zipWith(c, (first, second) -> second);", + " // BUG: Diagnostic contains: `Mono#zip` and `Mono#zipWith` should not be executed against", + " // `Mono#empty` or `Mono` parameter; please revisit the parameters used and make sure to", + " // supply correct publishers instead", + " c.zipWith(Mono.empty());", " }", "", " private Mono publisher() {",