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 index 49335432408..ff93c479ff1 100644 --- 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 index 020e9fd9226..3adb431a9c7 100644 --- 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/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonStaticImportTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonStaticImportTest.java index 982a87f942d..86ef74d4e6f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NonStaticImportTest.java +++ b/error-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 optional1 = empty();", - " Optional optional2 = Optional.empty();", - "", - " ImmutableList list1 = copyOf(ImmutableList.of());", - " ImmutableList 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 list = ImmutableList.of(builder);", + " Instant minInstant = MIN;", + " Map 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 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);