Ensure the order of copied annotations is deterministic.

Fixes #1176. RELNOTES=The order of annotations copied into generated code is now deterministic. PiperOrigin-RevId: 401086698
google · Oct 5, 2021 · 8ad800e · 8ad800e
1 parent 206b673
commit 8ad800e
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 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
@@ -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<String> annotationStrings(List<? extends AnnotationMirror> 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<String> nullableAnnotationFor(Element element, TypeMirror elemen
     List<? extends AnnotationMirror> elementAnnotations = element.getAnnotationMirrors();
     OptionalInt nullableAnnotationIndex = nullableAnnotationIndex(elementAnnotations);
     if (nullableAnnotationIndex.isPresent()) {
-      ImmutableList<String> 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
@@ -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
@@ -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 =