Skip to content

Commit 9c5f725

Browse files
cushonError Prone Team
authored andcommitted
Optimize snippet logic
Instead of applying all replacements and keeping the first edited line, only apply edits to a single line of the replacement. This improves performance for very large generated files with thousands of findings. PiperOrigin-RevId: 828443769
1 parent b063711 commit 9c5f725

File tree

2 files changed

+48
-7
lines changed

2 files changed

+48
-7
lines changed

check_api/src/main/java/com/google/errorprone/fixes/AppliedFix.java

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919
import static com.google.common.base.Preconditions.checkArgument;
2020

2121
import com.google.common.collect.ImmutableSet;
22-
import com.google.common.collect.Iterables;
22+
import com.google.common.collect.Range;
2323
import com.sun.tools.javac.tree.EndPosTable;
24+
import java.util.ArrayList;
25+
import java.util.Collection;
26+
import java.util.List;
2427
import java.util.Set;
2528
import org.jspecify.annotations.Nullable;
2629

@@ -45,20 +48,58 @@ public record AppliedFix(String snippet, boolean isRemoveLine) {
4548
return null;
4649
}
4750

48-
String replaced = applyReplacements(source, endPositions, suggestedFix);
49-
50-
// Find the changed line containing the first edit
51-
String snippet = firstEditedLine(replaced, Iterables.get(replacements, 0));
51+
String snippet = snippet(source, replacements);
5252
if (snippet.isEmpty()) {
5353
return new AppliedFix("to remove this line", /* isRemoveLine= */ true);
5454
}
5555
return new AppliedFix(snippet, /* isRemoveLine= */ false);
5656
}
5757

58+
/**
59+
* The maximum distance we'll look for a newline before or after the snippet. If we don't find one
60+
* the snippet will just start or end in the middle of a line.
61+
*/
62+
public static final int MAX_LINE_LENGTH = 100;
63+
64+
private static String snippet(
65+
CharSequence sourceSequence, ImmutableSet<Replacement> replacements) {
66+
Replacement firstEdit = replacements.iterator().next();
67+
// Find a subrange of the source that should contain the entire first line that fixes are
68+
// applied to, and then only edit source and apply fixes in that range. This is a performance
69+
// optimization to avoid applying all of the fixes in very large files just to produce a
70+
// snippet.
71+
int startOffset = Math.max(0, firstEdit.startPosition() - MAX_LINE_LENGTH);
72+
int endOffset = Math.min(firstEdit.endPosition() + MAX_LINE_LENGTH, sourceSequence.length());
73+
Range<Integer> trimmed = Range.closedOpen(startOffset, endOffset);
74+
List<Replacement> shiftedReplacements = new ArrayList<>();
75+
for (Replacement replacement : replacements) {
76+
if (!replacement.range().isConnected(trimmed)) {
77+
continue;
78+
}
79+
if (replacement.endPosition() > endOffset) {
80+
endOffset = replacement.endPosition();
81+
}
82+
shiftedReplacements.add(
83+
Replacement.create(
84+
replacement.startPosition() - startOffset,
85+
replacement.endPosition() - startOffset,
86+
replacement.replaceWith()));
87+
}
88+
String replaced =
89+
applyReplacements(sourceSequence.subSequence(startOffset, endOffset), shiftedReplacements);
90+
// Find the changed line containing the first edit
91+
return firstEditedLine(replaced, shiftedReplacements.getFirst());
92+
}
93+
5894
public static String applyReplacements(CharSequence source, EndPosTable endPositions, Fix fix) {
95+
return applyReplacements(source, fix.getReplacements(endPositions));
96+
}
97+
98+
private static String applyReplacements(
99+
CharSequence source, Collection<Replacement> replacements) {
59100
StringBuilder replaced = new StringBuilder();
60101
int positionInOriginal = 0;
61-
for (Replacement repl : fix.getReplacements(endPositions)) {
102+
for (Replacement repl : replacements) {
62103
checkArgument(
63104
repl.endPosition() <= source.length(),
64105
"End [%s] should not exceed source length [%s]",

check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,6 @@ public void shouldApplyFixesInReverseOrder() {
163163
public void shouldThrowIfReplacementOutsideSource() {
164164
SuggestedFix fix = SuggestedFix.replace(0, 6, "World!");
165165
assertThrows(
166-
IllegalArgumentException.class, () -> AppliedFix.apply("Hello", endPositions, fix));
166+
StringIndexOutOfBoundsException.class, () -> AppliedFix.apply("Hello", endPositions, fix));
167167
}
168168
}

0 commit comments

Comments
 (0)