Redo the model of numeric signs to support multi-field values

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<UtcOffsetFieldContainer> {
+        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<IncompleteUtcOffset> {
-    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<UtcOffsetFieldContainer>(OffsetFields.totalHours, minDigits)
+    UnsignedIntFieldFormatDirective<UtcOffsetFieldContainer>(OffsetFields.totalHoursAbs, minDigits)
 
 internal class UtcOffsetMinuteOfHourDirective(minDigits: Int) :
-    SignedIntFieldFormatDirective<UtcOffsetFieldContainer>(OffsetFields.minutesOfHour, minDigits)
+    UnsignedIntFieldFormatDirective<UtcOffsetFieldContainer>(OffsetFields.minutesOfHour, minDigits)
 
 internal class UtcOffsetSecondOfMinuteDirective(minDigits: Int) :
-    SignedIntFieldFormatDirective<UtcOffsetFieldContainer>(OffsetFields.secondsOfMinute, minDigits)
+    UnsignedIntFieldFormatDirective<UtcOffsetFieldContainer>(OffsetFields.secondsOfMinute, minDigits)
 
 internal object UtcOffsetFormatBuilderSpec: BuilderSpec<UtcOffsetFieldContainer>(
     mapOf(

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

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

@@ -19,11 +19,6 @@ internal interface FieldFormatDirective<in Target> {
      */
     val field: FieldSpec<Target, *>
 
-    /**
-     * 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<in Target> {
     /**
      * The parser structure that parses the field.
      */
-    fun parser(signsInverted: Boolean): ParserStructure<Target>
+    fun parser(): ParserStructure<Target>
 }
 
 /**
@@ -45,8 +40,6 @@ internal abstract class UnsignedIntFieldFormatDirective<in Target>(
     private val minDigits: Int,
 ) : FieldFormatDirective<Target> {
 
-    final override val signGetter: ((Target) -> Int)? = null
-
     private val maxDigits: Int = field.maxDigits
 
     init {
@@ -62,7 +55,7 @@ internal abstract class UnsignedIntFieldFormatDirective<in Target>(
             zeroPadding = minDigits,
         )
 
-    override fun parser(signsInverted: Boolean): ParserStructure<Target> =
+    override fun parser(): ParserStructure<Target> =
         ParserStructure(
             listOf(
                 NumberSpanParserOperation(
@@ -94,8 +87,6 @@ internal abstract class NamedUnsignedIntFieldFormatDirective<in Target>(
         }
     }
 
-    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<in Target>(
     override fun formatter(): FormatterOperation<Target> =
         StringFormatterOperation(::getStringValue)
 
-    override fun parser(signsInverted: Boolean): ParserStructure<Target> =
+    override fun parser(): ParserStructure<Target> =
         ParserStructure(listOf(
             StringSetParserOperation(values, ::setStringValue, "One of $values for ${field.name}")
         ), emptyList())
@@ -119,8 +110,6 @@ internal abstract class NamedEnumIntFieldFormatDirective<in Target, Type>(
     private val mapping: Map<Type, String>,
 ) : FieldFormatDirective<Target> {
 
-    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<in Target, Type>(
     override fun formatter(): FormatterOperation<Target> =
         StringFormatterOperation(::getStringValue)
 
-    override fun parser(signsInverted: Boolean): ParserStructure<Target> =
+    override fun parser(): ParserStructure<Target> =
         ParserStructure(listOf(
             StringSetParserOperation(mapping.values, ::setStringValue, "One of ${mapping.values} for ${field.name}")
         ), emptyList())
@@ -147,16 +136,14 @@ internal abstract class StringFieldFormatDirective<in Target>(
     private val acceptedStrings: Set<String>,
 ) : FieldFormatDirective<Target> {
 
-    final override val signGetter: ((Target) -> Int)? = null
-
     init {
         require(acceptedStrings.isNotEmpty())
     }
 
     override fun formatter(): FormatterOperation<Target> =
         StringFormatterOperation(field::getNotNull)
 
-    override fun parser(signsInverted: Boolean): ParserStructure<Target> =
+    override fun parser(): ParserStructure<Target> =
         ParserStructure(
             listOf(StringSetParserOperation(acceptedStrings, field::setWithoutReassigning, field.name)),
             emptyList()
@@ -170,9 +157,6 @@ internal abstract class SignedIntFieldFormatDirective<in Target>(
     private val outputPlusOnExceededPadding: Boolean = false,
 ) : FieldFormatDirective<Target> {
 
-    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<in Target>(
             outputPlusOnExceedsPad = outputPlusOnExceededPadding,
         )
 
-    override fun parser(signsInverted: Boolean): ParserStructure<Target> =
+    override fun parser(): ParserStructure<Target> =
         SignedIntParser(
             minDigits = minDigits,
             maxDigits = maxDigits,
             field::setWithoutReassigning,
             field.name,
             plusOnExceedsPad = outputPlusOnExceededPadding,
-            signsInverted = signsInverted
         )
 }
 
@@ -201,12 +184,10 @@ internal abstract class DecimalFractionFieldFormatDirective<in Target>(
     private val minDigits: Int?,
     private val maxDigits: Int?,
 ) : FieldFormatDirective<Target> {
-    override val signGetter: ((Target) -> Int)? = null
-
     override fun formatter(): FormatterOperation<Target> =
         DecimalFractionFormatterOperation(field::getNotNull, minDigits, maxDigits)
 
-    override fun parser(signsInverted: Boolean): ParserStructure<Target> = ParserStructure(
+    override fun parser(): ParserStructure<Target> = ParserStructure(
         listOf(
             NumberSpanParserOperation(
                 listOf(FractionPartConsumer(minDigits, maxDigits, field::setWithoutReassigning, field.name))

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

@@ -9,6 +9,14 @@ import kotlin.reflect.*
 
 private typealias Accessor<Object, Field> = KMutableProperty1<Object, Field?>
 
+internal interface FieldSign<in Target> {
+    /**
+     * The field that is `true` if the value of the field is known to be negative, and `false` otherwise.
+     */
+    val isNegative: Accessor<in Target, Boolean>
+    fun isZero(obj: Target): Boolean
+}
+
 /**
  * A specification of a field.
  *
@@ -33,6 +41,11 @@ internal interface FieldSpec<in Target, Type> {
      * 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<Target>?
 }
 
 internal abstract class AbstractFieldSpec<in Target, Type>: FieldSpec<Target, Type> {
@@ -76,6 +89,7 @@ internal class GenericFieldSpec<in Target, Type>(
     override val accessor: Accessor<in Target, Type>,
     override val name: String = accessor.name,
     override val defaultValue: Type? = null,
+    override val sign: FieldSign<Target>? = null,
 ) : AbstractFieldSpec<Target, Type>()
 
 /**
@@ -93,6 +107,7 @@ internal class UnsignedFieldSpec<in Target>(
     val maxValue: Int,
     override val name: String = accessor.name,
     override val defaultValue: Int? = null,
+    override val sign: FieldSign<Target>? = null,
 ) : AbstractFieldSpec<Target, Int>() {
     /**
      * The maximum length of the field when represented as a decimal number.
@@ -110,6 +125,7 @@ internal class SignedFieldSpec<in Target>(
     val maxAbsoluteValue: Int?,
     override val name: String = accessor.name,
     override val defaultValue: Int? = null,
+    override val sign: FieldSign<Target>? = null,
 ) : AbstractFieldSpec<Target, Int>() {
     val maxDigits: Int? = when {
         maxAbsoluteValue == null -> null