Skip to content

Commit bcf4dcf

Browse files
cushonError Prone Team
authored andcommitted
Optimize checks that report exactly the same fix in multiple diagnostics, like SameNameButDifferent
RELNOTES=N/A PiperOrigin-RevId: 498574682
1 parent 8ddb7cb commit bcf4dcf

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

check_api/src/main/java/com/google/errorprone/JavacErrorDescriptionListener.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.io.IOException;
3535
import java.io.UncheckedIOException;
3636
import java.util.EnumSet;
37+
import java.util.IdentityHashMap;
3738
import java.util.List;
3839
import java.util.Objects;
3940
import java.util.function.Function;
@@ -81,10 +82,18 @@ private JavacErrorDescriptionListener(
8182
this.context = context;
8283
this.dontUseErrors = dontUseErrors;
8384
checkNotNull(endPositions);
85+
// Optimization for checks that emit the same fix multiple times. Consider a check that renames
86+
// all uses of a symbol, and reports the diagnostic on all occurrences of the symbol. This can
87+
// be useful in environments where diagnostics are only shown on changed lines, but can lead to
88+
// quadratic behaviour during fix application if we're not careful.
89+
//
90+
// Using IdentityHashMap is sufficient when the repeated fixes are exactly the same instance.
91+
// Fix doesn't implement value equality, and we don't want to pay for it here anyways.
92+
IdentityHashMap<Fix, AppliedFix> cache = new IdentityHashMap<>();
8493
try {
8594
CharSequence sourceFileContent = sourceFile.getCharContent(true);
8695
AppliedFix.Applier applier = AppliedFix.fromSource(sourceFileContent, endPositions);
87-
fixToAppliedFix = applier::apply;
96+
fixToAppliedFix = fix -> cache.computeIfAbsent(fix, applier::apply);
8897
} catch (IOException e) {
8998
throw new UncheckedIOException(e);
9099
}

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,17 +153,17 @@ private void handle(Tree tree) {
153153
String simpleName = row.getKey();
154154
Map<TypeSymbol, List<TreePath>> columns = row.getValue();
155155

156-
SuggestedFix.Builder fix = SuggestedFix.builder();
156+
SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
157157
if (columns.size() > 1) {
158158
for (Map.Entry<TypeSymbol, List<TreePath>> cell : columns.entrySet()) {
159159
for (TreePath treePath : cell.getValue()) {
160160
TypeSymbol typeSymbol = cell.getKey();
161161
getBetterImport(typeSymbol, simpleName)
162162
.ifPresent(
163163
imp -> {
164-
String qualifiedName = qualifyType(state.withPath(treePath), fix, imp);
164+
String qualifiedName = qualifyType(state.withPath(treePath), fixBuilder, imp);
165165
String newSimpleName = qualifiedName + "." + simpleName;
166-
fix.replace(treePath.getLeaf(), newSimpleName);
166+
fixBuilder.replace(treePath.getLeaf(), newSimpleName);
167167
});
168168
}
169169
}
@@ -175,13 +175,11 @@ private void handle(Tree tree) {
175175
columns.keySet().stream()
176176
.map(t -> t.getQualifiedName().toString())
177177
.collect(joining(", ", "[", "]")));
178+
SuggestedFix fix = fixBuilder.build();
178179
for (List<TreePath> treePaths : trimmedTable.row(simpleName).values()) {
179180
for (TreePath treePath : treePaths) {
180181
state.reportMatch(
181-
buildDescription(treePath.getLeaf())
182-
.setMessage(message)
183-
.addFix(fix.build())
184-
.build());
182+
buildDescription(treePath.getLeaf()).setMessage(message).addFix(fix).build());
185183
}
186184
}
187185
}

0 commit comments

Comments
 (0)