diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxCollect.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxCollect.java deleted file mode 100644 index 88b7d67ff89..00000000000 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FluxCollect.java +++ /dev/null @@ -1,64 +0,0 @@ -package tech.picnic.errorprone.bugpatterns; - -import static com.google.errorprone.BugPattern.LinkType.NONE; -import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; -import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; -import static com.google.errorprone.matchers.Matchers.instanceMethod; - -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; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import tech.picnic.errorprone.bugpatterns.util.SourceCode; - -/** - * A {@link BugChecker} which flags usages of Flux {@code collect(...)} (and similar) followed by - * empty source checks. - * - *

Flux collect methods like {@link Flux#collect(java.util.stream.Collector)} (and similar) - * always emit a value even on an empty source (in which case an empty collections is returned). - * Following such operations with methods like {@link Mono#single()} or {@link - * Mono#switchIfEmpty(Mono)} is unnecessary. - */ -@AutoService(BugChecker.class) -@BugPattern( - summary = "`Flux`'s collect operations always emit precisely one value", - linkType = NONE, - severity = WARNING, - tags = SIMPLIFICATION) -public final class FluxCollect extends BugChecker implements MethodInvocationTreeMatcher { - private static final long serialVersionUID = 1L; - private static final Matcher MONO_SIZE_CHECK = - instanceMethod() - .onDescendantOf("reactor.core.publisher.Mono") - .namedAnyOf("single", "defaultIfEmpty", "switchIfEmpty"); - private static final Matcher FLUX_COLLECT = - instanceMethod() - .onDescendantOf("reactor.core.publisher.Flux") - .namedAnyOf( - "collect", "collectList", "collectMap", "collectMultimap", "collectSortedList"); - - @Override - public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (!MONO_SIZE_CHECK.matches(tree, state)) { - return Description.NO_MATCH; - } - - ExpressionTree receiver = ASTHelpers.getReceiver(tree); - if (!FLUX_COLLECT.matches(receiver, state)) { - return Description.NO_MATCH; - } - - return describeMatch( - tree, SuggestedFix.replace(tree, SourceCode.treeToString(receiver, state))); - } -} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonEmptyMono.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonEmptyMono.java new file mode 100644 index 00000000000..d9835641cd7 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonEmptyMono.java @@ -0,0 +1,85 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.NONE; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.instanceMethod; + +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; +import reactor.core.publisher.Mono; +import tech.picnic.errorprone.bugpatterns.util.SourceCode; + +/** + * A {@link BugChecker} which flags {@link Mono} operations that are known to be vacuous, given that + * they are invoked on a {@link Mono} that is known not to complete empty. + */ +@AutoService(BugChecker.class) +@BugPattern( + summary = "Avoid vacuous operations on known non-empty `Mono`s", + linkType = NONE, + severity = WARNING, + tags = SIMPLIFICATION) +// XXX: Given more advanced analysis many more expressions could be flagged. Consider +// `Mono.just(someValue)`, `Flux.just(someNonEmptySequence)`, +// `someMono.switchIfEmpty(someProvablyNonEmptyMono)` and many other variants. +// XXX: Consider implementing a similar check for `Publisher`s that are known to complete without +// emitting a value (e.g. `Mono.empty()`, `someFlux.then()`, ...), or known not to complete normally +// (`Mono.never()`, `someFlux.repeat()`, `Mono.error(...)`, ...). The later category could +// potentially be split out further. +public final class NonEmptyMono extends BugChecker implements MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Matcher MONO_SIZE_CHECK = + instanceMethod() + .onDescendantOf("reactor.core.publisher.Mono") + .namedAnyOf("single", "defaultIfEmpty", "switchIfEmpty"); + private static final Matcher SINGLETON_MONO = + anyOf( + instanceMethod() + .onDescendantOf("reactor.core.publisher.Flux") + .namedAnyOf( + "all", + "any", + "collect", + "collectList", + "collectMap", + "collectMultimap", + "collectSortedList", + "count", + "defaultIfEmpty", + "elementAt", + "hasElement", + "hasElements", + "last", + "reduce", + "reduceWith", + "single"), + instanceMethod() + .onDescendantOf("reactor.core.publisher.Mono") + .namedAnyOf("defaultIfEmpty", "hasElement", "single")); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!MONO_SIZE_CHECK.matches(tree, state)) { + return Description.NO_MATCH; + } + + ExpressionTree receiver = ASTHelpers.getReceiver(tree); + if (!SINGLETON_MONO.matches(receiver, state)) { + return Description.NO_MATCH; + } + + return describeMatch( + tree, SuggestedFix.replace(tree, SourceCode.treeToString(receiver, state))); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxCollectTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonEmptyMonoTest.java similarity index 98% rename from error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxCollectTest.java rename to error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonEmptyMonoTest.java index bbaea1b2e78..d7fa54d423f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxCollectTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonEmptyMonoTest.java @@ -6,11 +6,11 @@ import com.google.errorprone.CompilationTestHelper; import org.junit.jupiter.api.Test; -final class FluxCollectTest { +final class NonEmptyMonoTest { private final CompilationTestHelper compilationTestHelper = - CompilationTestHelper.newInstance(FluxCollect.class, getClass()); + CompilationTestHelper.newInstance(NonEmptyMono.class, getClass()); private final BugCheckerRefactoringTestHelper refactoringTestHelper = - BugCheckerRefactoringTestHelper.newInstance(FluxCollect.class, getClass()); + BugCheckerRefactoringTestHelper.newInstance(NonEmptyMono.class, getClass()); @Test void identification() {