Skip to content

Commit 02e4e4d

Browse files
dkhalanskyjbdkhalanskyjb
authored
Improve error descriptiveness in the parsing API (#360)
Fixes #359 Fixes #361 Additionally, remove delayed initialization of parsing to ensure that creating ambigous formats fails and document the thrown exception.
1 parent cc8121a commit 02e4e4d

15 files changed

+101
-44
lines changed

‎core/common/src/LocalDate.kt

+2
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ public expect class LocalDate : Comparable<LocalDate> {
6565
* (for example, [dayOfMonth] is 31 for February), consider using [DateTimeComponents.Format] instead.
6666
*
6767
* There is a collection of predefined formats in [LocalDate.Formats].
68+
*
69+
* @throws IllegalArgumentException if parsing using this format is ambiguous.
6870
*/
6971
@Suppress("FunctionName")
7072
public fun Format(block: DateTimeFormatBuilder.WithDate.() -> Unit): DateTimeFormat<LocalDate>

‎core/common/src/LocalDateTime.kt

+2
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public expect class LocalDateTime : Comparable<LocalDateTime> {
6262
* (for example, [dayOfMonth] is 31 for February), consider using [DateTimeComponents.Format] instead.
6363
*
6464
* There is a collection of predefined formats in [LocalDateTime.Formats].
65+
*
66+
* @throws IllegalArgumentException if parsing using this format is ambiguous.
6567
*/
6668
@Suppress("FunctionName")
6769
public fun Format(builder: DateTimeFormatBuilder.WithDateTime.() -> Unit): DateTimeFormat<LocalDateTime>

‎core/common/src/LocalTime.kt

+2
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ public expect class LocalTime : Comparable<LocalTime> {
9797
* (for example, [second] is 60), consider using [DateTimeComponents.Format] instead.
9898
*
9999
* There is a collection of predefined formats in [LocalTime.Formats].
100+
*
101+
* @throws IllegalArgumentException if parsing using this format is ambiguous.
100102
*/
101103
@Suppress("FunctionName")
102104
public fun Format(builder: DateTimeFormatBuilder.WithTime.() -> Unit): DateTimeFormat<LocalTime>

‎core/common/src/UtcOffset.kt

+2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ public expect class UtcOffset {
6767
* [DateTimeFormatBuilder.WithUtcOffset.offset] in a format builder for a larger data structure.
6868
*
6969
* There is a collection of predefined formats in [UtcOffset.Formats].
70+
*
71+
* @throws IllegalArgumentException if parsing using this format is ambiguous.
7072
*/
7173
@Suppress("FunctionName")
7274
public fun Format(block: DateTimeFormatBuilder.WithUtcOffset.() -> Unit): DateTimeFormat<UtcOffset>

‎core/common/src/format/DateTimeComponents.kt

+2
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ public class DateTimeComponents internal constructor(internal val contents: Date
6969
* Creates a [DateTimeFormat] for [DateTimeComponents] values using [DateTimeFormatBuilder.WithDateTimeComponents].
7070
*
7171
* There is a collection of predefined formats in [DateTimeComponents.Formats].
72+
*
73+
* @throws IllegalArgumentException if parsing using this format is ambiguous.
7274
*/
7375
@Suppress("FunctionName")
7476
public fun Format(block: DateTimeFormatBuilder.WithDateTimeComponents.() -> Unit): DateTimeFormat<DateTimeComponents> {

‎core/common/src/format/DateTimeFormat.kt

+4-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ internal sealed class AbstractDateTimeFormat<T, U : Copyable<U>> : DateTimeForma
115115
try {
116116
return valueFromIntermediate(matched)
117117
} catch (e: IllegalArgumentException) {
118-
throw DateTimeFormatException(e.message!!)
118+
throw DateTimeFormatException(when (val message = e.message) {
119+
null -> "The value parsed from '$input' is invalid"
120+
else -> "$message (when parsing '$input')"
121+
}, e)
119122
}
120123
}
121124

‎core/common/src/internal/format/FieldFormatDirective.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ internal abstract class NamedUnsignedIntFieldFormatDirective<in Target>(
108108
override fun parser(): ParserStructure<Target> =
109109
ParserStructure(
110110
listOf(
111-
StringSetParserOperation(values, AssignableString(), "One of $values for $name")
111+
StringSetParserOperation(values, AssignableString(), "one of $values for $name")
112112
), emptyList()
113113
)
114114
}
@@ -142,7 +142,7 @@ internal abstract class NamedEnumIntFieldFormatDirective<in Target, Type>(
142142
override fun parser(): ParserStructure<Target> =
143143
ParserStructure(
144144
listOf(
145-
StringSetParserOperation(mapping.values, AssignableString(), "One of ${mapping.values} for $name")
145+
StringSetParserOperation(mapping.values, AssignableString(), "one of ${mapping.values} for $name")
146146
), emptyList()
147147
)
148148
}

‎core/common/src/internal/format/FormatStructure.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,8 @@ internal open class ConcatenatedFormatStructure<in T>(
231231

232232
internal class CachedFormatStructure<in T>(formats: List<NonConcatenatedFormatStructure<T>>) :
233233
ConcatenatedFormatStructure<T>(formats) {
234-
private val cachedFormatter: FormatterStructure<T> by lazy { super.formatter() }
235-
private val cachedParser: ParserStructure<T> by lazy { super.parser() }
234+
private val cachedFormatter: FormatterStructure<T> = super.formatter()
235+
private val cachedParser: ParserStructure<T> = super.parser()
236236

237237
override fun formatter(): FormatterStructure<T> = cachedFormatter
238238

‎core/common/src/internal/format/parser/ParserOperation.kt

+6-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ internal class NumberSpanParserOperation<Output>(
5050

5151
init {
5252
require(consumers.all { (it.length ?: Int.MAX_VALUE) > 0 })
53-
require(consumers.count { it.length == null } <= 1)
53+
require(consumers.count { it.length == null } <= 1) {
54+
val fieldNames = consumers.filter { it.length == null }.map { it.whatThisExpects }
55+
"At most one variable-length numeric field in a row is allowed, but got several: $fieldNames. " +
56+
"Parsing is undefined: for example, with variable-length month number " +
57+
"and variable-length day of month, '111' can be parsed as Jan 11th or Nov 1st."
58+
}
5459
}
5560

5661
private val whatThisExpects: String

‎core/common/test/format/DateTimeComponentsFormatTest.kt

+7-7
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ class DateTimeComponentsFormatTest {
3232
setOffset(UtcOffset.ZERO)
3333
},
3434
format.parse("Tue, 40 Jun 2008 11:05:30 GMT"))
35-
assertFailsWith<DateTimeFormatException> { format.parse("Bue, 3 Jun 2008 11:05:30 GMT") }
35+
format.assertCanNotParse("Bue, 3 Jun 2008 11:05:30 GMT")
3636
}
3737

3838
@Test
3939
fun testInconsistentLocalTime() {
4040
val formatTime = LocalTime.Format {
41-
hour(); char(':'); minute();
41+
hour(); char(':'); minute()
4242
chars(" ("); amPmHour(); char(':'); minute(); char(' '); amPmMarker("AM", "PM"); char(')')
4343
}
4444
val format = DateTimeComponents.Format { time(formatTime) }
@@ -53,16 +53,16 @@ class DateTimeComponentsFormatTest {
5353
DateTimeComponents().apply { hour = 23; hourOfAmPm = 11; minute = 15; amPm = AmPmMarker.AM },
5454
format.parse(time2)
5555
)
56-
assertFailsWith<IllegalArgumentException> { formatTime.parse(time2) }
56+
formatTime.assertCanNotParse(time2)
5757
val time3 = "23:15 (10:15 PM)" // a time with an inconsistent number of hours
5858
assertDateTimeComponentsEqual(
5959
DateTimeComponents().apply { hour = 23; hourOfAmPm = 10; minute = 15; amPm = AmPmMarker.PM },
6060
format.parse(time3)
6161
)
62-
assertFailsWith<IllegalArgumentException> { formatTime.parse(time3) }
62+
formatTime.assertCanNotParse(time3)
6363
val time4 = "23:15 (11:16 PM)" // a time with an inconsistently duplicated field
64-
assertFailsWith<IllegalArgumentException> { format.parse(time4) }
65-
assertFailsWith<IllegalArgumentException> { formatTime.parse(time4) }
64+
format.assertCanNotParse(time4)
65+
formatTime.assertCanNotParse(time4)
6666
}
6767

6868
@Test
@@ -95,7 +95,7 @@ class DateTimeComponentsFormatTest {
9595
assertEquals(dateTime, bag.toLocalDateTime())
9696
assertEquals(offset, bag.toUtcOffset())
9797
assertEquals(berlin, bag.timeZoneId)
98-
assertFailsWith<DateTimeFormatException> { format.parse("2008-06-03T11:05:30.123456789+01:00[Mars/New_York]") }
98+
format.assertCanNotParse("2008-06-03T11:05:30.123456789+01:00[Mars/New_York]")
9999
for (zone in TimeZone.availableZoneIds) {
100100
assertEquals(zone, format.parse("2008-06-03T11:05:30.123456789+01:00[$zone]").timeZoneId)
101101
}

‎core/common/test/format/DateTimeFormatTest.kt

+21
Original file line numberDiff line numberDiff line change
@@ -127,4 +127,25 @@ class DateTimeFormatTest {
127127
DateTimeComponents.Format { chars(format) }.parse(format)
128128
}
129129
}
130+
131+
@Test
132+
fun testCreatingAmbiguousFormat() {
133+
assertFailsWith<IllegalArgumentException> {
134+
DateTimeComponents.Format {
135+
monthNumber(Padding.NONE)
136+
dayOfMonth(Padding.NONE)
137+
}
138+
}
139+
}
140+
}
141+
142+
fun <T> DateTimeFormat<T>.assertCanNotParse(input: String) {
143+
val exception = assertFailsWith<DateTimeFormatException> { parse(input) }
144+
try {
145+
val message = exception.message ?: throw AssertionError("The parse exception didn't have a message")
146+
assertContains(message, input)
147+
} catch (e: AssertionError) {
148+
e.addSuppressed(exception)
149+
throw e
150+
}
130151
}

‎core/common/test/format/LocalDateFormatTest.kt

+5-5
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ class LocalDateFormatTest {
1515

1616
@Test
1717
fun testErrorHandling() {
18-
val format = LocalDate.Formats.ISO
19-
assertEquals(LocalDate(2023, 2, 28), format.parse("2023-02-28"))
20-
val error = assertFailsWith<DateTimeFormatException> { format.parse("2023-02-40") }
21-
assertContains(error.message!!, "40")
22-
assertFailsWith<DateTimeFormatException> { format.parse("2023-02-XX") }
18+
LocalDate.Formats.ISO.apply {
19+
assertEquals(LocalDate(2023, 2, 28), parse("2023-02-28"))
20+
assertCanNotParse("2023-02-40")
21+
assertCanNotParse("2023-02-XX")
22+
}
2323
}
2424

2525
@Test

‎core/common/test/format/LocalDateTimeFormatTest.kt

+17-16
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ class LocalDateTimeFormatTest {
1515

1616
@Test
1717
fun testErrorHandling() {
18-
val format = LocalDateTime.Formats.ISO
19-
assertEquals(LocalDateTime(2023, 2, 28, 15, 36), format.parse("2023-02-28T15:36"))
20-
val error = assertFailsWith<DateTimeFormatException> { format.parse("2023-02-40T15:36") }
21-
assertContains(error.message!!, "40")
22-
assertFailsWith<DateTimeFormatException> { format.parse("2023-02-XXT15:36") }
18+
LocalDateTime.Formats.ISO.apply {
19+
assertEquals(LocalDateTime(2023, 2, 28, 15, 36), parse("2023-02-28T15:36"))
20+
assertCanNotParse("2023-02-40T15:36")
21+
assertCanNotParse("2023-02-XXT15:36")
22+
}
2323
}
2424

2525
@Test
@@ -163,7 +163,7 @@ class LocalDateTimeFormatTest {
163163
put(LocalDateTime(123456, 1, 1, 13, 44, 0, 0), ("+123456- 1- 1 13:44: 0" to setOf()))
164164
put(LocalDateTime(-123456, 1, 1, 13, 44, 0, 0), ("-123456- 1- 1 13:44: 0" to setOf()))
165165
}
166-
val format = LocalDateTime.Format {
166+
LocalDateTime.Format {
167167
year(Padding.SPACE)
168168
char('-')
169169
monthNumber(Padding.SPACE)
@@ -175,17 +175,18 @@ class LocalDateTimeFormatTest {
175175
minute(Padding.SPACE)
176176
char(':')
177177
second(Padding.SPACE)
178+
}.apply {
179+
test(dateTimes, this)
180+
parse(" 008- 7- 5 0: 0: 0")
181+
assertCanNotParse(" 008- 7- 5 0: 0: 0")
182+
assertCanNotParse(" 8- 7- 5 0: 0: 0")
183+
assertCanNotParse(" 008- 7- 5 0: 0: 0")
184+
assertCanNotParse(" 008-7- 5 0: 0: 0")
185+
assertCanNotParse("+008- 7- 5 0: 0: 0")
186+
assertCanNotParse(" -08- 7- 5 0: 0: 0")
187+
assertCanNotParse(" -08- 7- 5 0: 0: 0")
188+
assertCanNotParse("-8- 7- 5 0: 0: 0")
178189
}
179-
test(dateTimes, format)
180-
format.parse(" 008- 7- 5 0: 0: 0")
181-
assertFailsWith<DateTimeFormatException> { format.parse(" 008- 7- 5 0: 0: 0") }
182-
assertFailsWith<DateTimeFormatException> { format.parse(" 8- 7- 5 0: 0: 0") }
183-
assertFailsWith<DateTimeFormatException> { format.parse(" 008- 7- 5 0: 0: 0") }
184-
assertFailsWith<DateTimeFormatException> { format.parse(" 008-7- 5 0: 0: 0") }
185-
assertFailsWith<DateTimeFormatException> { format.parse("+008- 7- 5 0: 0: 0") }
186-
assertFailsWith<DateTimeFormatException> { format.parse(" -08- 7- 5 0: 0: 0") }
187-
assertFailsWith<DateTimeFormatException> { format.parse(" -08- 7- 5 0: 0: 0") }
188-
assertFailsWith<DateTimeFormatException> { format.parse("-8- 7- 5 0: 0: 0") }
189190
}
190191

191192
@Test

‎core/common/test/format/LocalTimeFormatTest.kt

+22-5
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ class LocalTimeFormatTest {
1515

1616
@Test
1717
fun testErrorHandling() {
18-
val format = LocalTime.Formats.ISO
19-
assertEquals(LocalTime(15, 36), format.parse("15:36"))
20-
val error = assertFailsWith<DateTimeFormatException> { format.parse("40:36") }
21-
assertContains(error.message!!, "40")
22-
assertFailsWith<DateTimeFormatException> { format.parse("XX:36") }
18+
LocalTime.Formats.ISO.apply {
19+
assertEquals(LocalTime(15, 36), parse("15:36"))
20+
assertCanNotParse("40:36")
21+
assertCanNotParse("XX:36")
22+
}
2323
}
2424

2525
@Test
@@ -199,6 +199,23 @@ class LocalTimeFormatTest {
199199
assertEquals("12:34:56.123", format.format(LocalTime(12, 34, 56, 123000000)))
200200
}
201201

202+
@Test
203+
fun testParsingDisagreeingComponents() {
204+
LocalTime.Format {
205+
hour()
206+
char(':')
207+
minute()
208+
char('(')
209+
amPmHour()
210+
char(' ')
211+
amPmMarker("AM", "PM")
212+
char(')')
213+
}.apply {
214+
assertEquals(LocalTime(23, 59), parse("23:59(11 PM)"))
215+
assertCanNotParse("23:59(11 AM)")
216+
}
217+
}
218+
202219
private fun test(strings: Map<LocalTime, Pair<String, Set<String>>>, format: DateTimeFormat<LocalTime>) {
203220
for ((date, stringsForDate) in strings) {
204221
val (canonicalString, otherStrings) = stringsForDate

‎core/common/test/format/UtcOffsetFormatTest.kt

+5-5
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,18 @@ class UtcOffsetFormatTest {
1313

1414
@Test
1515
fun testErrorHandling() {
16-
val format = UtcOffset.Format {
16+
UtcOffset.Format {
1717
isoOffset(
1818
zOnZero = true,
1919
useSeparator = true,
2020
outputMinute = WhenToOutput.ALWAYS,
2121
outputSecond = WhenToOutput.IF_NONZERO
2222
)
23+
}.apply {
24+
assertEquals(UtcOffset(hours = -4, minutes = -30), parse("-04:30"))
25+
assertCanNotParse("-04:60")
26+
assertCanNotParse("-04:XX")
2327
}
24-
assertEquals(UtcOffset(hours = -4, minutes = -30), format.parse("-04:30"))
25-
val error = assertFailsWith<DateTimeFormatException> { format.parse("-04:60") }
26-
assertContains(error.message!!, "60")
27-
assertFailsWith<DateTimeFormatException> { format.parse("-04:XX") }
2828
}
2929

3030
@Test

0 commit comments

Comments
 (0)