Skip to content

Commit 30b182b

Browse files
committed
Ensure that the value-is-reassigned error is not an exception
This way, if one branch of parsing tries to reassign a value, this branch will get ignored instead of crashing the whole process.
1 parent 7ebeead commit 30b182b

File tree

10 files changed

+181
-146
lines changed

10 files changed

+181
-146
lines changed

core/common/src/format/DateTimeComponents.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ internal class DateTimeComponentsContents internal constructor(
442442
}
443443

444444
@SharedImmutable
445-
internal val timeZoneField = GenericFieldSpec(DateTimeComponentsContents::timeZoneId)
445+
internal val timeZoneField = GenericFieldSpec(PropertyAccessor(DateTimeComponentsContents::timeZoneId))
446446

447447
internal class TimeZoneIdDirective(private val knownZones: Set<String>) :
448448
StringFieldFormatDirective<DateTimeComponentsContents>(timeZoneField, knownZones) {

core/common/src/format/LocalDateFormat.kt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ internal interface DateFieldContainer {
131131
}
132132

133133
private object DateFields {
134-
val year = SignedFieldSpec(DateFieldContainer::year, maxAbsoluteValue = null)
135-
val month = UnsignedFieldSpec(DateFieldContainer::monthNumber, minValue = 1, maxValue = 12)
136-
val dayOfMonth = UnsignedFieldSpec(DateFieldContainer::dayOfMonth, minValue = 1, maxValue = 31)
137-
val isoDayOfWeek = UnsignedFieldSpec(DateFieldContainer::isoDayOfWeek, minValue = 1, maxValue = 7)
134+
val year = SignedFieldSpec(PropertyAccessor(DateFieldContainer::year), maxAbsoluteValue = null)
135+
val month = UnsignedFieldSpec(PropertyAccessor(DateFieldContainer::monthNumber), minValue = 1, maxValue = 12)
136+
val dayOfMonth = UnsignedFieldSpec(PropertyAccessor(DateFieldContainer::dayOfMonth), minValue = 1, maxValue = 31)
137+
val isoDayOfWeek = UnsignedFieldSpec(PropertyAccessor(DateFieldContainer::isoDayOfWeek), minValue = 1, maxValue = 7)
138138
}
139139

140140
/**
@@ -228,7 +228,7 @@ private class MonthDirective(private val padding: Padding) :
228228
}
229229

230230
private class MonthNameDirective(private val names: MonthNames) :
231-
NamedUnsignedIntFieldFormatDirective<DateFieldContainer>(DateFields.month, names.names) {
231+
NamedUnsignedIntFieldFormatDirective<DateFieldContainer>(DateFields.month, names.names, "monthName") {
232232
override val builderRepresentation: String get() =
233233
"${DateTimeFormatBuilder.WithDate::monthName.name}(${names.toKotlinCode()})"
234234

@@ -252,7 +252,7 @@ private class DayDirective(private val padding: Padding) :
252252
}
253253

254254
private class DayOfWeekDirective(private val names: DayOfWeekNames) :
255-
NamedUnsignedIntFieldFormatDirective<DateFieldContainer>(DateFields.isoDayOfWeek, names.names) {
255+
NamedUnsignedIntFieldFormatDirective<DateFieldContainer>(DateFields.isoDayOfWeek, names.names, "dayOfWeekName") {
256256

257257
override val builderRepresentation: String get() =
258258
"${DateTimeFormatBuilder.WithDate::dayOfWeek.name}(${names.toKotlinCode()})"
@@ -324,4 +324,4 @@ internal val ISO_DATE_BASIC by lazy {
324324
}
325325

326326
@SharedImmutable
327-
private val emptyIncompleteLocalDate = IncompleteLocalDate()
327+
private val emptyIncompleteLocalDate = IncompleteLocalDate()

core/common/src/format/LocalTimeFormat.kt

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ internal interface TimeFieldContainer {
2727
}
2828

2929
private object TimeFields {
30-
val hour = UnsignedFieldSpec(TimeFieldContainer::hour, minValue = 0, maxValue = 23)
31-
val minute = UnsignedFieldSpec(TimeFieldContainer::minute, minValue = 0, maxValue = 59)
32-
val second = UnsignedFieldSpec(TimeFieldContainer::second, minValue = 0, maxValue = 59, defaultValue = 0)
33-
val fractionOfSecond = GenericFieldSpec(TimeFieldContainer::fractionOfSecond, defaultValue = DecimalFraction(0, 9))
34-
val isPm = GenericFieldSpec(TimeFieldContainer::isPm)
35-
val hourOfAmPm = UnsignedFieldSpec(TimeFieldContainer::hourOfAmPm, minValue = 1, maxValue = 12)
30+
val hour = UnsignedFieldSpec(PropertyAccessor(TimeFieldContainer::hour), minValue = 0, maxValue = 23)
31+
val minute = UnsignedFieldSpec(PropertyAccessor(TimeFieldContainer::minute), minValue = 0, maxValue = 59)
32+
val second = UnsignedFieldSpec(PropertyAccessor(TimeFieldContainer::second), minValue = 0, maxValue = 59, defaultValue = 0)
33+
val fractionOfSecond = GenericFieldSpec(PropertyAccessor(TimeFieldContainer::fractionOfSecond), defaultValue = DecimalFraction(0, 9))
34+
val isPm = GenericFieldSpec(PropertyAccessor(TimeFieldContainer::isPm))
35+
val hourOfAmPm = UnsignedFieldSpec(PropertyAccessor(TimeFieldContainer::hourOfAmPm), minValue = 1, maxValue = 12)
3636
}
3737

3838
internal class IncompleteLocalTime(
@@ -149,7 +149,8 @@ private class AmPmMarkerDirective(private val amString: String, private val pmSt
149149
TimeFields.isPm, mapOf(
150150
false to amString,
151151
true to pmString,
152-
)
152+
),
153+
"AM/PM marker"
153154
) {
154155

155156
override val builderRepresentation: String get() =

core/common/src/format/UtcOffsetFormat.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,26 +121,26 @@ internal fun DateTimeFormatBuilder.WithUtcOffset.isoOffset(
121121

122122
private object OffsetFields {
123123
private val sign = object : FieldSign<UtcOffsetFieldContainer> {
124-
override val isNegative = UtcOffsetFieldContainer::isNegative
124+
override val isNegative = PropertyAccessor(UtcOffsetFieldContainer::isNegative)
125125
override fun isZero(obj: UtcOffsetFieldContainer): Boolean =
126126
(obj.totalHoursAbs ?: 0) == 0 && (obj.minutesOfHour ?: 0) == 0 && (obj.secondsOfMinute ?: 0) == 0
127127
}
128128
val totalHoursAbs = UnsignedFieldSpec(
129-
UtcOffsetFieldContainer::totalHoursAbs,
129+
PropertyAccessor(UtcOffsetFieldContainer::totalHoursAbs),
130130
defaultValue = 0,
131131
minValue = 0,
132132
maxValue = 18,
133133
sign = sign,
134134
)
135135
val minutesOfHour = UnsignedFieldSpec(
136-
UtcOffsetFieldContainer::minutesOfHour,
136+
PropertyAccessor(UtcOffsetFieldContainer::minutesOfHour),
137137
defaultValue = 0,
138138
minValue = 0,
139139
maxValue = 59,
140140
sign = sign,
141141
)
142142
val secondsOfMinute = UnsignedFieldSpec(
143-
UtcOffsetFieldContainer::secondsOfMinute,
143+
PropertyAccessor(UtcOffsetFieldContainer::secondsOfMinute),
144144
defaultValue = 0,
145145
minValue = 0,
146146
maxValue = 59,

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

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,14 @@ internal abstract class UnsignedIntFieldFormatDirective<in Target>(
6161

6262
override fun formatter(): FormatterStructure<Target> {
6363
val formatter = UnsignedIntFormatterStructure(
64-
number = field::getNotNull,
64+
number = field.accessor::getterNotNull,
6565
zeroPadding = minDigits,
6666
)
6767
return if (spacePadding != null) SpacePaddedFormatter(formatter, spacePadding) else formatter
6868
}
6969

7070
override fun parser(): ParserStructure<Target> =
71-
spaceAndZeroPaddedUnsignedInt(minDigits, maxDigits, spacePadding, field::setWithoutReassigning, field.name)
71+
spaceAndZeroPaddedUnsignedInt(minDigits, maxDigits, spacePadding, field.accessor, field.name)
7272
}
7373

7474
/**
@@ -77,6 +77,7 @@ internal abstract class UnsignedIntFieldFormatDirective<in Target>(
7777
internal abstract class NamedUnsignedIntFieldFormatDirective<in Target>(
7878
final override val field: UnsignedFieldSpec<Target>,
7979
private val values: List<String>,
80+
private val name: String,
8081
) : FieldFormatDirective<Target> {
8182

8283
init {
@@ -85,10 +86,15 @@ internal abstract class NamedUnsignedIntFieldFormatDirective<in Target>(
8586
}
8687
}
8788

88-
private fun getStringValue(target: Target): String = values[field.getNotNull(target) - field.minValue]
89+
private fun getStringValue(target: Target): String = values[field.accessor.getterNotNull(target) - field.minValue]
8990

90-
private fun setStringValue(target: Target, value: String) {
91-
field.setWithoutReassigning(target, values.indexOf(value) + field.minValue)
91+
private inner class AssignableString: AssignableField<Target, String> {
92+
override fun trySetWithoutReassigning(container: Target, newValue: String): String? =
93+
field.accessor.trySetWithoutReassigning(container, values.indexOf(newValue) + field.minValue)?.let {
94+
values[it - field.minValue]
95+
}
96+
97+
override val name: String get() = this@NamedUnsignedIntFieldFormatDirective.name
9298
}
9399

94100
override fun formatter(): FormatterStructure<Target> =
@@ -97,7 +103,7 @@ internal abstract class NamedUnsignedIntFieldFormatDirective<in Target>(
97103
override fun parser(): ParserStructure<Target> =
98104
ParserStructure(
99105
listOf(
100-
StringSetParserOperation(values, ::setStringValue, "One of $values for ${field.name}")
106+
StringSetParserOperation(values, AssignableString(), "One of $values for $name")
101107
), emptyList()
102108
)
103109
}
@@ -108,20 +114,21 @@ internal abstract class NamedUnsignedIntFieldFormatDirective<in Target>(
108114
internal abstract class NamedEnumIntFieldFormatDirective<in Target, Type>(
109115
final override val field: FieldSpec<Target, Type>,
110116
private val mapping: Map<Type, String>,
117+
private val name: String,
111118
) : FieldFormatDirective<Target> {
112119

113120
private val reverseMapping = mapping.entries.associate { it.value to it.key }
114121

115-
private fun getStringValue(target: Target): String = mapping[field.getNotNull(target)]
122+
private fun getStringValue(target: Target): String = mapping[field.accessor.getterNotNull(target)]
116123
?: throw IllegalStateException(
117-
"The value ${field.getNotNull(target)} is does not have a corresponding string representation"
124+
"The value ${field.accessor.getterNotNull(target)} is does not have a corresponding string representation"
118125
)
119126

120-
private fun setStringValue(target: Target, value: String) {
121-
field.setWithoutReassigning(
122-
target, reverseMapping[value]
123-
?: throw IllegalStateException("The string value $value does not have a corresponding enum value")
124-
)
127+
private inner class AssignableString: AssignableField<Target, String> {
128+
override fun trySetWithoutReassigning(container: Target, newValue: String): String? =
129+
field.accessor.trySetWithoutReassigning(container, reverseMapping[newValue]!!)?.let { mapping[it] }
130+
131+
override val name: String get() = this@NamedEnumIntFieldFormatDirective.name
125132
}
126133

127134
override fun formatter(): FormatterStructure<Target> =
@@ -130,7 +137,7 @@ internal abstract class NamedEnumIntFieldFormatDirective<in Target, Type>(
130137
override fun parser(): ParserStructure<Target> =
131138
ParserStructure(
132139
listOf(
133-
StringSetParserOperation(mapping.values, ::setStringValue, "One of ${mapping.values} for ${field.name}")
140+
StringSetParserOperation(mapping.values, AssignableString(), "One of ${mapping.values} for $name")
134141
), emptyList()
135142
)
136143
}
@@ -145,11 +152,11 @@ internal abstract class StringFieldFormatDirective<in Target>(
145152
}
146153

147154
override fun formatter(): FormatterStructure<Target> =
148-
StringFormatterStructure(field::getNotNull)
155+
StringFormatterStructure(field.accessor::getterNotNull)
149156

150157
override fun parser(): ParserStructure<Target> =
151158
ParserStructure(
152-
listOf(StringSetParserOperation(acceptedStrings, field::setWithoutReassigning, field.name)),
159+
listOf(StringSetParserOperation(acceptedStrings, field.accessor, field.name)),
153160
emptyList()
154161
)
155162
}
@@ -169,7 +176,7 @@ internal abstract class SignedIntFieldFormatDirective<in Target>(
169176

170177
override fun formatter(): FormatterStructure<Target> {
171178
val formatter = SignedIntFormatterStructure(
172-
number = field::getNotNull,
179+
number = field.accessor::getterNotNull,
173180
zeroPadding = minDigits ?: 0,
174181
outputPlusOnExceededWidth = outputPlusOnExceededWidth,
175182
)
@@ -181,7 +188,7 @@ internal abstract class SignedIntFieldFormatDirective<in Target>(
181188
minDigits = minDigits,
182189
maxDigits = maxDigits,
183190
spacePadding = spacePadding,
184-
field::setWithoutReassigning,
191+
field.accessor,
185192
field.name,
186193
plusOnExceedsWidth = outputPlusOnExceededWidth,
187194
)
@@ -194,12 +201,12 @@ internal abstract class DecimalFractionFieldFormatDirective<in Target>(
194201
private val zerosToAdd: List<Int>,
195202
) : FieldFormatDirective<Target> {
196203
override fun formatter(): FormatterStructure<Target> =
197-
DecimalFractionFormatterStructure(field::getNotNull, minDigits, maxDigits, zerosToAdd)
204+
DecimalFractionFormatterStructure(field.accessor::getterNotNull, minDigits, maxDigits, zerosToAdd)
198205

199206
override fun parser(): ParserStructure<Target> = ParserStructure(
200207
listOf(
201208
NumberSpanParserOperation(
202-
listOf(FractionPartConsumer(minDigits, maxDigits, field::setWithoutReassigning, field.name))
209+
listOf(FractionPartConsumer(minDigits, maxDigits, field.accessor, field.name))
203210
)
204211
),
205212
emptyList()
@@ -214,16 +221,11 @@ internal abstract class ReducedIntFieldDirective<in Target>(
214221

215222
override fun formatter(): FormatterStructure<Target> =
216223
ReducedIntFormatterStructure(
217-
number = field::getNotNull,
224+
number = field.accessor::getterNotNull,
218225
digits = digits,
219226
base = base,
220227
)
221228

222229
override fun parser(): ParserStructure<Target> =
223-
ReducedIntParser(
224-
digits = digits,
225-
base = base,
226-
field::setWithoutReassigning,
227-
field.name,
228-
)
230+
ReducedIntParser(digits = digits, base = base, field.accessor, field.name)
229231
}

core/common/src/internal/format/FieldSpec.kt

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,45 @@
55

66
package kotlinx.datetime.internal.format
77

8+
import kotlinx.datetime.internal.format.parser.AssignableField
89
import kotlin.reflect.*
910

10-
private typealias Accessor<Object, Field> = KMutableProperty1<Object, Field?>
11+
internal interface Accessor<in Object, Field>: AssignableField<Object, Field> {
12+
fun getter(container: Object): Field?
13+
14+
/**
15+
* Function that returns the value of the field in the given object,
16+
* or throws [IllegalStateException] if the field is not set.
17+
*
18+
* This function is used to access fields during formatting.
19+
*/
20+
fun getterNotNull(container: Object): Field =
21+
getter(container) ?: throw IllegalStateException("Field $name is not set")
22+
}
23+
24+
internal class PropertyAccessor<Object, Field>(private val property: KMutableProperty1<Object, Field?>): Accessor<Object, Field> {
25+
override val name: String get() = property.name
26+
27+
override fun trySetWithoutReassigning(container: Object, newValue: Field): Field? {
28+
val oldValue = property.get(container)
29+
return when {
30+
oldValue === null -> {
31+
property.set(container, newValue)
32+
null
33+
}
34+
oldValue == newValue -> null
35+
else -> oldValue
36+
}
37+
}
38+
39+
override fun getter(container: Object): Field? = property.get(container)
40+
}
1141

