Fix various issues with datetime arithmetics

dkhalanskyjb · dkhalanskyjb · commit 63d3b4e45710 · 2025-06-11T12:23:53.000+02:00
Fixes #488 Fixes #487 Fixes #484
diff --git a/core/common/src/Instant.kt b/core/common/src/Instant.kt
@@ -450,27 +450,10 @@ public fun String.toInstant(): Instant = Instant.parse(this)
 public fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Instant = try {
     with(period) {
         val initialOffset = offsetIn(timeZone)
-        val initialLdt = toLocalDateTimeFailing(initialOffset)
-        val instantAfterMonths: Instant
-        val offsetAfterMonths: UtcOffset
-        val ldtAfterMonths: LocalDateTime
-        if (totalMonths != 0L) {
-            val unresolvedLdtWithMonths = initialLdt.plus(totalMonths, DateTimeUnit.MONTH)
-            instantAfterMonths = localDateTimeToInstant(unresolvedLdtWithMonths, timeZone, preferred = initialOffset)
-            offsetAfterMonths = instantAfterMonths.offsetIn(timeZone)
-            ldtAfterMonths = instantAfterMonths.toLocalDateTime(offsetAfterMonths)
-        } else {
-            instantAfterMonths = this@plus
-            offsetAfterMonths = initialOffset
-            ldtAfterMonths = initialLdt
-        }
-        val instantAfterMonthsAndDays = if (days != 0) {
-            val unresolvedLdtWithDays = ldtAfterMonths.plus(days, DateTimeUnit.DAY)
-            localDateTimeToInstant(unresolvedLdtWithDays, timeZone, preferred = offsetAfterMonths)
-        } else {
-            instantAfterMonths
-        }
-        instantAfterMonthsAndDays
+        val ldtPlusDate = toLocalDateTimeFailing(initialOffset)
+            .run { if (totalMonths != 0L) { plus(totalMonths, DateTimeUnit.MONTH) } else { this } }
+            .run { if (days != 0) { this.plus(days, DateTimeUnit.DAY) } else { this } }
+        localDateTimeToInstant(ldtPlusDate, timeZone, preferred = initialOffset)
             .run { if (totalNanoseconds != 0L) plus(0, totalNanoseconds).check(timeZone) else this }
     }.check(timeZone)
 } catch (e: ArithmeticException) {
@@ -522,21 +505,21 @@ public fun Instant.minus(period: DateTimePeriod, timeZone: TimeZone): Instant =
 public fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeriod {
     val initialOffset = offsetIn(timeZone)
     val initialLdt = toLocalDateTimeFailing(initialOffset)
-    val otherLdt = other.toLocalDateTimeFailing(other.offsetIn(timeZone))
-
-    val months = initialLdt.until(otherLdt, DateTimeUnit.MONTH) // `until` on dates never fails
-    val unresolvedLdtWithMonths = initialLdt.plus(months, DateTimeUnit.MONTH)
-        // won't throw: thisLdt + months <= otherLdt, which is known to be valid
-    val instantWithMonths = localDateTimeToInstant(unresolvedLdtWithMonths, timeZone, preferred = initialOffset)
-    val offsetWithMonths = instantWithMonths.offsetIn(timeZone)
-    val ldtWithMonths = instantWithMonths.toLocalDateTime(offsetWithMonths)
-    val days = ldtWithMonths.until(otherLdt, DateTimeUnit.DAY) // `until` on dates never fails
-    val unresolvedLdtWithDays = ldtWithMonths.plus(days, DateTimeUnit.DAY)
+    val otherLdt = other.toLocalDateTimeFailing(timeZone)
+    val timeAfterAddingDate =
+        localDateTimeToInstant(otherLdt.date.atTime(initialLdt.time), timeZone, preferred = initialOffset)
+    val delta = when {
+        other > this && timeAfterAddingDate > other -> -1 // addition won't throw: end date - date >= 1
+        other < this && timeAfterAddingDate < other -> 1 // addition won't throw: date - end date >= 1
+        else -> 0
+    }
+    val endDate = otherLdt.date.plus(delta, DateTimeUnit.DAY) // `endDate` is guaranteed to be valid
+    val unresolvedLdtWithDays = endDate.atTime(initialLdt.time)
     val newInstant = localDateTimeToInstant(unresolvedLdtWithDays, timeZone, preferred = initialOffset)
         // won't throw: thisLdt + days <= otherLdt
     val nanoseconds = newInstant.until(other, DateTimeUnit.NANOSECOND) // |otherLdt - thisLdt| < 24h
-
-    return buildDateTimePeriod(months, days.toInt(), nanoseconds)
+    val datePeriod = endDate - initialLdt.date
+    return buildDateTimePeriod(datePeriod.totalMonths, datePeriod.days, nanoseconds)
 }
 
 /**
@@ -555,8 +538,18 @@ public fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeri
  */
 public fun Instant.until(other: Instant, unit: DateTimeUnit, timeZone: TimeZone): Long =
     when (unit) {
-        is DateTimeUnit.DateBased ->
-            toLocalDateTimeFailing(offsetIn(timeZone)).until(other.toLocalDateTimeFailing(other.offsetIn(timeZone)), unit)
+        is DateTimeUnit.DateBased -> {
+            val start = toLocalDateTimeFailing(timeZone)
+            val end = other.toLocalDateTimeFailing(timeZone)
+            val timeAfterAddingDate =
+                localDateTimeToInstant(end.date.atTime(start.time), timeZone, preferred = this.offsetIn(timeZone))
+            val delta = when {
+                other > this && timeAfterAddingDate > other -> -1 // addition won't throw: end date - date >= 1
+                other < this && timeAfterAddingDate < other -> 1 // addition won't throw: date - end date >= 1
+                else -> 0
+            }
+            start.date.until(end.date.plus(delta, DateTimeUnit.DAY), unit)
+        }
         is DateTimeUnit.TimeBased -> {
             check(timeZone); other.check(timeZone)
             until(other, unit)
@@ -890,11 +883,14 @@ private fun Instant.toLocalDateTimeFailing(offset: UtcOffset): LocalDateTime = t
     throw DateTimeArithmeticException("Can not convert instant $this to LocalDateTime to perform computations", e)
 }
 
+private fun Instant.toLocalDateTimeFailing(timeZone: TimeZone): LocalDateTime =
+    toLocalDateTimeFailing(offsetIn(timeZone))
+
 /** Check that [Instant] fits in [LocalDateTime].
  * This is done on the results of computations for consistency with other platforms.
  */
 private fun Instant.check(zone: TimeZone): Instant = this@check.also {
-    toLocalDateTimeFailing(offsetIn(zone))
+    toLocalDateTimeFailing(zone)
 }
 
 private fun LocalDateTime.plus(value: Long, unit: DateTimeUnit.DateBased) =
@@ -908,18 +904,3 @@ private fun LocalDateTime.plus(value: Int, unit: DateTimeUnit.DateBased) =
  * @throws IllegalArgumentException if the boundaries of Instant are overflown
  */
 internal expect fun Instant.plus(secondsToAdd: Long, nanosToAdd: Long): Instant
-
-// org.threeten.bp.LocalDateTime#until
-internal fun LocalDateTime.until(other: LocalDateTime, unit: DateTimeUnit.DateBased): Long {
-    val otherDate = other.date
-    val delta = when {
-        otherDate > date && other.time < time -> -1 // addition won't throw: endDate - date >= 1
-        otherDate < date && other.time > time -> 1 // addition won't throw: date - endDate >= 1
-        else -> 0
-    }
-    val endDate = otherDate.plus(delta, DateTimeUnit.DAY)
-    return when (unit) {
-        is DateTimeUnit.MonthBased -> date.until(endDate, DateTimeUnit.MONTH) / unit.months
-        is DateTimeUnit.DayBased -> date.until(endDate, DateTimeUnit.DAY) / unit.days
-    }
-}
diff --git a/core/common/test/InstantTest.kt b/core/common/test/InstantTest.kt
@@ -16,6 +16,7 @@ import kotlin.time.Duration.Companion.hours
 import kotlin.time.Duration.Companion.milliseconds
 import kotlin.time.Duration.Companion.nanoseconds
 import kotlin.time.Duration.Companion.seconds
+import kotlin.time.TimeSource
 
 class InstantTest {
 
@@ -282,6 +283,104 @@ class InstantTest {
         assertEquals(end, end2)
     }
 
+    @Test
+    fun periodUntilSameSign() {
+        val tz = TimeZone.of("Europe/Berlin")
+        // Same sign when the period is positive but smaller than the non-DST-aware date-based period
+        assertPeriodSameSign(
+            LocalDateTime(2025, 3, 29, 2, 30).toInstant(tz).periodUntil(
+                LocalDateTime(2025, 3, 30, 3, 10).toInstant(tz), tz))
+        // Same sign when the period is negative but bigger than the non-DST-aware date-based period
+        assertPeriodSameSign(
+            Instant.parse("2025-07-27T00:59:00Z").periodUntil(
+                Instant.parse("2024-10-27T01:00:00Z"), tz))
+    }
+
+    @Test
+    @Ignore
+    fun periodUntilSameSignStressTest() {
+        val tz = TimeZone.of("Europe/Berlin")
+        val endMoment = TimeSource.Monotonic.markNow() + STRESS_TEST_DURATION
+        while (endMoment.elapsedNow().isNegative()) {
+            val start = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
+            val end = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
+            val period = start.periodUntil(end, tz)
+            assertPeriodSameSign(period)
+        }
+    }
+
+    @Test
+    fun untilDays() {
+        val tz = TimeZone.of("Europe/Berlin")
+        // No overshooting when the distance is positive but smaller than the non-DST-aware date-based distance
+        run {
+            val i1 = LocalDateTime(2025, 3, 29, 2, 30).toInstant(tz)
+            val i2 = LocalDateTime(2025, 3, 30, 3, 10).toInstant(tz)
+            val distance = i1.until(i2, DateTimeUnit.DAY, tz)
+            assertTrue(i2 > i1.plus(distance, DateTimeUnit.DAY, tz))
+        }
+        // No overshooting when the distance is negative but bigger than the non-DST-aware date-based distance
+        run {
+            val i1 = Instant.parse("2025-07-27T00:59:00Z")
+            val i2 = Instant.parse("2024-10-27T01:00:00Z")
+            val distance = i1.until(i2, DateTimeUnit.DAY, tz)
+            assertTrue(i2 < i1.plus(distance, DateTimeUnit.DAY, tz))
+        }
+    }
+
+    @Test
+    @Ignore
+    fun untilDaysStressTest() {
+        val tz = TimeZone.of("Europe/Berlin")
+        val endMoment = TimeSource.Monotonic.markNow() + STRESS_TEST_DURATION
+        while (endMoment.elapsedNow().isNegative()) {
+            val start = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
+            val end = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
+            val period = start.until(end, DateTimeUnit.DAY, tz)
+            val afterAdding = start.plus(period, DateTimeUnit.DAY, tz)
+            if (afterAdding > end && start < end || afterAdding < end && start > end) {
+                error("start: $start (${start.toLocalDateTime(tz)}), end: $end (${end.toLocalDateTime(tz)}), period: $period, " +
+                        "afterAdding: $afterAdding (${afterAdding.toLocalDateTime(tz)})")
+            }
+        }
+    }
+
+    @Test
+    fun dateTimePeriodWithGapBetweenMonthsAndDays() {
+        val zone = TimeZone.of("America/New_York")
+        // LocalDateTime(2019, 3, 10, 2, 0) is a gap.
+        // If months and days are not added atomically, the result will be adjusted.
+        val start = Instant.parse("2019-02-10T02:00:00-05:00")
+        val expectedEnd = Instant.parse("2019-03-11T02:00:00-04:00")
+        val end = start.plus(DateTimePeriod(months = 1, days = 1), zone)
+        // assertEquals(expectedEnd, end)
+        val period = start.periodUntil(end, zone)
+        assertEquals(end, start.plus(period, zone))
+    }
+
+    @Test
+    fun periodUntilWithGapBetweenMonthsAndDays() {
+        val start = Instant.parse("2024-01-30T01:10:00Z")
+        val end = Instant.parse("2025-04-01T01:10:00Z")
+        val tz = TimeZone.of("Europe/Berlin")
+        val period = start.periodUntil(end, tz)
+        assertEquals(DateTimePeriod(years = 1, months = 2, days = 2, hours = 1), period)
+        assertEquals(end, start.plus(period, tz), "start: $start, end: $end, period: $period")
+    }
+
+    @Test
+    @Ignore
+    fun periodUntilWithGapBetweenMonthsAndDaysStressTest() {
+        val tz = TimeZone.of("Europe/Berlin")
+        val endMoment = TimeSource.Monotonic.markNow() + STRESS_TEST_DURATION
+        while (endMoment.elapsedNow().isNegative()) {
+            val start = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
+            val end = Instant.fromEpochSeconds(Random.nextLong(1700000000, 1767222000))
+            val period = start.periodUntil(end, tz)
+            assertEquals(end, start.plus(period, tz), "start: $start, end: $end, period: $period")
+        }
+    }
+
     @Test
     fun diffInvariant() {
         repeat(STRESS_TEST_ITERATIONS) {
@@ -445,6 +544,12 @@ class InstantTest {
         assertFalse(Instant.MIN.isDistantFuture)
     }
 
+    private fun DateTimePeriod.hasSameSign() =
+        totalMonths >= 0 && days >= 0 && totalNanoseconds >= 0 ||
+                totalMonths <= 0 && days <= 0 && totalNanoseconds <= 0
+    private fun assertPeriodSameSign(period: DateTimePeriod) {
+        assertTrue(period.hasSameSign(), "Period $period has different signs for months, days and nanoseconds")
+    }
 }
 
 class InstantRangeTest {
diff --git a/core/common/test/assertions.kt b/core/common/test/assertions.kt
@@ -8,6 +8,7 @@ import kotlinx.datetime.DateTimeArithmeticException
 import kotlinx.datetime.DateTimeFormatException
 import kotlin.test.assertFailsWith
 import kotlin.test.fail
+import kotlin.time.Duration.Companion.minutes
 
 @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
 @kotlin.internal.InlineOnly
@@ -39,4 +40,9 @@ inline fun <T> assertIllegalArgument(message: String? = null, f: () -> T) {
 /**
  * The number of iterations to perform in nondeterministic tests.
  */
-const val STRESS_TEST_ITERATIONS = 1000
+const val STRESS_TEST_ITERATIONS = 1000
+
+/**
+ * How long to spin nondeterministic tests before giving up.
+ */
+val STRESS_TEST_DURATION = 5.minutes
