Have LexicographicalAnnotationListing sort TYPE_USE annotations last (#182)

This ensures compatibility with Error Prone's `AnnotationPosition`
check.

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListing.java

@@ -4,10 +4,13 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
 import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
 import static com.google.errorprone.BugPattern.StandardTags.STYLE;
+import static com.sun.tools.javac.code.TypeAnnotations.AnnotationType.DECLARATION;
+import static com.sun.tools.javac.code.TypeAnnotations.AnnotationType.TYPE;
 import static java.util.Comparator.comparing;
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
+import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Streams;
 import com.google.errorprone.BugPattern;
@@ -17,10 +20,14 @@ import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
 import com.google.errorprone.fixes.Fix;
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.AnnotationTree;
 import com.sun.source.tree.MethodTree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.TypeAnnotations.AnnotationType;
+import java.util.Comparator;
 import java.util.List;
-import java.util.Optional;
+import org.jspecify.nullness.Nullable;
 import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 
 /**
@@ -39,6 +46,9 @@ import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 public final class LexicographicalAnnotationListing extends BugChecker
     implements MethodTreeMatcher {
   private static final long serialVersionUID = 1L;
+  private static final Comparator<@Nullable AnnotationType> BY_ANNOTATION_TYPE =
+      (a, b) ->
+          (a == null || a == DECLARATION) && b == TYPE ? -1 : a == TYPE && b == DECLARATION ? 1 : 0;
 
   /** Instantiates a new {@link LexicographicalAnnotationListing} instance. */
   public LexicographicalAnnotationListing() {}
@@ -50,26 +60,29 @@ public final class LexicographicalAnnotationListing extends BugChecker
       return Description.NO_MATCH;
     }
 
-    ImmutableList<? extends AnnotationTree> sortedAnnotations = sort(originalOrdering, state);
+    ImmutableList<? extends AnnotationTree> sortedAnnotations =
+        sort(originalOrdering, ASTHelpers.getSymbol(tree), state);
     if (originalOrdering.equals(sortedAnnotations)) {
       return Description.NO_MATCH;
     }
 
-    Optional<Fix> fix = tryFixOrdering(originalOrdering, sortedAnnotations, state);
-
-    Description.Builder description = buildDescription(originalOrdering.get(0));
-    fix.ifPresent(description::addFix);
-    return description.build();
+    return describeMatch(
+        originalOrdering.get(0), fixOrdering(originalOrdering, sortedAnnotations, state));
   }
 
   private static ImmutableList<? extends AnnotationTree> sort(
-      List<? extends AnnotationTree> annotations, VisitorState state) {
+      List<? extends AnnotationTree> annotations, Symbol symbol, VisitorState state) {
     return annotations.stream()
-        .sorted(comparing(annotation -> SourceCode.treeToString(annotation, state)))
+        .sorted(
+            comparing(
+                    (AnnotationTree annotation) ->
+                        ASTHelpers.getAnnotationType(annotation, symbol, state),
+                    BY_ANNOTATION_TYPE)
+                .thenComparing(annotation -> SourceCode.treeToString(annotation, state)))
         .collect(toImmutableList());
   }
 
-  private static Optional<Fix> tryFixOrdering(
+  private static Fix fixOrdering(
       List<? extends AnnotationTree> originalAnnotations,
       ImmutableList<? extends AnnotationTree> sortedAnnotations,
       VisitorState state) {
@@ -80,6 +93,7 @@ public final class LexicographicalAnnotationListing extends BugChecker
                 SuggestedFix.builder()
                     .replace(original, SourceCode.treeToString(replacement, state)))
         .reduce(SuggestedFix.Builder::merge)
-        .map(SuggestedFix.Builder::build);
+        .map(SuggestedFix.Builder::build)
+        .orElseThrow(() -> new VerifyException("No annotations were provided"));
   }
 }

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingTest.java

@@ -1,7 +1,5 @@
 package tech.picnic.errorprone.bugpatterns;
 
-import static com.google.common.base.Predicates.containsPattern;
-
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
 import com.google.errorprone.CompilationTestHelper;
