Skip to content

Commit 29b0e54

Browse files
cushonError Prone Team
authored and
Error Prone Team
committed
Handle overlapping deletions for unused parameters
PiperOrigin-RevId: 551969690
1 parent b18fd72 commit 29b0e54

File tree

2 files changed

+32
-4
lines changed

2 files changed

+32
-4
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@
4848
import com.google.common.collect.Iterables;
4949
import com.google.common.collect.ListMultimap;
5050
import com.google.common.collect.Multimaps;
51+
import com.google.common.collect.Range;
52+
import com.google.common.collect.RangeSet;
5153
import com.google.common.collect.Streams;
54+
import com.google.common.collect.TreeRangeSet;
5255
import com.google.errorprone.BugPattern;
5356
import com.google.errorprone.ErrorProneFlags;
5457
import com.google.errorprone.VisitorState;
@@ -475,9 +478,10 @@ private static ImmutableList<SuggestedFix> buildUnusedParameterFixes(
475478
Symbol varSymbol, List<TreePath> usagePaths, VisitorState state) {
476479
MethodSymbol methodSymbol = (MethodSymbol) varSymbol.owner;
477480
int index = methodSymbol.params.indexOf(varSymbol);
478-
SuggestedFix.Builder fix = SuggestedFix.builder();
481+
RangeSet<Integer> deletions = TreeRangeSet.create();
479482
for (TreePath path : usagePaths) {
480-
fix.delete(path.getLeaf());
483+
deletions.add(
484+
Range.closed(getStartPosition(path.getLeaf()), state.getEndPosition(path.getLeaf())));
481485
}
482486
new TreePathScanner<Void, Void>() {
483487
@Override
@@ -507,7 +511,7 @@ private void removeByIndex(List<? extends Tree> trees) {
507511
// TODO(b/118437729): handle bogus source positions in enum declarations
508512
return;
509513
}
510-
fix.delete(tree);
514+
deletions.add(Range.closed(getStartPosition(tree), state.getEndPosition(tree)));
511515
return;
512516
}
513517
int startPos;
@@ -526,9 +530,11 @@ private void removeByIndex(List<? extends Tree> trees) {
526530
// TODO(b/118437729): handle bogus source positions in enum declarations
527531
return;
528532
}
529-
fix.replace(startPos, endPos, "");
533+
deletions.add(Range.closed(startPos, endPos));
530534
}
531535
}.scan(state.getPath().getCompilationUnit(), null);
536+
SuggestedFix.Builder fix = SuggestedFix.builder();
537+
deletions.asRanges().forEach(x -> fix.replace(x.lowerEndpoint(), x.upperEndpoint(), ""));
532538
return ImmutableList.of(fix.build());
533539
}
534540

core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,4 +1472,26 @@ public void parcelableCreator_noFinding() {
14721472
"}")
14731473
.doTest();
14741474
}
1475+
1476+
@Test
1477+
public void nestedParameterRemoval() {
1478+
refactoringHelper
1479+
.addInputLines(
1480+
"Test.java",
1481+
"class Test {",
1482+
" private int foo(int a, int b) { return b; }",
1483+
" void test() {",
1484+
" foo(foo(1, 2), 2);",
1485+
" }",
1486+
"}")
1487+
.addOutputLines(
1488+
"Test.java",
1489+
"class Test {",
1490+
" private int foo(int b) { return b; }",
1491+
" void test() {",
1492+
" foo(2);",
1493+
" }",
1494+
"}")
1495+
.doTest();
1496+
}
14751497
}

0 commit comments

Comments
 (0)