Make UtcOffsets with the same offset value indistinguishable

On the contrary, two FixedOffsetTimeZones with the same offset can differ by id
(this aspect is native-only for now).

Author: ilya-g

core/common/test/TimeZoneTest.kt

@@ -99,6 +99,16 @@ class TimeZoneTest {
         }
     }
 
+    @Test
+    fun utcOffsetNormalization() {
+        val sameOffsetTZs = listOf("+04", "+04:00", "UTC+4", "UT+04", "GMT+04:00:00").map { TimeZone.of(it) }
+        val instant = Instant.fromEpochSeconds(0)
+        val offsets = sameOffsetTZs.map { it.offsetAt(instant) }
+
+        assertTrue(offsets.distinct().size == 1, "Expected all offsets to be equal: $offsets")
+        assertTrue(offsets.map { it.toString() }.distinct().size == 1, "Expected all offsets to have the same string representation: $offsets")
+    }
+
     // from 310bp
     @Test
     fun newYorkOffset() {

core/native/src/TimeZone.kt

@@ -33,19 +33,23 @@ public actual open class TimeZone internal constructor(internal val value: TimeZ
                 return UtcOffset.parse(zoneId).asTimeZone()
             }
             if (zoneId == "UTC" || zoneId == "GMT" || zoneId == "UT") {
-                // TODO: Should we allow non-normalized zone ids in UtcOffset id?
-                return UtcOffset(0, zoneId).asTimeZone()
+                return FixedOffsetTimeZone(UtcOffset(0), zoneId)
             }
             if (zoneId.startsWith("UTC+") || zoneId.startsWith("GMT+") ||
                 zoneId.startsWith("UTC-") || zoneId.startsWith("GMT-")) {
+                val prefix = zoneId.take(3)
                 val offset = UtcOffset.parse(zoneId.substring(3))
-                return (if (offset.totalSeconds == 0) UtcOffset(0, zoneId.substring(0, 3))
-                else UtcOffset(offset.totalSeconds, zoneId.substring(0, 3) + offset.id)).asTimeZone()
+                return when (offset.totalSeconds) {
+                    0 -> FixedOffsetTimeZone(offset, prefix)
+                    else -> FixedOffsetTimeZone(offset, "$prefix$offset")
+                }
             }
             if (zoneId.startsWith("UT+") || zoneId.startsWith("UT-")) {
                 val offset = UtcOffset.parse(zoneId.substring(2))
-                return (if (offset.totalSeconds == 0) UtcOffset(0, "UT")
-                else UtcOffset(offset.totalSeconds, "UT" + offset.id)).asTimeZone()
+                return when (offset.totalSeconds) {
+                    0 -> FixedOffsetTimeZone(offset, "UT")
+                    else -> FixedOffsetTimeZone(offset, "UT$offset")
+                }
             }
             return TimeZone(PlatformTimeZoneImpl.of(zoneId))
         }
