From 8ad800edde0e8c743666a5d814e86ecd6c6f39c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 5 Oct 2021 15:34:20 -0700 Subject: [PATCH] Ensure the order of copied annotations is deterministic. Fixes https://github.com/google/auto/issues/1176. RELNOTES=The order of annotations copied into generated code is now deterministic. PiperOrigin-RevId: 401086698 --- .../processor/AutoValueishProcessor.java | 7 ++- .../processor/AutoValueCompilationTest.java | 57 +++++++++++++++++++ .../processor/PropertyAnnotationsTest.java | 15 +++-- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java index ea08557552..55a94a7128 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java @@ -501,9 +501,9 @@ final void defineSharedVarsForType( /** Returns the spelling to be used in the generated code for the given list of annotations. */ static ImmutableList annotationStrings(List annotations) { - // TODO(b/68008628): use ImmutableList.toImmutableList() when that works. return annotations.stream() .map(AnnotationOutput::sourceFormForAnnotation) + .sorted() // ensures deterministic order .collect(toImmutableList()); } @@ -623,8 +623,9 @@ static Optional nullableAnnotationFor(Element element, TypeMirror elemen List elementAnnotations = element.getAnnotationMirrors(); OptionalInt nullableAnnotationIndex = nullableAnnotationIndex(elementAnnotations); if (nullableAnnotationIndex.isPresent()) { - ImmutableList annotations = annotationStrings(elementAnnotations); - return Optional.of(annotations.get(nullableAnnotationIndex.getAsInt()) + " "); + AnnotationMirror annotation = elementAnnotations.get(nullableAnnotationIndex.getAsInt()); + String annotationString = AnnotationOutput.sourceFormForAnnotation(annotation); + return Optional.of(annotationString + " "); } else { return Optional.empty(); } diff --git a/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java index ab6690fd33..9d7f78562b 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java @@ -3051,6 +3051,63 @@ public void visibleProtectedAnnotationFromOtherPackage() { .containsMatch("(?s:@Parent.ProtectedAnnotation\\s*@Override\\s*public String foo\\(\\))"); } + @Test + public void methodAnnotationsCopiedInLexicographicalOrder() { + JavaFileObject bazFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoValue;", + "import com.package1.Annotation1;", + "import com.package2.Annotation0;", + "", + "@AutoValue", + "public abstract class Baz extends Parent {", + " @Annotation0", + " @Annotation1", + " @Override", + " public abstract String foo();", + "}"); + JavaFileObject parentFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Parent", + "package foo.bar;", + "", + "public abstract class Parent {", + " public abstract String foo();", + "}"); + JavaFileObject annotation1FileObject = + JavaFileObjects.forSourceLines( + "com.package1.Annotation1", + "package com.package1;", + "", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "", + "@Target({ElementType.FIELD, ElementType.METHOD})", + "public @interface Annotation1 {}"); + JavaFileObject annotation0FileObject = + JavaFileObjects.forSourceLines( + "com.package2.Annotation0", + "package com.package2;", + "", + "public @interface Annotation0 {}"); + Compilation compilation = + javac() + .withProcessors(new AutoValueProcessor()) + .withOptions("-Xlint:-processing", "-implicit:none") + .compile(bazFileObject, parentFileObject, annotation1FileObject, annotation0FileObject); + assertThat(compilation).succeededWithoutWarnings(); + assertThat(compilation) + .generatedSourceFile("foo.bar.AutoValue_Baz") + .contentsAsUtf8String() + .containsMatch( + "(?s:@Annotation1\\s+@Annotation0\\s+@Override\\s+public String foo\\(\\))"); + // @Annotation1 precedes @Annotation 0 because + // @com.package2.Annotation1 precedes @com.package1.Annotation0 + } + @Test public void nonVisibleProtectedAnnotationFromOtherPackage() { JavaFileObject bazFileObject = diff --git a/value/src/test/java/com/google/auto/value/processor/PropertyAnnotationsTest.java b/value/src/test/java/com/google/auto/value/processor/PropertyAnnotationsTest.java index 48d8cd6e25..1d7e89f50b 100644 --- a/value/src/test/java/com/google/auto/value/processor/PropertyAnnotationsTest.java +++ b/value/src/test/java/com/google/auto/value/processor/PropertyAnnotationsTest.java @@ -510,11 +510,14 @@ public void testCopyingMethodAnnotations() { "@PropertyAnnotationsTest.InheritedAnnotation") .build(); + // Annotations are in lexicographical order of FQN: + // @com.google.auto.value.processor.PropertyAnnotationsTest.InheritedAnnotation precedes + // @java.lang.Deprecated JavaFileObject outputFile = new OutputFileBuilder() .setImports(imports) - .addMethodAnnotations("@Deprecated", "@PropertyAnnotationsTest.InheritedAnnotation") - .addFieldAnnotations("@Deprecated", "@PropertyAnnotationsTest.InheritedAnnotation") + .addMethodAnnotations("@PropertyAnnotationsTest.InheritedAnnotation", "@Deprecated") + .addFieldAnnotations("@PropertyAnnotationsTest.InheritedAnnotation", "@Deprecated") .build(); Compilation compilation = @@ -548,12 +551,16 @@ public void testCopyingMethodAnnotationsToGeneratedFields() { .addInnerTypes("@Target(ElementType.METHOD) @interface MethodsOnly {}") .build(); + // Annotations are in lexicographical order of FQN: + // @com.google.auto.value.processor.PropertyAnnotationsTest.InheritedAnnotation precedes + // @foo.bar.Baz.MethodsOnly precedes + // @java.lang.Deprecated JavaFileObject outputFile = new OutputFileBuilder() .setImports(getImports(PropertyAnnotationsTest.class)) - .addFieldAnnotations("@Deprecated", "@PropertyAnnotationsTest.InheritedAnnotation") + .addFieldAnnotations("@PropertyAnnotationsTest.InheritedAnnotation", "@Deprecated") .addMethodAnnotations( - "@Deprecated", "@PropertyAnnotationsTest.InheritedAnnotation", "@Baz.MethodsOnly") + "@PropertyAnnotationsTest.InheritedAnnotation", "@Baz.MethodsOnly", "@Deprecated") .build(); Compilation compilation =