Skip to content

Commit

Permalink
In NullArgumentForNonNullParameter, don't trigger completion for `N…
Browse files Browse the repository at this point in the history
…ullMarked`.

Completion can fail under `--release 8`, leading to `warning: unknown enum constant ElementType.MODULE`.

This CL is one of a variety of ways that I'll be addressing google/truth#1320. It alone should be sufficient (unless there are other problems that I'm unaware of), but I'll do more, both for people who might not upgrade Error Prone immediately and for anyone else (NullAway?) who writes a check that calls `hasAnnotation(nullMarked)`.

PiperOrigin-RevId: 651801963
  • Loading branch information
cpovirk authored and Error Prone Team committed Jul 12, 2024
1 parent c41ee92 commit 77e4f72
Showing 1 changed file with 18 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import static com.google.errorprone.util.ASTHelpers.enclosingClass;
import static com.google.errorprone.util.ASTHelpers.enclosingPackage;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static java.util.Arrays.stream;
import static javax.lang.model.type.TypeKind.TYPEVAR;

Expand Down Expand Up @@ -70,6 +69,10 @@ public final class NullArgumentForNonNullParameter extends BugChecker
memoize(state -> state.getName("com.google.common.collect.Immutable"));
private static final Supplier<Name> GUAVA_GRAPH_IMMUTABLE_PREFIX =
memoize(state -> state.getName("com.google.common.graph.Immutable"));
private static final Supplier<Name> PROTO_NONNULL_API_NAME =
memoize(state -> state.getName("com.google.protobuf.Internal$ProtoNonnullApi"));
private static final Supplier<Name> NULL_MARKED_NAME =
memoize(state -> state.getName("org.jspecify.annotations.NullMarked"));
private static final Supplier<ImmutableSet<Name>> NULL_MARKED_PACKAGES_WE_TRUST =
memoize(
state ->
Expand Down Expand Up @@ -245,17 +248,29 @@ private static boolean isParameterOfMethodOnTypeStartingWith(
private boolean enclosingAnnotationDefaultsNonTypeVariablesToNonNull(
Symbol sym, VisitorState state) {
for (; sym != null; sym = sym.getEnclosingElement()) {
if (hasAnnotation(sym, "com.google.protobuf.Internal$ProtoNonnullApi", state)) {
if (hasDirectAnnotation(sym, PROTO_NONNULL_API_NAME.get(state))) {
return true;
}
if (hasAnnotation(sym, "org.jspecify.annotations.NullMarked", state)
if (hasDirectAnnotation(sym, NULL_MARKED_NAME.get(state))
&& weTrustNullMarkedOn(sym, state)) {
return true;
}
}
return false;
}

/*
* ASTHelpers has hasAnnotation and hasDirectAnnotationWithSimpleName but I think not this.
*
* We avoid hasAnnotation not just because it's unnecessary but also because it would cause issues
* under --release 8 on account of NullMarked's use of @Target(MODULE, ...).
*/
private static boolean hasDirectAnnotation(Symbol sym, Name name) {
return sym.getAnnotationMirrors().stream()
.anyMatch(
a -> ((Symbol) a.getAnnotationType().asElement()).getQualifiedName().equals(name));
}

private boolean weTrustNullMarkedOn(Symbol sym, VisitorState state) {
/*
* Similar to @NonNull (discussed above), the "default to non-null" annotation @NullMarked is
Expand Down

0 comments on commit 77e4f72

Please sign in to comment.