Don't flag things which probably shouldn't be static anyway.

There's probably a case to be made that we should remove the `static` modifier instead, but this will stop us making _broken_ suggestions. PiperOrigin-RevId: 572927264
google · Oct 12, 2023 · 8d62eb6 · 8d62eb6
1 parent 3cdff25
commit 8d62eb6
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 0 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/NonFinalStaticField.java b/core/src/main/java/com/google/errorprone/bugpatterns/NonFinalStaticField.java
@@ -24,12 +24,14 @@
 import static com.google.errorprone.util.ASTHelpers.canBeRemoved;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.hasDirectAnnotationWithSimpleName;
 import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static com.google.errorprone.util.ASTHelpers.isSameType;
 import static com.google.errorprone.util.ASTHelpers.isStatic;
 import static javax.lang.model.element.ElementKind.FIELD;
 import static javax.lang.model.element.Modifier.FINAL;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
@@ -48,6 +50,9 @@
 /** A BugPattern; see the summary. */
 @BugPattern(summary = "Static fields should almost always be final.", severity = WARNING)
 public final class NonFinalStaticField extends BugChecker implements VariableTreeMatcher {
+  private static final ImmutableSet<String> ANNOTATIONS_TO_AVOID =
+      ImmutableSet.of("Captor", "Inject", "Mock", "TestParameter");
+
   @Override
   public Description matchVariable(VariableTree tree, VisitorState state) {
     var symbol = getSymbol(tree);
@@ -60,6 +65,10 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
     if (isConsideredFinal(symbol)) {
       return NO_MATCH;
     }
+    if (ANNOTATIONS_TO_AVOID.stream()
+        .anyMatch(anno -> hasDirectAnnotationWithSimpleName(tree, anno))) {
+      return NO_MATCH;
+    }
     if (!canBeRemoved(symbol, state) || isEverMutatedInSameCompilationUnit(symbol, state)) {
       return describeMatch(tree);
     }

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/NonFinalStaticFieldTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/NonFinalStaticFieldTest.java
@@ -183,4 +183,16 @@ public void negativeInterface() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void exemptedAnnotation_noFinding() {
+    compilationTestHelper
+        .addSourceLines(
+            "Test.java", //
+            "import org.mockito.Mock;",
+            "public class Test {",
+            "  @Mock private static String foo;",
+            "}")
+        .doTest();
+  }
 }