Skip to content

Commit 1858d35

Browse files
cushonGoogle Java Core Libraries
authored andcommitted
Fix type-use annotation duplication on nested classes and arrays
Type-use annotations placed at the beginning of a return type (e.g. `@Nullable Enclosing.Inner` or `@Nullable byte[]`) are associated with the left-most type—the enclosing class type and array component type, respectively. AutoValue's getReturnTypeAnnotations previously only checked annotations on the root return type (e.g. `Inner` or `byte[]`), failing to check annotations on the left-most type as return type. As a result, it treated them as method declaration annotations and duplicated them onto the generated method. This fix updates getReturnTypeAnnotations to inspect the left-most type (unwrapping enclosing declared types and array component types) so type annotations are correctly excluded from being copied to method declarations. RELNOTES=n/a PiperOrigin-RevId: 960390407
1 parent a1751e3 commit 1858d35

4 files changed

Lines changed: 109 additions & 14 deletions

File tree

value/src/main/java/com/google/auto/value/processor/AnnotatedTypeMirror.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@
1515
*/
1616
package com.google.auto.value.processor;
1717

18+
import static com.google.auto.common.MoreTypes.asArray;
19+
import static com.google.auto.common.MoreTypes.asDeclared;
20+
1821
import com.google.common.base.Joiner;
1922
import com.google.common.collect.ImmutableList;
2023
import java.util.Objects;
2124
import javax.lang.model.element.AnnotationMirror;
2225
import javax.lang.model.type.TypeKind;
2326
import javax.lang.model.type.TypeMirror;
27+
import org.jspecify.annotations.Nullable;
2428

