Revert a few files and apply feedback

PicnicSupermarket · Jan 6, 2023 · 6be6585 · 6be6585
1 parent d4ce9e6
commit 6be6585
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 74 deletions.
diff --git a/...one-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparisonTest.java b/...one-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparisonTest.java
@@ -11,7 +11,9 @@
 // - During actual compilation only the first replacement is applied.
 // XXX: Can we perhaps work-around this by describing the fixes in reverse order?
 final class PrimitiveComparisonTest {
-  /* The logic for `char` and `short` is exactly analogous to the `byte` case. */
+  /**
+   * @implNote The logic for `char` and `short` is exactly analogous to the `byte` case.
+   */
   @Test
   void byteComparison() {
     CompilationTestHelper.newInstance(PrimitiveComparison.class, getClass())

diff --git a/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionTest.java b/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionTest.java
@@ -10,9 +10,19 @@
 // `String.valueOf(null)` may not. That is because the latter matches `String#valueOf(char[])`. We
 // could special-case `null` arguments, but that doesn't seem worth the trouble.
 final class RedundantStringConversionTest {
+  private final CompilationTestHelper compilationTestHelper =
+      CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass());
+  private final CompilationTestHelper customizedCompilationTestHelper =
+      CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+          .setArgs(
+              ImmutableList.of(
+                  "-XepOpt:RedundantStringConversion:ExtraConversionMethods=java.lang.Enum#name(),A#name(),A.B#toString(int)"));
+  private final BugCheckerRefactoringTestHelper refactoringTestHelper =
+      BugCheckerRefactoringTestHelper.newInstance(RedundantStringConversion.class, getClass());
+
   @Test
   void identificationOfIdentityTransformation() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "class A {",
@@ -35,7 +45,7 @@ void identificationOfIdentityTransformation() {
 
   @Test
   void identificationWithinMutatingAssignment() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import java.math.BigInteger;",
@@ -96,7 +106,7 @@ void identificationWithinMutatingAssignment() {
 
   @Test
   void identificationWithinBinaryOperation() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import java.math.BigInteger;",
@@ -191,7 +201,7 @@ void identificationWithinBinaryOperation() {
 
   @Test
   void identificationWithinStringBuilderMethod() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import java.math.BigInteger;",
@@ -246,7 +256,7 @@ void identificationWithinStringBuilderMethod() {
   // XXX: Also test the other formatter methods.
   @Test
   void identificationWithinFormatterMethod() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import java.util.Formattable;",
@@ -291,7 +301,7 @@ void identificationWithinFormatterMethod() {
 
   @Test
   void identificationWithinGuavaGuardMethod() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import static com.google.common.base.Preconditions.checkArgument;",
@@ -351,7 +361,7 @@ void identificationWithinGuavaGuardMethod() {
 
   @Test
   void identificationWithinSlf4jLoggerMethod() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import java.util.Formattable;",
@@ -406,10 +416,7 @@ void identificationWithinSlf4jLoggerMethod() {
 
   @Test
   void identificationOfCustomConversionMethod() {
-    CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass())
-        .setArgs(
-            ImmutableList.of(
-                "-XepOpt:RedundantStringConversion:ExtraConversionMethods=java.lang.Enum#name(),A#name(),A.B#toString(int)"))
+    customizedCompilationTestHelper
         .addSourceLines(
             "A.java",
             "import java.math.RoundingMode;",
@@ -502,7 +509,7 @@ void identificationOfCustomConversionMethod() {
 
   @Test
   void replacement() {
-    BugCheckerRefactoringTestHelper.newInstance(RedundantStringConversion.class, getClass())
+    refactoringTestHelper
         .addInputLines(
             "A.java",
             "class A {",

diff --git a/...e-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java b/...e-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StringCaseLocaleUsageTest.java
@@ -1,7 +1,6 @@
 package tech.picnic.errorprone.bugpatterns;
 
-import static com.google.errorprone.BugCheckerRefactoringTestHelper.newInstance;
-
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
 import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
 import com.google.errorprone.CompilationTestHelper;
@@ -48,7 +47,7 @@ void identification() {
 
   @Test
   void replacementFirstSuggestedFix() {
-    newInstance(StringCaseLocaleUsage.class, getClass())
+    BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass())
         .setFixChooser(FixChoosers.FIRST)
         .addInputLines(
             "A.java",
@@ -87,7 +86,7 @@ void replacementFirstSuggestedFix() {
 
   @Test
   void replacementSecondSuggestedFix() {
-    newInstance(StringCaseLocaleUsage.class, getClass())
+    BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass())
         .setFixChooser(FixChoosers.SECOND)
         .addInputLines(
             "A.java",

diff --git a/...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java b/...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ThirdPartyLibraryTest.java
@@ -18,9 +18,12 @@
 import reactor.core.publisher.Flux;
 
 final class ThirdPartyLibraryTest {
+  private final CompilationTestHelper compilationTestHelper =
+      CompilationTestHelper.newInstance(TestChecker.class, getClass());
+
   @Test
   void isIntroductionAllowed() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "// BUG: Diagnostic contains: ASSERTJ: true, GUAVA: true, NEW_RELIC_AGENT_API: true, REACTOR: true",
@@ -30,7 +33,7 @@ void isIntroductionAllowed() {
 
   @Test
   void isIntroductionAllowedWitnessClassesInSymtab() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    compilationTestHelper
         .addSourceLines(
             "A.java",
             "import com.google.common.collect.ImmutableList;",
@@ -52,7 +55,7 @@ void isIntroductionAllowedWitnessClassesInSymtab() {
 
   @Test
   void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    compilationTestHelper
         .withClasspath(ImmutableList.class, Flux.class)
         .addSourceLines(
             "A.java",
@@ -63,7 +66,7 @@ void isIntroductionAllowedWitnessClassesPartiallyOnClassPath() {
 
   @Test
   void isIntroductionAllowedWitnessClassesNotOnClassPath() {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    compilationTestHelper
         .withClasspath()
         .addSourceLines(
             "A.java",
@@ -76,7 +79,7 @@ void isIntroductionAllowedWitnessClassesNotOnClassPath() {
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void isIntroductionAllowedIgnoreClasspathCompat(boolean ignoreClassPath) {
-    CompilationTestHelper.newInstance(TestChecker.class, getClass())
+    compilationTestHelper
         .setArgs("-XepOpt:ErrorProneSupport:IgnoreClasspathCompat=" + ignoreClassPath)
         .withClasspath(ImmutableList.class, Flux.class)
         .addSourceLines(

diff --git a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java
@@ -25,32 +25,41 @@
 import tech.picnic.errorprone.refaster.ErrorProneFork;
 
 final class RefasterTest {
+  private final CompilationTestHelper compilationHelper =
+      CompilationTestHelper.newInstance(Refaster.class, getClass())
+          .matchAllDiagnostics()
+          .expectErrorMessage(
+              "StringOfSizeZeroRule",
+              containsPattern(
+                  "\\[Refaster Rule\\] FooRules\\.StringOfSizeZeroRule: Refactoring opportunity\\s+.+\\s+"))
+          .expectErrorMessage(
+              "StringOfSizeOneRule",
+              containsPattern(
+                  "\\[Refaster Rule\\] FooRules\\.StringOfSizeOneRule: "
+                      + "A custom description about matching single-char strings\\s+.+\\s+"
+                      + "\\(see https://error-prone.picnic.tech/refasterrules/FooRules#StringOfSizeOneRule\\)"))
+          .expectErrorMessage(
+              "StringOfSizeTwoRule",
+              containsPattern(
+                  "\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeTwoRule: "
+                      + "A custom subgroup description\\s+.+\\s+"
+                      + "\\(see https://example.com/rule/FooRules#ExtraGrouping.StringOfSizeTwoRule\\)"))
+          .expectErrorMessage(
+              "StringOfSizeThreeRule",
+              containsPattern(
+                  "\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeThreeRule: "
+                      + "A custom description about matching three-char strings\\s+.+\\s+"
+                      + "\\(see https://example.com/custom\\)"));
+  private final BugCheckerRefactoringTestHelper refactoringTestHelper =
+      BugCheckerRefactoringTestHelper.newInstance(Refaster.class, getClass());
+  private final BugCheckerRefactoringTestHelper restrictedRefactoringTestHelper =
+      BugCheckerRefactoringTestHelper.newInstance(Refaster.class, getClass())
+          .setArgs(
+              "-XepOpt:Refaster:NamePattern=.*\\$(StringOfSizeZeroVerboseRule|StringOfSizeTwoRule)$");
+
   @Test
   void identification() {
-    CompilationTestHelper.newInstance(Refaster.class, getClass())
-        .matchAllDiagnostics()
-        .expectErrorMessage(
-            "StringOfSizeZeroRule",
-            containsPattern(
-                "\\[Refaster Rule\\] FooRules\\.StringOfSizeZeroRule: Refactoring opportunity\\s+.+\\s+"))
-        .expectErrorMessage(
-            "StringOfSizeOneRule",
-            containsPattern(
-                "\\[Refaster Rule\\] FooRules\\.StringOfSizeOneRule: "
-                    + "A custom description about matching single-char strings\\s+.+\\s+"
-                    + "\\(see https://error-prone.picnic.tech/refasterrules/FooRules#StringOfSizeOneRule\\)"))
-        .expectErrorMessage(
-            "StringOfSizeTwoRule",
-            containsPattern(
-                "\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeTwoRule: "
-                    + "A custom subgroup description\\s+.+\\s+"
-                    + "\\(see https://example.com/rule/FooRules#ExtraGrouping.StringOfSizeTwoRule\\)"))
-        .expectErrorMessage(
-            "StringOfSizeThreeRule",
-            containsPattern(
-                "\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeThreeRule: "
-                    + "A custom description about matching three-char strings\\s+.+\\s+"
-                    + "\\(see https://example.com/custom\\)"))
+    compilationHelper
         .addSourceLines(
             "A.java",
             "class A {",
@@ -160,30 +169,7 @@ void severityAssignment(
       ImmutableList<String> arguments, ImmutableList<SeverityLevel> expectedSeverities) {
     assertThatThrownBy(
             () ->
-                CompilationTestHelper.newInstance(Refaster.class, getClass())
-                    .matchAllDiagnostics()
-                    .expectErrorMessage(
-                        "StringOfSizeZeroRule",
-                        containsPattern(
-                            "\\[Refaster Rule\\] FooRules\\.StringOfSizeZeroRule: Refactoring opportunity\\s+.+\\s+"))
-                    .expectErrorMessage(
-                        "StringOfSizeOneRule",
-                        containsPattern(
-                            "\\[Refaster Rule\\] FooRules\\.StringOfSizeOneRule: "
-                                + "A custom description about matching single-char strings\\s+.+\\s+"
-                                + "\\(see https://error-prone.picnic.tech/refasterrules/FooRules#StringOfSizeOneRule\\)"))
-                    .expectErrorMessage(
-                        "StringOfSizeTwoRule",
-                        containsPattern(
-                            "\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeTwoRule: "
-                                + "A custom subgroup description\\s+.+\\s+"
-                                + "\\(see https://example.com/rule/FooRules#ExtraGrouping.StringOfSizeTwoRule\\)"))
-                    .expectErrorMessage(
-                        "StringOfSizeThreeRule",
-                        containsPattern(
-                            "\\[Refaster Rule\\] FooRules\\.ExtraGrouping\\.StringOfSizeThreeRule: "
-                                + "A custom description about matching three-char strings\\s+.+\\s+"
-                                + "\\(see https://example.com/custom\\)"))
+                compilationHelper
                     .setArgs(arguments)
                     .addSourceLines(
                         "A.java",
@@ -234,7 +220,7 @@ private static SeverityLevel toSeverityLevel(String compilerDiagnosticsPrefix) {
 
   @Test
   void replacement() {
-    BugCheckerRefactoringTestHelper.newInstance(Refaster.class, getClass())
+    refactoringTestHelper
         .addInputLines(
             "A.java",
             "class A {",
@@ -260,9 +246,7 @@ void replacement() {
 
   @Test
   void restrictedReplacement() {
-    BugCheckerRefactoringTestHelper.newInstance(Refaster.class, getClass())
-        .setArgs(
-            "-XepOpt:Refaster:NamePattern=.*\\$(StringOfSizeZeroVerboseRule|StringOfSizeTwoRule)$")
+    restrictedRefactoringTestHelper
         .addInputLines(
             "A.java",
             "class A {",