UnusedMethod: Support additional exempting method annotations

Fixes #3144 & #3297. Fixes #3428 FUTURE_COPYBARA_INTEGRATE_REVIEW=#3428 from PicnicSupermarket:rossendrijver/unusedmethod_improvements 99f8878 PiperOrigin-RevId: 495640502
google · Dec 19, 2022 · 324780d · 324780d
1 parent bdfd015
commit 324780d
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 4 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java
@@ -45,6 +45,7 @@
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.BugPattern;
+import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
@@ -97,30 +98,63 @@ public final class UnusedMethod extends BugChecker implements CompilationUnitTre
   private static final ImmutableSet<String> EXEMPTING_METHOD_ANNOTATIONS =
       ImmutableSet.of(
           "com.fasterxml.jackson.annotation.JsonCreator",
+          "com.fasterxml.jackson.annotation.JsonValue",
           "com.google.inject.Provides",
           "com.google.inject.Inject",
           "com.google.inject.multibindings.ProvidesIntoMap",
           "com.google.inject.multibindings.ProvidesIntoSet",
           "com.tngtech.java.junit.dataprovider.DataProvider",
+          "jakarta.annotation.PreDestroy",
+          "jakarta.annotation.PostConstruct",
+          "jakarta.inject.Inject",
+          "jakarta.persistence.PostLoad",
+          "jakarta.persistence.PostPersist",
+          "jakarta.persistence.PostRemove",
+          "jakarta.persistence.PostUpdate",
+          "jakarta.persistence.PrePersist",
+          "jakarta.persistence.PreRemove",
+          "jakarta.persistence.PreUpdate",
           "javax.annotation.PreDestroy",
           "javax.annotation.PostConstruct",
           "javax.inject.Inject",
           "javax.persistence.PostLoad",
+          "javax.persistence.PostPersist",
+          "javax.persistence.PostRemove",
+          "javax.persistence.PostUpdate",
+          "javax.persistence.PrePersist",
+          "javax.persistence.PreRemove",
+          "javax.persistence.PreUpdate",
           "org.apache.beam.sdk.transforms.DoFn.ProcessElement",
           "org.aspectj.lang.annotation.Pointcut",
+          "org.aspectj.lang.annotation.After",
           "org.aspectj.lang.annotation.Before",
           "org.springframework.context.annotation.Bean",
           "org.testng.annotations.AfterClass",
           "org.testng.annotations.AfterMethod",
           "org.testng.annotations.BeforeClass",
           "org.testng.annotations.BeforeMethod",
           "org.testng.annotations.DataProvider",
-          "org.junit.AfterClass",
-          "org.junit.BeforeClass");
+          "org.junit.jupiter.api.BeforeAll",
+          "org.junit.jupiter.api.AfterAll",
+          "org.junit.jupiter.api.AfterEach",
+          "org.junit.jupiter.api.BeforeEach",
+          "org.junit.jupiter.api.RepeatedTest",
+          "org.junit.jupiter.api.Test",
+          "org.junit.jupiter.params.ParameterizedTest");
 
   /** The set of types exempting a type that is extending or implementing them. */
   private static final ImmutableSet<String> EXEMPTING_SUPER_TYPES = ImmutableSet.of();
 
+  private final ImmutableSet<String> additionalExemptingMethodAnnotations;
+
+  public UnusedMethod(ErrorProneFlags errorProneFlags) {
+    this.additionalExemptingMethodAnnotations =
+        errorProneFlags
+            .getList("UnusedMethod:ExemptingMethodAnnotations")
+            .map(ImmutableSet::copyOf)
+            .orElseGet(ImmutableSet::of);
+  }
+
   @Override
   public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
     // Map of symbols to method declarations. Initially this is a map of all of the methods. As we
@@ -430,14 +464,16 @@ public Void visitMethod(MethodTree tree, Void unused) {
    * Looks at the list of {@code annotations} and see if there is any annotation which exists {@code
    * exemptingAnnotations}.
    */
-  private static boolean exemptedByAnnotation(List<? extends AnnotationTree> annotations) {
+  private boolean exemptedByAnnotation(List<? extends AnnotationTree> annotations) {
     for (AnnotationTree annotation : annotations) {
       Type annotationType = getType(annotation);
       if (annotationType == null) {
         continue;
       }
       TypeSymbol tsym = annotationType.tsym;
-      if (EXEMPTING_METHOD_ANNOTATIONS.contains(tsym.getQualifiedName().toString())) {
+      String annotationName = tsym.getQualifiedName().toString();
+      if (EXEMPTING_METHOD_ANNOTATIONS.contains(annotationName)
+          || additionalExemptingMethodAnnotations.contains(annotationName)) {
         return true;
       }
     }

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java
@@ -138,6 +138,24 @@ public void exemptedByName() {
         .doTest();
   }
 
+  @Test
+  public void exemptedByCustomAnnotation() {
+    helper
+        .addSourceLines(
+            "Foo.java", //
+            "package example;",
+            "@interface Foo {}")
+        .addSourceLines(
+            "ExemptedByCustomAnnotation.java",
+            "package example;",
+            "class ExemptedByCustomAnnotation {",
+            "  @Foo",
+            "  private void bar() {}",
+            "}")
+        .setArgs("-XepOpt:UnusedMethod:ExemptingMethodAnnotations=example.Foo")
+        .doTest();
+  }
+
   @Test
   public void suppressions() {
     helper