From 35884331592899a49c692bd5d0b3aca518869f34 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Wed, 14 Oct 2026 09:02:46 -0700 Subject: [PATCH] Introduce `-XepOpt:Nullness:LikelyPublicApiOnly=true` for annotating only APIs that are likely to be accessible to users outside the package. (It's possible to "trick" the check by having a `private` or package-private class that is extended by a more visible class. But the point here is just to identify cases that are very _likely_ to be "public API.") PiperOrigin-RevId: 999407550 --- .../nullness/EqualsMissingNullable.java | 7 ++ .../nullness/FieldMissingNullable.java | 7 ++ .../bugpatterns/nullness/NullnessUtils.java | 70 ++++++++++++ .../nullness/ParameterMissingNullable.java | 11 ++ .../nullness/ReturnMissingNullable.java | 10 ++ .../nullness/VoidMissingNullable.java | 14 +++ .../nullness/EqualsMissingNullableTest.java | 43 ++++++++ .../nullness/FieldMissingNullableTest.java | 26 +++++ .../ParameterMissingNullableTest.java | 57 ++++++++++ .../nullness/ReturnMissingNullableTest.java | 101 ++++++++++++++++++ .../nullness/VoidMissingNullableTest.java | 86 ++++++++++++--- 11 files changed, 420 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullable.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullable.java index 0bac0332366..72ed17386e7 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullable.java @@ -21,7 +21,9 @@ import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAddingNullableAnnotationToType; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isAlreadyAnnotatedNullable; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isInNullMarkedScope; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isLikelyToBePublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.nullnessChecksShouldBeConservative; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.onlyLikelyPublicApi; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Matchers.equalsMethodDeclaration; import static com.google.errorprone.util.ASTHelpers.getSymbol; @@ -44,10 +46,12 @@ severity = SUGGESTION) public class EqualsMissingNullable extends BugChecker implements MethodTreeMatcher { private final boolean beingConservative; + private final boolean onlyLikelyPublicApi; @Inject EqualsMissingNullable(ErrorProneFlags flags) { this.beingConservative = nullnessChecksShouldBeConservative(flags); + this.onlyLikelyPublicApi = onlyLikelyPublicApi(flags); } @Override @@ -62,6 +66,9 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) { VariableTree parameterTree = getOnlyElement(methodTree.getParameters()); VarSymbol parameter = getSymbol(parameterTree); + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(parameter)) { + return NO_MATCH; + } if (isAlreadyAnnotatedNullable(parameter)) { return NO_MATCH; } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullable.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullable.java index 98a1cd57890..bdb5c405388 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullable.java @@ -22,7 +22,9 @@ import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.getNullCheck; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.hasDefinitelyNullBranch; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isAlreadyAnnotatedNullable; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isLikelyToBePublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.nullnessChecksShouldBeConservative; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.onlyLikelyPublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.varsProvenNullByParentIf; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.ASTHelpers.getSymbol; @@ -56,10 +58,12 @@ public class FieldMissingNullable extends BugChecker implements BinaryTreeMatcher, AssignmentTreeMatcher, VariableTreeMatcher { private final boolean beingConservative; + private final boolean onlyLikelyPublicApi; @Inject FieldMissingNullable(ErrorProneFlags flags) { this.beingConservative = nullnessChecksShouldBeConservative(flags); + this.onlyLikelyPublicApi = onlyLikelyPublicApi(flags); } @Override @@ -125,6 +129,9 @@ private Description matchIfLocallyDeclaredReferenceFieldWithoutNullable( if (assigned == null || assigned.getKind() != FIELD || assigned.type.isPrimitive()) { return NO_MATCH; } + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(assigned)) { + return NO_MATCH; + } /* * TODO(cpovirk): In conservative mode, consider skipping fields whose type is a type-variable 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 22973dfdc51..597cf8ffef6 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 @@ -26,6 +26,7 @@ import static com.google.errorprone.matchers.Matchers.staticMethod; import static com.google.errorprone.suppliers.Suppliers.JAVA_LANG_VOID_TYPE; import static com.google.errorprone.util.ASTHelpers.enclosingClass; +import static com.google.errorprone.util.ASTHelpers.enclosingElements; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; import static com.google.errorprone.util.ASTHelpers.hasAnnotation; @@ -34,8 +35,11 @@ import static com.sun.tools.javac.parser.Tokens.TokenKind.DOT; import static java.lang.Boolean.TRUE; import static java.util.Objects.requireNonNull; +import static javax.lang.model.element.Modifier.PROTECTED; +import static javax.lang.model.element.Modifier.PUBLIC; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.nullness.NullnessUtils.NullCheck.Polarity; @@ -50,10 +54,12 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.BinaryTree; import com.sun.source.tree.BlockTree; +import com.sun.source.tree.ClassTree; import com.sun.source.tree.ConditionalExpressionTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.IfTree; +import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.ParameterizedTypeTree; @@ -107,6 +113,70 @@ static boolean nullnessChecksShouldBeConservative(ErrorProneFlags flags) { return flags.getBoolean("Nullness:Conservative").orElse(true); } + static boolean onlyLikelyPublicApi(ErrorProneFlags flags) { + return flags.getBoolean("Nullness:LikelyPublicApiOnly").orElse(false); + } + + static boolean isLikelyToBePublicApi(Symbol sym) { + return enclosingElements(sym) + .allMatch( + s -> + switch (s) { + case ClassSymbol c -> isPublicOrProtected(c); + case MethodSymbol m -> isPublicOrProtected(m); + case VarSymbol v -> + switch (v.getKind()) { + case FIELD, ENUM_CONSTANT -> isPublicOrProtected(v); + case RECORD_COMPONENT -> true; + case PARAMETER -> + /* + * We have to make sure it's a method parameter, not a lambda parameter. + * (A lambda parameter has its "enclosing element" set to the actually + * enclosing method or to a synthetic MethodSymbol if the lambda is in a + * field initializer or initializer block.) + */ + ((MethodSymbol) v.getEnclosingElement()).getParameters().contains(v); + default -> false; + }; + default -> true; + }); + } + + static boolean isLikelyToBePublicApi(TreePath path) { + Tree child = null; + for (Tree tree : path) { + if (tree instanceof BlockTree || tree instanceof LambdaExpressionTree) { + return false; + } + if (tree instanceof VariableTree variableTree) { + return child != variableTree.getInitializer() + && isLikelyToBePublicApi(getSymbol(variableTree)); + } + if (tree instanceof MethodTree methodTree) { + /* + * Checking whether we are inside getDefaultValue is irrelevant for adding annotations, but + * it's future-proofing for any other isLikelyToBePublicApi use case in the future. + * + * Incidentally, Gemini notices that Java allows `int i() default (@Nullable int) 0;`! But + * to JSpecify, an annotation on the root type of a cast is already inapplicable, and a cast + * like `(Class<@NonNull Foo>) Foo.class` in an annotation default value is forbidden by + * javac. + */ + return child != methodTree.getDefaultValue() + && isLikelyToBePublicApi(getSymbol(methodTree)); + } + if (tree instanceof ClassTree) { + return isLikelyToBePublicApi(getSymbol(tree)); + } + child = tree; + } + throw new AssertionError("tree without enclosing element: " + Iterables.toString(path)); + } + + private static boolean isPublicOrProtected(Symbol symbol) { + return symbol.getModifiers().contains(PUBLIC) || symbol.getModifiers().contains(PROTECTED); + } + /* * TODO(cpovirk): Walking up the tree of enclosing elements may be more expensive than we'd like. * (But I haven't checked.) To improve upon that, would we go so far as to build special tracking diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullable.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullable.java index 4978095a187..91369acb45b 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullable.java @@ -24,7 +24,9 @@ import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.hasDefinitelyNullBranch; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.hasExtraParameterForEnclosingInstance; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isAlreadyAnnotatedNullable; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isLikelyToBePublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.nullnessChecksShouldBeConservative; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.onlyLikelyPublicApi; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; @@ -66,10 +68,12 @@ public final class ParameterMissingNullable extends BugChecker implements BinaryTreeMatcher, MethodInvocationTreeMatcher, NewClassTreeMatcher { private final boolean beingConservative; + private final boolean onlyLikelyPublicApi; @Inject ParameterMissingNullable(ErrorProneFlags flags) { this.beingConservative = nullnessChecksShouldBeConservative(flags); + this.onlyLikelyPublicApi = onlyLikelyPublicApi(flags); } @Override @@ -113,6 +117,9 @@ public Description matchBinary(BinaryTree tree, VisitorState state) { if (!isParameterWithoutNullable(symbol)) { return NO_MATCH; } + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(symbol)) { + return NO_MATCH; + } /* * OK, it's `param == null` or `param != null` for some `param` that is not `@Nullable`. @@ -245,6 +252,10 @@ public Description matchNewClass(NewClassTree tree, VisitorState state) { private Description matchCall( MethodSymbol methodSymbol, List arguments, VisitorState state) { + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(methodSymbol)) { + return NO_MATCH; + } + if (hasExtraParameterForEnclosingInstance(methodSymbol)) { // TODO(b/232103314): Figure out the right way to handle the implicit outer `this` parameter. return NO_MATCH; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java index 50775e21b58..b20edaf19d4 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java @@ -24,8 +24,10 @@ import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAddingNullableAnnotationToReturnType; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.hasDefinitelyNullBranch; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isAlreadyAnnotatedNullable; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isLikelyToBePublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isVoid; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.nullnessChecksShouldBeConservative; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.onlyLikelyPublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.varsProvenNullByParentIf; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Matchers.anyMethod; @@ -200,10 +202,12 @@ public class ReturnMissingNullable extends BugChecker implements CompilationUnit .collect(toImmutableSet())); private final boolean beingConservative; + private final boolean onlyLikelyPublicApi; @Inject ReturnMissingNullable(ErrorProneFlags flags) { this.beingConservative = nullnessChecksShouldBeConservative(flags); + this.onlyLikelyPublicApi = onlyLikelyPublicApi(flags); } @Override @@ -287,6 +291,9 @@ void doVisitMethod(MethodTree tree) { } MethodSymbol possibleOverride = getSymbol(tree); + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(possibleOverride)) { + return; + } /* * TODO(cpovirk): Provide an option to shortcut if !hasAnnotation(possibleOverride, @@ -358,6 +365,9 @@ void doVisitReturn(ReturnTree returnTree) { } MethodSymbol method = getSymbol(methodTree); + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(method)) { + return; + } List statements = methodTree.getBody().getStatements(); if (beingConservative && statements.size() == 1 diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullable.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullable.java index 28f5eee9fe5..07507252207 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullable.java @@ -20,8 +20,10 @@ import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAddingNullableAnnotationToReturnType; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAddingNullableAnnotationToType; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAnnotatingTypeUseOnlyLocationWithNullableAnnotation; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isLikelyToBePublicApi; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.isVoid; import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.nullnessChecksShouldBeConservative; +import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.onlyLikelyPublicApi; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Matchers.anyOf; import static com.google.errorprone.matchers.Matchers.isSubtypeOf; @@ -69,10 +71,12 @@ public class VoidMissingNullable extends BugChecker anyOf(isSubtypeOf(CLASS_TYPE), isSubtypeOf("com.google.common.reflect.TypeToken")); private final boolean beingConservative; + private final boolean onlyLikelyPublicApi; @Inject VoidMissingNullable(ErrorProneFlags flags) { this.beingConservative = nullnessChecksShouldBeConservative(flags); + this.onlyLikelyPublicApi = onlyLikelyPublicApi(flags); } /* @@ -106,6 +110,10 @@ public Description matchParameterizedType( return NO_MATCH; } + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(state.getPath())) { + return NO_MATCH; + } + for (Tree tree : parameterizedTypeTree.getTypeArguments()) { if (tree instanceof WildcardTree wildcardTree) { tree = wildcardTree.getBound(); @@ -140,6 +148,9 @@ public Description matchMethod(MethodTree tree, VisitorState state) { if (beingConservative && !NullnessUtils.isInNullMarkedScope(sym, state)) { return NO_MATCH; } + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(sym)) { + return NO_MATCH; + } return describeMatch(tree, fixByAddingNullableAnnotationToReturnType(state, tree)); } @@ -164,6 +175,9 @@ public Description matchVariable(VariableTree tree, VisitorState state) { if (beingConservative && !NullnessUtils.isInNullMarkedScope(sym, state)) { return NO_MATCH; } + if (onlyLikelyPublicApi && !isLikelyToBePublicApi(sym)) { + return NO_MATCH; + } SuggestedFix fix = fixByAddingNullableAnnotationToType(state, tree); if (fix.isEmpty()) { return NO_MATCH; diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullableTest.java index ec0eb60d12e..deb50df119c 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullableTest.java @@ -186,4 +186,47 @@ abstract class Foo { """) .doTest(); } + + @Test + public void likelyPublicApiOnly() { + CompilationTestHelper.newInstance(EqualsMissingNullable.class, getClass()) + .setArgs("-XepOpt:Nullness:Conservative=false", "-XepOpt:Nullness:LikelyPublicApiOnly=true") + .addSourceLines( + "PublicFoo.java", + """ + public abstract class PublicFoo { + // BUG: Diagnostic contains: @Nullable + public abstract boolean equals(Object o); + + protected abstract static class ProtectedInner { + // BUG: Diagnostic contains: @Nullable + public abstract boolean equals(Object o); + } + + abstract static class PackagePrivateInner { + public abstract boolean equals(Object o); + } + + public Object anonymous() { + return new Object() { + public boolean equals(Object o) { + return false; + } + }; + } + } + """) + .addSourceLines( + "PackagePrivateFoo.java", + """ + abstract class PackagePrivateFoo { + public abstract boolean equals(Object o); + + public abstract static class PublicInnerInPackagePrivateOuter { + public abstract boolean equals(Object o); + } + } + """) + .doTest(); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullableTest.java index 545794227d8..96702926957 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullableTest.java @@ -630,6 +630,32 @@ class Nullable {} .doTest(); } + @Test + public void likelyPublicApiOnly() { + createCompilationTestHelper() + .setArgs("-XepOpt:Nullness:LikelyPublicApiOnly=true") + .addSourceLines( + "PublicFoo.java", + """ + public class PublicFoo { + // BUG: Diagnostic contains: @Nullable + public Object publicField = null; + // BUG: Diagnostic contains: @Nullable + protected Object protectedField = null; + Object packagePrivateField = null; + private Object privateField = null; + } + """) + .addSourceLines( + "PackagePrivateFoo.java", + """ + class PackagePrivateFoo { + public Object publicFieldInPackagePrivateClass = null; + } + """) + .doTest(); + } + private CompilationTestHelper createCompilationTestHelper() { return CompilationTestHelper.newInstance(FieldMissingNullable.class, getClass()); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullableTest.java index 63c3d497b9d..39a85e0fbe6 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullableTest.java @@ -585,4 +585,61 @@ void bar() { """) .doTest(); } + + @Test + public void likelyPublicApiOnly() { + CompilationTestHelper.newInstance(ParameterMissingNullable.class, getClass()) + .setArgs("-XepOpt:Nullness:Conservative=false", "-XepOpt:Nullness:LikelyPublicApiOnly=true") + .addSourceLines( + "PublicFoo.java", + """ + import java.util.function.Consumer; + + public class PublicFoo { + public void publicNullCheck(Integer i) { + // BUG: Diagnostic contains: @Nullable + if (i == null) { + i = 0; + } + } + + void packagePrivateNullCheck(Integer i) { + if (i == null) { + i = 0; + } + } + + public void lambdaNullCheck() { + Consumer c = + (Integer i) -> { + if (i == null) { + i = 0; + } + }; + } + + public void publicCalledWithNull(Integer i) {} + + void packagePrivateCalledWithNull(Integer i) {} + + void caller() { + // BUG: Diagnostic contains: @Nullable + publicCalledWithNull(null); + packagePrivateCalledWithNull(null); + } + } + """) + .addSourceLines( + "PackagePrivateFoo.java", + """ + class PackagePrivateFoo { + public void publicMethodInPackagePrivateClass(Integer i) { + if (i == null) { + i = 0; + } + } + } + """) + .doTest(); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java index f3891b42bd6..06931fe2b7d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java @@ -2573,6 +2573,107 @@ String test(String x) { .doTest(); } + @Test + public void likelyPublicApiOnly() { + createCompilationTestHelper() + .setArgs("-XepOpt:Nullness:LikelyPublicApiOnly=true") + .addSourceLines( + "PublicFoo.java", + """ + public class PublicFoo { + public String publicMethod(boolean b) { + if (b) { + // BUG: Diagnostic contains: @Nullable + return null; + } + return ""; + } + + protected String protectedMethod(boolean b) { + if (b) { + // BUG: Diagnostic contains: @Nullable + return null; + } + return ""; + } + + String packagePrivateMethod(boolean b) { + if (b) { + return null; + } + return ""; + } + + private String privateMethod(boolean b) { + if (b) { + return null; + } + return ""; + } + + protected static class ProtectedInner { + public String methodInProtectedInner(boolean b) { + if (b) { + // BUG: Diagnostic contains: @Nullable + return null; + } + return ""; + } + } + + static class PackagePrivateInner { + public String methodInPackagePrivateInner(boolean b) { + if (b) { + return null; + } + return ""; + } + } + + public Object anonymousAndLocal() { + class Local { + public String methodInLocal(boolean b) { + if (b) { + return null; + } + return ""; + } + } + return new Object() { + public String methodInAnonymous(boolean b) { + if (b) { + return null; + } + return ""; + } + }; + } + } + """) + .addSourceLines( + "PackagePrivateFoo.java", + """ + class PackagePrivateFoo { + public String publicMethodInPackagePrivateOuter(boolean b) { + if (b) { + return null; + } + return ""; + } + + public static class PublicInnerInPackagePrivateOuter { + public String publicMethod(boolean b) { + if (b) { + return null; + } + return ""; + } + } + } + """) + .doTest(); + } + private CompilationTestHelper createCompilationTestHelper() { return CompilationTestHelper.newInstance(ReturnMissingNullable.class, getClass()); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullableTest.java index 62e9870924e..e18369771b4 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullableTest.java @@ -40,8 +40,6 @@ public void positive() { .addSourceLines( "Test.java", """ - import javax.annotation.Nullable; - class Test { // BUG: Diagnostic contains: @Nullable Void v; @@ -64,6 +62,8 @@ public void declarationAnnotatedLocation() { import javax.annotation.Nullable; abstract class Foo { + @Nullable Object existingDeclarationUsage; + java.lang.Void v; final Void f() { @@ -77,6 +77,8 @@ final Void f() { import javax.annotation.Nullable; abstract class Foo { + @Nullable Object existingDeclarationUsage; + @Nullable java.lang.Void v; @Nullable @@ -94,8 +96,6 @@ public void typeAnnotatedLocation() { .addInputLines( "in/Foo.java", """ - import org.checkerframework.checker.nullness.qual.Nullable; - abstract class Foo { java.lang.Void v; @@ -107,7 +107,7 @@ final Void f() { .addOutputLines( "out/Foo.java", """ - import org.checkerframework.checker.nullness.qual.Nullable; + import org.jspecify.annotations.Nullable; abstract class Foo { java.lang.@Nullable Void v; @@ -146,8 +146,6 @@ public void negativeNotVoid() { .addSourceLines( "Test.java", """ - import javax.annotation.Nullable; - class Test { String s; @@ -166,7 +164,6 @@ public void positiveTypeArgument() { "Test.java", """ import java.util.List; - import org.checkerframework.checker.nullness.qual.Nullable; class Test { // BUG: Diagnostic contains: @Nullable @@ -200,7 +197,6 @@ public void positiveTypeArgumentOtherAnnotation() { "Test.java", """ import java.util.List; - import org.checkerframework.checker.nullness.qual.Nullable; class Test { // BUG: Diagnostic contains: @Nullable @@ -334,6 +330,8 @@ public void negativeTypeArgumentDeclarationNullable() { import javax.annotation.Nullable; class Test { + @Nullable Object existingDeclarationUsage; + List a; List b; List c; @@ -441,7 +439,6 @@ public void positiveConservativeNullMarked() { .addSourceLines( "Test.java", """ - import javax.annotation.Nullable; import org.jspecify.annotations.NullMarked; @NullMarked @@ -459,12 +456,77 @@ public void negativeConservativeNotNullMarked() { .addSourceLines( "Test.java", """ - import javax.annotation.Nullable; - class Test { Void v; } """) .doTest(); } + + @Test + public void likelyPublicApiOnly() { + CompilationTestHelper.newInstance(VoidMissingNullable.class, getClass()) + .setArgs("-XepOpt:Nullness:Conservative=false", "-XepOpt:Nullness:LikelyPublicApiOnly=true") + .addSourceLines( + "PublicFoo.java", + """ + import java.util.List; + import java.util.function.Consumer; + import java.util.function.Supplier; + + // BUG: Diagnostic contains: @Nullable + public abstract class PublicFoo implements Iterable { + // BUG: Diagnostic contains: @Nullable + public Void publicField; + Void packagePrivateField; + + // BUG: Diagnostic contains: @Nullable + public Void publicMethod() { + return null; + } + + Void packagePrivateMethod() { + return null; + } + + // BUG: Diagnostic contains: @Nullable + public void publicMethodWithVoidParam(Void v) {} + + void packagePrivateMethodWithVoidParam(Void v) {} + + // BUG: Diagnostic contains: @Nullable + public List publicTypeArgField; + List packagePrivateTypeArgField; + + // BUG: Diagnostic contains: @Nullable + public List publicTypeArgMethod() { + return null; + } + + List packagePrivateTypeArgMethod() { + return null; + } + + public Object initializerTypeArg = (List) null; + + public void localAndLambda() { + List local = null; + local = (List) null; + Consumer c = (Void v) -> {}; + Supplier s = () -> (List) null; + } + } + """) + .addSourceLines( + "PackagePrivateFoo.java", + """ + import java.util.List; + + abstract class PackagePrivateFoo implements Iterable { + public Void publicField; + public List publicTypeArgField; + } + """) + .doTest(); + } }