Skip to content

Commit

Permalink
Update NullnessAnnotations API to use annotation mirrors and trees …
Browse files Browse the repository at this point in the history 
…instead of strings

PiperOrigin-RevId: 579319948
  • Loading branch information
@cushon
cushon authored and Error Prone Team committed Nov 3, 2023
1 parent 09a9aa6 commit 99bfc0b
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 49 deletions.
5 changes: 3 additions & 2 deletions check_api/src/main/java/com/google/errorprone/dataflow/nullnesspropagation/MethodInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@

package com.google.errorprone.dataflow.nullnesspropagation;

import java.util.List;
import com.google.common.collect.ImmutableList;
import javax.lang.model.element.AnnotationMirror;

/** Represents a Java method. Used for custom predicates to match non-null-returning methods. */
public interface MethodInfo {
String clazz();

String method();

List<String> annotations();
ImmutableList<AnnotationMirror> annotations();

boolean isStatic();

Expand Down
66 changes: 42 additions & 24 deletions ...src/main/java/com/google/errorprone/dataflow/nullnesspropagation/NullnessAnnotations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,22 @@
import static javax.lang.model.element.ElementKind.TYPE_PARAMETER;

import com.google.errorprone.util.MoreAnnotations;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Name;
import javax.lang.model.element.Parameterizable;
import javax.lang.model.element.TypeParameterElement;
import javax.lang.model.type.IntersectionType;
Expand All @@ -41,28 +46,44 @@ public class NullnessAnnotations {
// TODO(kmb): Correctly handle JSR 305 @Nonnull(NEVER) etc.
private static final Predicate<String> ANNOTATION_RELEVANT_TO_NULLNESS =
Pattern.compile(
".*\\b("
+ "(Recently)?NonNull(Decl|Type)?|NotNull|Nonnull|"
"(Recently)?NonNull(Decl|Type)?|NotNull|Nonnull|"
+ "(Recently)?Nullable(Decl|Type)?|CheckForNull|PolyNull|MonotonicNonNull(Decl)?|"
+ "ProtoMethodMayReturnNull|ProtoMethodAcceptsNullParameter|"
+ "ProtoPassThroughNullness"
+ ")$")
.asPredicate();
+ "ProtoPassThroughNullness")
.asMatchPredicate();
private static final Predicate<String> NULLABLE_ANNOTATION =
Pattern.compile(
".*\\b("
+ "(Recently)?Nullable(Decl|Type)?|CheckForNull|PolyNull|MonotonicNonNull(Decl)?|"
"(Recently)?Nullable(Decl|Type)?|CheckForNull|PolyNull|MonotonicNonNull(Decl)?|"
+ "ProtoMethodMayReturnNull|ProtoMethodAcceptsNullParameter|"
+ "ProtoPassThroughNullness"
+ ")$")
.asPredicate();
+ "ProtoPassThroughNullness")
.asMatchPredicate();

private NullnessAnnotations() {} // static methods only

