From 197c3d66b47067b2abc7c8198c2c4969debdb1ff Mon Sep 17 00:00:00 2001 From: ghm Date: Tue, 30 Dec 2025 08:04:27 -0800 Subject: [PATCH] Refactor SuggestedFixesTest to use text blocks and simplify test file paths. This was giving me a stroke trying to edit these. Some other misc cleaning up too. PiperOrigin-RevId: 850417102 --- .../errorprone/fixes/SuggestedFixesTest.java | 326 ++++++++++-------- 1 file changed, 180 insertions(+), 146 deletions(-) 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 dee79ea9bdb..33143ae6d50 100644 --- a/core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java +++ b/core/src/test/java/com/google/errorprone/fixes/SuggestedFixesTest.java @@ -74,6 +74,7 @@ import java.util.Optional; import java.util.function.Function; import javax.lang.model.element.Modifier; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -99,8 +100,8 @@ enum EditKind { } /** Test checker that adds or removes modifiers. */ - @BugPattern(name = "EditModifiers", summary = "Edits modifiers", severity = ERROR) - public static class EditModifiersChecker extends BugChecker + @BugPattern(summary = "Edits modifiers", severity = ERROR) + public static final class EditModifiersChecker extends BugChecker implements VariableTreeMatcher, MethodTreeMatcher { static final ImmutableMap MODIFIERS_BY_NAME = createModifiersByName(); @@ -144,27 +145,33 @@ private Description editModifiers(Tree tree, VisitorState state) { public void addAtBeginningOfLine() { BugCheckerRefactoringTestHelper.newInstance(EditModifiersChecker.class, getClass()) .addInputLines( - "in/Test.java", - "import javax.annotation.Nullable;", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.ADD)", - "class Test {", - " @Nullable", - " int foo() {", - " return 10;", - " }", - "}") + "Test.java", + """ + import javax.annotation.Nullable; + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + + @EditModifiers(value = "final", kind = EditModifiers.EditKind.ADD) + class Test { + @Nullable + int foo() { + return 10; + } + } + """) .addOutputLines( - "out/Test.java", - "import javax.annotation.Nullable;", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.ADD)", - "class Test {", - " @Nullable", - " final int foo() {", - " return 10;", - " }", - "}") + "Test.java", + """ + import javax.annotation.Nullable; + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + + @EditModifiers(value = "final", kind = EditModifiers.EditKind.ADD) + class Test { + @Nullable + final int foo() { + return 10; + } + } + """) .doTest(TestMode.TEXT_MATCH); } @@ -173,21 +180,24 @@ public void addModifiers() { CompilationTestHelper.newInstance(EditModifiersChecker.class, getClass()) .addSourceLines( "Test.java", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "import javax.annotation.Nullable;", - "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.ADD)", - "class Test {", - " // BUG: Diagnostic contains: final Object one", - " Object one;", - " // BUG: Diagnostic contains: @Nullable final Object two", - " @Nullable Object two;", - " // BUG: Diagnostic contains: @Nullable public final Object three", - " @Nullable public Object three;", - " // BUG: Diagnostic contains: public final Object four", - " public Object four;", - " // BUG: Diagnostic contains: public final @Nullable Object five", - " public @Nullable Object five;", - "}") + """ + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + import javax.annotation.Nullable; + + @EditModifiers(value = "final", kind = EditModifiers.EditKind.ADD) + class Test { + // BUG: Diagnostic contains: final Object one + Object one; + // BUG: Diagnostic contains: @Nullable final Object two + @Nullable Object two; + // BUG: Diagnostic contains: @Nullable public final Object three + @Nullable public Object three; + // BUG: Diagnostic contains: public final Object four + public Object four; + // BUG: Diagnostic contains: public final @Nullable Object five + public @Nullable Object five; + } + """) .doTest(); } @@ -196,17 +206,18 @@ public void addModifiersComment() { CompilationTestHelper.newInstance(EditModifiersChecker.class, getClass()) .addSourceLines( "Test.java", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "import javax.annotation.Nullable;", - "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.ADD)", - "class Test {", - " // BUG: Diagnostic contains:" - + " private @Deprecated /*comment*/ final volatile Object one;", - " private @Deprecated /*comment*/ volatile Object one;", - " // BUG: Diagnostic contains:" - + " private @Deprecated /*comment*/ static final Object two = null;", - " private @Deprecated /*comment*/ static Object two = null;", - "}") + """ + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + import javax.annotation.Nullable; + + @EditModifiers(value = "final", kind = EditModifiers.EditKind.ADD) + class Test { + // BUG: Diagnostic contains: private @Deprecated /*comment*/ final volatile Object one; + private @Deprecated /*comment*/ volatile Object one; + // BUG: Diagnostic contains: private @Deprecated /*comment*/ static final Object two = null; + private @Deprecated /*comment*/ static Object two = null; + } + """) .doTest(); } @@ -215,13 +226,16 @@ public void addModifiersFirst() { CompilationTestHelper.newInstance(EditModifiersChecker.class, getClass()) .addSourceLines( "Test.java", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "import javax.annotation.Nullable;", - "@EditModifiers(value=\"public\", kind=EditModifiers.EditKind.ADD)", - "class Test {", - " // BUG: Diagnostic contains: public static final transient Object one", - " static final transient Object one = null;", - "}") + """ + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + import javax.annotation.Nullable; + + @EditModifiers(value = "public", kind = EditModifiers.EditKind.ADD) + class Test { + // BUG: Diagnostic contains: public static final transient Object one + static final transient Object one = null; + } + """) .doTest(); } @@ -230,19 +244,22 @@ public void removeModifiers() { CompilationTestHelper.newInstance(EditModifiersChecker.class, getClass()) .addSourceLines( "Test.java", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "import javax.annotation.Nullable;", - "@EditModifiers(value=\"final\", kind=EditModifiers.EditKind.REMOVE)", - "class Test {", - " // BUG: Diagnostic contains: Object one", - " final Object one = null;", - " // BUG: Diagnostic contains: @Nullable Object two", - " @Nullable final Object two = null;", - " // BUG: Diagnostic contains: @Nullable public Object three", - " @Nullable public final Object three = null;", - " // BUG: Diagnostic contains: public Object four", - " public final Object four = null;", - "}") + """ + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + import javax.annotation.Nullable; + + @EditModifiers(value = "final", kind = EditModifiers.EditKind.REMOVE) + class Test { + // BUG: Diagnostic contains: Object one + final Object one = null; + // BUG: Diagnostic contains: @Nullable Object two + @Nullable final Object two = null; + // BUG: Diagnostic contains: @Nullable public Object three + @Nullable public final Object three = null; + // BUG: Diagnostic contains: public Object four + public final Object four = null; + } + """) .doTest(); } @@ -251,13 +268,18 @@ public void removeMultipleModifiers() { CompilationTestHelper.newInstance(EditModifiersChecker.class, getClass()) .addSourceLines( "Test.java", - String.format("import %s;", EditModifiers.class.getCanonicalName()), - "import javax.annotation.Nullable;", - "@EditModifiers(value={\"final\", \"static\"}, kind=EditModifiers.EditKind.REMOVE)", - "class Test {", - " // BUG: Diagnostic contains: private Object one = null;", - " private static final Object one = null;", - "}") + """ + import com.google.errorprone.fixes.SuggestedFixesTest.EditModifiers; + import javax.annotation.Nullable; + + @EditModifiers( + value = {"final", "static"}, + kind = EditModifiers.EditKind.REMOVE) + class Test { + // BUG: Diagnostic contains: private Object one = null; + private static final Object one = null; + } + """) .doTest(); } @@ -281,7 +303,7 @@ public Description matchReturn(ReturnTree tree, VisitorState state) { } /** Test checker that casts returned expression. */ - @BugPattern(name = "CastReturn", severity = ERROR, summary = "Adds casts to returned expressions") + @BugPattern(severity = ERROR, summary = "Adds casts to returned expressions") public static class CastReturnFullType extends BugChecker implements ReturnTreeMatcher { @Override @@ -1167,7 +1189,7 @@ private static DCDocComment getCommentTree(DocCommentTable docCommentTable, JCTr public void qualifyJavadocTest() { BugCheckerRefactoringTestHelper.newInstance(JavadocQualifier.class, getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ import java.util.List; import java.util.Map; @@ -1178,7 +1200,7 @@ void foo() {} } """) .addOutputLines( - "out/Test.java", + "Test.java", """ import java.util.List; import java.util.Map; @@ -1220,13 +1242,13 @@ public Description matchVariable(VariableTree tree, VisitorState state) { } @Test - @org.junit.Ignore("There appears to be an issue parsing lambda comments") + @Ignore("There appears to be an issue parsing lambda comments") public void suppressWarningsFix() { BugCheckerRefactoringTestHelper refactorTestHelper = BugCheckerRefactoringTestHelper.newInstance(SuppressMe.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { static final int BEST_NUMBER = 42; @@ -1255,7 +1277,7 @@ public void bar() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { @SuppressWarnings("SuppressMe") @@ -1314,7 +1336,7 @@ public void suppressWarningsWithCommentFix() { new SuppressMeWithComment("b/XXXX: fix me!"), getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { int BEST_NUMBER = 42; @@ -1324,7 +1346,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { // b/XXXX: fix me! @@ -1346,7 +1368,7 @@ public void suppressWarningsWithCommentFix_existingComment() { new SuppressMeWithComment("b/XXXX: fix me!"), getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { // This comment was here already. @@ -1358,7 +1380,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { // This comment was here already. @@ -1385,14 +1407,14 @@ public void suppressWarningsWithCommentFix_commentHasToBeLineWrapped() { getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { int BEST = 42; } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut @@ -1422,7 +1444,7 @@ public void removeSuppressWarnings_singleWarning_removesEntireAnnotation() { BugCheckerRefactoringTestHelper.newInstance(RemoveSuppressFromMe.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { @SuppressWarnings("RemoveMe") @@ -1430,7 +1452,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { int BEST = 42; @@ -1445,7 +1467,7 @@ public void removeSuppressWarnings_twoWarning_removesWarningAndNewArray() { BugCheckerRefactoringTestHelper.newInstance(RemoveSuppressFromMe.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { @SuppressWarnings({"RemoveMe", "KeepMe"}) @@ -1453,7 +1475,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { @SuppressWarnings("KeepMe") @@ -1469,7 +1491,7 @@ public void removeSuppressWarnings_threeWarning_removesOnlyOneAndKeepsArray() { BugCheckerRefactoringTestHelper.newInstance(RemoveSuppressFromMe.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { @SuppressWarnings({"RemoveMe", "KeepMe1", "KeepMe2"}) @@ -1477,7 +1499,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { @SuppressWarnings({"KeepMe1", "KeepMe2"}) @@ -1493,7 +1515,7 @@ public void removeSuppressWarnings_oneWarningInArray_removesWholeAnnotation() { BugCheckerRefactoringTestHelper.newInstance(RemoveSuppressFromMe.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { @SuppressWarnings({"RemoveMe"}) @@ -1501,7 +1523,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { int BEST = 42; @@ -1516,7 +1538,7 @@ public void removeSuppressWarnings_withValueInit_retainsValue() { BugCheckerRefactoringTestHelper.newInstance(RemoveSuppressFromMe.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ public class Test { @SuppressWarnings(value = {"RemoveMe", "KeepMe"}) @@ -1524,7 +1546,7 @@ public class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ public class Test { @SuppressWarnings(value = "KeepMe") @@ -1535,8 +1557,8 @@ public class Test { } /** A {@link BugChecker} for testing. */ - @BugPattern(name = "UpdateDoNotCallArgument", summary = "", severity = ERROR) - public static final class UpdateDoNotCallArgumentChecker extends BugChecker + @BugPattern(summary = "", severity = ERROR) + public static final class UpdateDoNotCallArgument extends BugChecker implements AnnotationTreeMatcher { @Override public Description matchAnnotation(AnnotationTree tree, VisitorState state) { @@ -1550,11 +1572,10 @@ public Description matchAnnotation(AnnotationTree tree, VisitorState state) { @Test public void updateAnnotationArgumentValues_noArguments() { BugCheckerRefactoringTestHelper refactorTestHelper = - BugCheckerRefactoringTestHelper.newInstance( - UpdateDoNotCallArgumentChecker.class, getClass()); + BugCheckerRefactoringTestHelper.newInstance(UpdateDoNotCallArgument.class, getClass()); refactorTestHelper .addInputLines( - "in/Test.java", + "Test.java", """ import com.google.errorprone.annotations.DoNotCall; @@ -1564,7 +1585,7 @@ void m() {} } """) .addOutputLines( - "out/Test.java", + "Test.java", """ import com.google.errorprone.annotations.DoNotCall; @@ -1592,7 +1613,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { public void compilesWithFixTest() { BugCheckerRefactoringTestHelper.newInstance(CompilesWithFixChecker.class, getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ class Test { void f() { @@ -1603,7 +1624,7 @@ void f() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ class Test { void f() { @@ -1620,7 +1641,7 @@ public void compilesWithFix_releaseFlag() { BugCheckerRefactoringTestHelper.newInstance(CompilesWithFixChecker.class, getClass()) .setArgs("--release", "9") .addInputLines( - "in/Test.java", + "Test.java", """ class Test { void f() { @@ -1631,7 +1652,7 @@ void f() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ class Test { void f() { @@ -1644,7 +1665,7 @@ void f() { } /** A test bugchecker that deletes an exception from throws. */ - @BugPattern(name = "RemovesExceptionChecker", summary = "", severity = ERROR) + @BugPattern(summary = "", severity = ERROR) public static class RemovesExceptionsChecker extends BugChecker implements MethodTreeMatcher { private final int index; @@ -1669,7 +1690,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) { public void deleteExceptionsRemoveFirstCheckerTest() { BugCheckerRefactoringTestHelper.newInstance(new RemovesExceptionsChecker(0), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ import java.io.IOException; @@ -1684,7 +1705,7 @@ void h() throws RuntimeException, Exception, IOException {} } """) .addOutputLines( - "out/Test.java", + "Test.java", """ import java.io.IOException; @@ -1705,7 +1726,7 @@ void h() throws Exception, IOException {} public void deleteExceptionsRemoveSecondCheckerTest() { BugCheckerRefactoringTestHelper.newInstance(new RemovesExceptionsChecker(1), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ import java.io.IOException; @@ -1720,7 +1741,7 @@ void h() throws RuntimeException, Exception, IOException {} } """) .addOutputLines( - "out/Test.java", + "Test.java", """ import java.io.IOException; @@ -1779,7 +1800,7 @@ public void renameVariable_renamesLocalVariable_withNestedScope() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", Integer.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ class Test { void m() { @@ -1792,7 +1813,7 @@ void m() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ class Test { void m() { @@ -1812,7 +1833,7 @@ public void renameVariable_ignoresMatchingNames_whenNotInScopeOfReplacement() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", Integer.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ class Test { void m() { @@ -1826,7 +1847,7 @@ void m() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ class Test { void m() { @@ -1847,7 +1868,7 @@ public void renameVariable_renamesMethodParameter() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", Integer.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ class Test { void m(Integer replace) { @@ -1856,7 +1877,7 @@ void m(Integer replace) { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ class Test { void m(Integer renamed) { @@ -1872,7 +1893,7 @@ public void renameVariable_renamesTryWithResourcesParameter() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", AutoCloseable.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ abstract class Test { abstract AutoCloseable open(); @@ -1886,7 +1907,7 @@ void m() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ abstract class Test { abstract AutoCloseable open(); @@ -1907,7 +1928,7 @@ public void renameVariable_renamesLambdaParameter_explicitlyTyped() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", Integer.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ import java.util.function.Function; @@ -1916,7 +1937,7 @@ class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ import java.util.function.Function; @@ -1932,7 +1953,7 @@ public void renameVariable_renamesLambdaParameter_notExplicitlyTyped() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", Integer.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ import java.util.function.Function; @@ -1941,7 +1962,7 @@ class Test { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ import java.util.function.Function; @@ -1957,7 +1978,7 @@ public void renameVariable_renamesCatchParameter() { BugCheckerRefactoringTestHelper.newInstance( new RenamesVariableChecker("replace", "renamed", Throwable.class), getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ class Test { void m() { @@ -1969,7 +1990,7 @@ void m() { } """) .addOutputLines( - "out/Test.java", + "Test.java", """ class Test { void m() { @@ -2000,8 +2021,16 @@ public Description matchClass(ClassTree tree, VisitorState state) { @Test public void removeAddModifier_rangesCompatible() { BugCheckerRefactoringTestHelper.newInstance(RemoveAddModifier.class, getClass()) - .addInputLines("in/Test.java", "public class Test {}") - .addOutputLines("out/Test.java", "abstract class Test {}") + .addInputLines( + "Test.java", + """ + public class Test {} + """) + .addOutputLines( + "Test.java", + """ + abstract class Test {} + """) .doTest(); } @@ -2023,14 +2052,14 @@ public Description matchClass(ClassTree tree, VisitorState state) { public void prefixAddImport() throws IOException { BugCheckerRefactoringTestHelper.newInstance(PrefixAddImportCheck.class, getClass()) .addInputLines( - "in/Test.java", + "Test.java", """ package p; class Test {} """) .addOutputLines( - "out/Test.java", + "Test.java", """ package p; @@ -2282,12 +2311,13 @@ public Description matchClass(ClassTree tree, VisitorState state) { @Test public void compilesWithFix_onlyInSameCompilationUnit() { - String[] unrelatedFile = { - "class ClassContainingRawType {", - // This unsuppressed raw type would prevent compilation. - " java.util.List list;", - "}", - }; + String unrelatedFile = + """ + class ClassContainingRawType { + // This unsuppressed raw type would prevent compilation. + java.util.List list; + } + """; // This compilation will succeed because we only consider the compilation errors in the first // class. @@ -2306,11 +2336,13 @@ class OnlyInSameCompilationUnit {} CompilationTestHelper.newInstance( AddSuppressWarningsIfCompilationSucceedsOnlyInSameCompilationUnit.class, getClass()) .addSourceLines( - "OnlyInSameCompilationUnit.java", // - "class OnlyInSameCompilationUnit {", - // This unsuppressed raw type prevents compilation. - " java.util.List list;", - "}") + "OnlyInSameCompilationUnit.java", + """ + class OnlyInSameCompilationUnit { + // This unsuppressed raw type prevents compilation. + java.util.List list; + } + """) .addSourceLines("ClassContainingRawType.java", unrelatedFile) .doTest(); } @@ -2337,11 +2369,13 @@ AddSuppressWarningsIfCompilationSucceedsInAllCompilationUnits.class, getClass()) class InAllCompilationUnits {} """) .addSourceLines( - "ClassContainingRawType.java", // - "class ClassContainingRawType {", - // This unsuppressed raw type prevents re-compilation, so the other class is unchanged. - " java.util.List list;", - "}") + "ClassContainingRawType.java", + """ + class ClassContainingRawType { + // This unsuppressed raw type prevents re-compilation, so the other class is unchanged. + java.util.List list; + } + """) .doTest(); }