Skip to content

Introduce -XepOpt:Nullness:LikelyPublicApiOnly=true for annotating only APIs that are likely to be accessible to users outside the package. #6206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Oct 14, 2026
Merged

Introduce -XepOpt:Nullness:LikelyPublicApiOnly=true for annotating only APIs that are likely to be accessible to users outside the package. #6206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}
Expand Down
7 changes: 7 additions & 0 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
70 changes: 70 additions & 0 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/NullnessUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/ParameterMissingNullable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -245,6 +252,10 @@ public Description matchNewClass(NewClassTree tree, VisitorState state) {

private Description matchCall(
MethodSymbol methodSymbol, List<? extends ExpressionTree> 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;
Expand Down
10 changes: 10 additions & 0 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -358,6 +365,9 @@ void doVisitReturn(ReturnTree returnTree) {
}

MethodSymbol method = getSymbol(methodTree);
if (onlyLikelyPublicApi && !isLikelyToBePublicApi(method)) {
return;
}
List<? extends StatementTree> statements = methodTree.getBody().getStatements();
if (beingConservative
&& statements.size() == 1
Expand Down
14 changes: 14 additions & 0 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/*
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}

Expand All @@ -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;
Expand Down
43 changes: 43 additions & 0 deletions core/src/test/java/com/google/errorprone/bugpatterns/nullness/EqualsMissingNullableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
26 changes: 26 additions & 0 deletions core/src/test/java/com/google/errorprone/bugpatterns/nullness/FieldMissingNullableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Loading
Loading