Improve the error messages

Author: dkhalanskyjb

core/native/src/Instant.kt

@@ -240,11 +240,18 @@ private class UnboundedLocalDateTime(
 }
 
 internal fun parseIso(isoString: String): Instant {
+    fun parseFailure(error: String): Nothing {
+        throw IllegalArgumentException("$error when parsing an Instant from $isoString")
+    }
+    inline fun expect(what: String, where: Int, predicate: (Char) -> Boolean) {
+        val c = isoString[where]
+        if (!predicate(c)) {
+            parseFailure("Expected $what, but got $c at position $where")
+        }
+    }
     val s = isoString
     var i = 0
-    if (s.isEmpty()) {
-        TODO()
-    }
+    require(s.isNotEmpty()) { "An empty string is not a valid Instant" }
     val yearSign = when (s[i]) {
         '+' -> { ++i; '+' }
         '-' -> { ++i; '-' }
@@ -257,36 +264,34 @@ internal fun parseIso(isoString: String): Instant {
         ++i
     }
     val year = when {
-        i - yearStart > 9 -> {
-            TODO()
+        i > yearStart + 9 -> {
+            parseFailure("Expected at most 9 digits for the year number, got ${i - yearStart}")
         }
         i - yearStart < 4 -> {
-            TODO()
+            parseFailure("The year number must be padded to 4 digits, got ${i - yearStart} digits")
         }
         else -> {
             if (yearSign == '+' && i - yearStart == 4) {
-                TODO()
+                parseFailure("The '+' sign at the start is only valid for year numbers longer than 4 digits")
             }
             if (yearSign == ' ' && i - yearStart != 4) {
-                TODO()
+                parseFailure("The '+' sign is required for year numbers longer than 4 digits")
             }
             if (yearSign == '-') -absYear else absYear
         }
     }
     // reading at least -MM-DDTHH:MM:SSZ
     //                  0123456789012345 16 chars
     if (s.length < i + 16) {
-        TODO()
+        parseFailure("The input string is too short")
     }
-    if (s[i] != '-') { TODO() }
-    if (s[i + 3] != '-') { TODO() }
-    if (s[i + 6] != 'T' && s[i + 6] != 't') { TODO() }
-    if (s[i + 9] != ':') { TODO() }
-    if (s[i + 12] != ':') { TODO() }
+    expect("'-'", i) { it == '-' }
+    expect("'-'", i + 3) { it == '-' }
+    expect("'T' or 't'", i + 6) { it == 'T' || it == 't' }
+    expect("':'", i + 9) { it == ':' }
+    expect("':'", i + 12) { it == ':' }
     for (j in listOf(1, 2, 4, 5, 7, 8, 10, 11, 13, 14)) {
-        if (s[i + j] !in '0'..'9') {
-            TODO()
-        }
+        expect("an ASCII digit", i + j) { it in '0'..'9' }
     }
     fun twoDigitNumber(index: Int) = s[index].code * 10 + s[index + 1].code - '0'.code * 11
     val month = twoDigitNumber(i + 1)
@@ -305,52 +310,54 @@ internal fun parseIso(isoString: String): Instant {
         if (i <= fractionStart + 9) {
             fraction * POWERS_OF_TEN[fractionStart + 9 - i]
         } else {
-            TODO()
+            parseFailure("At most 9 digits are supported for the fraction of the second, got {i - fractionStart}")
         }
     } else {
         i += 15
         0
     }
     val offsetSeconds = when (val sign = s.getOrNull(i)) {
         null -> {
-            TODO()
+            parseFailure("The UTC offset at the end of the string is missing")
         }
         'z', 'Z' -> if (s.length == i + 1) {
             0
         } else {
-            TODO()
+            parseFailure("Extra text after the instant at position ${i + 1}")
         }
         '-', '+' -> {
             val offsetStrLength = s.length - i
-            if (offsetStrLength % 3 != 0) { TODO() }
-            if (offsetStrLength > 9) { TODO() }
+            if (offsetStrLength % 3 != 0) { parseFailure("Invalid UTC offset string '${s.substring(i)}'") }
+            if (offsetStrLength > 9) { parseFailure("The UTC offset string '${s.substring(i)}' is too long") }
             for (j in listOf(3, 6)) {
                 if (s.getOrNull(i + j) ?: break != ':')
-                    TODO("Expected ':' at index ${i + j}, got '${s[i + j]}' when parsing instant from $s")
+                    parseFailure("Expected ':' at index ${i + j}, got '${s[i + j]}'")
             }
             for (j in listOf(1, 2, 4, 5, 7, 8)) {
                 if (s.getOrNull(i + j) ?: break !in '0'..'9')
-                    TODO("Expected a digit at index ${i + j}, got '${s[i + j]}' when parsing instant from $s")
+                    parseFailure("Expected a digit at index ${i + j}, got '${s[i + j]}'")
             }
             val offsetHour = twoDigitNumber(i + 1)
             val offsetMinute = if (offsetStrLength > 3) { twoDigitNumber(i + 4) } else { 0 }
             val offsetSecond = if (offsetStrLength > 6) { twoDigitNumber(i + 7) } else { 0 }
-            if (offsetMinute !in 0..59) { TODO("Expected offset-minute-of-hour in 0..59, got $offsetMinute when parsing instant from $s") }
-            if (offsetSecond !in 0..59) { TODO("Expected offset-second-of-minute in 0..59, got $offsetSecond when parsing instant from $s") }
+            if (offsetMinute !in 0..59) { parseFailure("Expected offset-minute-of-hour in 0..59, got $offsetMinute") }
+            if (offsetSecond !in 0..59) { parseFailure("Expected offset-second-of-minute in 0..59, got $offsetSecond") }
             if (offsetHour !in 0..17 && !(offsetHour == 18 && offsetMinute == 0 && offsetMinute == 0)) {
-                TODO()
+                parseFailure("Expected an offset in -18:00..+18:00, got $sign$offsetHour:$offsetMinute:$offsetSecond")
             }
             (offsetHour * 3600 + offsetMinute * 60 + offsetSecond) * if (sign == '-') -1 else 1
         }
         else -> {
-            TODO()
+            parseFailure("Expected the UTC offset at position $i, got '$sign'")
         }
     }
-    if (month !in 1..12) { TODO() }
-    if (day !in 1..month.monthLength(isLeapYear(year))) { TODO() }
-    if (hour !in 0..23) { TODO() }
-    if (minute !in 0..59) { TODO() }
-    if (second !in 0..59) { TODO() }
+    if (month !in 1..12) { parseFailure("Expected a month number in 1..12, got $month") }
+    if (day !in 1..month.monthLength(isLeapYear(year))) {
+        parseFailure("Expected a valid day-of-month for $year-$month, got $day")
+    }
+    if (hour !in 0..23) { parseFailure("Expected hour in 0..23, got $hour") }
+    if (minute !in 0..59) { parseFailure("Expected minute-of-hour in 0..59, got $minute") }
+    if (second !in 0..59) { parseFailure("Expected second-of-minute in 0..59, got $second") }
     return UnboundedLocalDateTime(year, month, day, hour, minute, second, nanosecond).toInstant(offsetSeconds)
 }
 

core/native/test/InstantIsoStringsTest.kt

@@ -99,7 +99,7 @@ class InstantIsoStringsTest {
 @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
 @kotlin.internal.InlineOnly
 private fun <T> assertInvalidFormat(message: String? = null, f: () -> T) {
-    assertFailsWith<NotImplementedError>(message) {
+    assertFailsWith<IllegalArgumentException>(message) {
         val result = f()
         fail(result.toString())
     }