From 5de7e01b71dcb75b524aa3a02fbf4e51348dff62 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 14 Apr 2023 15:50:51 +0200 Subject: [PATCH] Redo the model of numeric signs to support multi-field values --- core/common/src/format/UtcOffsetFormat.kt | 64 ++++++++++++++----- core/common/src/format/ValueBagFormat.kt | 19 ++++-- .../internal/format/FieldFormatDirective.kt | 33 ++-------- core/common/src/internal/format/FieldSpec.kt | 16 +++++ core/common/src/internal/format/Format.kt | 44 ++++++------- .../src/internal/format/FormatStrings.kt | 4 +- .../internal/format/parser/ParserOperation.kt | 59 +++++++++++------ 7 files changed, 144 insertions(+), 95 deletions(-) diff --git a/core/common/src/format/UtcOffsetFormat.kt b/core/common/src/format/UtcOffsetFormat.kt index 278c781f..49c1b626 100644 --- a/core/common/src/format/UtcOffsetFormat.kt +++ b/core/common/src/format/UtcOffsetFormat.kt @@ -9,9 +9,12 @@ import kotlinx.datetime.* import kotlinx.datetime.internal.* import kotlinx.datetime.internal.format.* import kotlinx.datetime.internal.format.parser.* +import kotlin.math.* +import kotlin.reflect.* internal interface UtcOffsetFieldContainer { - var totalHours: Int? + var isNegative: Boolean? + var totalHoursAbs: Int? var minutesOfHour: Int? var secondsOfMinute: Int? } @@ -87,51 +90,78 @@ public fun UtcOffset.Companion.parse(input: String, formatString: String): UtcOf public fun UtcOffset.Companion.parse(input: String, format: UtcOffsetFormat): UtcOffset = format.parse(input) internal fun UtcOffset.toIncompleteUtcOffset(): IncompleteUtcOffset = - IncompleteUtcOffset(totalSeconds / 3600, (totalSeconds / 60) % 60, totalSeconds % 60) + IncompleteUtcOffset( + totalSeconds < 0, + totalSeconds.absoluteValue / 3600, + (totalSeconds.absoluteValue / 60) % 60, + totalSeconds.absoluteValue % 60 + ) internal object OffsetFields { - val totalHours = SignedFieldSpec( - UtcOffsetFieldContainer::totalHours, + private val sign = object: FieldSign { + override val isNegative = UtcOffsetFieldContainer::isNegative + override fun isZero(obj: UtcOffsetFieldContainer): Boolean = + (obj.totalHoursAbs ?: 0) == 0 && (obj.minutesOfHour ?: 0) == 0 && (obj.secondsOfMinute ?: 0) == 0 + } + val totalHoursAbs = UnsignedFieldSpec( + UtcOffsetFieldContainer::totalHoursAbs, defaultValue = 0, - maxAbsoluteValue = 18, + minValue = 0, + maxValue = 18, + sign = sign, ) - val minutesOfHour = SignedFieldSpec( + val minutesOfHour = UnsignedFieldSpec( UtcOffsetFieldContainer::minutesOfHour, defaultValue = 0, - maxAbsoluteValue = 59, + minValue = 0, + maxValue = 59, + sign = sign, ) - val secondsOfMinute = SignedFieldSpec( + val secondsOfMinute = UnsignedFieldSpec( UtcOffsetFieldContainer::secondsOfMinute, defaultValue = 0, - maxAbsoluteValue = 59, + minValue = 0, + maxValue = 59, + sign = sign, ) } internal class IncompleteUtcOffset( - override var totalHours: Int? = null, + override var isNegative: Boolean? = null, + override var totalHoursAbs: Int? = null, override var minutesOfHour: Int? = null, override var secondsOfMinute: Int? = null, ) : UtcOffsetFieldContainer, Copyable { - fun toUtcOffset(): UtcOffset = UtcOffset(totalHours, minutesOfHour, secondsOfMinute) + + fun toUtcOffset(): UtcOffset { + val sign = if (isNegative == true) -1 else 1 + return UtcOffset( + totalHoursAbs?.let { it * sign }, minutesOfHour?.let { it * sign }, secondsOfMinute?.let { it * sign } + ) + } override fun equals(other: Any?): Boolean = - other is IncompleteUtcOffset && totalHours == other.totalHours && + other is IncompleteUtcOffset && isNegative == other.isNegative && totalHoursAbs == other.totalHoursAbs && minutesOfHour == other.minutesOfHour && secondsOfMinute == other.secondsOfMinute override fun hashCode(): Int = - totalHours.hashCode() * 31 + minutesOfHour.hashCode() * 31 + secondsOfMinute.hashCode() + isNegative.hashCode() + totalHoursAbs.hashCode() + minutesOfHour.hashCode() + secondsOfMinute.hashCode() + + override fun copy(): IncompleteUtcOffset = + IncompleteUtcOffset(isNegative, totalHoursAbs, minutesOfHour, secondsOfMinute) - override fun copy(): IncompleteUtcOffset = IncompleteUtcOffset(totalHours, minutesOfHour, secondsOfMinute) + override fun toString(): String = + "${isNegative?.let { if (it) "-" else "+" } ?: " " }${totalHoursAbs ?: "??"}:${minutesOfHour ?: "??"}:${secondsOfMinute ?: "??"}" } internal class UtcOffsetWholeHoursDirective(minDigits: Int) : - SignedIntFieldFormatDirective(OffsetFields.totalHours, minDigits) + UnsignedIntFieldFormatDirective(OffsetFields.totalHoursAbs, minDigits) internal class UtcOffsetMinuteOfHourDirective(minDigits: Int) : - SignedIntFieldFormatDirective(OffsetFields.minutesOfHour, minDigits) + UnsignedIntFieldFormatDirective(OffsetFields.minutesOfHour, minDigits) internal class UtcOffsetSecondOfMinuteDirective(minDigits: Int) : - SignedIntFieldFormatDirective(OffsetFields.secondsOfMinute, minDigits) + UnsignedIntFieldFormatDirective(OffsetFields.secondsOfMinute, minDigits) internal object UtcOffsetFormatBuilderSpec: BuilderSpec( mapOf( diff --git a/core/common/src/format/ValueBagFormat.kt b/core/common/src/format/ValueBagFormat.kt index 894c4dce..93a0a33b 100644 --- a/core/common/src/format/ValueBagFormat.kt +++ b/core/common/src/format/ValueBagFormat.kt @@ -11,6 +11,7 @@ import kotlinx.datetime.internal.* import kotlinx.datetime.internal.format.* import kotlinx.datetime.internal.format.parser.* import kotlinx.datetime.internal.safeMultiply +import kotlin.math.* /** * A collection of date-time fields. @@ -69,9 +70,11 @@ public class ValueBag internal constructor(internal val contents: ValueBagConten * If any of the fields are already set, they will be overwritten. */ public fun populateFrom(utcOffset: UtcOffset) { - offsetTotalHours = utcOffset.totalSeconds / 3600 - offsetMinutesOfHour = (utcOffset.totalSeconds % 3600) / 60 - offsetSecondsOfMinute = utcOffset.totalSeconds % 60 + offsetIsNegative = utcOffset.totalSeconds < 0 + val seconds = utcOffset.totalSeconds.absoluteValue + offsetTotalHours = seconds / 3600 + offsetMinutesOfHour = (seconds % 3600) / 60 + offsetSecondsOfMinute = seconds % 60 } /** @@ -123,11 +126,13 @@ public class ValueBag internal constructor(internal val contents: ValueBagConten /** Returns the nanosecond-of-second time component of this date/time value. */ public var nanosecond: Int? by contents.time::nanosecond - /** The total amount of full hours in the UTC offset. */ - public var offsetTotalHours: Int? by contents.offset::totalHours - /** The amount of minutes that don't add to a whole hour in the UTC offset. */ + /** True if the offset is negative. */ + public var offsetIsNegative: Boolean? by contents.offset::isNegative + /** The total amount of full hours in the UTC offset, in the range [0; 18]. */ + public var offsetTotalHours: Int? by contents.offset::totalHoursAbs + /** The amount of minutes that don't add to a whole hour in the UTC offset, in the range [0; 59]. */ public var offsetMinutesOfHour: Int? by contents.offset::minutesOfHour - /** The amount of seconds that don't add to a whole minute in the UTC offset. */ + /** The amount of seconds that don't add to a whole minute in the UTC offset, in the range [0; 59]. */ public var offsetSecondsOfMinute: Int? by contents.offset::secondsOfMinute public var timeZoneId: String? by contents::timeZoneId diff --git a/core/common/src/internal/format/FieldFormatDirective.kt b/core/common/src/internal/format/FieldFormatDirective.kt index 4b56e73f..221d7177 100644 --- a/core/common/src/internal/format/FieldFormatDirective.kt +++ b/core/common/src/internal/format/FieldFormatDirective.kt @@ -19,11 +19,6 @@ internal interface FieldFormatDirective { */ val field: FieldSpec - /** - * For numeric signed values, the way to check if the field is negative. For everything else, `null`. - */ - val signGetter: ((Target) -> Int)? - /** * The formatter operation that formats the field. */ @@ -32,7 +27,7 @@ internal interface FieldFormatDirective { /** * The parser structure that parses the field. */ - fun parser(signsInverted: Boolean): ParserStructure + fun parser(): ParserStructure } /** @@ -45,8 +40,6 @@ internal abstract class UnsignedIntFieldFormatDirective( private val minDigits: Int, ) : FieldFormatDirective { - final override val signGetter: ((Target) -> Int)? = null - private val maxDigits: Int = field.maxDigits init { @@ -62,7 +55,7 @@ internal abstract class UnsignedIntFieldFormatDirective( zeroPadding = minDigits, ) - override fun parser(signsInverted: Boolean): ParserStructure = + override fun parser(): ParserStructure = ParserStructure( listOf( NumberSpanParserOperation( @@ -94,8 +87,6 @@ internal abstract class NamedUnsignedIntFieldFormatDirective( } } - final override val signGetter: ((Target) -> Int)? = null - private fun getStringValue(target: Target): String = values[field.getNotNull(target) - field.minValue] private fun setStringValue(target: Target, value: String) { @@ -105,7 +96,7 @@ internal abstract class NamedUnsignedIntFieldFormatDirective( override fun formatter(): FormatterOperation = StringFormatterOperation(::getStringValue) - override fun parser(signsInverted: Boolean): ParserStructure = + override fun parser(): ParserStructure = ParserStructure(listOf( StringSetParserOperation(values, ::setStringValue, "One of $values for ${field.name}") ), emptyList()) @@ -119,8 +110,6 @@ internal abstract class NamedEnumIntFieldFormatDirective( private val mapping: Map, ) : FieldFormatDirective { - final override val signGetter: ((Target) -> Int)? = null - private val reverseMapping = mapping.entries.associate { it.value to it.key } private fun getStringValue(target: Target): String = mapping[field.getNotNull(target)] @@ -136,7 +125,7 @@ internal abstract class NamedEnumIntFieldFormatDirective( override fun formatter(): FormatterOperation = StringFormatterOperation(::getStringValue) - override fun parser(signsInverted: Boolean): ParserStructure = + override fun parser(): ParserStructure = ParserStructure(listOf( StringSetParserOperation(mapping.values, ::setStringValue, "One of ${mapping.values} for ${field.name}") ), emptyList()) @@ -147,8 +136,6 @@ internal abstract class StringFieldFormatDirective( private val acceptedStrings: Set, ) : FieldFormatDirective { - final override val signGetter: ((Target) -> Int)? = null - init { require(acceptedStrings.isNotEmpty()) } @@ -156,7 +143,7 @@ internal abstract class StringFieldFormatDirective( override fun formatter(): FormatterOperation = StringFormatterOperation(field::getNotNull) - override fun parser(signsInverted: Boolean): ParserStructure = + override fun parser(): ParserStructure = ParserStructure( listOf(StringSetParserOperation(acceptedStrings, field::setWithoutReassigning, field.name)), emptyList() @@ -170,9 +157,6 @@ internal abstract class SignedIntFieldFormatDirective( private val outputPlusOnExceededPadding: Boolean = false, ) : FieldFormatDirective { - final override val signGetter: ((Target) -> Int) = ::signGetterImpl - private fun signGetterImpl(target: Target): Int = (field.accessor.get(target) ?: 0).sign - init { require(minDigits == null || minDigits >= 0) require(maxDigits == null || minDigits == null || maxDigits >= minDigits) @@ -185,14 +169,13 @@ internal abstract class SignedIntFieldFormatDirective( outputPlusOnExceedsPad = outputPlusOnExceededPadding, ) - override fun parser(signsInverted: Boolean): ParserStructure = + override fun parser(): ParserStructure = SignedIntParser( minDigits = minDigits, maxDigits = maxDigits, field::setWithoutReassigning, field.name, plusOnExceedsPad = outputPlusOnExceededPadding, - signsInverted = signsInverted ) } @@ -201,12 +184,10 @@ internal abstract class DecimalFractionFieldFormatDirective( private val minDigits: Int?, private val maxDigits: Int?, ) : FieldFormatDirective { - override val signGetter: ((Target) -> Int)? = null - override fun formatter(): FormatterOperation = DecimalFractionFormatterOperation(field::getNotNull, minDigits, maxDigits) - override fun parser(signsInverted: Boolean): ParserStructure = ParserStructure( + override fun parser(): ParserStructure = ParserStructure( listOf( NumberSpanParserOperation( listOf(FractionPartConsumer(minDigits, maxDigits, field::setWithoutReassigning, field.name)) diff --git a/core/common/src/internal/format/FieldSpec.kt b/core/common/src/internal/format/FieldSpec.kt index 31d5fdaa..d5505c18 100644 --- a/core/common/src/internal/format/FieldSpec.kt +++ b/core/common/src/internal/format/FieldSpec.kt @@ -9,6 +9,14 @@ import kotlin.reflect.* private typealias Accessor = KMutableProperty1 +internal interface FieldSign { + /** + * The field that is `true` if the value of the field is known to be negative, and `false` otherwise. + */ + val isNegative: Accessor + fun isZero(obj: Target): Boolean +} + /** * A specification of a field. * @@ -33,6 +41,11 @@ internal interface FieldSpec { * The name of the field. */ val name: String + + /** + * The sign corresponding to the field value, or `null` if the field has none. + */ + val sign: FieldSign? } internal abstract class AbstractFieldSpec: FieldSpec { @@ -76,6 +89,7 @@ internal class GenericFieldSpec( override val accessor: Accessor, override val name: String = accessor.name, override val defaultValue: Type? = null, + override val sign: FieldSign? = null, ) : AbstractFieldSpec() /** @@ -93,6 +107,7 @@ internal class UnsignedFieldSpec( val maxValue: Int, override val name: String = accessor.name, override val defaultValue: Int? = null, + override val sign: FieldSign? = null, ) : AbstractFieldSpec() { /** * The maximum length of the field when represented as a decimal number. @@ -110,6 +125,7 @@ internal class SignedFieldSpec( val maxAbsoluteValue: Int?, override val name: String = accessor.name, override val defaultValue: Int? = null, + override val sign: FieldSign? = null, ) : AbstractFieldSpec() { val maxDigits: Int? = when { maxAbsoluteValue == null -> null diff --git a/core/common/src/internal/format/Format.kt b/core/common/src/internal/format/Format.kt index ff0573d6..152cb75f 100644 --- a/core/common/src/internal/format/Format.kt +++ b/core/common/src/internal/format/Format.kt @@ -25,13 +25,13 @@ internal class ConstantFormatStructure( // TODO: should itself also be a field with the default value "not negative" internal class SignedFormatStructure( val format: FormatStructure, - val plusSignRequired: Boolean, + val withPlusSign: Boolean, ) : NonConcatenatedFormatStructure { - internal val fields = basicFormats(format).mapNotNull(FieldFormatDirective::signGetter).toSet() + internal val fields = basicFormats(format).mapNotNull { it.field.sign }.toSet() init { - require(fields.isNotEmpty()) { "Signed format must contain at least one field with a negative sign" } + require(fields.isNotEmpty()) { "Signed format must contain at least one field with a sign" } } override fun toString(): String = "SignedFormatStructure($format)" @@ -60,16 +60,18 @@ internal fun FormatStructure.formatter(): FormatterStructure { fun checkIfAllNegative(value: T): Boolean { var seenNonZero = false for (check in fields) { - val sign = check(value) - if (sign > 0) return false - if (sign < 0) seenNonZero = true + when { + check.isNegative.get(value) == true -> seenNonZero = true + check.isZero(value) -> continue + else -> return false + } } return seenNonZero } SignedFormatter( innerFormat, ::checkIfAllNegative, - plusSignRequired + withPlusSign ) to fieldSpecs } @@ -123,24 +125,20 @@ private fun FormatStructure.parser(signsInverted: Boolean = false): Parse is ConstantFormatStructure -> ParserStructure(operations = listOf(PlainStringParserOperation(string)), followedBy = emptyList()) is SignedFormatStructure -> { - ParserStructure( - operations = emptyList(), - followedBy = listOf( - format.parser(signsInverted = signsInverted).let { - if (!plusSignRequired) it else - listOf( - ConstantFormatStructure("+").parser(signsInverted), - it - ).concat() + listOf(ParserStructure( + operations = listOf(SignParser( + isNegativeSetter = { value -> + for (field in fields) { + field.isNegative.set(value, true) + } }, - listOf( - ConstantFormatStructure("-").parser(signsInverted), - format.parser(signsInverted = !signsInverted), - ).concat() - ) - ) + withPlusSign = withPlusSign, + whatThisExpects = "sign for ${this.fields}" + )), + emptyList() + ), format.parser()).concat() } - is BasicFormatStructure -> directive.parser(signsInverted) + is BasicFormatStructure -> directive.parser() is AlternativesFormatStructure -> ParserStructure(operations = emptyList(), followedBy = formats.map { it.parser(signsInverted) }) is ConcatenatedFormatStructure -> formats.map { it.parser(signsInverted) }.concat() diff --git a/core/common/src/internal/format/FormatStrings.kt b/core/common/src/internal/format/FormatStrings.kt index 6385bd5b..c6e88d3d 100644 --- a/core/common/src/internal/format/FormatStrings.kt +++ b/core/common/src/internal/format/FormatStrings.kt @@ -42,8 +42,8 @@ internal fun AppendableFormatStructure.appendFormatString(format: String, currentBuilder.add( when (sign) { null -> format - '+' -> SignedFormatStructure(format, plusSignRequired = true) - '-' -> SignedFormatStructure(format, plusSignRequired = false) + '+' -> SignedFormatStructure(format, withPlusSign = true) + '-' -> SignedFormatStructure(format, withPlusSign = false) else -> throw IllegalArgumentException("Unexpected sign $sign") } ) diff --git a/core/common/src/internal/format/parser/ParserOperation.kt b/core/common/src/internal/format/parser/ParserOperation.kt index c7b24512..b1aa7bee 100644 --- a/core/common/src/internal/format/parser/ParserOperation.kt +++ b/core/common/src/internal/format/parser/ParserOperation.kt @@ -102,6 +102,28 @@ internal class NumberSpanParserOperation( override fun toString(): String = whatThisExpects } +internal class SignParser( + private val isNegativeSetter: (Output) -> Unit, + private val withPlusSign: Boolean, + private val whatThisExpects: String, +) : ParserOperation { + override fun Output.consume(input: CharSequence, startIndex: Int): ParseResult { + if (startIndex >= input.length) + return ParseResult.Ok(startIndex) + val char = input[startIndex] + if (char == '-') { + isNegativeSetter(this) + return ParseResult.Ok(startIndex + 1) + } + if (char == '+' && withPlusSign) { + return ParseResult.Ok(startIndex + 1) + } + return ParseResult.Error("Expected $whatThisExpects but got $char", startIndex) + } + + override fun toString(): String = whatThisExpects +} + /** * Matches the longest suitable string from `strings` and calls [consume] with the matched string. */ @@ -182,28 +204,25 @@ internal fun SignedIntParser( setter: (Output, Int) -> Unit, name: String, plusOnExceedsPad: Boolean = false, - signsInverted: Boolean = false, ): ParserStructure { val parsers = mutableListOf>>( ) - if (!signsInverted) { - parsers.add( - listOf( - PlainStringParserOperation("-"), - NumberSpanParserOperation( - listOf( - UnsignedIntConsumer( - minDigits, - maxDigits, - setter, - name, - multiplyByMinus1 = !signsInverted - ) + parsers.add( + listOf( + PlainStringParserOperation("-"), + NumberSpanParserOperation( + listOf( + UnsignedIntConsumer( + minDigits, + maxDigits, + setter, + name, + multiplyByMinus1 = true ) ) - ), - ) - } + ) + ), + ) if (plusOnExceedsPad) { parsers.add( listOf( @@ -214,7 +233,7 @@ internal fun SignedIntParser( minDigits, setter, name, - multiplyByMinus1 = signsInverted + multiplyByMinus1 = false ) ) ) @@ -230,7 +249,7 @@ internal fun SignedIntParser( maxDigits, setter, name, - multiplyByMinus1 = signsInverted + multiplyByMinus1 = false ) ) ) @@ -246,7 +265,7 @@ internal fun SignedIntParser( maxDigits, setter, name, - multiplyByMinus1 = signsInverted + multiplyByMinus1 = false ) ) )