public static Optional<Nullness> fromAnnotations(Collection<String> annotations) {
public static Optional<Nullness> fromAnnotationTrees(List<? extends AnnotationTree> annotations) {
return fromAnnotationSimpleNames(annotations.stream().map(a -> simpleName(a)));
}

public static Optional<Nullness> fromAnnotationMirrors(
List<? extends AnnotationMirror> annotations) {
return fromAnnotationStream(annotations.stream());
}

private static String simpleName(AnnotationTree annotation) {
Tree annotationType = annotation.getAnnotationType();
if (annotationType instanceof IdentifierTree) {
return ((IdentifierTree) annotationType).getName().toString();
} else if (annotationType instanceof MemberSelectTree) {
return ((MemberSelectTree) annotationType).getIdentifier().toString();
} else {
throw new AssertionError(annotationType.getKind());
}
}

private static Name simpleName(AnnotationMirror annotation) {
return annotation.getAnnotationType().asElement().getSimpleName();
}

public static Optional<Nullness> fromAnnotationsOn(@Nullable Symbol sym) {
if (sym == null) {
return Optional.empty();
Expand Down Expand Up @@ -97,13 +118,12 @@ public static Optional<Nullness> fromAnnotationsOn(@Nullable Symbol sym) {
return fromElement;
}

return fromAnnotationStream(
MoreAnnotations.getDeclarationAndTypeAttributes(sym).map(Object::toString));
return fromAnnotationStream(MoreAnnotations.getDeclarationAndTypeAttributes(sym));
}

public static Optional<Nullness> fromAnnotationsOn(@Nullable TypeMirror type) {
if (type != null) {
return fromAnnotationList(type.getAnnotationMirrors());
return fromAnnotationStream(type.getAnnotationMirrors().stream());
}
return Optional.empty();
}
Expand All @@ -119,8 +139,7 @@ public static Optional<Nullness> fromDefaultAnnotations(@Nullable Element sym) {
// type annotations. For now we're just using a hard-coded simple name.
// TODO(b/121272440): Look for existing default annotations
if (sym.getAnnotationMirrors().stream()
.map(Object::toString)
.anyMatch(it -> it.endsWith(".DefaultNotNull"))) {
.anyMatch(a -> simpleName(a).contentEquals("DefaultNotNull"))) {
return Optional.of(Nullness.NONNULL);
}
sym = sym.getEnclosingElement();
Expand All @@ -141,9 +160,7 @@ public static Optional<Nullness> getUpperBound(TypeVariable typeVar) {
result =
fromAnnotationStream(
((IntersectionType) typeVar.getUpperBound())
.getBounds().stream()
.map(TypeMirror::getAnnotationMirrors)
.map(Object::toString));
.getBounds().stream().flatMap(t -> t.getAnnotationMirrors().stream()));
} else {
result = fromAnnotationsOn(typeVar.getUpperBound());
}
Expand All @@ -168,7 +185,7 @@ public static Optional<Nullness> getUpperBound(TypeVariable typeVar) {
typeParam.getSimpleName().equals(typeVar.asElement().getSimpleName()))
.findFirst()
// Annotations at class/interface/method type variable declaration
.flatMap(decl -> fromAnnotationList(decl.getAnnotationMirrors()));
.flatMap(decl -> fromAnnotationStream(decl.getAnnotationMirrors().stream()));
}
}

Expand All @@ -177,11 +194,12 @@ public static Optional<Nullness> getUpperBound(TypeVariable typeVar) {
return result.isPresent() ? result : fromDefaultAnnotations(typeVar.asElement());
}

