Skip to content

Commit

Permalink
Tidy some SuggestedFix usages.
Browse files Browse the repository at this point in the history
I added a `toBuilder` method to avoid the `SuggestedFix.builder().merge(x)` dance, and made use of the newer static `merge` method in a few places.

PiperOrigin-RevId: 617895251
  • Loading branch information
graememorgan authored and Error Prone Team committed Mar 26, 2024
1 parent af9dad8 commit 98da9d5
Show file tree
Hide file tree
Showing 18 changed files with 34 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ public static Builder builder() {
return new Builder();
}

public Builder toBuilder() {
return SuggestedFix.builder().merge(this);
}

/** Builds {@link SuggestedFix}s. */
public static class Builder {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ public static SuggestedFix renameMethod(MethodTree tree, String replacement, Vis
*/
public static SuggestedFix renameMethodWithInvocations(
MethodTree tree, String replacement, VisitorState state) {
SuggestedFix.Builder fix = SuggestedFix.builder().merge(renameMethod(tree, replacement, state));
SuggestedFix.Builder fix = renameMethod(tree, replacement, state).toBuilder();
MethodSymbol sym = getSymbol(tree);
new TreeScanner<Void, Void>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ && matchingMethods(
}
}
return fixes.buildOrThrow().entrySet().stream()
.map(
e -> SuggestedFix.builder().merge(e.getValue()).setShortDescription(e.getKey()).build())
.map(e -> e.getValue().toBuilder().setShortDescription(e.getKey()).build())
.collect(toImmutableList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
argument,
tree,
() ->
SuggestedFix.builder()
.merge(renameMethodInvocation(tree, "getLoopbackAddress", state))
renameMethodInvocation(tree, "getLoopbackAddress", state).toBuilder()
.delete(argument)
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
getStartPosition(tree.getThrows().get(0)),
state.getEndPosition(getLast(tree.getThrows())),
canActuallyBeThrown.stream().map(state::getSourceForNode).collect(joining(", ")));
SuggestedFix fix =
SuggestedFix.builder().merge(fixJavadoc(thrownTypes, state)).merge(throwsFix).build();
SuggestedFix fix = fixJavadoc(thrownTypes, state).toBuilder().merge(throwsFix).build();
return buildDescription(tree.getThrows().get(0)).setMessage(description).addFix(fix).build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ private static SuggestedFix fixLocal(
state.getSourceForNode(tree.getType()),
name,
state.getSourceForNode(tree.getInitializer()));
return SuggestedFix.builder()
.merge(renameVariableUsages(tree, name, state))
return renameVariableUsages(tree, name, state).toBuilder()
.postfixWith(outerMethodTree, replacement)
.delete(tree)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.suppliers.Supplier;
Expand Down Expand Up @@ -80,11 +79,10 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
.build();
}

private static Fix threadLocalFix(
private static SuggestedFix threadLocalFix(
VariableTree tree, VisitorState state, VarSymbol sym, SuggestedFix rename) {
SuggestedFix.Builder fix =
SuggestedFix.builder()
.merge(rename)
rename.toBuilder()
.replace(
tree.getType(),
String.format("ThreadLocal<%s>", state.getSourceForNode(tree.getType())))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ private static Fix buildFix(
return finishFix(fix.build(), exceptionType, newAsserts, suffix, state);
}

private static Fix finishFix(
private static SuggestedFix finishFix(
SuggestedFix baseFix,
Type exceptionType,
List<String> newAsserts,
Expand All @@ -262,7 +262,7 @@ private static Fix finishFix(
if (throwingStatements.isEmpty()) {
return baseFix;
}
SuggestedFix.Builder fix = SuggestedFix.builder().merge(baseFix);
SuggestedFix.Builder fix = baseFix.toBuilder();
fix.addStaticImport("org.junit.Assert.assertThrows");
StringBuilder fixPrefix = new StringBuilder();
String exceptionTypeName = SuggestedFixes.qualifyType(state, fix, exceptionType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,9 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
return NO_MATCH;
}
SuggestedFix fix =
SuggestedFix.builder()
.merge(renameVariable(tree, state))
.merge(addModifiers(tree, state, STATIC).orElse(SuggestedFix.emptyFix()))
.build();
SuggestedFix.merge(
renameVariable(tree, state),
addModifiers(tree, state, STATIC).orElse(SuggestedFix.emptyFix()));
return describeMatch(tree, fix);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
String fieldName = field.getSimpleName().toString();
VariableTree parameterTree = (VariableTree) parameterPath.getLeaf();
SuggestedFix.Builder fix =
SuggestedFix.builder()
.merge(SuggestedFixes.renameVariable(parameterTree, fieldName, state));
SuggestedFixes.renameVariable(parameterTree, fieldName, state).toBuilder();

if (parameterPath.getParentPath() != null) {
String qualifiedName =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.JUnitMatchers.containsTestMethod;
import static com.google.errorprone.matchers.JUnitMatchers.isJUnit4TestClass;
Expand Down Expand Up @@ -191,23 +192,20 @@ private Description describeFixes(MethodTree methodTree, VisitorState state) {
Optional<SuggestedFix> removeStatic =
SuggestedFixes.removeModifiers(methodTree, state, Modifier.STATIC);
SuggestedFix testFix =
SuggestedFix.builder()
.merge(removeStatic.orElse(null))
removeStatic.orElse(emptyFix()).toBuilder()
.addImport("org.junit.Test")
.prefixWith(methodTree, "@Test ")
.build();
SuggestedFix ignoreFix =
SuggestedFix.builder()
.merge(testFix)
testFix.toBuilder()
.addImport("org.junit.Ignore")
.prefixWith(methodTree, "@Ignore ")
.build();

SuggestedFix visibilityFix =
SuggestedFix.builder()
.merge(SuggestedFixes.removeModifiers(methodTree, state, Modifier.PUBLIC).orElse(null))
.merge(SuggestedFixes.addModifiers(methodTree, state, Modifier.PRIVATE).orElse(null))
.build();
SuggestedFix.merge(
SuggestedFixes.removeModifiers(methodTree, state, Modifier.PUBLIC).orElse(emptyFix()),
SuggestedFixes.addModifiers(methodTree, state, Modifier.PRIVATE).orElse(emptyFix()));

// Suggest @Ignore first if test method is named like a purposely disabled test.
String methodName = methodTree.getName().toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private static boolean referencesExternalMethods(
* `EnclosingClass.foo(...)` and `this::foo` to `EnclosingClass::foo`).
*/
private SuggestedFix fixQualifiers(VisitorState state, MethodSymbol sym, SuggestedFix f) {
SuggestedFix.Builder builder = SuggestedFix.builder().merge(f);
SuggestedFix.Builder builder = f.toBuilder();
new TreeScanner<Void, Void>() {
@Override
public Void visitMemberSelect(MemberSelectTree tree, Void unused) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ public Description matchClass(ClassTree classTree, VisitorState state) {
return NO_MATCH;
}
SuggestedFix.Builder fix =
SuggestedFix.builder()
.merge(addMembers(classTree, state, createPrivateConstructor(classTree)));
addMembers(classTree, state, createPrivateConstructor(classTree)).toBuilder();
SuggestedFixes.addModifiers(classTree, state, Modifier.FINAL).ifPresent(fix::merge);
return describeMatch(classTree, fix.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ private Description handleStatements(
MethodTree tree, VisitorState state, JCExpression expectedException, SuggestedFix baseFix) {
return describeMatch(
tree,
buildFix(
state,
SuggestedFix.builder().merge(baseFix),
expectedException,
tree.getBody().getStatements()));
buildFix(state, baseFix.toBuilder(), expectedException, tree.getBody().getStatements()));
}

private static SuggestedFix buildFix(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ private static SuggestedFix refactor(
ImmutableList<ExpressionTree> arguments, MethodInvocationTree tree, VisitorState state) {
// First we replace the containsExactlyElementsIn method with containsExactly.
SuggestedFix.Builder fix =
SuggestedFix.builder()
.merge(SuggestedFixes.renameMethodInvocation(tree, "containsExactly", state));
SuggestedFixes.renameMethodInvocation(tree, "containsExactly", state).toBuilder();
// Finally, we use the arguments from the new iterable to build the containsExactly arguments.
ExpressionTree expressionToReplace = tree.getArguments().get(0);
if (!arguments.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,7 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
symbol.name))
.addAllFixes(
fixes.stream()
.map(
f ->
SuggestedFix.builder()
.merge(makeFirstAssignmentDeclaration)
.merge(f)
.build())
.map(f -> SuggestedFix.merge(makeFirstAssignmentDeclaration, f))
.collect(toImmutableList()))
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ private Description generateReturnFix(
return NO_MATCH;
}
SuggestedFix fix =
SuggestedFix.builder()
.merge(Utils.replace(returnTree, "", state))
Utils.replace(returnTree, "", state).toBuilder()
.replace(
pos,
pos,
Expand All @@ -156,8 +155,7 @@ private Description generateSeeFix(DocTreePath docTreePath, SeeTree seeTree, Vis
SuggestedFix fix =
replacement.isEmpty()
? replacement
: SuggestedFix.builder()
.merge(replacement)
: replacement.toBuilder()
.replace(
pos,
pos,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.staticMethod;
Expand Down Expand Up @@ -1604,10 +1605,9 @@ public static class RemoveAddModifier extends BugChecker implements ClassTreeMat
public Description matchClass(ClassTree tree, VisitorState state) {
return describeMatch(
tree,
SuggestedFix.builder()
.merge(SuggestedFixes.removeModifiers(tree, state, Modifier.PUBLIC).orElse(null))
.merge(SuggestedFixes.addModifiers(tree, state, Modifier.ABSTRACT).orElse(null))
.build());
SuggestedFix.merge(
SuggestedFixes.removeModifiers(tree, state, Modifier.PUBLIC).orElse(emptyFix()),
SuggestedFixes.addModifiers(tree, state, Modifier.ABSTRACT).orElse(emptyFix())));
}
}

Expand Down

0 comments on commit 98da9d5

Please sign in to comment.