Skip to content

Commit

Permalink
Directly return an AnnotationMirror for @Nullable, rather than a …
Browse files Browse the repository at this point in the history
…`DeclaredType`.

The old logic was unnecessarily tortuous, extracting the `DeclaredType` for the `@Nullable` annotation we found, then later synthesizing a new `AnnotationMirror` for that type. We can just use the original `AnnotationMirror`.

Later we will probably introduce logic to use the [JSpecify](http://jspecify.org) `@Nullable` if it is available and if the `@AutoValue` class doesn't mention its own `@Nullable`. At that point we can go back to synthesizing an `AnnotationMirror`, but only for that particular case.

RELNOTES=n/a
PiperOrigin-RevId: 364834935
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Mar 24, 2021
1 parent 4d01ce6 commit 0b4fd6a
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ final void defineSharedVarsForType(
vars.toString = methodsToGenerate.containsKey(ObjectMethod.TO_STRING);
vars.equals = methodsToGenerate.containsKey(ObjectMethod.EQUALS);
vars.hashCode = methodsToGenerate.containsKey(ObjectMethod.HASH_CODE);
Optional<DeclaredType> nullable = Nullables.nullableMentionedInMethods(methods);
Optional<AnnotationMirror> nullable = Nullables.nullableMentionedInMethods(methods);
vars.equalsParameterType = equalsParameterType(methodsToGenerate, nullable);
vars.serialVersionUID = getSerialVersionUID(type);
}
Expand Down Expand Up @@ -710,7 +710,7 @@ private static Map<ObjectMethod, ExecutableElement> determineObjectMethodsToGene
* @param nullable the type of a {@code @Nullable} type annotation that we have found, if any
*/
static String equalsParameterType(
Map<ObjectMethod, ExecutableElement> methodsToGenerate, Optional<DeclaredType> nullable) {
Map<ObjectMethod, ExecutableElement> methodsToGenerate, Optional<AnnotationMirror> nullable) {
ExecutableElement equals = methodsToGenerate.get(ObjectMethod.EQUALS);
if (equals == null) {
return ""; // this will not be referenced because no equals method will be generated
Expand All @@ -721,26 +721,11 @@ static String equalsParameterType(
// then that might be a type annotation or an annotation on the parameter.
ImmutableList<AnnotationMirror> extraAnnotations =
nullable.isPresent() && !nullableAnnotationFor(equals, parameterType).isPresent()
? ImmutableList.of(annotationMirror(nullable.get()))
? ImmutableList.of(nullable.get())
: ImmutableList.of();
return TypeEncoder.encodeWithAnnotations(parameterType, extraAnnotations, ImmutableSet.of());
}

private static AnnotationMirror annotationMirror(DeclaredType annotationType) {
return new AnnotationMirror() {
@Override
public DeclaredType getAnnotationType() {
return annotationType;
}

@Override
public ImmutableMap<? extends ExecutableElement, ? extends AnnotationValue>
getElementValues() {
return ImmutableMap.of();
}
};
}

/**
* Returns the subset of all abstract methods in the given set of methods. A given method
* signature is only mentioned once, even if it is inherited on more than one path. If any of the
Expand Down
26 changes: 14 additions & 12 deletions value/src/main/java/com/google/auto/value/processor/Nullables.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class Nullables {
* Returns the type of a {@code @Nullable} type-annotation, if one is found anywhere in the
* signatures of the given methods.
*/
static Optional<DeclaredType> nullableMentionedInMethods(Collection<ExecutableElement> methods) {
static Optional<AnnotationMirror> nullableMentionedInMethods(
Collection<ExecutableElement> methods) {
return methods.stream()
.flatMap(
method ->
Expand All @@ -52,18 +53,19 @@ static Optional<DeclaredType> nullableMentionedInMethods(Collection<ExecutableEl
.orElse(Optional.empty());
}

private static Optional<DeclaredType> nullableIn(TypeMirror type) {
private static Optional<AnnotationMirror> nullableIn(TypeMirror type) {
return new NullableFinder().visit(type);
}

private static Optional<DeclaredType> nullableIn(List<? extends AnnotationMirror> annotations) {
private static Optional<AnnotationMirror> nullableIn(
List<? extends AnnotationMirror> annotations) {
return annotations.stream()
.map(AnnotationMirror::getAnnotationType)
.filter(t -> t.asElement().getSimpleName().contentEquals("Nullable"))
.filter(a -> a.getAnnotationType().asElement().getSimpleName().contentEquals("Nullable"))
.map(a -> (AnnotationMirror) a) // get rid of the pesky wildcard
.findFirst();
}

private static class NullableFinder extends SimpleTypeVisitor8<Optional<DeclaredType>, Void> {
private static class NullableFinder extends SimpleTypeVisitor8<Optional<AnnotationMirror>, Void> {
private final TypeMirrorSet visiting = new TypeMirrorSet();

NullableFinder() {
Expand All @@ -73,7 +75,7 @@ private static class NullableFinder extends SimpleTypeVisitor8<Optional<Declared
// Primitives can't be @Nullable so we don't check that.

@Override
public Optional<DeclaredType> visitDeclared(DeclaredType t, Void unused) {
public Optional<AnnotationMirror> visitDeclared(DeclaredType t, Void unused) {
if (!visiting.add(t)) {
return Optional.empty();
}
Expand All @@ -83,7 +85,7 @@ public Optional<DeclaredType> visitDeclared(DeclaredType t, Void unused) {
}

@Override
public Optional<DeclaredType> visitTypeVariable(TypeVariable t, Void unused) {
public Optional<AnnotationMirror> visitTypeVariable(TypeVariable t, Void unused) {
if (!visiting.add(t)) {
return Optional.empty();
}
Expand All @@ -93,14 +95,14 @@ public Optional<DeclaredType> visitTypeVariable(TypeVariable t, Void unused) {
}

@Override
public Optional<DeclaredType> visitArray(ArrayType t, Void unused) {
public Optional<AnnotationMirror> visitArray(ArrayType t, Void unused) {
return nullableIn(t.getAnnotationMirrors())
.map(Optional::of)
.orElseGet(() -> visit(t.getComponentType()));
}

@Override
public Optional<DeclaredType> visitWildcard(WildcardType t, Void unused) {
public Optional<AnnotationMirror> visitWildcard(WildcardType t, Void unused) {
return nullableIn(t.getAnnotationMirrors())
.map(Optional::of)
.orElseGet(
Expand All @@ -112,13 +114,13 @@ public Optional<DeclaredType> visitWildcard(WildcardType t, Void unused) {
}

@Override
public Optional<DeclaredType> visitIntersection(IntersectionType t, Void unused) {
public Optional<AnnotationMirror> visitIntersection(IntersectionType t, Void unused) {
return nullableIn(t.getAnnotationMirrors())
.map(Optional::of)
.orElseGet(() -> visitAll(t.getBounds()));
}

private Optional<DeclaredType> visitAll(List<? extends TypeMirror> types) {
private Optional<AnnotationMirror> visitAll(List<? extends TypeMirror> types) {
return types.stream()
.map(this::visit)
.filter(Optional::isPresent)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import javax.annotation.processing.RoundEnvironment;
import javax.annotation.processing.SupportedAnnotationTypes;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.DeclaredType;
Expand Down Expand Up @@ -165,7 +166,9 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
expect
.withMessage("method %s should have @Nullable", nullableMethod)
.about(optionals())
.that(Nullables.nullableMentionedInMethods(notNullablePlusNullable))
.that(
Nullables.nullableMentionedInMethods(notNullablePlusNullable)
.map(AnnotationMirror::getAnnotationType))
.hasValue(nullableType);
}
ran = true;
Expand Down

0 comments on commit 0b4fd6a

Please sign in to comment.