Make `AbstractAsyncTypeReturnsNull` use `hasDefinitelyNullBranch` instead of looking only for null literals.
PiperOrigin-RevId: 852311211
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/AbstractAsyncTypeReturnsNull.java
similarity index 74%
rename from core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java
rename to core/src/main/java/com/google/errorprone/bugpatterns/nullness/AbstractAsyncTypeReturnsNull.java
index 5ec7dc1..45323da 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractAsyncTypeReturnsNull.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/AbstractAsyncTypeReturnsNull.java
@@ -14,10 +14,12 @@
* limitations under the License.
*/
-package com.google.errorprone.bugpatterns;
+package com.google.errorprone.bugpatterns.nullness;
import static com.google.common.collect.Streams.concat;
import static com.google.common.collect.Streams.stream;
+import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.hasDefinitelyNullBranch;
+import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
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;
@@ -25,8 +27,10 @@
import static com.google.errorprone.util.ASTHelpers.streamSuperMethods;
import static com.sun.source.tree.Tree.Kind.NULL_LITERAL;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.Futures;
import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.LambdaExpressionTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.ReturnTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
@@ -41,8 +45,8 @@
import java.util.stream.Stream;
/**
- * Superclass for checks that {@code AsyncCallable} and {@code AsyncFunction} implementations do not
- * directly {@code return null}.
+ * Superclass for checks that implementations of null-hostile abstract methods do not return
+ * definitely null values.
*/
abstract class AbstractAsyncTypeReturnsNull extends BugChecker
implements ReturnTreeMatcher, LambdaExpressionTreeMatcher {
@@ -54,7 +58,12 @@
@Override
public final Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
- if (tree.getBody().getKind() != NULL_LITERAL) {
+ if (!(tree.getBody() instanceof ExpressionTree body)
+ || !hasDefinitelyNullBranch(
+ body,
+ /* definitelyNullVars= */ ImmutableSet.of(),
+ /* varsProvenNullByParentIf= */ ImmutableSet.of(),
+ state)) {
return NO_MATCH;
}
Type functionalInterfaceType = ASTHelpers.getType(tree);
@@ -66,12 +75,18 @@
return NO_MATCH;
}
- return describeMatch(tree, provideFix((ExpressionTree) tree.getBody()));
+ return describeMatch(tree, provideFix(body));
}
@Override
public final Description matchReturn(ReturnTree tree, VisitorState state) {
- if (tree.getExpression() == null || tree.getExpression().getKind() != NULL_LITERAL) {
+ if (tree.getExpression() == null
+ || !hasDefinitelyNullBranch(
+ tree.getExpression(),
+ // TODO: cpovirk - Maybe populate sets to pass here and even above.
+ /* definitelyNullVars= */ ImmutableSet.of(),
+ /* varsProvenNullByParentIf= */ ImmutableSet.of(),
+ state)) {
return NO_MATCH;
}
Type asyncType = state.getTypeFromString(asyncClass.getName());
@@ -116,9 +131,15 @@
}
protected SuggestedFix provideFix(ExpressionTree tree) {
- return SuggestedFix.builder()
- .replace(tree, "immediateFuture(null)")
- .addStaticImport(Futures.class.getName() + ".immediateFuture")
- .build();
+ return tree.getKind() == NULL_LITERAL
+ ? SuggestedFix.builder()
+ .replace(tree, "immediateFuture(null)")
+ .addStaticImport(Futures.class.getName() + ".immediateFuture")
+ .build()
+ /*
+ * TODO: cpovirk - Provide a variant of hasDefinitelyNullBranch that returns a Set<Tree> of
+ * definitely null locations, and then generate a fix for those locations.
+ */
+ : emptyFix();
}
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AsyncCallableReturnsNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/AsyncCallableReturnsNull.java
similarity index 87%
rename from core/src/main/java/com/google/errorprone/bugpatterns/AsyncCallableReturnsNull.java
rename to core/src/main/java/com/google/errorprone/bugpatterns/nullness/AsyncCallableReturnsNull.java
index c3f5c70..f52fa19 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/AsyncCallableReturnsNull.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/AsyncCallableReturnsNull.java
@@ -14,14 +14,14 @@
* limitations under the License.
*/
-package com.google.errorprone.bugpatterns;
+package com.google.errorprone.bugpatterns.nullness;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import com.google.common.util.concurrent.AsyncCallable;
import com.google.errorprone.BugPattern;
-/** Checks that {@link AsyncCallable} implementations do not directly {@code return null}. */
+/** Checks that {@link AsyncCallable} implementations do not return definitely null values. */
@BugPattern(
summary = "AsyncCallable should not return a null Future, only a Future whose result is null.",
severity = ERROR)
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AsyncFunctionReturnsNull.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/AsyncFunctionReturnsNull.java
similarity index 87%
rename from core/src/main/java/com/google/errorprone/bugpatterns/AsyncFunctionReturnsNull.java
rename to core/src/main/java/com/google/errorprone/bugpatterns/nullness/AsyncFunctionReturnsNull.java
index 30d8e7e..31f517e 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/AsyncFunctionReturnsNull.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/AsyncFunctionReturnsNull.java
@@ -14,14 +14,14 @@
* limitations under the License.
*/
-package com.google.errorprone.bugpatterns;
+package com.google.errorprone.bugpatterns.nullness;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import com.google.common.util.concurrent.AsyncFunction;
import com.google.errorprone.BugPattern;
-/** Checks that {@link AsyncFunction} implementations do not directly {@code return null}. */
+/** Checks that {@link AsyncFunction} implementations do not return definitely null values. */
@BugPattern(
summary = "AsyncFunction should not return a null Future, only a Future whose result is null.",
severity = ERROR)
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullnessUtils.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullnessUtils.java
index 59ec970..087dadb 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullnessUtils.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullnessUtils.java
@@ -85,7 +85,7 @@
*
* @author awturner@google.com (Andy Turner)
*/
-class NullnessUtils {
+final class NullnessUtils {
private NullnessUtils() {}
private static final Matcher<ExpressionTree> OPTIONAL_OR_NULL =
diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
index 33aff8d..ed2e535 100644
--- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
+++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
@@ -40,8 +40,6 @@
import com.google.errorprone.bugpatterns.AssertThrowsMultipleStatements;
import com.google.errorprone.bugpatterns.AssertionFailureIgnored;
import com.google.errorprone.bugpatterns.AssignmentExpression;
-import com.google.errorprone.bugpatterns.AsyncCallableReturnsNull;
-import com.google.errorprone.bugpatterns.AsyncFunctionReturnsNull;
import com.google.errorprone.bugpatterns.AttemptedNegativeZero;
import com.google.errorprone.bugpatterns.AutoValueBoxedValues;
import com.google.errorprone.bugpatterns.AutoValueBuilderDefaultsInConstructor;
@@ -579,6 +577,8 @@
import com.google.errorprone.bugpatterns.javadoc.UrlInSee;
import com.google.errorprone.bugpatterns.nullness.AddNullMarkedToClass;
import com.google.errorprone.bugpatterns.nullness.AddNullMarkedToPackageInfo;
+import com.google.errorprone.bugpatterns.nullness.AsyncCallableReturnsNull;
+import com.google.errorprone.bugpatterns.nullness.AsyncFunctionReturnsNull;
import com.google.errorprone.bugpatterns.nullness.DereferenceWithNullBranch;
import com.google.errorprone.bugpatterns.nullness.EqualsBrokenForNull;
import com.google.errorprone.bugpatterns.nullness.EqualsMissingNullable;
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AsyncFunctionReturnsNullTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/AsyncFunctionReturnsNullTest.java
similarity index 98%
rename from core/src/test/java/com/google/errorprone/bugpatterns/AsyncFunctionReturnsNullTest.java
rename to core/src/test/java/com/google/errorprone/bugpatterns/nullness/AsyncFunctionReturnsNullTest.java
index 811f98d..1e1ee30 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/AsyncFunctionReturnsNullTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/AsyncFunctionReturnsNullTest.java
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-package com.google.errorprone.bugpatterns;
+package com.google.errorprone.bugpatterns.nullness;
import com.google.errorprone.CompilationTestHelper;
import org.junit.Test;
