diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java index b8b831d5915..3bee542f95d 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java +++ b/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 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 overridesMethodOfClass(Class clazz) { - checkNotNull(clazz); - return new Matcher() { - @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(); } } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMethodReturnsNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMethodReturnsNull.java deleted file mode 100644 index faaa5c5e20a..00000000000 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMethodReturnsNull.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2020 The Error Prone Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.errorprone.bugpatterns; - -import static com.google.errorprone.matchers.Description.NO_MATCH; -import static com.sun.source.tree.Tree.Kind.NULL_LITERAL; - -import com.google.errorprone.VisitorState; -import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher; -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 com.sun.source.tree.StatementTree; -import com.sun.source.util.TreePath; -import java.util.Optional; - -/** Superclass for checks that method implementations that directly {@code return null}. */ -abstract class AbstractMethodReturnsNull extends BugChecker implements ReturnTreeMatcher { - private final Matcher methodTreeMatcher; - - AbstractMethodReturnsNull(Matcher methodTreeMatcher) { - this.methodTreeMatcher = methodTreeMatcher; - } - - protected abstract Optional provideFix(ReturnTree tree); - - @Override - 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)) { - return NO_MATCH; - } - if (!methodTreeMatcher.matches((MethodTree) path.getLeaf(), state)) { - return NO_MATCH; - } - return provideFix(tree) - .map(fix -> describeMatch(tree, fix)) - .orElseGet(() -> describeMatch(tree)); - } -} diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ReturnsNullCollection.java b/core/src/main/java/com/google/errorprone/bugpatterns/ReturnsNullCollection.java index 425258ae8c4..96f9dcd3ecc 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ReturnsNullCollection.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ReturnsNullCollection.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 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); } }