Handle overlapping deletions for unused parameters

PiperOrigin-RevId: 551969690
google · Jul 28, 2023 · 29b0e54 · 29b0e54
1 parent b18fd72
commit 29b0e54
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
@@ -48,7 +48,10 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Multimaps;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
 import com.google.common.collect.Streams;
+import com.google.common.collect.TreeRangeSet;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.VisitorState;
@@ -475,9 +478,10 @@ private static ImmutableList<SuggestedFix> buildUnusedParameterFixes(
       Symbol varSymbol, List<TreePath> usagePaths, VisitorState state) {
     MethodSymbol methodSymbol = (MethodSymbol) varSymbol.owner;
     int index = methodSymbol.params.indexOf(varSymbol);
-    SuggestedFix.Builder fix = SuggestedFix.builder();
+    RangeSet<Integer> deletions = TreeRangeSet.create();
     for (TreePath path : usagePaths) {
-      fix.delete(path.getLeaf());
+      deletions.add(
+          Range.closed(getStartPosition(path.getLeaf()), state.getEndPosition(path.getLeaf())));
     }
     new TreePathScanner<Void, Void>() {
       @Override
@@ -507,7 +511,7 @@ private void removeByIndex(List<? extends Tree> trees) {
             // TODO(b/118437729): handle bogus source positions in enum declarations
             return;
           }
-          fix.delete(tree);
+          deletions.add(Range.closed(getStartPosition(tree), state.getEndPosition(tree)));
           return;
         }
         int startPos;
@@ -526,9 +530,11 @@ private void removeByIndex(List<? extends Tree> trees) {
           // TODO(b/118437729): handle bogus source positions in enum declarations
           return;
         }
-        fix.replace(startPos, endPos, "");
+        deletions.add(Range.closed(startPos, endPos));
       }
     }.scan(state.getPath().getCompilationUnit(), null);
+    SuggestedFix.Builder fix = SuggestedFix.builder();
+    deletions.asRanges().forEach(x -> fix.replace(x.lowerEndpoint(), x.upperEndpoint(), ""));
     return ImmutableList.of(fix.build());
   }
 

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
@@ -1472,4 +1472,26 @@ public void parcelableCreator_noFinding() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void nestedParameterRemoval() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "class Test {",
+            "  private int foo(int a, int b) { return b; }",
+            "  void test() {",
+            "    foo(foo(1, 2), 2);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "class Test {",
+            "  private int foo(int b) { return b; }",
+            "  void test() {",
+            "    foo(2);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }