From 5947748455da689aee8a022bea899b8edeb59288 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 11 Dec 2020 16:39:43 +0300 Subject: [PATCH 01/12] Change DateTimePeriod to be normalized Now, it only has three components: months, days, and nanoseconds, and all the other properties are just representations of these ones. This way, for each DateTimePeriod there exists a well-defined ISO-8601 representation, and `toString()` behaves correctly. Fixes https://github.com/Kotlin/kotlinx-datetime/issues/79 --- core/common/src/DateTimePeriod.kt | 130 ++++++++++++++----------- core/common/src/Instant.kt | 12 +-- core/common/src/math.kt | 19 ++++ core/common/test/DateTimePeriodTest.kt | 11 ++- core/common/test/InstantTest.kt | 19 ++-- core/js/src/Instant.kt | 6 +- core/jvm/src/Instant.kt | 6 +- core/jvm/test/ConvertersTest.kt | 10 +- core/native/src/Instant.kt | 6 +- 9 files changed, 134 insertions(+), 85 deletions(-) diff --git a/core/common/src/DateTimePeriod.kt b/core/common/src/DateTimePeriod.kt index bc52a732d..82fda429b 100644 --- a/core/common/src/DateTimePeriod.kt +++ b/core/common/src/DateTimePeriod.kt @@ -5,26 +5,29 @@ package kotlinx.datetime +import kotlin.math.* import kotlin.time.Duration import kotlin.time.ExperimentalTime // TODO: could be error-prone without explicitly named params sealed class DateTimePeriod { - abstract val years: Int - abstract val months: Int + internal abstract val totalMonths: Int abstract val days: Int - abstract val hours: Int - abstract val minutes: Int - abstract val seconds: Long - abstract val nanoseconds: Long + internal abstract val totalNanoseconds: Long - private fun allNotPositive() = - years <= 0 && months <= 0 && days <= 0 && hours <= 0 && minutes <= 0 && seconds <= 0 && nanoseconds <= 0 && - (years or months or days or hours or minutes != 0 || seconds or nanoseconds != 0L) + val years: Int get() = totalMonths / 12 + val months: Int get() = totalMonths % 12 + open val hours: Int get() = (totalNanoseconds / 3_600_000_000_000).toInt() + open val minutes: Int get() = ((totalNanoseconds % 3_600_000_000_000) / 60_000_000_000).toInt() + open val seconds: Int get() = ((totalNanoseconds % 60_000_000_000) / NANOS_PER_ONE).toInt() + open val nanoseconds: Int get() = (totalNanoseconds % NANOS_PER_ONE).toInt() + + private fun allNonpositive() = + totalMonths <= 0 && days <= 0 && totalNanoseconds <= 0 && (totalMonths or days != 0 || totalNanoseconds != 0L) override fun toString(): String = buildString { - val sign = if (allNotPositive()) { append('-'); -1 } else 1 + val sign = if (allNonpositive()) { append('-'); -1 } else 1 append('P') if (years != 0) append(years * sign).append('Y') if (months != 0) append(months * sign).append('M') @@ -32,9 +35,14 @@ sealed class DateTimePeriod { var t = "T" if (hours != 0) append(t).append(hours * sign).append('H').also { t = "" } if (minutes != 0) append(t).append(minutes * sign).append('M').also { t = "" } - if (seconds != 0L || nanoseconds != 0L) { - append(t).append(seconds * sign) - if (nanoseconds != 0L) append('.').append((nanoseconds * sign).toString().padStart(9, '0')) + if (seconds or nanoseconds != 0) { + append(t) + append(when { + seconds != 0 -> seconds * sign + nanoseconds * sign < 0 -> "-0" + else -> "0" + }) + if (nanoseconds != 0) append('.').append((nanoseconds.absoluteValue).toString().padStart(9, '0')) append('S') } @@ -45,83 +53,95 @@ sealed class DateTimePeriod { if (this === other) return true if (other !is DateTimePeriod) return false - if (years != other.years) return false - if (months != other.months) return false + if (totalMonths != other.totalMonths) return false if (days != other.days) return false - if (hours != other.hours) return false - if (minutes != other.minutes) return false - if (seconds != other.seconds) return false - if (nanoseconds != other.nanoseconds) return false + if (totalNanoseconds != other.totalNanoseconds) return false return true } override fun hashCode(): Int { - var result = years - result = 31 * result + months + var result = totalMonths result = 31 * result + days - result = 31 * result + hours - result = 31 * result + minutes - result = 31 * result + seconds.hashCode() - result = 31 * result + nanoseconds.hashCode() + result = 31 * result + totalNanoseconds.hashCode() return result } // TODO: parsing from iso string } -class DatePeriod( - override val years: Int = 0, - override val months: Int = 0, - override val days: Int = 0 +class DatePeriod internal constructor( + internal override val totalMonths: Int, + override val days: Int = 0, ) : DateTimePeriod() { + constructor(years: Int = 0, months: Int = 0, days: Int = 0): this(totalMonths(years, months), days) + // avoiding excessive computations override val hours: Int get() = 0 override val minutes: Int get() = 0 - override val seconds: Long get() = 0 - override val nanoseconds: Long get() = 0 + override val seconds: Int get() = 0 + override val nanoseconds: Int get() = 0 + override val totalNanoseconds: Long get() = 0 } private class DateTimePeriodImpl( - override val years: Int = 0, - override val months: Int = 0, - override val days: Int = 0, - override val hours: Int = 0, - override val minutes: Int = 0, - override val seconds: Long = 0, - override val nanoseconds: Long = 0 + internal override val totalMonths: Int = 0, + override val days: Int = 0, + override val totalNanoseconds: Long = 0, ) : DateTimePeriod() +// TODO: these calculations fit in a JS Number. Possible to do an expect/actual here. +private fun totalMonths(years: Int, months: Int): Int = + when (val totalMonths = years.toLong() * 12 + months.toLong()) { + in Int.MIN_VALUE..Int.MAX_VALUE -> totalMonths.toInt() + else -> throw IllegalArgumentException("The total number of months in $years years and $months months overflows an Int") + } + +private fun totalNanoseconds(hours: Int, minutes: Int, seconds: Int, nanoseconds: Long): Long { + val totalMinutes: Long = hours.toLong() * 60 + minutes + // absolute value at most 61 * Int.MAX_VALUE + val totalMinutesAsSeconds: Long = totalMinutes * 60 + // absolute value at most 61 * 60 * Int.MAX_VALUE < 64 * 64 * 2^31 = 2^43 + val minutesAndNanosecondsAsSeconds: Long = totalMinutesAsSeconds + nanoseconds / NANOS_PER_ONE + // absolute value at most 2^43 + 2^63 / 10^9 < 2^43 + 2^34 < 2^44 + val totalSeconds = minutesAndNanosecondsAsSeconds + seconds + // absolute value at most 2^44 + 2^31 < 2^45 + return try { + multiplyAndAdd(totalSeconds, 1_000_000_000, nanoseconds % NANOS_PER_ONE) + } catch (e: ArithmeticException) { + throw IllegalArgumentException("The total number of nanoseconds in $hours hours, $minutes minutes, $seconds seconds, and $nanoseconds nanoseconds overflows a Long") + } +} + +internal fun buildDateTimePeriod(totalMonths: Int = 0, days: Int = 0, totalNanoseconds: Long): DateTimePeriod = + if (totalNanoseconds != 0L) + DateTimePeriodImpl(totalMonths, days, totalNanoseconds) + else + DatePeriod(totalMonths, days) + fun DateTimePeriod( years: Int = 0, months: Int = 0, days: Int = 0, hours: Int = 0, minutes: Int = 0, - seconds: Long = 0, + seconds: Int = 0, nanoseconds: Long = 0 -): DateTimePeriod = if (hours or minutes != 0 || seconds or nanoseconds != 0L) - DateTimePeriodImpl(years, months, days, hours, minutes, seconds, nanoseconds) -else - DatePeriod(years, months, days) +): DateTimePeriod = buildDateTimePeriod(totalMonths(years, months), days, + totalNanoseconds(hours, minutes, seconds, nanoseconds)) @OptIn(ExperimentalTime::class) fun Duration.toDateTimePeriod(): DateTimePeriod = toComponents { hours, minutes, seconds, nanoseconds -> - DateTimePeriod(hours = hours, minutes = minutes, seconds = seconds.toLong(), nanoseconds = nanoseconds.toLong()) + buildDateTimePeriod(totalNanoseconds = totalNanoseconds(hours, minutes, seconds, nanoseconds.toLong())) } -operator fun DateTimePeriod.plus(other: DateTimePeriod): DateTimePeriod = DateTimePeriod( - safeAdd(this.years, other.years), - safeAdd(this.months, other.months), - safeAdd(this.days, other.days), - safeAdd(this.hours, other.hours), - safeAdd(this.minutes, other.minutes), - safeAdd(this.seconds, other.seconds), - safeAdd(this.nanoseconds, other.nanoseconds) +operator fun DateTimePeriod.plus(other: DateTimePeriod): DateTimePeriod = buildDateTimePeriod( + safeAdd(totalMonths, other.totalMonths), + safeAdd(days, other.days), + safeAdd(totalNanoseconds, other.totalNanoseconds), ) operator fun DatePeriod.plus(other: DatePeriod): DatePeriod = DatePeriod( - safeAdd(this.years, other.years), - safeAdd(this.months, other.months), - safeAdd(this.days, other.days) + safeAdd(totalMonths, other.totalMonths), + safeAdd(days, other.days), ) diff --git a/core/common/src/Instant.kt b/core/common/src/Instant.kt index 46b8fce0a..ed19d0991 100644 --- a/core/common/src/Instant.kt +++ b/core/common/src/Instant.kt @@ -192,19 +192,15 @@ public fun Instant.minus(period: DateTimePeriod, timeZone: TimeZone): Instant = /* An overflow can happen for any component, but we are only worried about nanoseconds, as having an overflow in any other component means that `plus` will throw due to the minimum value of the numeric type overflowing the platform-specific limits. */ - if (period.nanoseconds != Long.MIN_VALUE) { - val negatedPeriod = with(period) { - DateTimePeriod(-years, -months, -days, -hours, -minutes, -seconds, -nanoseconds) - } + if (period.totalNanoseconds != Long.MIN_VALUE) { + val negatedPeriod = with(period) { buildDateTimePeriod(-totalMonths, -days, -totalNanoseconds) } plus(negatedPeriod, timeZone) } else { - val negatedPeriod = with(period) { - DateTimePeriod(-years, -months, -days, -hours, -minutes, -seconds, -(nanoseconds+1)) - } + val negatedPeriod = with(period) { buildDateTimePeriod(-totalMonths, -days, -(totalNanoseconds+1)) } plus(negatedPeriod, timeZone).plus(DateTimeUnit.NANOSECOND) } - /** +/** * Returns a [DateTimePeriod] representing the difference between `this` and [other] instants. * * The components of [DateTimePeriod] are calculated so that adding it to `this` instant results in the [other] instant. diff --git a/core/common/src/math.kt b/core/common/src/math.kt index 31e245509..c164f2ddd 100644 --- a/core/common/src/math.kt +++ b/core/common/src/math.kt @@ -161,3 +161,22 @@ internal fun multiplyAddAndDivide(d: Long, n: Long, r: Long, m: Long): Long { val (rd, rr) = multiplyAndDivide(md, n, m) return safeAdd(rd, safeAdd(mr / m, safeAdd(mr % m, rr) / m)) } + +/** + * Calculates [d] * [n] + [r], where [n] > 0 and |[r]| <= [n]. + * + * @throws ArithmeticException if the result overflows a long + */ +internal fun multiplyAndAdd(d: Long, n: Long, r: Long): Long { + var md = d + var mr = r + // make sure [md] and [mr] have the same sign + if (d > 0 && r < 0) { + md-- + mr += n + } else if (d < 0 && r > 0) { + md++ + mr -= n + } + return safeAdd(safeMultiply(md, n), mr) +} \ No newline at end of file diff --git a/core/common/test/DateTimePeriodTest.kt b/core/common/test/DateTimePeriodTest.kt index aa044fd0f..e03b1e5d7 100644 --- a/core/common/test/DateTimePeriodTest.kt +++ b/core/common/test/DateTimePeriodTest.kt @@ -17,7 +17,7 @@ class DateTimePeriodTest { assertEquals("P1Y", DateTimePeriod(years = 1).toString()) assertEquals("P1Y1M", DatePeriod(years = 1, months = 1).toString()) assertEquals("P11M", DateTimePeriod(months = 11).toString()) - assertEquals("P14M", DateTimePeriod(months = 14).toString()) // TODO: normalize or not + assertEquals("P1Y2M", DateTimePeriod(months = 14).toString()) // TODO: normalize or not assertEquals("P10M5D", DateTimePeriod(months = 10, days = 5).toString()) assertEquals("P1Y40D", DateTimePeriod(years = 1, days = 40).toString()) @@ -29,8 +29,15 @@ class DateTimePeriodTest { assertEquals("-P1DT1H", DateTimePeriod(days = -1, hours = -1).toString()) assertEquals("-P1M", DateTimePeriod(months = -1).toString()) - assertEquals("P-1Y-2M-3DT-4H-5M0.500000000S", + assertEquals("-P1Y2M3DT4H4M59.500000000S", DateTimePeriod(years = -1, months = -2, days = -3, hours = -4, minutes = -5, seconds = 0, nanoseconds = 500_000_000).toString()) + + assertEquals("PT277H46M39.999999999S", DateTimePeriod(nanoseconds = 999_999_999_999_999L).toString()) + assertEquals("PT0.999999999S", DateTimePeriod(seconds = 1, nanoseconds = -1L).toString()) + assertEquals("-PT0.000000001S", DateTimePeriod(nanoseconds = -1L).toString()) + assertEquals("P1DT-0.000000001S", DateTimePeriod(days = 1, nanoseconds = -1L).toString()) + assertEquals("-PT0.999999999S", DateTimePeriod(seconds = -1, nanoseconds = 1L).toString()) + assertEquals("P1DT-0.999999999S", DateTimePeriod(days = 1, seconds = -1, nanoseconds = 1L).toString()) } @Test diff --git a/core/common/test/InstantTest.kt b/core/common/test/InstantTest.kt index 2f5cba334..8f0e13948 100644 --- a/core/common/test/InstantTest.kt +++ b/core/common/test/InstantTest.kt @@ -468,9 +468,11 @@ class InstantRangeTest { fun periodArithmeticOutOfRange() { // Instant.plus(DateTimePeriod(), TimeZone) // Arithmetic overflow - for (instant in smallInstants + largeNegativeInstants + largePositiveInstants) { - assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(seconds = Long.MAX_VALUE), UTC) } - assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(seconds = Long.MIN_VALUE), UTC) } + for (instant in largePositiveInstants) { + assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(nanoseconds = Long.MAX_VALUE), UTC) } + } + for (instant in largeNegativeInstants) { + assertArithmeticFails("$instant") { instant.plus(DateTimePeriod(nanoseconds = Long.MIN_VALUE), UTC) } } // Arithmetic overflow in an Int for (instant in smallInstants + listOf(maxValidInstant)) { @@ -494,9 +496,8 @@ class InstantRangeTest { assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(nanoseconds = 1), UTC) } assertArithmeticFails { minValidInstant.plus(DateTimePeriod(nanoseconds = -1), UTC) } // Overflowing a LocalDateTime in intermediate computations - assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(seconds = 1, nanoseconds = -1_000_000_001), UTC) } - assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(hours = 1, minutes = -61), UTC) } - assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(days = 1, hours = -48), UTC) } + assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(days = 1, nanoseconds = -1_000_000_001), UTC) } + assertArithmeticFails { maxValidInstant.plus(DateTimePeriod(months = 1, days = -48), UTC) } } @Test @@ -540,9 +541,9 @@ class InstantRangeTest { @Test fun periodUntilOutOfRange() { // Instant.periodUntil - maxValidInstant.periodUntil(minValidInstant, UTC) - assertArithmeticFails { (maxValidInstant + 1.nanoseconds).periodUntil(minValidInstant, UTC) } - assertArithmeticFails { maxValidInstant.periodUntil(minValidInstant - 1.nanoseconds, UTC) } + maxValidInstant.periodUntil(maxValidInstant, UTC) + assertArithmeticFails { (maxValidInstant + 1.nanoseconds).periodUntil(maxValidInstant, UTC) } + assertArithmeticFails { minValidInstant.periodUntil(minValidInstant - 1.nanoseconds, UTC) } } @Test diff --git a/core/js/src/Instant.kt b/core/js/src/Instant.kt index a4f2e5073..b59ece296 100644 --- a/core/js/src/Instant.kt +++ b/core/js/src/Instant.kt @@ -116,8 +116,8 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst .run { if (days != 0) plusDays(days) as ZonedDateTime else this } .run { if (hours != 0) plusHours(hours) else this } .run { if (minutes != 0) plusMinutes(minutes) else this } - .run { if (seconds != 0L) plusSeconds(seconds.toDouble()) else this } - .run { if (nanoseconds != 0L) plusNanos(nanoseconds.toDouble()) else this } + .run { if (seconds != 0) plusSeconds(seconds) else this } + .run { if (nanoseconds != 0) plusNanos(nanoseconds.toDouble()) else this } }.toInstant().let(::Instant) } catch (e: Throwable) { if (e.isJodaDateTimeException()) throw DateTimeArithmeticException(e) @@ -191,7 +191,7 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT val time = thisZdt.until(otherZdt, ChronoUnit.NANOS).toDouble().nanoseconds time.toComponents { hours, minutes, seconds, nanoseconds -> - return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds.toLong(), nanoseconds.toLong()) + return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds, nanoseconds.toLong()) } } catch (e: Throwable) { if (e.isJodaDateTimeException()) throw DateTimeArithmeticException(e) else throw e diff --git a/core/jvm/src/Instant.kt b/core/jvm/src/Instant.kt index dfcace595..98f7b39b5 100644 --- a/core/jvm/src/Instant.kt +++ b/core/jvm/src/Instant.kt @@ -99,8 +99,8 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst .run { if (days != 0) plusDays(days.toLong()) else this } .run { if (hours != 0) plusHours(hours.toLong()) else this } .run { if (minutes != 0) plusMinutes(minutes.toLong()) else this } - .run { if (seconds != 0L) plusSeconds(seconds) else this } - .run { if (nanoseconds != 0L) plusNanos(nanoseconds) else this } + .run { if (seconds != 0) plusSeconds(seconds.toLong()) else this } + .run { if (nanoseconds != 0) plusNanos(nanoseconds.toLong()) else this } }.toInstant().let(::Instant) } catch (e: DateTimeException) { throw DateTimeArithmeticException(e) @@ -152,7 +152,7 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT val time = thisZdt.until(otherZdt, ChronoUnit.NANOS).nanoseconds time.toComponents { hours, minutes, seconds, nanoseconds -> - return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds.toLong(), nanoseconds.toLong()) + return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds, nanoseconds.toLong()) } } diff --git a/core/jvm/test/ConvertersTest.kt b/core/jvm/test/ConvertersTest.kt index cc1f8d63f..a638e6848 100644 --- a/core/jvm/test/ConvertersTest.kt +++ b/core/jvm/test/ConvertersTest.kt @@ -85,15 +85,21 @@ class ConvertersTest { @Test fun datePeriod() { + + fun assertJtPeriodEqual(a: JTPeriod, b: JTPeriod) { + assertEquals(a.days, b.days) + assertEquals(a.months + a.years * 12, b.months + b.years * 12) + } + fun test(years: Int, months: Int, days: Int) { val ktPeriod = DatePeriod(years, months, days) val jtPeriod = JTPeriod.of(years, months, days) assertEquals(ktPeriod, jtPeriod.toKotlinDatePeriod()) - assertEquals(jtPeriod, ktPeriod.toJavaPeriod()) + assertJtPeriodEqual(jtPeriod, ktPeriod.toJavaPeriod()) // TODO: assertEquals(ktPeriod, jtPeriod.toString().let(DatePeriod::parse)) - assertEquals(jtPeriod, ktPeriod.toString().let(JTPeriod::parse)) + assertJtPeriodEqual(jtPeriod, ktPeriod.toString().let(JTPeriod::parse)) } repeat(1000) { diff --git a/core/native/src/Instant.kt b/core/native/src/Instant.kt index e0011eb9e..2e01c3a8f 100644 --- a/core/native/src/Instant.kt +++ b/core/native/src/Instant.kt @@ -274,8 +274,8 @@ actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Instant = t plus(hours.toLong() * SECONDS_PER_HOUR, 0).check(timeZone) else this } .run { if (minutes != 0) plus(minutes.toLong() * SECONDS_PER_MINUTE, 0).check(timeZone) else this } - .run { if (seconds != 0L) plus(seconds, 0).check(timeZone) else this } - .run { if (nanoseconds != 0L) plus(0, nanoseconds).check(timeZone) else this } + .run { if (seconds != 0) plus(seconds.toLong(), 0).check(timeZone) else this } + .run { if (nanoseconds != 0) plus(0, nanoseconds.toLong()).check(timeZone) else this } }.check(timeZone) } catch (e: ArithmeticException) { throw DateTimeArithmeticException("Arithmetic overflow when adding CalendarPeriod to an Instant", e) @@ -328,7 +328,7 @@ actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeri val time = thisLdt.until(otherLdt, DateTimeUnit.NANOSECOND).nanoseconds // |otherLdt - thisLdt| < 24h time.toComponents { hours, minutes, seconds, nanoseconds -> - return DateTimePeriod((months / 12), (months % 12), days, hours, minutes, seconds.toLong(), nanoseconds.toLong()) + return DateTimePeriod((months / 12), (months % 12), days, hours, minutes, seconds, nanoseconds.toLong()) } } From 3caca4c0baef07d4f2e1a173f0c5827a1afea2c9 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 4 Dec 2020 17:10:06 +0300 Subject: [PATCH 02/12] Parse DateTimePeriod from ISO-8601 strings --- core/common/src/DateTimePeriod.kt | 177 ++++++++++++++++++++++++- core/common/test/DateTimePeriodTest.kt | 30 ++++- core/jvm/test/ConvertersTest.kt | 2 +- 3 files changed, 205 insertions(+), 4 deletions(-) diff --git a/core/common/src/DateTimePeriod.kt b/core/common/src/DateTimePeriod.kt index 82fda429b..3c143760f 100644 --- a/core/common/src/DateTimePeriod.kt +++ b/core/common/src/DateTimePeriod.kt @@ -9,7 +9,6 @@ import kotlin.math.* import kotlin.time.Duration import kotlin.time.ExperimentalTime - // TODO: could be error-prone without explicitly named params sealed class DateTimePeriod { internal abstract val totalMonths: Int @@ -67,9 +66,173 @@ sealed class DateTimePeriod { return result } - // TODO: parsing from iso string + companion object { + fun parse(text: String): DateTimePeriod { + fun parseException(message: String, position: Int): Nothing = + throw DateTimeFormatException("Parse error at char $position: $message") + val START = 0 + val AFTER_P = 1 + val AFTER_YEAR = 2 + val AFTER_MONTH = 3 + val AFTER_WEEK = 4 + val AFTER_DAY = 5 + val AFTER_T = 6 + val AFTER_HOUR = 7 + val AFTER_MINUTE = 8 + val AFTER_SECOND_AND_NANO = 9 + + var state = START + // next unread character + var i = 0 + var sign = 1 + var years = 0 + var months = 0 + var weeks = 0 + var days = 0 + var hours = 0 + var minutes = 0 + var seconds = 0 + var nanoseconds = 0 + while (true) { + if (i >= text.length) { + if (state == START) + parseException("Unexpected end of input; 'P' designator is required", i) + if (state == AFTER_T) + parseException("Unexpected end of input; at least one time component is required after 'T'", i) + val daysTotal = when (val n = days.toLong() + weeks * 7) { + in Int.MIN_VALUE..Int.MAX_VALUE -> n.toInt() + else -> parseException("The total number of days under 'D' and 'W' designators should fit into an Int", 0) + } + return DateTimePeriod(years, months, daysTotal, hours, minutes, seconds, nanoseconds.toLong()) + } + if (state == START) { + if (i + 1 >= text.length && (text[i] == '+' || text[i] == '-')) + parseException("Unexpected end of string; 'P' designator is required", i) + when (text[i]) { + '+', '-' -> { + if (text[i] == '-') + sign = -1 + if (text[i + 1] != 'P') + parseException("Expected 'P', got '${text[i + 1]}'", i + 1) + i += 2 + } + 'P' -> { i += 1 } + else -> parseException("Expected '+', '-', 'P', got '${text[i]}'", i) + } + state = AFTER_P + continue + } + var localSign = sign + val iStart = i + when (text[i]) { + '+', '-' -> { + if (text[i] == '-') localSign *= -1 + i += 1 + if (i >= text.length || text[i] !in '0'..'9') + parseException("A number expected after '${text[i]}'", i) + } + in '0'..'9' -> { } + 'T' -> { + if (state >= AFTER_T) + parseException("Only one 'T' designator is allowed", i) + state = AFTER_T + i += 1 + continue + } + } + var number = 0L + while (i < text.length && text[i] in '0'..'9') { + try { + number = safeAdd(safeMultiply(number, 10), (text[i] - '0').toLong()) + } catch (e: ArithmeticException) { + parseException("The number is too large", iStart) + } + i += 1 + } + number *= localSign + if (i == text.length) + parseException("Expected a designator after the numerical value", i) + val wrongOrder = "Wrong component order: should be 'Y', 'M', 'W', 'D', then designator 'T', then 'H', 'M', 'S'" + fun Long.toIntThrowing(component: Char): Int { + if (this < Int.MIN_VALUE || this > Int.MAX_VALUE) + parseException("Value $this does not fit into an Int, which is required for component '$component'", iStart) + return toInt() + } + when (text[i].toUpperCase()) { + 'Y' -> { + if (state >= AFTER_YEAR) + parseException(wrongOrder, i) + state = AFTER_YEAR + years = number.toIntThrowing('Y') + } + 'M' -> { + if (state >= AFTER_T) { + // Minutes + if (state >= AFTER_MINUTE) + parseException(wrongOrder, i) + state = AFTER_MINUTE + minutes = number.toIntThrowing('M') + } else { + // Months + if (state >= AFTER_MONTH) + parseException(wrongOrder, i) + state = AFTER_MONTH + months = number.toIntThrowing('M') + } + } + 'W' -> { + if (state >= AFTER_WEEK) + parseException(wrongOrder, i) + state = AFTER_WEEK + weeks = number.toIntThrowing('W') + } + 'D' -> { + if (state >= AFTER_DAY) + parseException(wrongOrder, i) + state = AFTER_DAY + days = number.toIntThrowing('D') + } + 'H' -> { + if (state >= AFTER_HOUR || state < AFTER_T) + parseException(wrongOrder, i) + state = AFTER_HOUR + hours = number.toIntThrowing('H') + } + 'S' -> { + if (state >= AFTER_SECOND_AND_NANO || state < AFTER_T) + parseException(wrongOrder, i) + state = AFTER_SECOND_AND_NANO + seconds = number.toIntThrowing('S') + } + '.', ',' -> { + i += 1 + if (i >= text.length) + parseException("Expected designator 'S' after ${text[i - 1]}", i) + val iStartFraction = i + while (i < text.length && text[i] in '0'..'9') + i += 1 + val fractionLength = i - iStartFraction + if (fractionLength > 9) + parseException("Only the nanosecond fractions of a second are supported", iStartFraction) + val fractionalPart = text.substring(iStartFraction, i) + "0".repeat(9 - fractionLength) + nanoseconds = fractionalPart.toInt(10) * localSign + if (text[i] != 'S') + parseException("Expected the 'S' designator after a fraction", i) + if (state >= AFTER_SECOND_AND_NANO || state < AFTER_T) + parseException(wrongOrder, i) + state = AFTER_SECOND_AND_NANO + seconds = number.toIntThrowing('S') + } + else -> parseException("Expected a designator after the numerical value", i) + } + i += 1 + } + } + } } +public fun String.toDateTimePeriod(): DateTimePeriod = DateTimePeriod.parse(this) + class DatePeriod internal constructor( internal override val totalMonths: Int, override val days: Int = 0, @@ -81,8 +244,18 @@ class DatePeriod internal constructor( override val seconds: Int get() = 0 override val nanoseconds: Int get() = 0 override val totalNanoseconds: Long get() = 0 + + companion object { + fun parse(text: String): DatePeriod = + when (val period = DateTimePeriod.parse(text)) { + is DatePeriod -> period + else -> throw DateTimeFormatException("Period $period (parsed from string $text) is not date-based") + } + } } +public fun String.toDatePeriod(): DatePeriod = DatePeriod.parse(this) + private class DateTimePeriodImpl( internal override val totalMonths: Int = 0, override val days: Int = 0, diff --git a/core/common/test/DateTimePeriodTest.kt b/core/common/test/DateTimePeriodTest.kt index e03b1e5d7..f7b1f7090 100644 --- a/core/common/test/DateTimePeriodTest.kt +++ b/core/common/test/DateTimePeriodTest.kt @@ -40,6 +40,34 @@ class DateTimePeriodTest { assertEquals("P1DT-0.999999999S", DateTimePeriod(days = 1, seconds = -1, nanoseconds = 1L).toString()) } + @Test + fun parseIsoString() { + assertEquals(DateTimePeriod(years = 1), DateTimePeriod.parse("P1Y")) + assertEquals(DatePeriod(years = 1, months = 1), DateTimePeriod.parse("P1Y1M")) + assertEquals(DateTimePeriod(months = 11), DateTimePeriod.parse("P11M")) + assertEquals(DateTimePeriod(months = 14), DateTimePeriod.parse("P14M")) // TODO: normalize or not + assertEquals(DateTimePeriod(months = 10, days = 5), DateTimePeriod.parse("P10M5D")) + assertEquals(DateTimePeriod(years = 1, days = 40), DateTimePeriod.parse("P1Y40D")) + + assertEquals(DateTimePeriod(hours = 1), DateTimePeriod.parse("PT1H")) + assertEquals(DateTimePeriod(), DateTimePeriod.parse("P0D")) + assertEquals(DatePeriod(), DateTimePeriod.parse("P0D")) + + assertEquals(DateTimePeriod(days = 1, hours = -1), DateTimePeriod.parse("P1DT-1H")) + assertEquals(DateTimePeriod(days = -1, hours = -1), DateTimePeriod.parse("-P1DT1H")) + assertEquals(DateTimePeriod(months = -1), DateTimePeriod.parse("-P1M")) + + assertEquals(DateTimePeriod(years = -1, months = -2, days = -3, hours = -4, minutes = -5, seconds = 0, nanoseconds = 500_000_000), + DateTimePeriod.parse("P-1Y-2M-3DT-4H-5M0.500000000S")) + + assertEquals(DateTimePeriod(nanoseconds = 999_999_999_999_999L), DateTimePeriod.parse("PT277H46M39.999999999S")) + assertEquals(DateTimePeriod(seconds = 1, nanoseconds = -1L), DateTimePeriod.parse("PT0.999999999S")) + assertEquals(DateTimePeriod(nanoseconds = -1L), DateTimePeriod.parse("-PT0.000000001S")) + assertEquals(DateTimePeriod(days = 1, nanoseconds = -1L), DateTimePeriod.parse("P1DT-0.000000001S")) + assertEquals(DateTimePeriod(seconds = -1, nanoseconds = 1L), DateTimePeriod.parse("-PT0.999999999S")) + assertEquals(DateTimePeriod(days = 1, seconds = -1, nanoseconds = 1L), DateTimePeriod.parse("P1DT-0.999999999S")) + } + @Test fun periodArithmetic() { val p1 = DateTimePeriod(years = 10) @@ -76,4 +104,4 @@ class DateTimePeriodTest { assertEquals(period, duration.toDateTimePeriod()) } } -} \ No newline at end of file +} diff --git a/core/jvm/test/ConvertersTest.kt b/core/jvm/test/ConvertersTest.kt index a638e6848..01eb65494 100644 --- a/core/jvm/test/ConvertersTest.kt +++ b/core/jvm/test/ConvertersTest.kt @@ -98,7 +98,7 @@ class ConvertersTest { assertEquals(ktPeriod, jtPeriod.toKotlinDatePeriod()) assertJtPeriodEqual(jtPeriod, ktPeriod.toJavaPeriod()) - // TODO: assertEquals(ktPeriod, jtPeriod.toString().let(DatePeriod::parse)) + assertEquals(ktPeriod, jtPeriod.toString().let(DatePeriod::parse)) assertJtPeriodEqual(jtPeriod, ktPeriod.toString().let(JTPeriod::parse)) } From d7a8141db64bb6c8615cc048f23742b029909393 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 11 Dec 2020 17:21:39 +0300 Subject: [PATCH 03/12] Use orthogonal DateTimePeriod components internally --- core/js/src/Instant.kt | 9 +++------ core/js/src/LocalDate.kt | 5 ++--- core/jvm/src/Instant.kt | 14 ++++---------- core/jvm/src/LocalDate.kt | 5 ++--- core/native/src/Instant.kt | 16 ++++------------ core/native/src/LocalDate.kt | 5 ++--- 6 files changed, 17 insertions(+), 37 deletions(-) diff --git a/core/js/src/Instant.kt b/core/js/src/Instant.kt index b59ece296..d371808ea 100644 --- a/core/js/src/Instant.kt +++ b/core/js/src/Instant.kt @@ -111,8 +111,7 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst val thisZdt = this.value.atZone(timeZone.zoneId) with(period) { thisZdt - .run { if (years != 0 && months == 0) plusYears(years) else this } - .run { if (months != 0) plusMonths(years * 12.0 + months) else this } + .run { if (totalMonths != 0) plusMonths(totalMonths) else this } .run { if (days != 0) plusDays(days) as ZonedDateTime else this } .run { if (hours != 0) plusHours(hours) else this } .run { if (minutes != 0) plusMinutes(minutes) else this } @@ -188,11 +187,9 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT val months = thisZdt.until(otherZdt, ChronoUnit.MONTHS).toDouble(); thisZdt = thisZdt.plusMonths(months) val days = thisZdt.until(otherZdt, ChronoUnit.DAYS).toDouble(); thisZdt = thisZdt.plusDays(days) as ZonedDateTime - val time = thisZdt.until(otherZdt, ChronoUnit.NANOS).toDouble().nanoseconds + val nanoseconds = thisZdt.until(otherZdt, ChronoUnit.NANOS).toDouble() - time.toComponents { hours, minutes, seconds, nanoseconds -> - return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds, nanoseconds.toLong()) - } + buildDateTimePeriod(months.toInt(), days.toInt(), nanoseconds.toLong()) } catch (e: Throwable) { if (e.isJodaDateTimeException()) throw DateTimeArithmeticException(e) else throw e } diff --git a/core/js/src/LocalDate.kt b/core/js/src/LocalDate.kt index 2132b7cd2..b384b86b0 100644 --- a/core/js/src/LocalDate.kt +++ b/core/js/src/LocalDate.kt @@ -68,8 +68,7 @@ private fun LocalDate.plusNumber(value: Number, unit: DateTimeUnit.DateBased): L public actual operator fun LocalDate.plus(period: DatePeriod): LocalDate = try { with(period) { return@with value - .run { if (years != 0 && months == 0) plusYears(years) else this } - .run { if (months != 0) plusMonths(years.toDouble() * 12 + months) else this } + .run { if (totalMonths != 0) plusMonths(totalMonths) else this } .run { if (days != 0) plusDays(days) else this } }.let(::LocalDate) @@ -86,7 +85,7 @@ public actual fun LocalDate.periodUntil(other: LocalDate): DatePeriod { val months = startD.until(endD, ChronoUnit.MONTHS).toInt(); startD = startD.plusMonths(months) val days = startD.until(endD, ChronoUnit.DAYS).toInt() - return DatePeriod(months / 12, months % 12, days) + return DatePeriod(totalMonths = months, days) } public actual fun LocalDate.until(other: LocalDate, unit: DateTimeUnit.DateBased): Int = when(unit) { diff --git a/core/jvm/src/Instant.kt b/core/jvm/src/Instant.kt index 98f7b39b5..9215dde46 100644 --- a/core/jvm/src/Instant.kt +++ b/core/jvm/src/Instant.kt @@ -94,13 +94,9 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst val thisZdt = atZone(timeZone) return with(period) { thisZdt - .run { if (years != 0 && months == 0) plusYears(years.toLong()) else this } - .run { if (months != 0) plusMonths(years * 12L + months.toLong()) else this } + .run { if (totalMonths != 0) plusMonths(totalMonths.toLong()) else this } .run { if (days != 0) plusDays(days.toLong()) else this } - .run { if (hours != 0) plusHours(hours.toLong()) else this } - .run { if (minutes != 0) plusMinutes(minutes.toLong()) else this } - .run { if (seconds != 0) plusSeconds(seconds.toLong()) else this } - .run { if (nanoseconds != 0) plusNanos(nanoseconds.toLong()) else this } + .run { if (totalNanoseconds != 0L) plusNanos(totalNanoseconds) else this } }.toInstant().let(::Instant) } catch (e: DateTimeException) { throw DateTimeArithmeticException(e) @@ -149,11 +145,9 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT val months = thisZdt.until(otherZdt, ChronoUnit.MONTHS); thisZdt = thisZdt.plusMonths(months) val days = thisZdt.until(otherZdt, ChronoUnit.DAYS); thisZdt = thisZdt.plusDays(days) - val time = thisZdt.until(otherZdt, ChronoUnit.NANOS).nanoseconds + val nanoseconds = thisZdt.until(otherZdt, ChronoUnit.NANOS) - time.toComponents { hours, minutes, seconds, nanoseconds -> - return DateTimePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt(), hours, minutes, seconds, nanoseconds.toLong()) - } + return buildDateTimePeriod(months.toInt(), days.toInt(), nanoseconds) } public actual fun Instant.until(other: Instant, unit: DateTimeUnit, timeZone: TimeZone): Long = try { diff --git a/core/jvm/src/LocalDate.kt b/core/jvm/src/LocalDate.kt index 8c3625845..c93fd6c03 100644 --- a/core/jvm/src/LocalDate.kt +++ b/core/jvm/src/LocalDate.kt @@ -85,8 +85,7 @@ private fun ofEpochDayChecked(epochDay: Long): java.time.LocalDate { public actual operator fun LocalDate.plus(period: DatePeriod): LocalDate = try { with(period) { return@with value - .run { if (years != 0 && months == 0) plusYears(years.toLong()) else this } - .run { if (months != 0) plusMonths(years * 12L + months.toLong()) else this } + .run { if (totalMonths != 0) plusMonths(totalMonths.toLong()) else this } .run { if (days != 0) plusDays(days.toLong()) else this } }.let(::LocalDate) @@ -101,7 +100,7 @@ public actual fun LocalDate.periodUntil(other: LocalDate): DatePeriod { val months = startD.until(endD, ChronoUnit.MONTHS); startD = startD.plusMonths(months) val days = startD.until(endD, ChronoUnit.DAYS) - return DatePeriod((months / 12).toInt(), (months % 12).toInt(), days.toInt()) + return DatePeriod(totalMonths = months.toInt(), days.toInt()) } public actual fun LocalDate.until(other: LocalDate, unit: DateTimeUnit.DateBased): Int = when(unit) { diff --git a/core/native/src/Instant.kt b/core/native/src/Instant.kt index 2e01c3a8f..32b924a31 100644 --- a/core/native/src/Instant.kt +++ b/core/native/src/Instant.kt @@ -266,16 +266,10 @@ private fun Instant.check(zone: TimeZone): Instant = this@check.also { actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Instant = try { with(period) { val withDate = toZonedLocalDateTimeFailing(timeZone) - .run { if (years != 0 && months == 0) plus(years, DateTimeUnit.YEAR) else this } - .run { if (months != 0) plus(safeAdd(safeMultiply(years, 12), months), DateTimeUnit.MONTH) else this } + .run { if (totalMonths != 0) plus(totalMonths, DateTimeUnit.MONTH) else this } .run { if (days != 0) plus(days, DateTimeUnit.DAY) else this } withDate.toInstant() - .run { if (hours != 0) - plus(hours.toLong() * SECONDS_PER_HOUR, 0).check(timeZone) else this } - .run { if (minutes != 0) - plus(minutes.toLong() * SECONDS_PER_MINUTE, 0).check(timeZone) else this } - .run { if (seconds != 0) plus(seconds.toLong(), 0).check(timeZone) else this } - .run { if (nanoseconds != 0) plus(0, nanoseconds.toLong()).check(timeZone) else this } + .run { if (totalNanoseconds != 0L) plus(0, totalNanoseconds).check(timeZone) else this } }.check(timeZone) } catch (e: ArithmeticException) { throw DateTimeArithmeticException("Arithmetic overflow when adding CalendarPeriod to an Instant", e) @@ -325,11 +319,9 @@ actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeri thisLdt = thisLdt.plus(months, DateTimeUnit.MONTH) // won't throw: thisLdt + months <= otherLdt, which is known to be valid val days = thisLdt.until(otherLdt, DateTimeUnit.DAY).toInt() // `until` on dates never fails thisLdt = thisLdt.plus(days, DateTimeUnit.DAY) // won't throw: thisLdt + days <= otherLdt - val time = thisLdt.until(otherLdt, DateTimeUnit.NANOSECOND).nanoseconds // |otherLdt - thisLdt| < 24h + val nanoseconds = thisLdt.until(otherLdt, DateTimeUnit.NANOSECOND) // |otherLdt - thisLdt| < 24h - time.toComponents { hours, minutes, seconds, nanoseconds -> - return DateTimePeriod((months / 12), (months % 12), days, hours, minutes, seconds, nanoseconds.toLong()) - } + return buildDateTimePeriod(months, days, nanoseconds) } public actual fun Instant.until(other: Instant, unit: DateTimeUnit, timeZone: TimeZone): Long = diff --git a/core/native/src/LocalDate.kt b/core/native/src/LocalDate.kt index dc575a1d1..c73446756 100644 --- a/core/native/src/LocalDate.kt +++ b/core/native/src/LocalDate.kt @@ -270,8 +270,7 @@ actual operator fun LocalDate.plus(period: DatePeriod): LocalDate = with(period) { try { this@plus - .run { if (years != 0 && months == 0) plusYears(years) else this } - .run { if (months != 0) plusMonths(safeAdd(safeMultiply(years, 12), months)) else this } + .run { if (totalMonths != 0) plusMonths(totalMonths) else this } .run { if (days != 0) plusDays(days) else this } } catch (e: ArithmeticException) { throw DateTimeArithmeticException("Arithmetic overflow when adding a period to a date", e) @@ -305,5 +304,5 @@ public actual fun LocalDate.yearsUntil(other: LocalDate): Int = actual fun LocalDate.periodUntil(other: LocalDate): DatePeriod { val months = monthsUntil(other) val days = plusMonths(months).daysUntil(other) - return DatePeriod(months / 12, months % 12, days) + return DatePeriod(totalMonths = months, days) } From 512e0213e22b369e40ca1bcf9987e50fcd15e49c Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 14 Dec 2020 15:14:42 +0300 Subject: [PATCH 04/12] JVM: document and handle overflow in DateTimePeriod's months --- core/common/src/Instant.kt | 6 ++++-- core/common/src/LocalDate.kt | 4 ++++ core/jvm/src/Instant.kt | 3 +++ core/jvm/src/LocalDate.kt | 3 +++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/core/common/src/Instant.kt b/core/common/src/Instant.kt index ed19d0991..d414fbac9 100644 --- a/core/common/src/Instant.kt +++ b/core/common/src/Instant.kt @@ -210,7 +210,8 @@ public fun Instant.minus(period: DateTimePeriod, timeZone: TimeZone): Instant = * - negative or zero if this instant is later than the other, * - exactly zero if this instant is equal to the other. * - * @throws DateTimeArithmeticException if `this` or [other] instant is too large to fit in [LocalDateTime]. + * @throws DateTimeArithmeticException if `this` or [other] instant is too large to fit in [LocalDateTime]. Also (only + * on JVM) if the number of months between the two dates exceeds an Int. */ public expect fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateTimePeriod @@ -292,7 +293,8 @@ public fun Instant.yearsUntil(other: Instant, timeZone: TimeZone): Int = * - positive or zero if this instant is later than the other, * - exactly zero if this instant is equal to the other. * - * @throws DateTimeArithmeticException if `this` or [other] instant is too large to fit in [LocalDateTime]. + * @throws DateTimeArithmeticException if `this` or [other] instant is too large to fit in [LocalDateTime]. Also (only + * on JVM) if the number of months between the two dates exceeds an Int. * @see Instant.periodUntil */ public fun Instant.minus(other: Instant, timeZone: TimeZone): DateTimePeriod = diff --git a/core/common/src/LocalDate.kt b/core/common/src/LocalDate.kt index 70c55c57b..d941c4f3b 100644 --- a/core/common/src/LocalDate.kt +++ b/core/common/src/LocalDate.kt @@ -137,6 +137,8 @@ public operator fun LocalDate.minus(period: DatePeriod): LocalDate = * - negative or zero if this date is later than the other, * - exactly zero if this date is equal to the other. * + * @throws DateTimeArithmeticException if the number of months between the two dates exceeds an Int (JVM only) + * * @see LocalDate.minus */ expect fun LocalDate.periodUntil(other: LocalDate): DatePeriod @@ -151,6 +153,8 @@ expect fun LocalDate.periodUntil(other: LocalDate): DatePeriod * - positive or zero if this date is later than the other, * - exactly zero if this date is equal to the other. * + * @throws DateTimeArithmeticException if the number of months between the two dates exceeds an Int (JVM only) + * * @see LocalDate.periodUntil */ operator fun LocalDate.minus(other: LocalDate): DatePeriod = other.periodUntil(this) diff --git a/core/jvm/src/Instant.kt b/core/jvm/src/Instant.kt index 9215dde46..988742bc3 100644 --- a/core/jvm/src/Instant.kt +++ b/core/jvm/src/Instant.kt @@ -147,6 +147,9 @@ public actual fun Instant.periodUntil(other: Instant, timeZone: TimeZone): DateT val days = thisZdt.until(otherZdt, ChronoUnit.DAYS); thisZdt = thisZdt.plusDays(days) val nanoseconds = thisZdt.until(otherZdt, ChronoUnit.NANOS) + if (months > Int.MAX_VALUE || months < Int.MIN_VALUE) { + throw DateTimeArithmeticException("The number of months between $this and $other does not fit in an Int") + } return buildDateTimePeriod(months.toInt(), days.toInt(), nanoseconds) } diff --git a/core/jvm/src/LocalDate.kt b/core/jvm/src/LocalDate.kt index c93fd6c03..d76f5f576 100644 --- a/core/jvm/src/LocalDate.kt +++ b/core/jvm/src/LocalDate.kt @@ -100,6 +100,9 @@ public actual fun LocalDate.periodUntil(other: LocalDate): DatePeriod { val months = startD.until(endD, ChronoUnit.MONTHS); startD = startD.plusMonths(months) val days = startD.until(endD, ChronoUnit.DAYS) + if (months > Int.MAX_VALUE || months < Int.MIN_VALUE) { + throw DateTimeArithmeticException("The number of months between $this and $other does not fit in an Int") + } return DatePeriod(totalMonths = months.toInt(), days.toInt()) } From d4c99c0c92cd13172848f77542c7e3e4c33f0dd0 Mon Sep 17 00:00:00 2001 From: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com> Date: Mon, 14 Dec 2020 15:49:35 +0300 Subject: [PATCH 05/12] Better name of an internal function Co-authored-by: ilya-g --- core/jvm/test/ConvertersTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/jvm/test/ConvertersTest.kt b/core/jvm/test/ConvertersTest.kt index 01eb65494..6016dca34 100644 --- a/core/jvm/test/ConvertersTest.kt +++ b/core/jvm/test/ConvertersTest.kt @@ -86,7 +86,7 @@ class ConvertersTest { @Test fun datePeriod() { - fun assertJtPeriodEqual(a: JTPeriod, b: JTPeriod) { + fun assertJtPeriodNormalizedEquals(a: JTPeriod, b: JTPeriod) { assertEquals(a.days, b.days) assertEquals(a.months + a.years * 12, b.months + b.years * 12) } From eb0f283db914041c3c687dbc4142655f4d455f7a Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 14 Dec 2020 15:57:31 +0300 Subject: [PATCH 06/12] Small fixes --- core/common/src/DateTimePeriod.kt | 14 ++++++-------- core/common/test/DateTimePeriodTest.kt | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/core/common/src/DateTimePeriod.kt b/core/common/src/DateTimePeriod.kt index 3c143760f..8792adffc 100644 --- a/core/common/src/DateTimePeriod.kt +++ b/core/common/src/DateTimePeriod.kt @@ -235,7 +235,7 @@ public fun String.toDateTimePeriod(): DateTimePeriod = DateTimePeriod.parse(this class DatePeriod internal constructor( internal override val totalMonths: Int, - override val days: Int = 0, + override val days: Int, ) : DateTimePeriod() { constructor(years: Int = 0, months: Int = 0, days: Int = 0): this(totalMonths(years, months), days) // avoiding excessive computations @@ -243,7 +243,7 @@ class DatePeriod internal constructor( override val minutes: Int get() = 0 override val seconds: Int get() = 0 override val nanoseconds: Int get() = 0 - override val totalNanoseconds: Long get() = 0 + internal override val totalNanoseconds: Long get() = 0 companion object { fun parse(text: String): DatePeriod = @@ -257,9 +257,9 @@ class DatePeriod internal constructor( public fun String.toDatePeriod(): DatePeriod = DatePeriod.parse(this) private class DateTimePeriodImpl( - internal override val totalMonths: Int = 0, - override val days: Int = 0, - override val totalNanoseconds: Long = 0, + internal override val totalMonths: Int, + override val days: Int, + override val totalNanoseconds: Long, ) : DateTimePeriod() // TODO: these calculations fit in a JS Number. Possible to do an expect/actual here. @@ -303,9 +303,7 @@ fun DateTimePeriod( totalNanoseconds(hours, minutes, seconds, nanoseconds)) @OptIn(ExperimentalTime::class) -fun Duration.toDateTimePeriod(): DateTimePeriod = toComponents { hours, minutes, seconds, nanoseconds -> - buildDateTimePeriod(totalNanoseconds = totalNanoseconds(hours, minutes, seconds, nanoseconds.toLong())) -} +fun Duration.toDateTimePeriod(): DateTimePeriod = buildDateTimePeriod(totalNanoseconds = toLongNanoseconds()) operator fun DateTimePeriod.plus(other: DateTimePeriod): DateTimePeriod = buildDateTimePeriod( safeAdd(totalMonths, other.totalMonths), diff --git a/core/common/test/DateTimePeriodTest.kt b/core/common/test/DateTimePeriodTest.kt index f7b1f7090..85f1b6530 100644 --- a/core/common/test/DateTimePeriodTest.kt +++ b/core/common/test/DateTimePeriodTest.kt @@ -17,7 +17,7 @@ class DateTimePeriodTest { assertEquals("P1Y", DateTimePeriod(years = 1).toString()) assertEquals("P1Y1M", DatePeriod(years = 1, months = 1).toString()) assertEquals("P11M", DateTimePeriod(months = 11).toString()) - assertEquals("P1Y2M", DateTimePeriod(months = 14).toString()) // TODO: normalize or not + assertEquals("P1Y2M", DateTimePeriod(months = 14).toString()) assertEquals("P10M5D", DateTimePeriod(months = 10, days = 5).toString()) assertEquals("P1Y40D", DateTimePeriod(years = 1, days = 40).toString()) @@ -45,7 +45,7 @@ class DateTimePeriodTest { assertEquals(DateTimePeriod(years = 1), DateTimePeriod.parse("P1Y")) assertEquals(DatePeriod(years = 1, months = 1), DateTimePeriod.parse("P1Y1M")) assertEquals(DateTimePeriod(months = 11), DateTimePeriod.parse("P11M")) - assertEquals(DateTimePeriod(months = 14), DateTimePeriod.parse("P14M")) // TODO: normalize or not + assertEquals(DateTimePeriod(months = 14), DateTimePeriod.parse("P14M")) assertEquals(DateTimePeriod(months = 10, days = 5), DateTimePeriod.parse("P10M5D")) assertEquals(DateTimePeriod(years = 1, days = 40), DateTimePeriod.parse("P1Y40D")) From 439336141aee8d8fbe2140abd1a48ddcca36b0ac Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 14 Dec 2020 15:59:50 +0300 Subject: [PATCH 07/12] Fix --- core/jvm/test/ConvertersTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/jvm/test/ConvertersTest.kt b/core/jvm/test/ConvertersTest.kt index 6016dca34..b59890815 100644 --- a/core/jvm/test/ConvertersTest.kt +++ b/core/jvm/test/ConvertersTest.kt @@ -96,10 +96,10 @@ class ConvertersTest { val jtPeriod = JTPeriod.of(years, months, days) assertEquals(ktPeriod, jtPeriod.toKotlinDatePeriod()) - assertJtPeriodEqual(jtPeriod, ktPeriod.toJavaPeriod()) + assertJtPeriodNormalizedEquals(jtPeriod, ktPeriod.toJavaPeriod()) assertEquals(ktPeriod, jtPeriod.toString().let(DatePeriod::parse)) - assertJtPeriodEqual(jtPeriod, ktPeriod.toString().let(JTPeriod::parse)) + assertJtPeriodNormalizedEquals(jtPeriod, ktPeriod.toString().let(JTPeriod::parse)) } repeat(1000) { From 54930c6ee7ce9ea83b5e985215183849e9b830a6 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 14 Dec 2020 16:32:32 +0300 Subject: [PATCH 08/12] Small fix --- core/common/src/DateTimePeriod.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/common/src/DateTimePeriod.kt b/core/common/src/DateTimePeriod.kt index 8792adffc..8d454ce97 100644 --- a/core/common/src/DateTimePeriod.kt +++ b/core/common/src/DateTimePeriod.kt @@ -259,7 +259,7 @@ public fun String.toDatePeriod(): DatePeriod = DatePeriod.parse(this) private class DateTimePeriodImpl( internal override val totalMonths: Int, override val days: Int, - override val totalNanoseconds: Long, + internal override val totalNanoseconds: Long, ) : DateTimePeriod() // TODO: these calculations fit in a JS Number. Possible to do an expect/actual here. From bf1195d2f88d6d78bcda6f92280255d0c46b476c Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 18 Jan 2021 13:59:05 +0300 Subject: [PATCH 09/12] Add tests for DateTimePeriod normalization --- core/common/test/DateTimePeriodTest.kt | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/core/common/test/DateTimePeriodTest.kt b/core/common/test/DateTimePeriodTest.kt index 85f1b6530..38fb8d201 100644 --- a/core/common/test/DateTimePeriodTest.kt +++ b/core/common/test/DateTimePeriodTest.kt @@ -12,6 +12,39 @@ import kotlin.time.* class DateTimePeriodTest { + @Test + fun normalization() { + assertPeriodComponents(DateTimePeriod(years = 1) as DatePeriod, years = 1) + assertPeriodComponents(DateTimePeriod(years = 1, months = 1) as DatePeriod, years = 1, months = 1) + assertPeriodComponents(DateTimePeriod(years = 1, months = -1) as DatePeriod, months = 11) + assertPeriodComponents(DateTimePeriod(years = -1, months = 1) as DatePeriod, months = -11) + assertPeriodComponents(DateTimePeriod(years = -1, months = -1) as DatePeriod, years = -1, months = -1) + assertPeriodComponents(DateTimePeriod(months = 11) as DatePeriod, months = 11) + assertPeriodComponents(DateTimePeriod(months = 14) as DatePeriod, years = 1, months = 2) + assertPeriodComponents(DateTimePeriod(months = -14) as DatePeriod, years = -1, months = -2) + assertPeriodComponents(DateTimePeriod(months = 10, days = 5) as DatePeriod, months = 10, days = 5) + assertPeriodComponents(DateTimePeriod(years = 1, days = 40) as DatePeriod, years = 1, days = 40) + assertPeriodComponents(DateTimePeriod(years = 1, days = -40) as DatePeriod, years = 1, days = -40) + assertPeriodComponents(DateTimePeriod(days = 5) as DatePeriod, days = 5) + + assertPeriodComponents(DateTimePeriod(hours = 3), hours = 3) + assertPeriodComponents(DateTimePeriod(hours = 1, minutes = 120), hours = 3) + assertPeriodComponents(DateTimePeriod(hours = 1, minutes = 119, seconds = 60), hours = 3) + assertPeriodComponents(DateTimePeriod(hours = 1, minutes = 119, seconds = 59, nanoseconds = 1_000_000_000), hours = 3) + assertPeriodComponents(DateTimePeriod(hours = 1, minutes = 121, seconds = -59, nanoseconds = -1_000_000_000), hours = 3) + assertPeriodComponents(DateTimePeriod()) + assertPeriodComponents(DatePeriod()) + + assertPeriodComponents(DateTimePeriod(days = 1, hours = -1), days = 1, hours = -1) + assertPeriodComponents(DateTimePeriod(days = -1, hours = -1), days = -1, hours = -1) + + assertPeriodComponents(DateTimePeriod(years = -1, months = -2, days = -3, hours = -4, minutes = -5, seconds = 0, nanoseconds = 500_000_000), + years = -1, months = -2, days = -3, hours = -4, minutes = -4, seconds = -59, nanoseconds = -500_000_000) + + assertPeriodComponents(DateTimePeriod(nanoseconds = 999_999_999_999_999L), hours = 277, minutes = 46, seconds = 39, nanoseconds = 999_999_999) + assertPeriodComponents(DateTimePeriod(nanoseconds = -999_999_999_999_999L), hours = -277, minutes = -46, seconds = -39, nanoseconds = -999_999_999) + } + @Test fun toStringConversion() { assertEquals("P1Y", DateTimePeriod(years = 1).toString()) @@ -104,4 +137,16 @@ class DateTimePeriodTest { assertEquals(period, duration.toDateTimePeriod()) } } + + private fun assertPeriodComponents(period: DateTimePeriod, + years: Int = 0, months: Int = 0, days: Int = 0, + hours: Int = 0, minutes: Int = 0, seconds: Int = 0, nanoseconds: Int = 0) { + assertEquals(years, period.years) + assertEquals(months, period.months) + assertEquals(days, period.days) + assertEquals(hours, period.hours) + assertEquals(minutes, period.minutes) + assertEquals(seconds, period.seconds) + assertEquals(nanoseconds, period.nanoseconds) + } } From 70d4db8001b45650c56801f7f6be3fb546016c56 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 18 Jan 2021 14:04:21 +0300 Subject: [PATCH 10/12] Add missing dots in the comments --- core/common/src/LocalDate.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/common/src/LocalDate.kt b/core/common/src/LocalDate.kt index d941c4f3b..43045a15e 100644 --- a/core/common/src/LocalDate.kt +++ b/core/common/src/LocalDate.kt @@ -137,7 +137,7 @@ public operator fun LocalDate.minus(period: DatePeriod): LocalDate = * - negative or zero if this date is later than the other, * - exactly zero if this date is equal to the other. * - * @throws DateTimeArithmeticException if the number of months between the two dates exceeds an Int (JVM only) + * @throws DateTimeArithmeticException if the number of months between the two dates exceeds an Int (JVM only). * * @see LocalDate.minus */ @@ -153,7 +153,7 @@ expect fun LocalDate.periodUntil(other: LocalDate): DatePeriod * - positive or zero if this date is later than the other, * - exactly zero if this date is equal to the other. * - * @throws DateTimeArithmeticException if the number of months between the two dates exceeds an Int (JVM only) + * @throws DateTimeArithmeticException if the number of months between the two dates exceeds an Int (JVM only). * * @see LocalDate.periodUntil */ From 3214ee3ef5628e9e6ad97b8962ff8b2eb6ab07fb Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 18 Jan 2021 14:43:32 +0300 Subject: [PATCH 11/12] Add tests for non-parseable strings for DateTimePeriod --- core/common/test/DateTimePeriodTest.kt | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/core/common/test/DateTimePeriodTest.kt b/core/common/test/DateTimePeriodTest.kt index 38fb8d201..65ad2a590 100644 --- a/core/common/test/DateTimePeriodTest.kt +++ b/core/common/test/DateTimePeriodTest.kt @@ -99,6 +99,29 @@ class DateTimePeriodTest { assertEquals(DateTimePeriod(days = 1, nanoseconds = -1L), DateTimePeriod.parse("P1DT-0.000000001S")) assertEquals(DateTimePeriod(seconds = -1, nanoseconds = 1L), DateTimePeriod.parse("-PT0.999999999S")) assertEquals(DateTimePeriod(days = 1, seconds = -1, nanoseconds = 1L), DateTimePeriod.parse("P1DT-0.999999999S")) + + // overflow of `Int.MAX_VALUE` months + assertFailsWith { DateTimePeriod.parse("P2000000000Y") } + assertFailsWith { DateTimePeriod.parse("P1Y2147483640M") } + + // too large a number in a field + assertFailsWith { DateTimePeriod.parse("P3000000000Y") } + assertFailsWith { DateTimePeriod.parse("P3000000000M") } + assertFailsWith { DateTimePeriod.parse("P3000000000D") } + assertFailsWith { DateTimePeriod.parse("P3000000000H") } + assertFailsWith { DateTimePeriod.parse("P3000000000M") } + assertFailsWith { DateTimePeriod.parse("P3000000000S") } + + // wrong order of signifiers + assertFailsWith { DateTimePeriod.parse("P1Y2D3M") } + assertFailsWith { DateTimePeriod.parse("P0DT1M2H") } + + // loss of precision in fractional seconds + assertFailsWith { DateTimePeriod.parse("P0.000000000001S") } + + // non-zero time components when parsing DatePeriod + assertFailsWith { DatePeriod.parse("P1DT1H") } + DatePeriod.parse("P1DT0H") } @Test From 5d7fb0410b18a0b146b4ebdaf89fbb8e8f418c65 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Wed, 17 Feb 2021 10:45:52 +0300 Subject: [PATCH 12/12] Enhance tests with documentation of properties resulting from parsing --- core/common/test/DateTimePeriodTest.kt | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/core/common/test/DateTimePeriodTest.kt b/core/common/test/DateTimePeriodTest.kt index 65ad2a590..8c70a8285 100644 --- a/core/common/test/DateTimePeriodTest.kt +++ b/core/common/test/DateTimePeriodTest.kt @@ -78,10 +78,12 @@ class DateTimePeriodTest { assertEquals(DateTimePeriod(years = 1), DateTimePeriod.parse("P1Y")) assertEquals(DatePeriod(years = 1, months = 1), DateTimePeriod.parse("P1Y1M")) assertEquals(DateTimePeriod(months = 11), DateTimePeriod.parse("P11M")) - assertEquals(DateTimePeriod(months = 14), DateTimePeriod.parse("P14M")) assertEquals(DateTimePeriod(months = 10, days = 5), DateTimePeriod.parse("P10M5D")) assertEquals(DateTimePeriod(years = 1, days = 40), DateTimePeriod.parse("P1Y40D")) + assertEquals(DateTimePeriod(months = 14), DateTimePeriod.parse("P14M")) + assertPeriodComponents(DateTimePeriod.parse("P14M") as DatePeriod, years = 1, months = 2) + assertEquals(DateTimePeriod(hours = 1), DateTimePeriod.parse("PT1H")) assertEquals(DateTimePeriod(), DateTimePeriod.parse("P0D")) assertEquals(DatePeriod(), DateTimePeriod.parse("P0D")) @@ -92,13 +94,19 @@ class DateTimePeriodTest { assertEquals(DateTimePeriod(years = -1, months = -2, days = -3, hours = -4, minutes = -5, seconds = 0, nanoseconds = 500_000_000), DateTimePeriod.parse("P-1Y-2M-3DT-4H-5M0.500000000S")) + assertPeriodComponents(DateTimePeriod.parse("P-1Y-2M-3DT-4H-5M0.500000000S"), + years = -1, months = -2, days = -3, hours = -4, minutes = -4, seconds = -59, nanoseconds = -500_000_000) assertEquals(DateTimePeriod(nanoseconds = 999_999_999_999_999L), DateTimePeriod.parse("PT277H46M39.999999999S")) - assertEquals(DateTimePeriod(seconds = 1, nanoseconds = -1L), DateTimePeriod.parse("PT0.999999999S")) - assertEquals(DateTimePeriod(nanoseconds = -1L), DateTimePeriod.parse("-PT0.000000001S")) - assertEquals(DateTimePeriod(days = 1, nanoseconds = -1L), DateTimePeriod.parse("P1DT-0.000000001S")) - assertEquals(DateTimePeriod(seconds = -1, nanoseconds = 1L), DateTimePeriod.parse("-PT0.999999999S")) - assertEquals(DateTimePeriod(days = 1, seconds = -1, nanoseconds = 1L), DateTimePeriod.parse("P1DT-0.999999999S")) + assertPeriodComponents(DateTimePeriod.parse("PT277H46M39.999999999S"), + hours = 277, minutes = 46, seconds = 39, nanoseconds = 999_999_999) + + assertEquals(DateTimePeriod(nanoseconds = 999_999_999), DateTimePeriod.parse("PT0.999999999S")) + assertEquals(DateTimePeriod(nanoseconds = -1), DateTimePeriod.parse("-PT0.000000001S")) + assertEquals(DateTimePeriod(days = 1, nanoseconds = -1), DateTimePeriod.parse("P1DT-0.000000001S")) + assertEquals(DateTimePeriod(nanoseconds = -999_999_999), DateTimePeriod.parse("-PT0.999999999S")) + assertEquals(DateTimePeriod(days = 1, nanoseconds = -999_999_999), DateTimePeriod.parse("P1DT-0.999999999S")) + assertPeriodComponents(DateTimePeriod.parse("P1DT-0.999999999S"), days = 1, nanoseconds = -999_999_999) // overflow of `Int.MAX_VALUE` months assertFailsWith { DateTimePeriod.parse("P2000000000Y") }