1242
internal interface FieldSign<in Target> {
1343
/**
1444
* The field that is `true` if the value of the field is known to be negative, and `false` otherwise.
1545
*/
16-
val isNegative: Accessor<in Target, Boolean>
46+
val isNegative: Accessor<Target, Boolean>
1747
fun isZero(obj: Target): Boolean
1848
}
1949

@@ -30,7 +60,7 @@ internal interface FieldSpec<in Target, Type> {
3060
/**
3161
* The function with which the field can be accessed.
3262
*/
33-
val accessor: Accessor<in Target, Type>
63+
val accessor: Accessor<Target, Type>
3464

3565
/**
3666
* The default value of the field, or `null` if the field has none.
@@ -52,41 +82,12 @@ internal abstract class AbstractFieldSpec<in Target, Type>: FieldSpec<Target, Ty
5282
override fun toString(): String = "The field $name (default value is $defaultValue)"
5383
}
5484

55-
/**
56-
* Function that returns the value of the field in the given object,
57-
* or throws [IllegalStateException] if the field is not set.
58-
*
59-
* This function is used to access fields during formatting.
60-
*/
61-
internal fun <Object, Field> FieldSpec<Object, Field>.getNotNull(obj: Object): Field =
62-
accessor.get(obj) ?: throw IllegalStateException("Field $name is not set")
63-
64-
/**
65-
* If the field is not set, sets it to the given value.
66-
* If the field is set to the given value, does nothing.
67-
* If the field is set to a different value, throws [IllegalArgumentException].
68-
*
69-
* This function is used to ensure internal consistency during parsing.
70-
* There exist formats where the same data is repeated several times in the same object, for example,
71-
* "14:15 (02:15 PM)". In such cases, we want to ensure that the values are consistent.
72-
*/
73-
internal fun <Object, Field> FieldSpec<Object, Field>.setWithoutReassigning(obj: Object, value: Field) {
74-
val oldValue = accessor.get(obj)
75-
if (oldValue != null) {
76-
require(oldValue == value) {
77-
"Attempting to assign conflicting values '$oldValue' and '$value' to field '$name'"
78-
}
79-
} else {
80-
accessor.set(obj, value)
81-
}
82-
}
83-
8485
/**
8586
* A specification of a field that can contain values of any kind.
8687
* Used for fields additional information about which is not that important for parsing/formatting.
8788
*/
8889
internal class GenericFieldSpec<in Target, Type>(
89-
override val accessor: Accessor<in Target, Type>,
90+
override val accessor: Accessor<Target, Type>,
9091
override val name: String = accessor.name,
9192
override val defaultValue: Type? = null,
9293
override val sign: FieldSign<Target>? = null,
@@ -96,7 +97,7 @@ internal class GenericFieldSpec<in Target, Type>(
9697
* A specification of a field that can only contain non-negative values.
9798
*/
9899
internal class UnsignedFieldSpec<in Target>(
99-
override val accessor: Accessor<in Target, Int>,
100+
override val accessor: Accessor<Target, Int>,
100101
/**
101102
* The minimum value of the field.
102103
*/
@@ -121,7 +122,7 @@ internal class UnsignedFieldSpec<in Target>(
121122
}
122123

123124
internal class SignedFieldSpec<in Target>(
124-
override val accessor: Accessor<in Target, Int>,
125+
override val accessor: Accessor<Target, Int>,
125126
val maxAbsoluteValue: Int?,
126127
override val name: String = accessor.name,
127128
override val defaultValue: Int? = null,

0 commit comments

Comments
 (0)