From f9c69dd726490ae0b6b1e90362998542dc31eda9 Mon Sep 17 00:00:00 2001 From: Bastien Diederichs Date: Fri, 10 Mar 2023 10:04:11 +0100 Subject: [PATCH 1/5] Introduce rule to drop default setFixChooser --- error-prone-contrib/pom.xml | 1 - .../refasterrules/BugCheckerRules.java | 29 +++++++++++++++++++ .../refasterrules/RefasterRulesTest.java | 1 + .../BugCheckerRulesTestInput.java | 19 ++++++++++++ .../BugCheckerRulesTestOutput.java | 18 ++++++++++++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java create mode 100644 error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index b16612d0e5..e15f6e58da 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -37,7 +37,6 @@ ${groupId.error-prone} error_prone_test_helpers - test ${project.groupId} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java new file mode 100644 index 0000000000..3bc4ee6308 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java @@ -0,0 +1,29 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; + +@OnlineDocumentation +final class BugCheckerRules { + private BugCheckerRules() {} + + /** + * Drop {@link + * BugCheckerRefactoringTestHelper#setFixChooser(BugCheckerRefactoringTestHelper.FixChooser)} when + * set to the default {@link FixChoosers#FIRST}. + */ + static final class SetFixChooserDefault { + @BeforeTemplate + BugCheckerRefactoringTestHelper before(BugCheckerRefactoringTestHelper helper) { + return helper.setFixChooser(FixChoosers.FIRST); + } + + @AfterTemplate + BugCheckerRefactoringTestHelper after(BugCheckerRefactoringTestHelper helper) { + return helper; + } + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java index c60bb8b286..d1b461536c 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/refasterrules/RefasterRulesTest.java @@ -35,6 +35,7 @@ final class RefasterRulesTest { AssertJThrowingCallableRules.class, AssortedRules.class, BigDecimalRules.class, + BugCheckerRules.class, CollectionRules.class, ComparatorRules.class, DoubleStreamRules.class, diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java new file mode 100644 index 0000000000..a6a6d16e20 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java @@ -0,0 +1,19 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.common.collect.ImmutableSet; +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; +import tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(FixChoosers.class); + } + + BugCheckerRefactoringTestHelper testSetFixChooserDefault() { + return BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass()) + .setFixChooser(FixChoosers.FIRST); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java new file mode 100644 index 0000000000..cb1ab7f433 --- /dev/null +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java @@ -0,0 +1,18 @@ +package tech.picnic.errorprone.refasterrules; + +import com.google.common.collect.ImmutableSet; +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; +import tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage; +import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; + +final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { + @Override + public ImmutableSet elidedTypesAndStaticImports() { + return ImmutableSet.of(FixChoosers.class); + } + + BugCheckerRefactoringTestHelper testSetFixChooserDefault() { + return BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass()); + } +} From 8277605a545666f7092a51275434259a20d70504 Mon Sep 17 00:00:00 2001 From: Bastien Diederichs Date: Fri, 10 Mar 2023 10:26:16 +0100 Subject: [PATCH 2/5] Apply the actual rule --- .../tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageTest.java | 1 - .../picnic/errorprone/bugpatterns/IdentityConversionTest.java | 1 - .../picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java | 1 - 3 files changed, 3 deletions(-) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageTest.java index 410993f614..bce223a2b4 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageTest.java @@ -67,7 +67,6 @@ void identification() { @Test void replacementFirstSuggestedFix() { BugCheckerRefactoringTestHelper.newInstance(FluxFlatMapUsage.class, getClass()) - .setFixChooser(FixChoosers.FIRST) .addInputLines( "A.java", "import reactor.core.publisher.Flux;", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java index 9341c76433..631acdad68 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java @@ -180,7 +180,6 @@ void identification() { @Test void replacementFirstSuggestedFix() { BugCheckerRefactoringTestHelper.newInstance(IdentityConversion.class, getClass()) - .setFixChooser(FixChoosers.FIRST) .addInputLines( "A.java", "import static com.google.errorprone.matchers.Matchers.staticMethod;", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java index b1865d6089..a3d087aef4 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java @@ -48,7 +48,6 @@ void identification() { @Test void replacementFirstSuggestedFix() { BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass()) - .setFixChooser(FixChoosers.FIRST) .addInputLines( "A.java", "class A {", From 23c51d1a81bd1f5b74ca5b67dc3806dd9906e67f Mon Sep 17 00:00:00 2001 From: Bastien Diederichs Date: Fri, 10 Mar 2023 10:31:31 +0100 Subject: [PATCH 3/5] Clean doc --- .../picnic/errorprone/refasterrules/BugCheckerRules.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java index 3bc4ee6308..224bf2d547 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java @@ -1,19 +1,20 @@ package tech.picnic.errorprone.refasterrules; import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChooser; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; +/** Refaster rules related to BugChecker classes. */ @OnlineDocumentation final class BugCheckerRules { private BugCheckerRules() {} /** - * Drop {@link - * BugCheckerRefactoringTestHelper#setFixChooser(BugCheckerRefactoringTestHelper.FixChooser)} when - * set to the default {@link FixChoosers#FIRST}. + * Drop {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} when set to the default + * {@link FixChoosers#FIRST}. */ static final class SetFixChooserDefault { @BeforeTemplate From fc064c5955a92ce284860371bc97da2822effe6d Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 12 Mar 2023 11:46:51 +0100 Subject: [PATCH 4/5] Suggestions --- error-prone-contrib/pom.xml | 1 + .../refasterrules/BugCheckerRules.java | 33 ++++++++++++++++--- .../BugCheckerRulesTestInput.java | 18 +++++++--- .../BugCheckerRulesTestOutput.java | 15 +++++++-- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index e15f6e58da..41f6b74e6d 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -37,6 +37,7 @@ ${groupId.error-prone} error_prone_test_helpers + provided ${project.groupId} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java index 224bf2d547..e3c55de509 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java @@ -3,23 +3,25 @@ import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChooser; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; +import com.google.errorprone.refaster.Refaster; import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; -/** Refaster rules related to BugChecker classes. */ +/** Refaster rules related to {@link com.google.errorprone.bugpatterns.BugChecker} classes. */ @OnlineDocumentation final class BugCheckerRules { private BugCheckerRules() {} /** - * Drop {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} when set to the default - * {@link FixChoosers#FIRST}. + * Avoid calling {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} with the + * default value of {@link FixChoosers#FIRST}. */ - static final class SetFixChooserDefault { + static final class BugCheckerRefactoringTestHelperIdentity { @BeforeTemplate BugCheckerRefactoringTestHelper before(BugCheckerRefactoringTestHelper helper) { - return helper.setFixChooser(FixChoosers.FIRST); + return Refaster.anyOf( + helper.setFixChooser(FixChoosers.FIRST), helper.setImportOrder("static-first")); } @AfterTemplate @@ -27,4 +29,25 @@ BugCheckerRefactoringTestHelper after(BugCheckerRefactoringTestHelper helper) { return helper; } } + + /** + * Prefer {@link BugCheckerRefactoringTestHelper.ExpectOutput#expectUnchanged()} over repeating + * the input. + */ + // XXX: This rule assumes that the full source code is specified as a single string, e.g. using a + // text block. Support for multi-line source code input would require a `BugChecker` + // implementation instead. + static final class BugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged { + @BeforeTemplate + BugCheckerRefactoringTestHelper before( + BugCheckerRefactoringTestHelper helper, String path, String source) { + return helper.addInputLines(path, source).addOutputLines(path, source); + } + + @AfterTemplate + BugCheckerRefactoringTestHelper after( + BugCheckerRefactoringTestHelper helper, String path, String source) { + return helper.addInputLines(path, source).expectUnchanged(); + } + } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java index a6a6d16e20..49eaa57945 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java @@ -3,7 +3,7 @@ import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; -import tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage; +import com.google.errorprone.bugpatterns.BugChecker; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { @@ -12,8 +12,18 @@ public ImmutableSet elidedTypesAndStaticImports() { return ImmutableSet.of(FixChoosers.class); } - BugCheckerRefactoringTestHelper testSetFixChooserDefault() { - return BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass()) - .setFixChooser(FixChoosers.FIRST); + ImmutableSet testBugCheckerRefactoringTestHelperIdentity() { + return ImmutableSet.of( + BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) + .setFixChooser(FixChoosers.FIRST), + BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) + .setImportOrder("static-first")); + } + + BugCheckerRefactoringTestHelper + testBugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged() { + return BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) + .addInputLines("A.java", "class A {}") + .addOutputLines("A.java", "class A {}"); } } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java index cb1ab7f433..39aef89159 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java @@ -3,7 +3,7 @@ import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; -import tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage; +import com.google.errorprone.bugpatterns.BugChecker; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { @@ -12,7 +12,16 @@ public ImmutableSet elidedTypesAndStaticImports() { return ImmutableSet.of(FixChoosers.class); } - BugCheckerRefactoringTestHelper testSetFixChooserDefault() { - return BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass()); + ImmutableSet testBugCheckerRefactoringTestHelperIdentity() { + return ImmutableSet.of( + BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()), + BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass())); + } + + BugCheckerRefactoringTestHelper + testBugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged() { + return BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) + .addInputLines("A.java", "class A {}") + .expectUnchanged(); } } From 7e05c7316715b85063c49b4be0d58bef688feee0 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Wed, 22 Mar 2023 18:30:06 +0100 Subject: [PATCH 5/5] Suggestions --- .../tech/picnic/errorprone/refasterrules/BugCheckerRules.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java index e3c55de509..a65dff6783 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java @@ -14,8 +14,8 @@ final class BugCheckerRules { private BugCheckerRules() {} /** - * Avoid calling {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} with the - * default value of {@link FixChoosers#FIRST}. + * Avoid calling {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} or {@link + * BugCheckerRefactoringTestHelper#setImportOrder(String)} with their respective default values. */ static final class BugCheckerRefactoringTestHelperIdentity { @BeforeTemplate