From ca1a9e4ca3bd92c5244ad6f270073ece03976ec0 Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Mon, 5 Jul 2021 21:24:24 +0300 Subject: [PATCH] Duration: longer than long values in ISO components, test negative cases --- libraries/stdlib/src/kotlin/time/Duration.kt | 39 ++++++--- libraries/stdlib/test/time/DurationTest.kt | 85 +++++++++++++++----- 2 files changed, 91 insertions(+), 33 deletions(-) diff --git a/libraries/stdlib/src/kotlin/time/Duration.kt b/libraries/stdlib/src/kotlin/time/Duration.kt index 2f167a2c24f..ad88fa25c1b 100644 --- a/libraries/stdlib/src/kotlin/time/Duration.kt +++ b/libraries/stdlib/src/kotlin/time/Duration.kt @@ -7,10 +7,7 @@ package kotlin.time import kotlin.contracts.* import kotlin.jvm.JvmInline -import kotlin.math.abs -import kotlin.math.roundToInt -import kotlin.math.roundToLong -import kotlin.math.sign +import kotlin.math.* /** * Represents the amount of time one instant of time is away from another instant. @@ -1080,13 +1077,13 @@ private fun parseDuration(value: String, strictIso: Boolean): Duration { length <= index -> throw IllegalArgumentException() value[index] == 'P' -> { - index++ + if (++index == length) throw IllegalArgumentException() val signedDigits = "+-0123456789." var isTimeComponent = false var prevUnit: DurationUnit? = null while (index < length) { if (value[index] == 'T') { - if (isTimeComponent || index++ == length) throw IllegalArgumentException() + if (isTimeComponent || ++index == length) throw IllegalArgumentException() isTimeComponent = true continue } @@ -1098,11 +1095,13 @@ private fun parseDuration(value: String, strictIso: Boolean): Duration { val unit = durationUnitByIsoChar(unitChar, isTimeComponent) if (prevUnit != null && prevUnit <= unit) throw IllegalArgumentException("Unexpected order of duration components") prevUnit = unit - if (unit == DurationUnit.SECONDS && '.' in component) { - // TODO: parse fractional part separately - result += component.toDouble().toDuration(unit) + val dotIndex = component.indexOf('.') + if (unit == DurationUnit.SECONDS && dotIndex > 0) { + val whole = component.substring(0, dotIndex) + result += parseOverLongIsoComponent(whole).toDuration(unit) + result += component.substring(dotIndex).toDouble().toDuration(unit) } else { - result += component.toLong().toDuration(unit) + result += parseOverLongIsoComponent(component).toDuration(unit) } } } @@ -1129,9 +1128,11 @@ private fun parseDuration(value: String, strictIso: Boolean): Duration { val unit = durationUnitByShortName(unitName) if (prevUnit != null && prevUnit <= unit) throw IllegalArgumentException("Unexpected order of duration components") prevUnit = unit - if ('.' in component) { - // TODO: parse fractional part separately - result += component.toDouble().toDuration(unit) + val dotIndex = component.indexOf('.') + if (dotIndex > 0) { + val whole = component.substring(0, dotIndex) + result += whole.toLong().toDuration(unit) + result += component.substring(dotIndex).toDouble().toDuration(unit) if (index < length) throw IllegalArgumentException("Fractional component must be last") } else { result += component.toLong().toDuration(unit) @@ -1143,6 +1144,18 @@ private fun parseDuration(value: String, strictIso: Boolean): Duration { } +private fun parseOverLongIsoComponent(value: String): Long { + val length = value.length + var startIndex = 0 + if (length > 0 && value[0] in "+-") startIndex++ + if ((length - startIndex) > 16 && (startIndex..value.lastIndex).all { value[it] in '0'..'9' }) { + // all chars are digits, but more than ceiling(log10(MAX_MILLIS / 1000)) of them + return if (value[0] == '-') Long.MIN_VALUE else Long.MAX_VALUE + } + return value.toLong() +} + + private inline fun String.substringWhile(startIndex: Int, predicate: (Char) -> Boolean): String = substring(startIndex, skipWhile(startIndex, predicate)) diff --git a/libraries/stdlib/test/time/DurationTest.kt b/libraries/stdlib/test/time/DurationTest.kt index ef158e551db..965f6872cc8 100644 --- a/libraries/stdlib/test/time/DurationTest.kt +++ b/libraries/stdlib/test/time/DurationTest.kt @@ -454,7 +454,7 @@ class DurationTest { } @Test - fun isoStringFormatAndParse() { + fun parseAndFormatIsoString() { fun test(duration: Duration, vararg isoStrings: String) { assertEquals(isoStrings.first(), duration.toIsoString()) for (isoString in isoStrings) { @@ -497,12 +497,30 @@ class DurationTest { test(Duration.ZERO, "PT0S", "P1DT-24H", "PT-1H60M", "-PT1M-60S") // infinite - test(Duration.INFINITE, "PT9999999999999H", "PT10000000000000H") - test(-Duration.INFINITE, "-PT9999999999999H", "-PT10000000000000H") + test(Duration.INFINITE, "PT9999999999999H", "PT10000000000000H", "-PT-9999999999999H", "-PT-1234567890123456789012S") + test(-Duration.INFINITE, "-PT9999999999999H", "-PT10000000000000H", "PT-1234567890123456789012S") } @Test - fun toStringInUnits() { + fun parseIsoStringFailing() { + for (invalidValue in listOf( + "", " ", "P", "PT", "P1DT", "P1", "PT1", "0", "+P", "+", "-", "h", "H", "something", + "1m", "1d", "2d 11s", "Infinity", "-Infinity", + "P+12+34D", "P12-34D", "PT1234567890-1234567890S", + " P1D", "PT1S ", + "P1Y", "P1M", "P1S", "PT1D", "PT1Y", + "PT1S2S", "PT1S2H", + "P9999999999999DT-9999999999999H", + "PT1.5H", "PT0.5D", "PT.5S", "PT0.25.25S", + )) { + assertNull(Duration.parseIsoStringOrNull(invalidValue), invalidValue) + assertFailsWith(invalidValue) { Duration.parseIsoString(invalidValue) } + } + + } + + @Test + fun formatInUnits() { var d = with(Duration) { days(1) + hours(15) + minutes(31) + seconds(45) + milliseconds(678) + microseconds(920) + nanoseconds(516.34) @@ -545,28 +563,32 @@ class DurationTest { @Test - fun toStringDefault() { - fun test(duration: Duration, vararg expectedOptions: String) { + fun parseAndFormatDefault() { + fun test(duration: Duration, vararg expected: String) { val actual = duration.toString() + assertEquals(expected.first(), actual) - if (!expectedOptions.contains(actual)) { - assertEquals(expectedOptions.toList(), duration.toString()) - } - if (duration > Duration.ZERO) + if (duration > Duration.ZERO) { assertEquals("-$actual", (-duration).toString()) + } + + for (string in expected) { + assertEquals(duration, Duration.parse(string), string) + assertEquals(duration, Duration.parseOrNull(string), string) + } } - test(Duration.days(101), "101d") - test(Duration.days(45.3), "45d 7h 12m") // 0.3d == 7.2h + test(Duration.days(101), "101d", "2424h") + test(Duration.days(45.3), "45d 7h 12m", "45.3d", "45d 7.2h") // 0.3d == 7.2h test(Duration.days(45), "45d") - test(Duration.days(40.5), "40d 12h") - test(Duration.days(40) + Duration.minutes(20), "40d 0h 20m") - test(Duration.days(40) + Duration.seconds(20), "40d 0h 0m 20s") - test(Duration.days(40) + Duration.nanoseconds(100), "40d 0h 0m 0.000000100s") + test(Duration.days(40.5), "40d 12h", "40.5d", "40d 720m") + test(Duration.days(40) + Duration.minutes(20), "40d 0h 20m", "40d 20m", "40d 1200s") + test(Duration.days(40) + Duration.seconds(20), "40d 0h 0m 20s", "40d 20s") + test(Duration.days(40) + Duration.nanoseconds(100), "40d 0h 0m 0.000000100s", "40d 100ns") - test(Duration.hours(40) + Duration.minutes(15), "1d 16h 15m") - test(Duration.hours(40), "1d 16h") + test(Duration.hours(40) + Duration.minutes(15), "1d 16h 15m", "40h 15m") + test(Duration.hours(40), "1d 16h", "40h") test(Duration.hours(12.5), "12h 30m") test(Duration.hours(12) + Duration.seconds(15), "12h 0m 15s") @@ -584,7 +606,7 @@ class DurationTest { test(Duration.seconds(0.5), "500ms") test(Duration.milliseconds(40.2), "40.200ms") test(Duration.milliseconds(4.225), "4.225ms") - test(Duration.milliseconds(4.24501), "4.245010ms") + test(Duration.milliseconds(4.24501), "4.245010ms", "4ms 245us 10ns") test(Duration.milliseconds(1), "1ms") test(Duration.milliseconds(0.75), "750us") @@ -592,10 +614,12 @@ class DurationTest { test(Duration.microseconds(7.25), "7.250us") test(Duration.microseconds(1.035), "1.035us") test(Duration.microseconds(1.005), "1.005us") + test(Duration.nanoseconds(1800), "1.800us", "1800ns", "0.0000000005h") test(Duration.nanoseconds(950.5), "951ns") test(Duration.nanoseconds(85.23), "85ns") test(Duration.nanoseconds(8.235), "8ns") + test(Duration.nanoseconds(1), "1ns", "0.9ns", "0.001us", "0.0009us") test(Duration.nanoseconds(1.3), "1ns") test(Duration.nanoseconds(0.75), "1ns") test(Duration.nanoseconds(0.7512), "1ns") @@ -605,7 +629,7 @@ class DurationTest { // test(Duration.nanoseconds(0.0034), "0.0034ns") // test(Duration.nanoseconds(0.0000035), "0.0000035ns") - test(Duration.ZERO, "0s") + test(Duration.ZERO, "0s", "0.4ns", "0000.0000ns") test(Duration.days(365) * 10000, "3650000d") test(Duration.days(300) * 100000, "30000000d") test(Duration.days(365) * 100000, "36500000d") @@ -621,4 +645,25 @@ class DurationTest { test(Duration.INFINITE, "Infinity") } + @Test + fun parseDefaultFailing() { + for (invalidValue in listOf( + "", " ", "P", "PT", "P1DT", "P1", "PT1", "0", "+P", "+", "-", "h", "H", "something", + "1234567890123456789012ns", "Inf", "-Infinity value", + "1s ", " 1s", + "1d 1m 1h", "1s 2s", + "-12m -15s", + "12.5m 11.5s", ".2s", "0.1553.39m", + "P+12+34D", "P12-34D", "PT1234567890-1234567890S", + " P1D", "PT1S ", + "P1Y", "P1M", "P1S", "PT1D", "PT1Y", + "PT1S2S", "PT1S2H", + "P9999999999999DT-9999999999999H", + "PT1.5H", "PT0.5D", "PT.5S", "PT0.25.25S", + )) { + assertNull(Duration.parseOrNull(invalidValue), invalidValue) + assertFailsWith(invalidValue) { Duration.parse(invalidValue) } + } + } + }