@@ -9,9 +7,7 @@ import org.junit.jupiter.api.Test;
 
 final class LexicographicalAnnotationListingTest {
   private final CompilationTestHelper compilationTestHelper =
-      CompilationTestHelper.newInstance(LexicographicalAnnotationListing.class, getClass())
-          .expectErrorMessage(
-              "X", containsPattern("Sort annotations lexicographically where possible"));
+      CompilationTestHelper.newInstance(LexicographicalAnnotationListing.class, getClass());
   private final BugCheckerRefactoringTestHelper refactoringTestHelper =
       BugCheckerRefactoringTestHelper.newInstance(
           LexicographicalAnnotationListing.class, getClass());
@@ -21,7 +17,9 @@ final class LexicographicalAnnotationListingTest {
     compilationTestHelper
         .addSourceLines(
             "A.java",
+            "import java.lang.annotation.ElementType;",
             "import java.lang.annotation.Repeatable;",
+            "import java.lang.annotation.Target;",
             "",
             "interface A {",
             "  @Repeatable(Foos.class)",
@@ -33,6 +31,7 @@ final class LexicographicalAnnotationListingTest {
             "    Bar[] anns() default {};",
             "  }",
             "",
+            "  @Target(ElementType.METHOD)",
             "  @interface Bar {",
             "    String[] value() default {};",
             "  }",
@@ -45,11 +44,21 @@ final class LexicographicalAnnotationListingTest {
             "    Foo[] value();",
             "  }",
             "",
-            "  // BUG: Diagnostic matches: X",
+            "  @Target(ElementType.TYPE_USE)",
+            "  @interface FooTypeUse {",
+            "    String[] value() default {};",
+            "  }",
+            "",
+            "  @Target(ElementType.TYPE_USE)",
+            "  @interface BarTypeUse {",
+            "    String[] value() default {};",
+            "  }",
+            "",
+            "  // BUG: Diagnostic contains:",
             "  @Foo",
             "  @Bar",
             "  A unsortedSimpleCase();",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Foo()",
             "  @Bar()",
             "  A unsortedWithParens();",
@@ -61,12 +70,12 @@ final class LexicographicalAnnotationListingTest {
             "  @Foo()",
             "  A sortedAnnotationsOneWithParens();",
             "",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Foo",
             "  @Baz",
             "  @Bar",
             "  A threeUnsortedAnnotationsSameInitialLetter();",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Bar",
             "  @Foo()",
             "  @Baz",
@@ -77,16 +86,16 @@ final class LexicographicalAnnotationListingTest {
             "  @Foo()",
             "  A threeSortedAnnotations();",
             "",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Foo({\"b\"})",
             "  @Bar({\"a\"})",
             "  A unsortedWithStringAttributes();",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Baz(str = {\"a\", \"b\"})",
             "  @Foo(ints = {1, 0})",
             "  @Bar",
             "  A unsortedWithAttributes();",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Bar",
             "  @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})",
             "  @Baz",
@@ -101,11 +110,23 @@ final class LexicographicalAnnotationListingTest {
             "  @Foo(ints = {1, 2})",
             "  @Foo({\"b\"})",
             "  A sortedRepeatableAnnotation();",
-            "  // BUG: Diagnostic matches: X",
+            "  // BUG: Diagnostic contains:",
             "  @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})",
             "  @Bar",
             "  @Foo(ints = {1, 2})",
             "  A unsortedRepeatableAnnotation();",
+            "",
+            "  // BUG: Diagnostic contains:",
+            "  default @FooTypeUse @BarTypeUse A unsortedTypeAnnotations() {",
+            "    return null;",
+            "  }",
+            "",
+            "  // BUG: Diagnostic contains:",
+            "  @Baz",
+            "  @Bar",
+            "  default @FooTypeUse @BarTypeUse A unsortedTypeUseAndOtherAnnotations() {",
+            "    return null;",
+            "  }",
             "}")
         .doTest();
   }
@@ -115,7 +136,9 @@ final class LexicographicalAnnotationListingTest {
     refactoringTestHelper
         .addInputLines(
             "A.java",
+            "import java.lang.annotation.ElementType;",
             "import java.lang.annotation.Repeatable;",
+            "import java.lang.annotation.Target;",
             "",
             "interface A {",
             "  @Repeatable(Foos.class)",
@@ -127,6 +150,7 @@ final class LexicographicalAnnotationListingTest {
             "    Bar[] anns() default {};",
             "  }",
             "",
+            "  @Target(ElementType.METHOD)",
             "  @interface Bar {",
             "    String[] value() default {};",
             "  }",
@@ -139,6 +163,16 @@ final class LexicographicalAnnotationListingTest {
             "    Foo[] value();",
             "  }",
             "",
+            "  @Target(ElementType.TYPE_USE)",
+            "  @interface FooTypeUse {",
+            "    String[] value() default {};",
+            "  }",
+            "",
+            "  @Target(ElementType.TYPE_USE)",
+            "  @interface BarTypeUse {",
+            "    String[] value() default {};",
+            "  }",
+            "",
             "  @Bar",
             "  A singleAnnotation();",
             "",
@@ -174,10 +208,18 @@ final class LexicographicalAnnotationListingTest {
             "  @Bar",
             "  @Foo(ints = {1, 2})",
             "  A unsortedRepeatableAnnotation();",
+            "",
+            "  @Baz",
+            "  @Bar",
+            "  default @FooTypeUse @BarTypeUse A unsortedWithTypeUseAnnotations() {",
+            "    return null;",
+            "  }",
             "}")
         .addOutputLines(
             "A.java",
+            "import java.lang.annotation.ElementType;",
             "import java.lang.annotation.Repeatable;",
+            "import java.lang.annotation.Target;",
             "",
             "interface A {",
             "  @Repeatable(Foos.class)",
@@ -189,6 +231,7 @@ final class LexicographicalAnnotationListingTest {
             "    Bar[] anns() default {};",
             "  }",
             "",
+            "  @Target(ElementType.METHOD)",
             "  @interface Bar {",
             "    String[] value() default {};",
             "  }",
@@ -201,6 +244,16 @@ final class LexicographicalAnnotationListingTest {
             "    Foo[] value();",
             "  }",
             "",
+            "  @Target(ElementType.TYPE_USE)",
+            "  @interface FooTypeUse {",
+            "    String[] value() default {};",
+            "  }",
+            "",
+            "  @Target(ElementType.TYPE_USE)",
+            "  @interface BarTypeUse {",
+            "    String[] value() default {};",
+            "  }",
+            "",
             "  @Bar",
             "  A singleAnnotation();",
             "",
@@ -236,6 +289,12 @@ final class LexicographicalAnnotationListingTest {
             "  @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})",
             "  @Foo(ints = {1, 2})",
             "  A unsortedRepeatableAnnotation();",
+            "",
+            "  @Bar",
+            "  @Baz",
+            "  default @BarTypeUse @FooTypeUse A unsortedWithTypeUseAnnotations() {",
+            "    return null;",
+            "  }",
             "}")
         .doTest(TestMode.TEXT_MATCH);
   }