Skip to content

Commit 8255d1e

Browse files
cushonError Prone Team
authored andcommitted
Avoid overlaps in UnnecessaryAsync fixes
PiperOrigin-RevId: 829361333
1 parent 4c0f01b commit 8255d1e

File tree

2 files changed

+61
-34
lines changed

2 files changed

+61
-34
lines changed

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

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
package com.google.errorprone.bugpatterns;
1818

1919
import static com.google.common.collect.ImmutableList.toImmutableList;
20+
import static com.google.common.collect.Iterables.getOnlyElement;
2021
import static com.google.errorprone.matchers.Description.NO_MATCH;
2122
import static com.google.errorprone.matchers.Matchers.anyOf;
2223
import static com.google.errorprone.matchers.Matchers.constructor;
24+
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
2325
import static com.google.errorprone.util.ASTHelpers.getSymbol;
2426
import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
2527
import static java.lang.String.format;
@@ -158,41 +160,34 @@ public Void visitIdentifier(IdentifierTree tree, Void unused) {
158160
if (parentTree instanceof MemberSelectTree memberSelectTree) {
159161
var grandparent =
160162
(MethodInvocationTree) getCurrentPath().getParentPath().getParentPath().getLeaf();
161-
if (memberSelectTree.getIdentifier().contentEquals("set")) {
162-
fix.replace(
163-
grandparent,
164-
format(
165-
"%s = %s",
166-
state.getSourceForNode(tree),
167-
state.getSourceForNode(grandparent.getArguments().get(0))));
168-
} else if (memberSelectTree.getIdentifier().contentEquals("get")) {
169-
fix.replace(grandparent, state.getSourceForNode(tree));
170-
} else if (memberSelectTree.getIdentifier().contentEquals("getAndIncrement")) {
171-
fix.replace(grandparent, format("%s++", state.getSourceForNode(tree)));
172-
} else if (memberSelectTree.getIdentifier().contentEquals("getAndDecrement")) {
173-
fix.replace(grandparent, format("%s--", state.getSourceForNode(tree)));
174-
} else if (memberSelectTree.getIdentifier().contentEquals("incrementAndGet")) {
175-
fix.replace(grandparent, format("++%s", state.getSourceForNode(tree)));
176-
} else if (memberSelectTree.getIdentifier().contentEquals("decrementAndGet")) {
177-
fix.replace(grandparent, format("--%s", state.getSourceForNode(tree)));
178-
} else if (memberSelectTree.getIdentifier().contentEquals("compareAndSet")) {
179-
fix.replace(
180-
grandparent,
181-
format(
182-
"%s = %s",
183-
state.getSourceForNode(tree),
184-
state.getSourceForNode(grandparent.getArguments().get(1))));
185-
} else if (memberSelectTree.getIdentifier().contentEquals("addAndGet")) {
186-
fix.replace(
187-
grandparent,
188-
format(
189-
"%s += %s",
190-
state.getSourceForNode(tree),
191-
state.getSourceForNode(grandparent.getArguments().get(0))));
192-
} else {
193-
fixable.set(false);
163+
String methodName = memberSelectTree.getIdentifier().toString();
164+
int receiverEndPos = state.getEndPosition(memberSelectTree.getExpression());
165+
int endPos = state.getEndPosition(grandparent);
166+
switch (methodName) {
167+
case "set" -> {
168+
var arg = grandparent.getArguments().getFirst();
169+
fix.replace(receiverEndPos, getStartPosition(arg), " = ")
170+
.replace(state.getEndPosition(arg), endPos, "");
171+
}
172+
case "get" -> fix.replace(receiverEndPos, endPos, "");
173+
case "getAndIncrement" -> fix.replace(receiverEndPos, endPos, "++");
174+
case "getAndDecrement" -> fix.replace(receiverEndPos, endPos, "--");
175+
case "incrementAndGet" ->
176+
fix.prefixWith(memberSelectTree, "++").replace(receiverEndPos, endPos, "");
177+
case "decrementAndGet" ->
178+
fix.prefixWith(memberSelectTree, "--").replace(receiverEndPos, endPos, "");
179+
case "compareAndSet" -> {
180+
var arg = grandparent.getArguments().get(1);
181+
fix.replace(receiverEndPos, getStartPosition(arg), " = ")
182+
.replace(state.getEndPosition(arg), endPos, "");
183+
}
184+
case "addAndGet" -> {
185+
var arg = getOnlyElement(grandparent.getArguments());
186+
fix.replace(receiverEndPos, getStartPosition(arg), " += ")
187+
.replace(state.getEndPosition(arg), endPos, "");
188+
}
189+
default -> fixable.set(false);
194190
}
195-
196191
} else {
197192
fixable.set(false);
198193
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,4 +260,36 @@ void f() {
260260
""")
261261
.doTest();
262262
}
263+
264+
@Test
265+
public void fixOverlap() {
266+
refactoring
267+
.addInputLines(
268+
"Test.java",
269+
"""
270+
import java.util.concurrent.atomic.AtomicInteger;
271+
272+
class Test {
273+
int test() {
274+
var ai = new AtomicInteger();
275+
ai.set(ai.get() + 1);
276+
return ai.get();
277+
}
278+
}
279+
""")
280+
.addOutputLines(
281+
"Test.java",
282+
"""
283+
import java.util.concurrent.atomic.AtomicInteger;
284+
285+
class Test {
286+
int test() {
287+
int ai = 0;
288+
ai = ai + 1;
289+
return ai;
290+
}
291+
}
292+
""")
293+
.doTest();
294+
}
263295
}

0 commit comments

Comments
 (0)