Simplify tests

PicnicSupermarket · Oct 26, 2023 · b6a4b78 · b6a4b78
1 parent 24c4bff
commit b6a4b78
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 44 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonStaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NonStaticImport.java
@@ -34,6 +34,7 @@
  * A {@link BugChecker} that flags static imports of type members that should *not* be statically
  * imported.
  */
+// XXX: This check is closely linked to `StaticImport`. Consider merging the two.
 // XXX: Add suppression support. If qualification of one more more identifiers is suppressed, then
 // the associated static import should *not* be removed.
 // XXX: Also introduce logic that disallows statically importing `ZoneOffset.ofHours` and other

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StaticImport.java
@@ -30,6 +30,7 @@
 import java.util.Optional;
 
 /** A {@link BugChecker} that flags type members that can and should be statically imported. */
+// XXX: This check is closely linked to `NonStaticImport`. Consider merging the two.
 // XXX: Tricky cases:
 // - `org.springframework.http.HttpStatus` (not always an improvement, and `valueOf` must
 //    certainly be excluded)

diff --git a/...r-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonStaticImportTest.java b/...r-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonStaticImportTest.java
@@ -38,21 +38,13 @@ void candidateIdentifiersDoNotClash() {
   @Test
   void identification() {
     CompilationTestHelper.newInstance(NonStaticImport.class, getClass())
-        .addSourceLines(
-            "pkg/B.java",
-            "package pkg;",
-            "",
-            "public final class B {",
-            "  public int MIN = 1;",
-            "",
-            "  public static class INSTANCE {}",
-            "}")
-        .addSourceLines("MAX_VALUE.java", "", "public final class MAX_VALUE {}")
         .addSourceLines(
             "pkg/A.java",
             "package pkg;",
             "",
             "// BUG: Diagnostic contains:",
+            "import static com.google.common.base.Strings.nullToEmpty;",
+            "// BUG: Diagnostic contains:",
             "import static com.google.common.collect.ImmutableList.copyOf;",
             "// BUG: Diagnostic contains:",
             "import static java.lang.Integer.MAX_VALUE;",
@@ -62,6 +54,8 @@ void identification() {
             "import static java.time.Clock.systemUTC;",
             "// BUG: Diagnostic contains:",
             "import static java.time.Instant.MIN;",
+            "// BUG: Diagnostic contains:",
+            "import static java.time.ZoneOffset.SHORT_IDS;",
             "import static java.time.ZoneOffset.UTC;",
             "// BUG: Diagnostic contains:",
             "import static java.util.Collections.min;",
@@ -70,55 +64,50 @@ void identification() {
             "import static java.util.Locale.ROOT;",
             "// BUG: Diagnostic contains:",
             "import static java.util.Optional.empty;",
+            "import static pkg.A.WithMethodThatIsSelectivelyFlagged.list;",
             "",
             "import com.google.common.collect.ImmutableList;",
             "import com.google.common.collect.ImmutableSet;",
-            "import java.time.Clock;",
             "import java.time.Instant;",
             "import java.time.ZoneOffset;",
             "import java.util.Locale;",
-            "import java.util.Optional;",
-            "import pkg.B.INSTANCE;",
+            "import java.util.Map;",
+            "import pkg.A.Wrapper.ZERO;",
             "",
             "class A {",
             "  private Integer MIN_VALUE = 12;",
             "",
             "  void m() {",
+            "    nullToEmpty(null);",
+            "    copyOf(ImmutableList.of());",
+            "    int max = MAX_VALUE;",
+            "    int min = MIN_VALUE;",
             "    systemUTC();",
-            "    Clock.systemUTC();",
-            "    ZoneOffset utcIsExempted = UTC;",
-            "",
-            "    Optional<Integer> optional1 = empty();",
-            "    Optional<Integer> optional2 = Optional.empty();",
-            "",
-            "    ImmutableList<Integer> list1 = copyOf(ImmutableList.of());",
-            "    ImmutableList<Integer> list2 = ImmutableList.copyOf(ImmutableList.of());",
-            "",
-            "    Locale locale1 = ROOT;",
-            "    Locale locale2 = Locale.ROOT;",
-            "    Locale isNotACandidate = ENGLISH;",
-            "",
-            "    Instant instant1 = MIN;",
-            "    Instant instant2 = Instant.MIN;",
-            "",
-            "    ImmutableSet.of(min(ImmutableSet.of()));",
-            "",
-            "    Object builder = null;",
-            "    ImmutableList<Object> list = ImmutableList.of(builder);",
+            "    Instant minInstant = MIN;",
+            "    Map<String, String> shortIds = SHORT_IDS;",
+            "    ZoneOffset utc = UTC;",
+            "    min(ImmutableSet.of());",
+            "    Locale english = ENGLISH;",
+            "    Locale root = ROOT;",
+            "    empty();",
+            "",
+            "    list();",
+            "    new ZERO();",
+            "  }",
             "",
-            "    Integer refersToMemberVariable = MIN_VALUE;",
-            "    Integer minIsNotStatic = new B().MIN;",
-            "    Object regularImport = new INSTANCE();",
-            "    MAX_VALUE maxValue = new MAX_VALUE();",
+            "  static final class WithMethodThatIsSelectivelyFlagged {",
+            "    static ImmutableList<String> list() {",
+            "      return ImmutableList.of();",
+            "    }",
+            "  }",
             "",
-            "    Integer from = 12;",
-            "    Integer i1 = from;",
+            "  static final class Wrapper {",
+            "    static final class ZERO {}",
             "  }",
             "}")
         .doTest();
   }
 
-  // XXX: Test multiple replacements of the same import.
   @Test
   void replacement() {
     BugCheckerRefactoringTestHelper.newInstance(NonStaticImport.class, getClass())
@@ -161,8 +150,9 @@ void replacement() {
             "  }",
             "",
             "  private static final class WithCustomConstant {",
-            "    private static final int MIN = 42;",
-            "    private static final int MAX = MIN;",
+            "    private static final Instant MIN = Instant.EPOCH;",
+            "    private static final Instant OTHER = MIN;",
+            "    private static final Instant OTHER_MAX = MAX;",
             "  }",
             "}")
         .addOutputLines(
@@ -196,8 +186,9 @@ void replacement() {
             "  }",
             "",
             "  private static final class WithCustomConstant {",
-            "    private static final int MIN = 42;",
-            "    private static final int MAX = MIN;",
+            "    private static final Instant MIN = Instant.EPOCH;",
+            "    private static final Instant OTHER = MIN;",
+            "    private static final Instant OTHER_MAX = Instant.MAX;",
             "  }",
             "}")
         .doTest(TestMode.TEXT_MATCH);