FieldCanBeLocal: don't emit findings if the only uses are assignments…

… (i.e. the field is effectively unused). PiperOrigin-RevId: 705632227
google · Dec 12, 2024 · 99cae77 · 99cae77
1 parent ccf54bb
commit 99cae77
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 3 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeLocal.java b/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeLocal.java
@@ -76,7 +76,7 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
     Map<VarSymbol, TreePath> potentialFields = new LinkedHashMap<>();
     SetMultimap<VarSymbol, TreePath> unconditionalAssignments =
         MultimapBuilder.linkedHashKeys().linkedHashSetValues().build();
-    SetMultimap<VarSymbol, Tree> uses =
+    SetMultimap<VarSymbol, TreePath> uses =
         MultimapBuilder.linkedHashKeys().linkedHashSetValues().build();
 
     new SuppressibleTreePathScanner<Void, Void>(state) {
@@ -194,7 +194,7 @@ private void handleIdentifier(Tree tree) {
               return;
             }
             VarSymbol varSymbol = (VarSymbol) symbol;
-            uses.put(varSymbol, tree);
+            uses.put(varSymbol, getCurrentPath());
             if (!unconditionallyAssigned.contains(varSymbol)) {
               potentialFields.remove(varSymbol);
             }
@@ -250,6 +250,15 @@ public Void visitMemberSelect(MemberSelectTree memberSelectTree, Void unused) {
       if (assignmentLocations.isEmpty()) {
         continue;
       }
+      // Don't emit findings if the _only_ uses of the field are assignments: we'll be overlapping
+      // with UnusedVariable.
+      if (uses.get(varSymbol).stream()
+          .allMatch(
+              tp ->
+                  tp.getParentPath().getLeaf() instanceof AssignmentTree parent
+                      && parent.getVariable() == tp.getLeaf())) {
+        continue;
+      }
       SuggestedFix.Builder fix = SuggestedFix.builder();
       VariableTree variableTree = (VariableTree) declarationSite.getLeaf();
       String type = state.getSourceForNode(variableTree.getType());
@@ -277,7 +286,8 @@ public Void visitMemberSelect(MemberSelectTree memberSelectTree, Void unused) {
         }
       }
       // Strip "this." off any uses of the field.
-      for (Tree usage : uses.get(varSymbol)) {
+      for (TreePath usagePath : uses.get(varSymbol)) {
+        var usage = usagePath.getLeaf();
         if (deletedTrees.contains(usage)
             || usage.getKind() == Kind.IDENTIFIER
             || usage.getKind() != Kind.MEMBER_SELECT) {

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/FieldCanBeLocalTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/FieldCanBeLocalTest.java
@@ -51,6 +51,23 @@ int foo() {
         .doTest();
   }
 
+  @Test
+  public void fullyUnused_noFinding() {
+    helper
+        .addSourceLines(
+            "Test.java",
+            """
+            class Test {
+              private int a;
+
+              void foo() {
+                a = 1;
+              }
+            }
+            """)
+        .doTest();
+  }
+
   @Test
   public void suppressedByUnusedPrefix() {
     helper