AutoValueFinalMethods: support method-level suppression.

And streamify the implementation a bit. PiperOrigin-RevId: 589088083
google · Dec 8, 2023 · ac52ca9 · ac52ca9
1 parent 336323a
commit ac52ca9
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 33 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueFinalMethods.java b/core/src/main/java/com/google/errorprone/bugpatterns/AutoValueFinalMethods.java
@@ -16,13 +16,14 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
 import static com.google.errorprone.matchers.Matchers.allOf;
 import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.not;
+import static java.util.stream.Collectors.joining;
 
-import com.google.common.base.Joiner;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
@@ -34,10 +35,6 @@
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.MethodTree;
-import com.sun.source.tree.Tree;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Optional;
 import javax.lang.model.element.Modifier;
 
 /**
@@ -71,34 +68,31 @@ public Description matchClass(ClassTree tree, VisitorState state) {
     if (!ASTHelpers.hasAnnotation(tree, "com.google.auto.value.AutoValue", state)) {
       return NO_MATCH;
     }
-    SuggestedFix.Builder fix = SuggestedFix.builder();
-    List<String> matchedMethods = new ArrayList<>();
-    MethodTree firstMatchedMethod = null;
-    for (Tree memberTree : tree.getMembers()) {
-      if (!(memberTree instanceof MethodTree)) {
-        continue;
-      }
-      MethodTree method = (MethodTree) memberTree;
-      if (METHOD_MATCHER.matches(method, state)) {
-        Optional<SuggestedFix> optionalSuggestedFix =
-            SuggestedFixes.addModifiers(method, state, Modifier.FINAL);
-        if (optionalSuggestedFix.isPresent()) {
-          matchedMethods.add(method.getName().toString());
-          fix.merge(optionalSuggestedFix.get());
-          if (firstMatchedMethod == null) {
-            firstMatchedMethod = method;
-          }
-        }
-      }
-    }
-    if (!fix.isEmpty()) {
-      String message =
-          String.format(
-              "Make %s final in AutoValue classes, "
-                  + "so it is clear to readers that AutoValue is not overriding them",
-              Joiner.on(", ").join(matchedMethods));
-      return buildDescription(firstMatchedMethod).setMessage(message).addFix(fix.build()).build();
+    var candidateMethods =
+        tree.getMembers().stream()
+            .filter(
+                t ->
+                    t instanceof MethodTree
+                        && METHOD_MATCHER.matches((MethodTree) t, state)
+                        && !isSuppressed(t, state))
+            .map(t -> (MethodTree) t)
+            .collect(toImmutableList());
+
+    var fix =
+        candidateMethods.stream()
+            .flatMap(t -> SuggestedFixes.addModifiers(t, state, Modifier.FINAL).stream())
+            .reduce(SuggestedFix.emptyFix(), SuggestedFix::merge);
+
+    if (fix.isEmpty()) {
+      return NO_MATCH;
     }
-    return NO_MATCH;
+    return buildDescription(candidateMethods.get(0))
+        .setMessage(
+            String.format(
+                "Make %s final in AutoValue classes, "
+                    + "so it is clear to readers that AutoValue is not overriding them",
+                candidateMethods.stream().map(t -> t.getName().toString()).collect(joining(", "))))
+        .addFix(fix)
+        .build();
   }
 }
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueFinalMethodsTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AutoValueFinalMethodsTest.java
@@ -86,6 +86,25 @@ public void finalAdditionToEqHcTs() {
         .doTest();
   }
 
+  @Test
+  public void suppression() {
+    compilationHelper
+        .addSourceLines(
+            "in/Test.java",
+            "import com.google.auto.value.AutoValue;",
+            "@AutoValue",
+            "abstract class Test {",
+            "  abstract String valueOne();",
+            "  abstract String valueTwo();",
+            "  @SuppressWarnings(\"AutoValueFinalMethods\")",
+            "  @Override",
+            "  public int hashCode() {",
+            "    return 1;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void negativeCases() {
     compilationHelper