CanIgnoreReturnValueSuggester: Support additional exempting method an…

…notations While there, excempt `@AfterTemplate` methods with analysis. Fixes #4009 COPYBARA_INTEGRATE_REVIEW=#4009 from PicnicSupermarket:rossendrijver/CIRV_Refaster fd25f02 PiperOrigin-RevId: 563515580
google · Sep 7, 2023 · 6ca3f0a · 6ca3f0a
1 parent a046d80
commit 6ca3f0a
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 3 deletions.
diff --git a/...ava/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggester.java b/...ava/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggester.java
@@ -32,7 +32,9 @@
 import static com.google.errorprone.util.ASTHelpers.stripParentheses;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import com.google.errorprone.BugPattern;
+import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
@@ -59,6 +61,7 @@
 import com.sun.tools.javac.code.Type;
 import java.util.HashSet;
 import java.util.Set;
+import javax.inject.Inject;
 
 /**
  * Checker that recommends annotating a method with {@code @CanIgnoreReturnValue} if the method
@@ -74,21 +77,39 @@ public final class CanIgnoreReturnValueSuggester extends BugChecker implements M
 
   private static final String AUTO_VALUE = "com.google.auto.value.AutoValue";
   private static final String IMMUTABLE = "com.google.errorprone.annotations.Immutable";
-  private static final String CRV = "com.google.errorprone.annotations.CheckReturnValue";
   private static final String CIRV = "com.google.errorprone.annotations.CanIgnoreReturnValue";
+  private static final ImmutableSet<String> EXEMPTING_METHOD_ANNOTATIONS =
+      ImmutableSet.of(
+          CIRV,
+          "com.google.errorprone.annotations.CheckReturnValue",
+          "com.google.errorprone.refaster.annotation.AfterTemplate");
 
   private static final Supplier<Type> PROTO_BUILDER =
       VisitorState.memoize(s -> s.getTypeFromString("com.google.protobuf.MessageLite.Builder"));
 
   private static final ImmutableSet<String> BANNED_METHOD_PREFIXES =
       ImmutableSet.of("get", "is", "has", "new", "clone", "copy");
 
+  private final ImmutableSet<String> exemptingMethodAnnotations;
+
+  @Inject
+  public CanIgnoreReturnValueSuggester(ErrorProneFlags errorProneFlags) {
+    this.exemptingMethodAnnotations =
+        Sets.union(
+                EXEMPTING_METHOD_ANNOTATIONS,
+                errorProneFlags
+                    .getSet("CanIgnoreReturnValue:ExemptingMethodAnnotations")
+                    .orElse(ImmutableSet.of()))
+            .immutableCopy();
+  }
+
   @Override
   public Description matchMethod(MethodTree methodTree, VisitorState state) {
     MethodSymbol methodSymbol = getSymbol(methodTree);
 
-    // if the method is already directly annotated w/ @CRV or @CIRV, bail out
-    if (hasAnnotation(methodSymbol, CRV, state) || hasAnnotation(methodSymbol, CIRV, state)) {
+    // If the method has an exempting annotation, then bail out.
+    if (exemptingMethodAnnotations.stream()
+        .anyMatch(annotation -> ASTHelpers.hasAnnotation(methodSymbol, annotation, state))) {
       return Description.NO_MATCH;
     }
 

diff --git a/...com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggesterTest.java b/...com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggesterTest.java
@@ -609,6 +609,24 @@ public void constructor() {
         .doTest();
   }
 
+  @Test
+  public void refasterAfterTemplate() {
+    helper
+        .addInputLines(
+            "A.java",
+            "import com.google.errorprone.refaster.annotation.AfterTemplate;",
+            "class A {",
+            "  static final class MethodLacksBeforeTemplateAnnotation {",
+            "    @AfterTemplate",
+            "    String after(String str) {",
+            "      return str;",
+            "    }",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
   @Test
   public void sometimesThrows() {
     helper
@@ -832,4 +850,26 @@ public void providesMethod_b267362954() {
         .expectUnchanged()
         .doTest();
   }
+
+  @Test
+  public void exemptedByCustomAnnotation() {
+    helper
+        .addInputLines("Foo.java", "package example;", "@interface Foo {}")
+        .expectUnchanged()
+        .addInputLines(
+            "ExemptedByCustomAnnotation.java",
+            "package example;",
+            "public final class ExemptedByCustomAnnotation {",
+            "  private String name;",
+            "",
+            "  @Foo",
+            "  public ExemptedByCustomAnnotation setName(String name) {",
+            "    this.name = name;",
+            "    return this;",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .setArgs("-XepOpt:CanIgnoreReturnValue:ExemptingMethodAnnotations=example.Foo")
+        .doTest();
+  }
 }