@@ -80,7 +84,9 @@ public actual open class TimeZone internal constructor(internal val value: TimeZ
 
 
 @Serializable(with = FixedOffsetTimeZoneSerializer::class)
-public actual class FixedOffsetTimeZone actual constructor(public actual val utcOffset: UtcOffset) : TimeZone(ZoneOffsetImpl(utcOffset, utcOffset.id)) {
+public actual class FixedOffsetTimeZone internal constructor(public actual val utcOffset: UtcOffset, id: String) : TimeZone(ZoneOffsetImpl(utcOffset, id)) {
+
+    public actual constructor(utcOffset: UtcOffset) : this(utcOffset, utcOffset.toString())
 
     @Deprecated("Use utcOffset.totalSeconds", ReplaceWith("utcOffset.totalSeconds"))
     public actual val totalSeconds: Int get() = utcOffset.totalSeconds

core/native/src/TimeZoneImpl.kt

@@ -38,5 +38,5 @@ internal class ZoneOffsetImpl(val utcOffset: UtcOffset, override val id: String)
 
     // org.threeten.bp.ZoneOffset#equals
     override fun equals(other: Any?): Boolean =
-        this === other || other is ZoneOffsetImpl && utcOffset == other.utcOffset
+        this === other || other is ZoneOffsetImpl && id == other.id
 }

core/native/src/UtcOffset.kt

@@ -8,15 +8,16 @@ package kotlinx.datetime
 import kotlin.math.abs
 import kotlin.native.concurrent.ThreadLocal
 
-public actual class UtcOffset internal constructor(public actual val totalSeconds: Int, internal val id: String) {
+public actual class UtcOffset internal constructor(public actual val totalSeconds: Int) {
+    private val id: String = zoneIdByOffset(totalSeconds)
 
     override fun hashCode(): Int = totalSeconds
     override fun equals(other: Any?): Boolean = other is UtcOffset && this.totalSeconds == other.totalSeconds
     override fun toString(): String = id
 
     public actual companion object {
 
-        internal val ZERO: UtcOffset = UtcOffset(0, "Z")
+        internal val ZERO: UtcOffset = UtcOffset(0)
 
         public actual fun parse(offsetString: String): UtcOffset {
             if (offsetString == "Z") {
@@ -59,8 +60,7 @@ public actual class UtcOffset internal constructor(public actual val totalSecond
             val first: Char = offsetString[0]
             if (first != '+' && first != '-') {
                 throw IllegalTimeZoneException(
-                    "Invalid ID for UtcOffset, plus/minus not found when expected: $offsetString"
-                )
+                    "Invalid ID for UtcOffset, plus/minus not found when expected: $offsetString")
             }
             return if (first == '-') {
                 ofHoursMinutesSeconds(-hours, -minutes, -seconds)
@@ -72,10 +72,8 @@ public actual class UtcOffset internal constructor(public actual val totalSecond
         // org.threeten.bp.ZoneOffset#validate
         private fun validate(hours: Int, minutes: Int, seconds: Int) {
             if (hours < -18 || hours > 18) {
-                throw IllegalTimeZoneException(
-                    "Zone offset hours not in valid range: value " + hours +
-                            " is not in the range -18 to 18"
-                )
+                throw IllegalTimeZoneException("Zone offset hours not in valid range: value " + hours +
+                    " is not in the range -18 to 18")
             }
             if (hours > 0) {
                 if (minutes < 0 || seconds < 0) {
@@ -89,16 +87,12 @@ public actual class UtcOffset internal constructor(public actual val totalSecond
                 throw IllegalTimeZoneException("Zone offset minutes and seconds must have the same sign")
             }
             if (abs(minutes) > 59) {
-                throw IllegalTimeZoneException(
-                    "Zone offset minutes not in valid range: abs(value) " +
-                            abs(minutes) + " is not in the range 0 to 59"
-                )
+                throw IllegalTimeZoneException("Zone offset minutes not in valid range: abs(value) " +
+                    abs(minutes) + " is not in the range 0 to 59")
             }
             if (abs(seconds) > 59) {
-                throw IllegalTimeZoneException(
-                    "Zone offset seconds not in valid range: abs(value) " +
-                            abs(seconds) + " is not in the range 0 to 59"
-                )
+                throw IllegalTimeZoneException("Zone offset seconds not in valid range: abs(value) " +
+                    abs(seconds) + " is not in the range 0 to 59")
             }
             if (abs(hours) == 18 && (abs(minutes) > 0 || abs(seconds) > 0)) {
                 throw IllegalTimeZoneException("Zone offset not in valid range: -18:00 to +18:00")
@@ -115,9 +109,9 @@ public actual class UtcOffset internal constructor(public actual val totalSecond
         // org.threeten.bp.ZoneOffset#ofTotalSeconds
         internal fun ofSeconds(seconds: Int): UtcOffset =
             if (seconds % (15 * SECONDS_PER_MINUTE) == 0) {
-                utcOffsetCache[seconds] ?: UtcOffset(seconds, zoneIdByOffset(seconds)).also { utcOffsetCache[seconds] = it }
+                utcOffsetCache[seconds] ?: UtcOffset(seconds).also { utcOffsetCache[seconds] = it }
             } else {
-                UtcOffset(seconds, zoneIdByOffset(seconds))
+                UtcOffset(seconds)
             }
 
         // org.threeten.bp.ZoneOffset#parseNumber