From 527171cd5c32f00aa304ff5c76c7ee2d9f053ceb Mon Sep 17 00:00:00 2001 From: ghm Date: Tue, 26 Mar 2024 09:42:36 -0700 Subject: [PATCH] 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 --- .../com/google/errorprone/fixes/SuggestedFix.java | 4 ++++ .../google/errorprone/fixes/SuggestedFixes.java | 2 +- .../bugpatterns/AbstractReturnValueIgnored.java | 3 +-- .../errorprone/bugpatterns/AddressSelection.java | 3 +-- .../bugpatterns/CheckedExceptionNotThrown.java | 3 +-- .../bugpatterns/ConstantPatternCompile.java | 3 +-- .../errorprone/bugpatterns/DateFormatConstant.java | 6 ++---- .../bugpatterns/ExpectedExceptionChecker.java | 4 ++-- .../errorprone/bugpatterns/FieldCanBeStatic.java | 7 +++---- .../bugpatterns/InconsistentCapitalization.java | 3 +-- .../errorprone/bugpatterns/JUnit4TestNotRun.java | 14 ++++++-------- .../errorprone/bugpatterns/MethodCanBeStatic.java | 2 +- .../PrivateConstructorForUtilityClass.java | 3 +-- .../bugpatterns/TestExceptionChecker.java | 6 +----- .../TruthContainsExactlyElementsInUsage.java | 3 +-- .../errorprone/bugpatterns/UnusedVariable.java | 7 +------ .../bugpatterns/javadoc/MissingSummary.java | 6 ++---- .../errorprone/fixes/SuggestedFixesTest.java | 8 ++++---- 18 files changed, 34 insertions(+), 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 index 8f05c9b2392..9dc0c715e7d 100644 --- 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 index 2f4b04d2435..b278de56b08 100644 --- 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() { @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 index f32e28ff033..256bfa1d025 100644 --- 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 index 5159374333d..518bdc7be0f 100644 --- 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 index b64053d5e6b..dbaccb55560 100644 --- 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 index cbe0de37c4f..543e6b8c88b 100644 --- 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 index fd99bbb5928..7f4700e663a 100644 --- 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 index dd05c0002ff..84e362651aa 100644 --- 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 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 index 5f93e4f0eaa..1232efb9745 100644 --- 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 index de360fb848e..eb98130b45f 100644 --- 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 index 0a26e9d4996..36a33d60766 100644 --- 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 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 index 696ecd00c8b..08ad96fbc5d 100644 --- 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() { @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 index f1e9ea6cecf..860fa2d6df9 100644 --- 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 index cafc90e9792..ccb646fcc9f 100644 --- 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/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java b/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java index 78e5c92aebb..79fc1c32bee 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java @@ -92,8 +92,7 @@ private static SuggestedFix refactor( ImmutableList 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 index 1c4a85509cb..8b0204b36a9 100644 --- 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 index 841fb703e9c..4be9493fea7 100644 --- 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 index c378abaa09b..ffe69a29e0e 100644 --- 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()))); } }