Remove AbstractMethodReturnsNull and inline its logic.

The two inheritors are sufficiently different that this isn't helping, and is counterproductive to what I'm about to do.

PiperOrigin-RevId: 849999819

Author: graememorgan

core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java

@@ -16,53 +16,59 @@
 
 package com.google.errorprone.bugpatterns;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.util.ASTHelpers.findSuperMethods;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.sun.source.tree.Tree.Kind.NULL_LITERAL;
 
 import com.google.common.util.concurrent.Futures;
 import com.google.errorprone.VisitorState;
-import com.google.errorprone.fixes.Fix;
+import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
-import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ReturnTree;
-import com.sun.tools.javac.code.Symbol.MethodSymbol;
-import java.util.Optional;
+import com.sun.source.tree.StatementTree;
+import com.sun.source.util.TreePath;
 
 /**
  * Superclass for checks that {@code AsyncCallable} and {@code AsyncFunction} implementations do not
  * directly {@code return null}.
  */
-abstract class AbstractAsyncTypeReturnsNull extends AbstractMethodReturnsNull {
+abstract class AbstractAsyncTypeReturnsNull extends BugChecker implements ReturnTreeMatcher {
+  private final Class<?> asyncClass;
 
   AbstractAsyncTypeReturnsNull(Class<?> asyncClass) {
-    super(overridesMethodOfClass(asyncClass));
+    this.asyncClass = asyncClass;
   }
 
   @Override
-  protected Optional<Fix> provideFix(ReturnTree tree) {
-    return Optional.of(
-        SuggestedFix.builder()
-            .replace(tree.getExpression(), "immediateFuture(null)")
-            .addStaticImport(Futures.class.getName() + ".immediateFuture")
-            .build());
+  public final Description matchReturn(ReturnTree tree, VisitorState state) {
+    if (tree.getExpression() == null || tree.getExpression().getKind() != NULL_LITERAL) {
+      return NO_MATCH;
+    }
+    TreePath path = state.getPath();
+    while (path != null && path.getLeaf() instanceof StatementTree) {
+      path = path.getParentPath();
+    }
+    if (path == null || !(path.getLeaf() instanceof MethodTree methodTree)) {
+      return NO_MATCH;
+    }
+    if (findSuperMethods(getSymbol(methodTree), state.getTypes()).stream()
+        .noneMatch(
+            superMethod ->
+                superMethod.owner != null
+                    && superMethod.owner.getQualifiedName().contentEquals(asyncClass.getName()))) {
+      return NO_MATCH;
+    }
+    return describeMatch(tree, provideFix(tree.getExpression()));
   }
 
-  private static Matcher<MethodTree> overridesMethodOfClass(Class<?> clazz) {
-    checkNotNull(clazz);
-    return new Matcher<MethodTree>() {
-      @Override
-      public boolean matches(MethodTree tree, VisitorState state) {
-        MethodSymbol symbol = getSymbol(tree);
-        for (MethodSymbol superMethod : findSuperMethods(symbol, state.getTypes())) {
-          if (superMethod.owner != null
-              && superMethod.owner.getQualifiedName().contentEquals(clazz.getName())) {
-            return true;
-          }
-        }
-        return false;
-      }
-    };
+  protected SuggestedFix provideFix(ExpressionTree tree) {
+    return SuggestedFix.builder()
+        .replace(tree, "immediateFuture(null)")
+        .addStaticImport(Futures.class.getName() + ".immediateFuture")
+        .build();
   }
 }

core/src/main/java/com/google/errorprone/bugpatterns/AbstractMethodReturnsNull.java

@@ -17,20 +17,24 @@
 package com.google.errorprone.bugpatterns;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.matchers.Matchers.allOf;
 import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
 import static com.google.errorprone.matchers.Matchers.methodReturns;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.sun.source.tree.Tree.Kind.NULL_LITERAL;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
 import com.google.errorprone.dataflow.nullnesspropagation.TrustingNullnessAnalysis;
-import com.google.errorprone.fixes.Fix;
+import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ReturnTree;
-import java.util.Optional;
+import com.sun.source.tree.StatementTree;
+import com.sun.source.util.TreePath;
 
 /**
  * Flags methods with collection return types which return {@code null} in some cases but don't
@@ -43,7 +47,7 @@
         "Method has a collection return type and returns {@code null} in some cases but does not"
             + " annotate the method as @Nullable. See Effective Java 3rd Edition Item 54.",
     severity = SUGGESTION)
-public class ReturnsNullCollection extends AbstractMethodReturnsNull {
+public class ReturnsNullCollection extends BugChecker implements ReturnTreeMatcher {
 
   private static boolean methodWithoutNullable(MethodTree tree, VisitorState state) {
     return !TrustingNullnessAnalysis.hasNullableAnnotation(getSymbol(tree));
@@ -58,12 +62,21 @@ private static boolean methodWithoutNullable(MethodTree tree, VisitorState state
               methodReturns(isSubtypeOf("com.google.common.collect.Table"))),
           ReturnsNullCollection::methodWithoutNullable);
 
-  public ReturnsNullCollection() {
-    super(METHOD_RETURNS_COLLECTION_WITHOUT_NULLABLE_ANNOTATION);
-  }
-
   @Override
-  protected Optional<Fix> provideFix(ReturnTree tree) {
-    return Optional.empty();
+  public final Description matchReturn(ReturnTree tree, VisitorState state) {
+    if (tree.getExpression() == null || tree.getExpression().getKind() != NULL_LITERAL) {
+      return NO_MATCH;
+    }
+    TreePath path = state.getPath();
+    while (path != null && path.getLeaf() instanceof StatementTree) {
+      path = path.getParentPath();
+    }
+    if (path == null || !(path.getLeaf() instanceof MethodTree methodTree)) {
+      return NO_MATCH;
+    }
+    if (!METHOD_RETURNS_COLLECTION_WITHOUT_NULLABLE_ANNOTATION.matches(methodTree, state)) {
+      return NO_MATCH;
+    }
+    return describeMatch(tree);
   }
 }