2529
/**
2630
* A {@link TypeMirror} and associated annotations.
@@ -89,6 +93,41 @@ public String toString() {
8993
return annotations.isEmpty() ? rewrittenType.toString() : annotations + " " + rewrittenType;
9094
}
9195

96+
@Nullable AnnotatedTypeMirror getEnclosingType() {
97+
if (originalType.getKind().equals(TypeKind.DECLARED)) {
98+
TypeMirror originalEnclosing = EclipseHack.getEnclosingType(asDeclared(originalType));
99+
if (originalEnclosing.getKind().equals(TypeKind.DECLARED)) {
100+
TypeMirror rewrittenEnclosing = EclipseHack.getEnclosingType(asDeclared(rewrittenType));
101+
return new AnnotatedTypeMirror(originalEnclosing, rewrittenEnclosing);
102+
}
103+
}
104+
return null;
105+
}
106+
107+
@Nullable AnnotatedTypeMirror getComponentType() {
108+
if (originalType.getKind().equals(TypeKind.ARRAY)) {
109+
TypeMirror originalComponent = asArray(originalType).getComponentType();
110+
TypeMirror rewrittenComponent = asArray(rewrittenType).getComponentType();
111+
return new AnnotatedTypeMirror(originalComponent, rewrittenComponent);
112+
}
113+
return null;
114+
}
115+
116+
AnnotatedTypeMirror leftMostType() {
117+
AnnotatedTypeMirror cur = this;
118+
while (cur.getKind().equals(TypeKind.ARRAY)) {
119+
cur = cur.getComponentType();
120+
}
121+
while (cur.getKind().equals(TypeKind.DECLARED)) {
122+
AnnotatedTypeMirror enclosing = cur.getEnclosingType();
123+
if (enclosing == null) {
124+
break;
125+
}
126+
cur = enclosing;
127+
}
128+
return cur;
129+
}
130+
92131
@Override
93132
public boolean equals(Object obj) {
94133
if (obj instanceof AnnotatedTypeMirror) {

value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,9 @@ ImmutableList<AnnotationMirror> propertyMethodAnnotations(
12361236

12371237
// We need to exclude type annotations from the ones being output on the method, since
12381238
// they will be output as part of the method's return type.
1239-
Set<String> returnTypeAnnotations = getReturnTypeAnnotations(method, a -> true);
1239+
AnnotatedTypeMirror returnType =
1240+
MethodSignature.asMemberOf(typeUtils(), type, method).returnType();
1241+
Set<String> returnTypeAnnotations = getReturnTypeAnnotations(returnType, a -> true);
12401242
Set<String> excluded = union(excludedAnnotations, returnTypeAnnotations);
12411243
return annotationsToCopy(type, method, excluded);
12421244
}
@@ -1253,10 +1255,12 @@ final ImmutableListMultimap<ExecutableElement, AnnotationMirror> propertyFieldAn
12531255

12541256
private ImmutableList<AnnotationMirror> propertyFieldAnnotations(
12551257
TypeElement type, ExecutableElement method) {
1258+
AnnotatedTypeMirror returnType =
1259+
MethodSignature.asMemberOf(typeUtils(), type, method).returnType();
12561260
// We need to exclude type annotations from the ones being output on the method, since
12571261
// they will be output as part of the field's type.
12581262
Set<String> returnTypeAnnotations =
1259-
getReturnTypeAnnotations(method, this::annotationAppliesToFields);
1263+
getReturnTypeAnnotations(returnType, this::annotationAppliesToFields);
12601264
if (!hasAnnotationMirror(method, COPY_ANNOTATIONS_NAME)) {
12611265
// If there's no @CopyAnnotations, we will still copy a @Nullable annotation, if (1) it is not
12621266
// a TYPE_USE annotation (those appear as part of the type in the generated code) and (2) it
@@ -1295,9 +1299,19 @@ private ImmutableList<AnnotationMirror> propertyFieldAnnotations(
12951299
return annotationsToCopy(type, method, excluded);
12961300
}
12971301

1302+
/**
1303+
* Returns annotations that appear at the start of the given method return type. For example, if
1304+
* the method is {@code abstract @NotNull Outer.@Nullable Inner foo()} then the method will return
1305+
* a set containing {@code @NotNull}. This is to prevent outputting {@code @NotNull} a second time
1306+
* if, for example, that annotation targets methods as well as types. Normally we copy annotations
1307+
* from abstract methods to their implementations, but here we would not want to because we are
1308+
* already outputting {@code @NotNull} as part of the return type. In the example, we don't need
1309+
* to return {@code @Nullable} because it is unambiguously a type annotation and cannot be
1310+
* interpreted as applying to the method.
1311+
*/
12981312
private static Set<String> getReturnTypeAnnotations(
1299-
ExecutableElement method, Predicate<TypeElement> typeFilter) {
1300-
return method.getReturnType().getAnnotationMirrors().stream()
1313+
AnnotatedTypeMirror methodReturnType, Predicate<TypeElement> typeFilter) {
1314+
return methodReturnType.leftMostType().annotations().stream()
13011315
.map(a -> a.getAnnotationType().asElement())
13021316
.map(MoreElements::asType)
13031317
.filter(typeFilter)

value/src/main/java/com/google/auto/value/processor/TypeEncoder.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -330,13 +330,17 @@ void appendTypeName(DeclaredType type, StringBuilder sb) {
330330
// add them with appendTypeArguments below. Of course, it's more usual for the outer class
331331
// not to have type arguments, but we'll still follow this path if the nested class is an
332332
// inner (not static) class.
333-
visit2(enclosing, sb);
333+
visitEnclosingDeclared(MoreTypes.asDeclared(enclosing), sb);
334334
sb.append(".").append(type.asElement().getSimpleName());
335335
} else {
336336
sb.append('`').append(className(type)).append('`');
337337
}
338338
}
339339

