TIL: There is a special value keyword for annotations

PicnicSupermarket · Mar 10, 2024 · f2f6a00 · f2f6a00
1 parent 3f3ae86
commit f2f6a00
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 20 deletions.
diff --git a/...src/main/java/tech/picnic/errorprone/bugpatterns/DuplicateAnnotationAttributeListing.java b/...src/main/java/tech/picnic/errorprone/bugpatterns/DuplicateAnnotationAttributeListing.java
@@ -97,7 +97,7 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
   private Optional<Fix> removeDuplicateAnnotationEntries(AnnotationTree tree, VisitorState state) {
     return matcher
         .extractMatchingArguments(tree)
-        .map(expr -> extractArray(expr).flatMap(arr -> removeDuplicates(arr, state)))
+        .map(expr -> extractArray(expr).flatMap(arr -> removeDuplicates(arr, tree, state)))
         .flatMap(Optional::stream)
         .reduce(SuggestedFix.Builder::merge)
         .map(SuggestedFix.Builder::build);
@@ -110,7 +110,7 @@ private static Optional<NewArrayTree> extractArray(ExpressionTree expr) {
   }
 
   private static Optional<SuggestedFix.Builder> removeDuplicates(
-      NewArrayTree array, VisitorState state) {
+      NewArrayTree array, AnnotationTree tree, VisitorState state) {
     if (array.getInitializers().size() < 2) {
       /* There's only one element, no duplicates are expected. */
       return Optional.empty();
@@ -125,11 +125,7 @@ private static Optional<SuggestedFix.Builder> removeDuplicates(
       return Optional.empty();
     }
 
-    // In the case of String entries, if after removing the duplicates in a listing array, there's
-    // only one element left, then we can omit the brackets.
-    boolean stringEntries =
-        nonDuplicateEntries.stream().map(Tree::getKind).allMatch(Tree.Kind.STRING_LITERAL::equals);
-    String prefix = stringEntries && nonDuplicateEntries.size() == 1 ? "" : "{";
+    String prefix = shouldOmitBrackets(tree, nonDuplicateEntries) ? "" : "{";
     String suffix = prefix.isBlank() ? "" : "}";
 
     String suggestion =
@@ -139,19 +135,6 @@ private static Optional<SuggestedFix.Builder> removeDuplicates(
     return Optional.of(SuggestedFix.builder().replace(array, suggestion));
   }
 
-  private static String extractName(VisitorState state, Tree expr) {
-    String exprString = SourceCode.treeToString(expr, state);
-
-    Symbol symbol = ASTHelpers.getSymbol(expr);
-    if (symbol != null) {
-      return (symbol.getKind() == INTERFACE || symbol.getKind() == CLASS)
-          ? exprString + Kind.CLASS.extension
-          : exprString;
-    }
-
-    return exprString;
-  }
-
   /**
    * Extracts from the given array element expression a distinct set of {@link Tree nodes} omitting
    * the duplicate entries.
@@ -235,6 +218,34 @@ private static ImmutableSet<Tree> getNonDuplicateEntries(
     return nodes.build();
   }
 
+  // In the case of entries defined for the special attribute name value, if after removing the
+  // duplicates in a listing array, there's only one element left, then we can omit the brackets.
+  private static boolean shouldOmitBrackets(
+      AnnotationTree tree, ImmutableSet<Tree> nonDuplicateEntries) {
+    boolean hasValueKeyword =
+        tree.getArguments().stream()
+            .filter(arg -> arg.getKind() == Tree.Kind.ASSIGNMENT)
+            .map(AssignmentTree.class::cast)
+            .map(AssignmentTree::getVariable)
+            .map(IdentifierTree.class::cast)
+            .map(IdentifierTree::getName)
+            .anyMatch(name -> name.contentEquals("value"));
+    return nonDuplicateEntries.size() == 1 && hasValueKeyword;
+  }
+
+  private static String extractName(VisitorState state, Tree expr) {
+    String exprString = SourceCode.treeToString(expr, state);
+
+    Symbol symbol = ASTHelpers.getSymbol(expr);
+    if (symbol != null) {
+      return (symbol.getKind() == INTERFACE || symbol.getKind() == CLASS)
+          ? exprString + Kind.CLASS.extension
+          : exprString;
+    }
+
+    return exprString;
+  }
+
   private static AnnotationAttributeMatcher createAnnotationAttributeMatcher(
       ErrorProneFlags flags) {
     return AnnotationAttributeMatcher.create(

diff --git a/...test/java/tech/picnic/errorprone/bugpatterns/DuplicateAnnotationAttributeListingTest.java b/...test/java/tech/picnic/errorprone/bugpatterns/DuplicateAnnotationAttributeListingTest.java
@@ -141,9 +141,16 @@ DuplicateAnnotationAttributeListing.class, getClass())
             "    String[] value() default {};",
             "  }",
             "",
+            "  @interface Baz {",
+            "    String[] declaredValue() default {};",
+            "  }",
+            "",
             "  @Foo({\"\", \" \", \"a\", \"a\"})",
             "  A duplicateString();",
             "",
+            "  @Baz(declaredValue = {\"a\", \"a\"})",
+            "  A duplicateStringWithNoValueKeyword();",
+            "",
             "  @Foo(cls = {A.class, A.class})",
             "  A duplicateClasses();",
             "",
@@ -177,9 +184,16 @@ DuplicateAnnotationAttributeListing.class, getClass())
             "    String[] value() default {};",
             "  }",
             "",
+            "  @interface Baz {",
+            "    String[] declaredValue() default {};",
+            "  }",
+            "",
             "  @Foo({\"\", \" \", \"a\"})",
             "  A duplicateString();",
             "",
+            "  @Baz(declaredValue = {\"a\"})",
+            "  A duplicateStringWithNoValueKeyword();",
+            "",
             "  @Foo(cls = {A.class})",
             "  A duplicateClasses();",
             "",