Re-apply @rickie suggestions

PicnicSupermarket · Aug 14, 2023 · 091c1ea · 091c1ea
1 parent 60e831e
commit 091c1ea
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 22 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NestedPublishers.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NestedPublishers.java
@@ -10,6 +10,7 @@
 import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.unbound;
 
 import com.google.auto.service.AutoService;
+import com.google.common.collect.Iterables;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
@@ -20,10 +21,8 @@
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.tools.javac.code.Type;
-import java.util.function.Function;
 import org.jspecify.annotations.Nullable;
 import org.reactivestreams.Publisher;
-import reactor.core.publisher.Flux;
 
 /** A {@link BugChecker} that flags nesting of {@link Publisher Publishers}. */
 @AutoService(BugChecker.class)
@@ -52,31 +51,18 @@ public NestedPublishers() {}
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
-    Type publisherOfPublisherType = PUBLISHER_OF_PUBLISHERS.get(state);
-    Type groupedFluxType = GROUPED_FLUX.get(state);
-    Type treeType = ASTHelpers.getType(tree);
-    if (!isNestedPublisher(state, publisherOfPublisherType, treeType)
-        || isTypeArgumentGroupedFlux(state, groupedFluxType, treeType)) {
+    Type type = ASTHelpers.getType(tree);
+
+    if (!isSubType(type, PUBLISHER_OF_PUBLISHERS.get(state), state)
+        || isSubType(
+            Iterables.getOnlyElement(type.getTypeArguments()), GROUPED_FLUX.get(state), state)) {
       return Description.NO_MATCH;
     }
 
     return describeMatch(tree);
   }
 
-  private static boolean isNestedPublisher(
-      VisitorState state, @Nullable Type publisherOfPublisherType, Type treeType) {
-    return publisherOfPublisherType != null
-        && state.getTypes().isSubtype(treeType, publisherOfPublisherType);
-  }
-
-  /**
-   * Excluding the type when it matches {@code Flux<GroupedFlux<K, V>>} to not flag usages of {@link
-   * Flux#groupBy(Function)}.
-   */
-  private static boolean isTypeArgumentGroupedFlux(
-      VisitorState state, @Nullable Type groupedFluxType, Type treeType) {
-    return groupedFluxType != null
-        && treeType.getTypeArguments().stream()
-            .anyMatch(typez -> ASTHelpers.isSameType(typez, groupedFluxType, state));
+  private static boolean isSubType(Type subType, @Nullable Type type, VisitorState state) {
+    return type != null && state.getTypes().isSubtype(subType, type);
   }
 }
diff --git a/...-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NestedPublishersTest.java b/...-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NestedPublishersTest.java
@@ -14,6 +14,8 @@ void identification() {
             "",
             "class A {",
             "  void m() {",
+            "    Mono.empty();",
+            "    Mono.just(1);",
             "    // BUG: Diagnostic contains:",
             "    Mono.just(Mono.empty());",
             "    // BUG: Diagnostic contains:",
@@ -27,6 +29,8 @@ void identification() {
             "    // BUG: Diagnostic contains:",
             "    Mono.just(1).map(Flux::just);",
             "",
+            "    Flux.empty();",
+            "    Flux.just(1);",
             "    // BUG: Diagnostic contains:",
             "    Flux.just(Flux.empty());",
             "    // BUG: Diagnostic contains:",