340+
void visitEnclosingDeclared(DeclaredType type, StringBuilder sb) {
341+
visit2(type, sb);
342+
}
343+
340344
void appendTypeArguments(DeclaredType type, StringBuilder sb) {
341345
List<? extends TypeMirror> arguments = type.getTypeArguments();
342346
if (!arguments.isEmpty()) {
@@ -465,11 +469,14 @@ public StringBuilder visitDeclared(DeclaredType type, StringBuilder sb) {
465469
return sb;
466470
}
467471

468-
private void visitEnclosingDeclared(DeclaredType enclosing, StringBuilder sb) {
472+
@Override
473+
void visitEnclosingDeclared(DeclaredType enclosing, StringBuilder sb) {
469474
List<? extends AnnotationMirror> annotationMirrors = getTypeAnnotations.apply(enclosing);
470-
// Enclosing instances should NEVER have @NotNull annotations.
475+
// Enclosing instances should NEVER have @NotNull or @Nullable annotations.
471476
annotationMirrors =
472-
annotationMirrors.stream().filter(a -> !Nullables.isNonNull(a)).collect(toList());
477+
annotationMirrors.stream()
478+
.filter(a -> !Nullables.isNonNull(a) && !Nullables.isNullable(a))
479+
.collect(toList());
473480
if (annotationMirrors.isEmpty()) {
474481
super.visitDeclared(enclosing, sb);
475482
} else {

value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4322,13 +4322,27 @@ public void notNullTypeUseAnnotationOnOuterClass() {
43224322
"class Outer {",
43234323
" class Inner {}",
43244324
"}");
4325-
// TODO(b/540040170): Fix the duplicate annotation bug that causes compilation failure.
43264325
Compilation compilation =
43274326
javac()
43284327
.withProcessors(new AutoValueProcessor(), new AutoValueBuilderProcessor())
43294328
.compile(javaFileObject);
4329+
assertThat(compilation).succeeded();
43304330
assertThat(compilation)
4331-
.hadErrorContaining("foo.bar.Baz.NotNull is not a repeatable annotation");
4331+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4332+
.contentsAsUtf8String()
4333+
.contains(
4334+
" @Override\n"
4335+
+ " Outer.Inner inner() {\n"
4336+
+ " return inner;\n"
4337+
+ " }\n");
4338+
assertThat(compilation)
4339+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4340+
.contentsAsUtf8String()
4341+
.doesNotContain("@Baz.NotNull\n @Override");
4342+
assertThat(compilation)
4343+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4344+
.contentsAsUtf8String()
4345+
.contains("private Outer.@Nullable Inner inner;");
43324346
}
43334347

43344348
@Test
@@ -4412,13 +4426,27 @@ public void nullableTypeUseAnnotationOnOuterClass() {
44124426
"class Outer {",
44134427
" class Inner {}",
44144428
"}");
4415-
// TODO(b/540040170): Fix the duplicate annotation bug that causes compilation failure.
44164429
Compilation compilation =
44174430
javac()
44184431
.withProcessors(new AutoValueProcessor(), new AutoValueBuilderProcessor())
44194432
.compile(javaFileObject);
4433+
assertThat(compilation).succeeded();
44204434
assertThat(compilation)
4421-
.hadErrorContaining("foo.bar.Baz.Nullable is not a repeatable annotation");
4435+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4436+
.contentsAsUtf8String()
4437+
.contains(
4438+
" @Override\n"
4439+
+ " Outer.Inner inner() {\n"
4440+
+ " return inner;\n"
4441+
+ " }\n");
4442+
assertThat(compilation)
4443+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4444+
.contentsAsUtf8String()
4445+
.doesNotContain("@Baz.Nullable\n @Override");
4446+
assertThat(compilation)
4447+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4448+
.contentsAsUtf8String()
4449+
.contains("private Outer.Inner inner;");
44224450
}
44234451

44244452
@Test
@@ -4453,13 +4481,20 @@ public void notNullTypeUseAnnotationOnArray() {
44534481
" public abstract Baz build();",
44544482
" }",
44554483
"}");
4456-
// TODO(b/540040170): Fix the duplicate annotation bug that causes compilation failure.
44574484
Compilation compilation =
44584485
javac()
44594486
.withProcessors(new AutoValueProcessor(), new AutoValueBuilderProcessor())
44604487
.compile(javaFileObject);
4488+
assertThat(compilation).succeeded();
44614489
assertThat(compilation)
4462-
.hadErrorContaining("foo.bar.Baz.NotNull is not a repeatable annotation");
4490+
.generatedSourceFile("foo.bar.AutoValue_Baz")
4491+
.contentsAsUtf8String()
4492+
.contains(
4493+
" @SuppressWarnings(\"mutable\")\n"
4494+
+ " @Override\n"
4495+
+ " @Baz.NotNull byte[] bytes() {\n"
4496+
+ " return bytes;\n"
4497+
+ " }\n");
44634498
}
44644499

44654500
private static String sorted(String... imports) {

0 commit comments

Comments
 (0)