From 6a50fe2d9c0e56457c9633e84b1d4040e42a0cba Mon Sep 17 00:00:00 2001 From: Phil Werli Date: Mon, 11 Nov 2024 10:42:05 +0100 Subject: [PATCH] Introduce `Instant{Identity,TruncatedTo{Milliseconds,Seconds}}` Refaster rules (#1395) While there, have `IdentityConversion` support `Instant#from`. --- .../bugpatterns/IdentityConversion.java | 2 + .../refasterrules/OptionalRules.java | 5 +- .../errorprone/refasterrules/TimeRules.java | 57 +++++++++++++++++++ .../bugpatterns/IdentityConversionTest.java | 6 ++ .../refasterrules/TimeRulesTestInput.java | 26 +++++++++ .../refasterrules/TimeRulesTestOutput.java | 25 ++++++++ 6 files changed, 117 insertions(+), 4 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java index 0c369404..14464da9 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversion.java @@ -39,6 +39,7 @@ import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; +import java.time.Instant; import java.util.Arrays; import java.util.List; import tech.picnic.errorprone.utils.SourceCode; @@ -84,6 +85,7 @@ public final class IdentityConversion extends BugChecker implements MethodInvoca ImmutableSetMultimap.class.getCanonicalName(), ImmutableTable.class.getCanonicalName()) .named("copyOf"), + staticMethod().onClass(Instant.class.getCanonicalName()).namedAnyOf("from"), staticMethod().onClass(Matchers.class.getCanonicalName()).namedAnyOf("allOf", "anyOf"), staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"), staticMethod() diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java index 99b7acd5..cc8928aa 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java @@ -398,10 +398,7 @@ final class OptionalRules { } } - /** - * Avoid unnecessary operations on an {@link Optional} that ultimately result in that very same - * {@link Optional}. - */ + /** Don't unnecessarily transform an {@link Optional} to an equivalent instance. */ static final class OptionalIdentity { @BeforeTemplate @SuppressWarnings("NestedOptionals") diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TimeRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TimeRules.java index 9e48cf80..be2382d5 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TimeRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/TimeRules.java @@ -142,6 +142,63 @@ final class TimeRules { } } + /** Don't unnecessarily transform an {@link Instant} to an equivalent instance. */ + static final class InstantIdentity { + @BeforeTemplate + Instant before(Instant instant, TemporalUnit temporalUnit) { + return Refaster.anyOf( + instant.plus(Duration.ZERO), + instant.plus(0, temporalUnit), + instant.plusNanos(0), + instant.plusMillis(0), + instant.plusSeconds(0), + instant.minus(Duration.ZERO), + instant.minus(0, temporalUnit), + instant.minusNanos(0), + instant.minusMillis(0), + instant.minusSeconds(0), + Instant.parse(instant.toString()), + instant.truncatedTo(ChronoUnit.NANOS), + Instant.ofEpochSecond(instant.getEpochSecond(), instant.getNano())); + } + + @AfterTemplate + Instant after(Instant instant) { + return instant; + } + } + + /** + * Prefer {@link Instant#truncatedTo(TemporalUnit)} over less obvious alternatives. + * + *

Note that {@link Instant#toEpochMilli()} throws an {@link ArithmeticException} for dates + * very far in the past or future, while the suggested alternative doesn't. + */ + static final class InstantTruncatedToMilliseconds { + @BeforeTemplate + Instant before(Instant instant) { + return Instant.ofEpochMilli(instant.toEpochMilli()); + } + + @AfterTemplate + Instant after(Instant instant) { + return instant.truncatedTo(ChronoUnit.MILLIS); + } + } + + /** Prefer {@link Instant#truncatedTo(TemporalUnit)} over less obvious alternatives. */ + static final class InstantTruncatedToSeconds { + @BeforeTemplate + Instant before(Instant instant) { + return Instant.ofEpochSecond(instant.getEpochSecond()); + } + + @AfterTemplate + Instant after(Instant instant) { + return instant.truncatedTo(ChronoUnit.SECONDS); + } + } + /** Prefer {@link Instant#atOffset(ZoneOffset)} over more verbose alternatives. */ static final class InstantAtOffset { @BeforeTemplate 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 631acdad..f9286f4e 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 @@ -28,6 +28,8 @@ final class IdentityConversionTest { "import com.google.common.collect.ImmutableTable;", "import com.google.errorprone.matchers.Matcher;", "import com.google.errorprone.matchers.Matchers;", + "import java.time.Instant;", + "import java.time.ZonedDateTime;", "import reactor.adapter.rxjava.RxJava2Adapter;", "import reactor.core.publisher.Flux;", "import reactor.core.publisher.Mono;", @@ -149,6 +151,10 @@ final class IdentityConversionTest { " // BUG: Diagnostic contains:", " ImmutableTable o11 = ImmutableTable.copyOf(ImmutableTable.of());", "", + " Instant instant1 = Instant.from(ZonedDateTime.now());", + " // BUG: Diagnostic contains:", + " Instant instant2 = Instant.from(Instant.now());", + "", " // BUG: Diagnostic contains:", " Matcher allOf1 = Matchers.allOf(instanceMethod());", " Matcher allOf2 = Matchers.allOf(instanceMethod(), staticMethod());", diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestInput.java index 58bac6c5..8b22e112 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestInput.java @@ -66,6 +66,32 @@ final class TimeRulesTest implements RefasterRuleCollectionTestCase { return Instant.EPOCH.atZone(ZoneOffset.UTC).toOffsetDateTime(); } + ImmutableSet testInstantIdentity() { + return ImmutableSet.of( + Instant.EPOCH.plusMillis(1).plus(Duration.ZERO), + Instant.EPOCH.plusMillis(2).plus(0, ChronoUnit.MILLIS), + Instant.EPOCH.plusMillis(3).plusNanos(0L), + Instant.EPOCH.plusMillis(4).plusMillis(0), + Instant.EPOCH.plusMillis(5).plusSeconds(0L), + Instant.EPOCH.plusMillis(6).minus(Duration.ZERO), + Instant.EPOCH.plusMillis(7).minus(0, ChronoUnit.SECONDS), + Instant.EPOCH.plusMillis(8).minusNanos(0L), + Instant.EPOCH.plusMillis(9).minusMillis(0), + Instant.EPOCH.plusMillis(10).minusSeconds(0L), + Instant.parse(Instant.EPOCH.plusMillis(11).toString()), + Instant.EPOCH.plusMillis(12).truncatedTo(ChronoUnit.NANOS), + Instant.ofEpochSecond( + Instant.EPOCH.plusMillis(13).getEpochSecond(), Instant.EPOCH.plusMillis(13).getNano())); + } + + Instant testInstantTruncatedToMilliseconds() { + return Instant.ofEpochMilli(Instant.EPOCH.toEpochMilli()); + } + + Instant testInstantTruncatedToSeconds() { + return Instant.ofEpochSecond(Instant.EPOCH.getEpochSecond()); + } + OffsetDateTime testInstantAtOffset() { return OffsetDateTime.ofInstant(Instant.EPOCH, ZoneOffset.UTC); } diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestOutput.java index ee4f347b..cd739835 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestOutput.java @@ -66,6 +66,31 @@ final class TimeRulesTest implements RefasterRuleCollectionTestCase { return OffsetDateTime.ofInstant(Instant.EPOCH, ZoneOffset.UTC); } + ImmutableSet testInstantIdentity() { + return ImmutableSet.of( + Instant.EPOCH.plusMillis(1), + Instant.EPOCH.plusMillis(2), + Instant.EPOCH.plusMillis(3), + Instant.EPOCH.plusMillis(4), + Instant.EPOCH.plusMillis(5), + Instant.EPOCH.plusMillis(6), + Instant.EPOCH.plusMillis(7), + Instant.EPOCH.plusMillis(8), + Instant.EPOCH.plusMillis(9), + Instant.EPOCH.plusMillis(10), + Instant.EPOCH.plusMillis(11), + Instant.EPOCH.plusMillis(12), + Instant.EPOCH.plusMillis(13)); + } + + Instant testInstantTruncatedToMilliseconds() { + return Instant.EPOCH.truncatedTo(ChronoUnit.MILLIS); + } + + Instant testInstantTruncatedToSeconds() { + return Instant.EPOCH.truncatedTo(ChronoUnit.SECONDS); + } + OffsetDateTime testInstantAtOffset() { return Instant.EPOCH.atOffset(ZoneOffset.UTC); }