private static Optional<Nullness> fromAnnotationList(List<?> annotations) {
return fromAnnotationStream(annotations.stream().map(Object::toString));
private static Optional<Nullness> fromAnnotationStream(
Stream<? extends AnnotationMirror> annotations) {
return fromAnnotationSimpleNames(annotations.map(a -> simpleName(a).toString()));
}

private static Optional<Nullness> fromAnnotationStream(Stream<String> annotations) {
private static Optional<Nullness> fromAnnotationSimpleNames(Stream<String> annotations) {
return annotations
.filter(ANNOTATION_RELEVANT_TO_NULLNESS)
.map(annot -> NULLABLE_ANNOTATION.test(annot) ? Nullness.NULLABLE : Nullness.NONNULL)
Expand Down
25 changes: 11 additions & 14 deletions .../java/com/google/errorprone/dataflow/nullnesspropagation/NullnessPropagationTransfer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
import java.util.Set;
import java.util.function.Predicate;
import javax.annotation.Nullable;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeVariable;
import org.checkerframework.errorprone.dataflow.analysis.Analysis;
Expand Down Expand Up @@ -203,7 +204,7 @@ private static class ReturnValueIsNonNull implements Predicate<MethodInfo>, Seri
public boolean test(MethodInfo methodInfo) {
// Any method explicitly annotated is trusted to behave as advertised.
Optional<Nullness> fromAnnotations =
NullnessAnnotations.fromAnnotations(methodInfo.annotations());
NullnessAnnotations.fromAnnotationMirrors(methodInfo.annotations());
if (fromAnnotations.isPresent()) {
return fromAnnotations.get() == NONNULL;
}
Expand Down Expand Up @@ -406,11 +407,8 @@ Nullness visitInstanceOf(

@Override
Nullness visitTypeCast(TypeCastNode node, SubNodeValues inputs) {
ImmutableList<String> annotations =
node.getType().getAnnotationMirrors().stream()
.map(Object::toString)
.collect(toImmutableList());
return NullnessAnnotations.fromAnnotations(annotations)
List<? extends AnnotationMirror> annotations = node.getType().getAnnotationMirrors();
return NullnessAnnotations.fromAnnotationMirrors(annotations)
.orElseGet(
() -> hasPrimitiveType(node) ? NONNULL : inputs.valueOfSubNode(node.getOperand()));
}
Expand Down Expand Up @@ -782,7 +780,8 @@ private Nullness returnValueNullness(MethodInvocationNode node, @Nullable ClassA
if (callee == null) {
return defaultAssumption;
}
Optional<Nullness> declaredNullness = NullnessAnnotations.fromAnnotations(callee.annotations);
Optional<Nullness> declaredNullness =
NullnessAnnotations.fromAnnotationMirrors(callee.annotations);
if (declaredNullness.isPresent()) {
return declaredNullness.get();
}
Expand Down Expand Up @@ -988,7 +987,7 @@ public int hashCode() {
static final class ClassAndMethod implements Member, MethodInfo {
final String clazz;
final String method;
final ImmutableList<String> annotations;
final ImmutableList<AnnotationMirror> annotations;
final boolean isStatic;
final boolean isPrimitive;
final boolean isBoolean;
Expand All @@ -998,7 +997,7 @@ static final class ClassAndMethod implements Member, MethodInfo {
private ClassAndMethod(
String clazz,
String method,
ImmutableList<String> annotations,
ImmutableList<AnnotationMirror> annotations,
boolean isStatic,
boolean isPrimitive,
boolean isBoolean,
Expand All @@ -1016,10 +1015,8 @@ private ClassAndMethod(

static ClassAndMethod make(MethodSymbol methodSymbol, @Nullable Types types) {
// TODO(b/71812955): consider just wrapping methodSymbol instead of copying everything out.
ImmutableList<String> annotations =
MoreAnnotations.getDeclarationAndTypeAttributes(methodSymbol)
.map(Object::toString)
.collect(toImmutableList());
ImmutableList<AnnotationMirror> annotations =
MoreAnnotations.getDeclarationAndTypeAttributes(methodSymbol).collect(toImmutableList());

ClassSymbol clazzSymbol = (ClassSymbol) methodSymbol.owner;
return new ClassAndMethod(
Expand Down Expand Up @@ -1087,7 +1084,7 @@ public String method() {
}

@Override
public ImmutableList<String> annotations() {
public ImmutableList<AnnotationMirror> annotations() {
return annotations;
}

Expand Down
2 changes: 1 addition & 1 deletion .../java/com/google/errorprone/dataflow/nullnesspropagation/TrustingNullnessPropagation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private enum TrustReturnAnnotation implements Predicate<MethodInfo> {

@Override
public boolean apply(MethodInfo input) {
return NullnessAnnotations.fromAnnotations(input.annotations()).orElse(Nullness.NONNULL)
return NullnessAnnotations.fromAnnotationMirrors(input.annotations()).orElse(Nullness.NONNULL)
== Nullness.NONNULL;
}
}
Expand Down
13 changes: 5 additions & 8 deletions core/src/main/java/com/google/errorprone/bugpatterns/nullness/VoidMissingNullable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.errorprone.bugpatterns.nullness;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAddingNullableAnnotationToReturnType;
import static com.google.errorprone.bugpatterns.nullness.NullnessUtils.fixByAddingNullableAnnotationToType;
Expand Down Expand Up @@ -44,8 +43,8 @@
import com.google.errorprone.dataflow.nullnesspropagation.NullnessAnnotations;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotatedTypeTree;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.ParameterizedTypeTree;
Expand All @@ -56,6 +55,7 @@
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Type;
import java.util.List;
import javax.inject.Inject;

/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
Expand Down Expand Up @@ -184,7 +184,7 @@ private void checkTree(Tree tree, VisitorState state) {
* TODO(cpovirk): Provide this pair of checks as NullnessAnnotations.fromAnnotationsOn(Tree),
* which might also be useful for a hypothetical future TypeArgumentMissingNullable?
*/
if (NullnessAnnotations.fromAnnotations(annotationsIfAnnotatedTypeTree(tree)).orElse(null)
if (NullnessAnnotations.fromAnnotationTrees(annotationsIfAnnotatedTypeTree(tree)).orElse(null)
== Nullness.NULLABLE) {
return;
}
Expand All @@ -209,12 +209,9 @@ private boolean typeMatches(Type type, VisitorState state) {
&& NullnessAnnotations.fromAnnotationsOn(type).orElse(null) != Nullness.NULLABLE;
}

private static ImmutableList<String> annotationsIfAnnotatedTypeTree(Tree tree) {
private static List<? extends AnnotationTree> annotationsIfAnnotatedTypeTree(Tree tree) {
if (tree instanceof AnnotatedTypeTree) {
AnnotatedTypeTree annotated = ((AnnotatedTypeTree) tree);
return annotated.getAnnotations().stream()
.map(ASTHelpers::getAnnotationName)
.collect(toImmutableList());
return ((AnnotatedTypeTree) tree).getAnnotations();
}
return ImmutableList.of();
}
Expand Down

0 comments on commit 99bfc0b

Please sign in to comment.