Introduce Instant{Identity,TruncatedTo{Milliseconds,Seconds}} Refas…

…ter rules (#1395) While there, have `IdentityConversion` support `Instant#from`.
PicnicSupermarket · Nov 11, 2024 · 6a50fe2 · 6a50fe2
1 parent ed32cba
commit 6a50fe2
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 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
@@ -39,6 +39,7 @@
 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
@@ -398,10 +398,7 @@ Optional<T> after(Optional<T> optional1, Optional<T> optional2) {
     }
   }
 
-  /**
-   * 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<T> {
     @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
@@ -142,6 +142,63 @@ OffsetDateTime after(Instant instant, ZoneId zoneId) {
     }
   }
 
+  /** 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.
+   *
+   * <p>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/...rone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java b/...rone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionTest.java
@@ -28,6 +28,8 @@ void identification() {
             "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 @@ void identification() {
             "    // BUG: Diagnostic contains:",
             "    ImmutableTable<Object, Object, Object> 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/...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestInput.java b/...e-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestInput.java
@@ -66,6 +66,32 @@ OffsetDateTime testOffsetDateTimeOfInstant() {
     return Instant.EPOCH.atZone(ZoneOffset.UTC).toOffsetDateTime();
   }
 
+  ImmutableSet<Instant> 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/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestOutput.java b/...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/TimeRulesTestOutput.java
@@ -66,6 +66,31 @@ OffsetDateTime testOffsetDateTimeOfInstant() {
     return OffsetDateTime.ofInstant(Instant.EPOCH, ZoneOffset.UTC);
   }
 
+  ImmutableSet<Instant> 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);
   }