Tidy some SuggestedFix usages.

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: 619220321
google · Mar 26, 2024 · 527171c · 527171c
1 parent af9dad8
commit 527171c
Show file tree

Hide file tree

Showing 18 changed files with 34 additions and 53 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFix.java b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFix.java
@@ -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 {
 

diff --git a/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java b/check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java
@@ -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

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReturnValueIgnored.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractReturnValueIgnored.java
@@ -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());
   }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java b/core/src/main/java/com/google/errorprone/bugpatterns/AddressSelection.java
@@ -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());
   }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/CheckedExceptionNotThrown.java b/core/src/main/java/com/google/errorprone/bugpatterns/CheckedExceptionNotThrown.java
@@ -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();
   }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java b/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java
@@ -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();

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/DateFormatConstant.java b/core/src/main/java/com/google/errorprone/bugpatterns/DateFormatConstant.java
@@ -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;
@@ -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())))

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ExpectedExceptionChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/ExpectedExceptionChecker.java
@@ -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,
@@ -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);

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeStatic.java b/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeStatic.java
@@ -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);
   }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/InconsistentCapitalization.java b/core/src/main/java/com/google/errorprone/bugpatterns/InconsistentCapitalization.java
@@ -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 =

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java b/core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java
@@ -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;
@@ -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();

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/MethodCanBeStatic.java b/core/src/main/java/com/google/errorprone/bugpatterns/MethodCanBeStatic.java
@@ -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) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/PrivateConstructorForUtilityClass.java b/core/src/main/java/com/google/errorprone/bugpatterns/PrivateConstructorForUtilityClass.java
@@ -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());
   }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TestExceptionChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/TestExceptionChecker.java
@@ -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(

diff --git a/.../src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java b/.../src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java
@@ -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()) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
@@ -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());
     }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/MissingSummary.java b/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/MissingSummary.java
@@ -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,
@@ -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,

diff --git a/core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java b/core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java
@@ -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;
@@ -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())));
     }
   }