Avoid extraneous range checks when constructors receive safe values

Author: dkhalanskyjb

core/common/src/Month.kt

@@ -72,6 +72,8 @@ public val Month.number: Int get() = ordinal + 1
  * @sample kotlinx.datetime.test.samples.MonthSamples.constructorFunction
  */
 public fun Month(number: Int): Month {
-    require(number in 1..12)
+    require(number in 1..12) {
+        "Month must be a number between 1 and 12, got $number"
+    }
     return Month.entries[number - 1]
 }

core/commonKotlin/src/LocalDate.kt

@@ -23,30 +23,38 @@ private fun isValidYear(year: Int): Boolean =
     year >= YEAR_MIN && year <= YEAR_MAX
 
 @Serializable(with = LocalDateIso8601Serializer::class)
-public actual class LocalDate actual constructor(public actual val year: Int, month: Int, public actual val day: Int) : Comparable<LocalDate> {
+public actual class LocalDate private constructor(
+    public actual val year: Int, month: Int, public actual val day: Int, unit: Unit
+) : Comparable<LocalDate> {
+
+    public actual constructor(year: Int, month: Int, day: Int): this(year, month, day, Unit) {
+        require(_month in 1..12) { "Invalid date: month must be a number between 1 and 12, got $_month" }
+        validateYearAndDay()
+    }
 
     private val _month: Int = month
     @Deprecated("Use the 'month' property instead", ReplaceWith("this.month.number"), level = DeprecationLevel.WARNING)
     public actual val monthNumber: Int get() = _month
     @Deprecated("Use the 'day' property instead", ReplaceWith("this.day"), level = DeprecationLevel.WARNING)
     public actual val dayOfMonth: Int get() = day
 
-    init {
+    public actual constructor(year: Int, month: Month, day: Int) : this(year, month.number, day, Unit) {
+        validateYearAndDay()
+    }
+
+    private fun validateYearAndDay() {
         // org.threeten.bp.LocalDate#create
         require(isValidYear(year)) { "Invalid date: the year is out of range" }
-        require(_month in 1..12) { "Invalid date: month must be a number between 1 and 12, got $_month" }
         require(day in 1..31) { "Invalid date: day of month must be a number between 1 and 31, got $day" }
         if (day > 28 && day > _month.monthLength(isLeapYear(year))) {
             if (day == 29) {
                 throw IllegalArgumentException("Invalid date 'February 29' as '$year' is not a leap year")
             } else {
-                throw IllegalArgumentException("Invalid date '${Month(month)} $day'")
+                throw IllegalArgumentException("Invalid date '$month $day'")
             }
         }
     }
 
-    public actual constructor(year: Int, month: Month, day: Int) : this(year, month.number, day)
-
     public actual companion object {
         public actual fun parse(input: CharSequence, format: DateTimeFormat<LocalDate>): LocalDate = format.parse(input)
 
@@ -58,7 +66,7 @@ public actual class LocalDate actual constructor(public actual val year: Int, mo
                 (day > 28 && day > month.monthLength(isLeapYear(year)))) {
                 null
             } else {
-                LocalDate(year, month, day)
+                LocalDate(year, month, day, Unit)
             }
 
         public actual fun orNull(year: Int, month: Month, day: Int): LocalDate? =
@@ -98,7 +106,8 @@ public actual class LocalDate actual constructor(public actual val year: Int, mo
             val dom = marchDoy0 - (marchMonth0 * 306 + 5) / 10 + 1
             yearEst += marchMonth0 / 10
 
-            return LocalDate(yearEst.toInt(), month, dom)
+            // The range of valid values was checked in the beginning
+            return LocalDate(yearEst.toInt(), month, dom, Unit)
         }
 
         public actual fun fromEpochDays(epochDays: Int): LocalDate = fromEpochDays(epochDays.toLong())
@@ -171,11 +180,13 @@ public actual class LocalDate actual constructor(public actual val year: Int, mo
 
     // org.threeten.bp.LocalDate#resolvePreviousValid
     /**
+     * May only be called if the year and month are valid.
+     *
      * @throws IllegalArgumentException if the result exceeds the boundaries
      */
     private fun resolvePreviousValid(year: Int, month: Int, day: Int): LocalDate {
         val newDay = min(day, month.monthLength(isLeapYear(year)))
-        return LocalDate(year, month, newDay)
+        return LocalDate(year, month, newDay, Unit)
     }
 
     // org.threeten.bp.LocalDate#plusMonths

core/commonKotlin/src/LocalDateTime.kt

@@ -65,10 +65,10 @@ public actual constructor(public actual val date: LocalDate, public actual val t
     }
 
     public actual constructor(year: Int, month: Int, day: Int, hour: Int, minute: Int, second: Int, nanosecond: Int) :
-        this(LocalDate(year, month, day), LocalTime.of(hour, minute, second, nanosecond))
+        this(LocalDate(year, month, day), LocalTime(hour, minute, second, nanosecond))
 
     public actual constructor(year: Int, month: Month, day: Int, hour: Int, minute: Int, second: Int, nanosecond: Int) :
-        this(LocalDate(year, month, day), LocalTime.of(hour, minute, second, nanosecond))
+        this(LocalDate(year, month, day), LocalTime(hour, minute, second, nanosecond))
 
     public actual val year: Int get() = date.year
     @Deprecated("Use the 'month' property instead", ReplaceWith("this.month.number"), level = DeprecationLevel.WARNING)
@@ -106,7 +106,7 @@ public actual constructor(public actual val date: LocalDate, public actual val t
 
     // org.threeten.bp.chrono.ChronoLocalDateTime#toEpochSecond
     internal fun toEpochSecond(offset: UtcOffset): Long {
-        val epochDay = date.toEpochDays().toLong()
+        val epochDay = date.toEpochDays()
         var secs: Long = epochDay * 86400 + time.toSecondOfDay()
         secs -= offset.totalSeconds
         return secs

core/commonKotlin/src/LocalTime.kt

@@ -14,14 +14,17 @@ import kotlinx.datetime.serializers.LocalTimeIso8601Serializer
 import kotlinx.serialization.Serializable
 
 @Serializable(LocalTimeIso8601Serializer::class)
-public actual class LocalTime actual constructor(
+public actual class LocalTime private constructor(
     public actual val hour: Int,
     public actual val minute: Int,
     public actual val second: Int,
-    public actual val nanosecond: Int
-    ) : Comparable<LocalTime> {
+    public actual val nanosecond: Int,
+    unit: Unit,
+) : Comparable<LocalTime> {
 
-    init {
+    public actual constructor(
+        hour: Int, minute: Int, second: Int, nanosecond: Int
+    ) : this(hour, minute, second, nanosecond, Unit) {
         fun check(value: Int, lower: Int, upper: Int, str: String) =
             require(value in lower..upper) {
                 "Invalid time: $str must be a number between $lower and $upper, got $value"
@@ -37,7 +40,7 @@ public actual class LocalTime actual constructor(
             if (hour !in 0..23 || minute !in 0..59 || second !in 0..59 || nanosecond !in 0 until NANOS_PER_ONE) {
                 null
             } else {
-                LocalTime(hour, minute, second, nanosecond)
+                LocalTime(hour, minute, second, nanosecond, Unit)
             }
 
         public actual fun parse(input: CharSequence, format: DateTimeFormat<LocalTime>): LocalTime = format.parse(input)
@@ -56,30 +59,34 @@ public actual class LocalTime actual constructor(
 
         // org.threeten.bp.LocalTime#ofSecondOfDay(long, int)
         internal fun ofSecondOfDay(secondOfDay: Int, nanoOfSecond: Int): LocalTime {
-            require(secondOfDay in 0 until SECONDS_PER_DAY)
-            require(nanoOfSecond in 0 until NANOS_PER_ONE)
+            require(secondOfDay in 0 until SECONDS_PER_DAY) {
+                "Invalid time: secondOfDay must be between 0 and $SECONDS_PER_DAY, got $secondOfDay"
+            }
+            require(nanoOfSecond in 0 until NANOS_PER_ONE) {
+                "Invalid time: nanosecondOfSecond must be between 0 and $NANOS_PER_ONE, got $nanoOfSecond"
+            }
             val hours = (secondOfDay / SECONDS_PER_HOUR)
             val secondWithoutHours = secondOfDay - hours * SECONDS_PER_HOUR
             val minutes = (secondWithoutHours / SECONDS_PER_MINUTE)
             val second = secondWithoutHours - minutes * SECONDS_PER_MINUTE
-            return LocalTime(hours, minutes, second, nanoOfSecond)
-        }
-
-        internal fun of(hour: Int, minute: Int, second: Int, nanosecond: Int): LocalTime {
-            return LocalTime(hour, minute, second, nanosecond)
+            // The range of valid values was checked in the require statements above
+            return LocalTime(hours, minutes, second, nanoOfSecond, Unit)
         }
 
         // org.threeten.bp.LocalTime#ofNanoOfDay
         internal fun ofNanoOfDay(nanoOfDay: Long): LocalTime {
-            require(nanoOfDay >= 0 && nanoOfDay < SECONDS_PER_DAY.toLong() * NANOS_PER_ONE)
+            require(nanoOfDay >= 0 && nanoOfDay < SECONDS_PER_DAY.toLong() * NANOS_PER_ONE) {
+                "Invalid time: nanosecondOfDay must be between 0 and 86_400_000_000_000, got $nanoOfDay"
+            }
             var newNanoOfDay = nanoOfDay
             val hours = (newNanoOfDay / NANOS_PER_HOUR).toInt()
             newNanoOfDay -= hours * NANOS_PER_HOUR
             val minutes = (newNanoOfDay / NANOS_PER_MINUTE).toInt()
             newNanoOfDay -= minutes * NANOS_PER_MINUTE
             val seconds = (newNanoOfDay / NANOS_PER_ONE).toInt()
             newNanoOfDay -= seconds * NANOS_PER_ONE
-            return LocalTime(hours, minutes, seconds, newNanoOfDay.toInt())
+            // The range of valid values was checked in the require statement
+            return LocalTime(hours, minutes, seconds, newNanoOfDay.toInt(), Unit)
         }
 
         internal actual val MIN: LocalTime = LocalTime(0, 0, 0, 0)

core/commonKotlin/src/UtcOffset.kt

@@ -23,21 +23,25 @@ public actual class UtcOffset private constructor(public actual val totalSeconds
 
         public actual val ZERO: UtcOffset = UtcOffset(totalSeconds = 0)
 
-        public actual fun orNull(hours: Int?, minutes: Int?, seconds: Int?): UtcOffset? = try {
-            UtcOffset(hours, minutes, seconds)
-        } catch (_: IllegalArgumentException) {
-            null
+        public actual fun orNull(hours: Int?, minutes: Int?, seconds: Int?): UtcOffset? = when {
+            hours != null ->
+                ofHoursMinutesSecondsOrNull(hours, minutes ?: 0, seconds ?: 0)
+            minutes != null ->
+                ofHoursMinutesSecondsOrNull(minutes / MINUTES_PER_HOUR, minutes % MINUTES_PER_HOUR, seconds ?: 0)
+            else ->
+                ofSecondsOrNull(seconds ?: 0)
         }
 
         public actual fun parse(input: CharSequence, format: DateTimeFormat<UtcOffset>): UtcOffset = format.parse(input)
 
         @Deprecated("This overload is only kept for binary compatibility", level = DeprecationLevel.HIDDEN)
         public fun parse(offsetString: String): UtcOffset = parse(input = offsetString)
 
+        private fun totalSecondsValid(totalSeconds: Int): Boolean =
+            totalSeconds in -18 * SECONDS_PER_HOUR..18 * SECONDS_PER_HOUR
+
         private fun validateTotal(totalSeconds: Int) {
-            if (totalSeconds !in -18 * SECONDS_PER_HOUR .. 18 * SECONDS_PER_HOUR) {
-                throw IllegalArgumentException("Total seconds value is out of range: $totalSeconds")
-            }
+            require(totalSecondsValid(totalSeconds)) { "Total seconds value is out of range: $totalSeconds" }
         }
 
         // org.threeten.bp.ZoneOffset#validate
@@ -70,23 +74,48 @@ public actual class UtcOffset private constructor(public actual val totalSeconds
             }
         }
 
+        private fun hoursMinutesSecondsValid(hours: Int, minutes: Int, seconds: Int): Boolean =
+            // valid range for hours, minutes and seconds
+            hours in -18..18 && minutes in -59..59 && seconds in -59..59
+                    // same sign for all components
+                    && !(hours > 0 && (minutes < 0 || seconds < 0))
+                    && !(hours < 0 && (minutes > 0 || seconds > 0))
+                    // same sign for minutes and seconds
+                    && !(minutes > 0 && seconds < 0 || minutes < 0 && seconds > 0)
+                    // valid range for total seconds
+                    && !(abs(hours) == 18 && (abs(minutes) > 0 || abs(seconds) > 0))
+
         // org.threeten.bp.ZoneOffset#ofHoursMinutesSeconds
+        internal fun ofHoursMinutesSecondsUnsafe(hours: Int, minutes: Int, seconds: Int): UtcOffset =
+            if (hours == 0 && minutes == 0 && seconds == 0) ZERO
+            else ofSeconds(hours * SECONDS_PER_HOUR + minutes * SECONDS_PER_MINUTE + seconds)
+
         internal fun ofHoursMinutesSeconds(hours: Int, minutes: Int, seconds: Int): UtcOffset {
             validate(hours, minutes, seconds)
-            return if (hours == 0 && minutes == 0 && seconds == 0) ZERO
-            else ofSeconds(hours * SECONDS_PER_HOUR + minutes * SECONDS_PER_MINUTE + seconds)
+            return ofHoursMinutesSecondsUnsafe(hours, minutes, seconds)
         }
 
-        // org.threeten.bp.ZoneOffset#ofTotalSeconds
-        internal fun ofSeconds(seconds: Int): UtcOffset {
-            validateTotal(seconds)
-            return if (seconds % (15 * SECONDS_PER_MINUTE) == 0) {
-                utcOffsetCache[seconds] ?: UtcOffset(totalSeconds = seconds).also { utcOffsetCache[seconds] = it }
+        internal fun ofHoursMinutesSecondsOrNull(hours: Int, minutes: Int, seconds: Int): UtcOffset? =
+            if (hoursMinutesSecondsValid(hours, minutes, seconds)) {
+                ofHoursMinutesSecondsUnsafe(hours, minutes, seconds)
             } else {
-                UtcOffset(totalSeconds = seconds)
+                null
             }
+
+        // org.threeten.bp.ZoneOffset#ofTotalSeconds
+        private fun ofSecondsUnsafe(seconds: Int): UtcOffset = if (seconds % (15 * SECONDS_PER_MINUTE) == 0) {
+            utcOffsetCache[seconds] ?: UtcOffset(totalSeconds = seconds).also { utcOffsetCache[seconds] = it }
+        } else {
+            UtcOffset(totalSeconds = seconds)
         }
 
+        internal fun ofSeconds(seconds: Int): UtcOffset = validateTotal(seconds).let {
+            ofSecondsUnsafe(seconds)
+        }
+
+        internal fun ofSecondsOrNull(seconds: Int): UtcOffset? =
+            if (totalSecondsValid(seconds)) ofSecondsUnsafe(seconds) else null
+
         @Suppress("FunctionName")
         public actual fun Format(block: DateTimeFormatBuilder.WithUtcOffset.() -> Unit): DateTimeFormat<UtcOffset> =
             UtcOffsetFormat.build(block)