Skip to content

Commit 7a1d01d

Browse files
dkhalanskyjbdkhalanskyjb
committed
Avoid extraneous range checks when constructors receive safe values
1 parent b4df4b8 commit 7a1d01d

File tree

5 files changed

+91
-42
lines changed

5 files changed

+91
-42
lines changed

‎core/common/src/Month.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ public val Month.number: Int get() = ordinal + 1
7272
* @sample kotlinx.datetime.test.samples.MonthSamples.constructorFunction
7373
*/
7474
public fun Month(number: Int): Month {
75-
require(number in 1..12)
75+
require(number in 1..12) {
76+
"Month must be a number between 1 and 12, got $number"
77+
}
7678
return Month.entries[number - 1]
7779
}

‎core/commonKotlin/src/LocalDate.kt‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,30 +23,38 @@ private fun isValidYear(year: Int): Boolean =
2323
year >= YEAR_MIN && year <= YEAR_MAX
2424

2525
@Serializable(with = LocalDateSerializer::class)
26-
public actual class LocalDate actual constructor(public actual val year: Int, month: Int, public actual val day: Int) : Comparable<LocalDate> {
26+
public actual class LocalDate private constructor(
27+
public actual val year: Int, month: Int, public actual val day: Int, unit: Unit
28+
) : Comparable<LocalDate> {
29+
30+
public actual constructor(year: Int, month: Int, day: Int): this(year, month, day, Unit) {
31+
require(_month in 1..12) { "Invalid date: month must be a number between 1 and 12, got $_month" }
32+
validateYearAndDay()
33+
}
2734

2835
private val _month: Int = month
2936
@Deprecated("Use the 'month' property instead", ReplaceWith("this.month.number"), level = DeprecationLevel.WARNING)
3037
public actual val monthNumber: Int get() = _month
3138
@Deprecated("Use the 'day' property instead", ReplaceWith("this.day"), level = DeprecationLevel.WARNING)
3239
public actual val dayOfMonth: Int get() = day
3340

34-
init {
41+
public actual constructor(year: Int, month: Month, day: Int) : this(year, month.number, day, Unit) {
42+
validateYearAndDay()
43+
}
44+
45+
private fun validateYearAndDay() {
3546
// org.threeten.bp.LocalDate#create
3647
require(isValidYear(year)) { "Invalid date: the year is out of range" }
37-
require(_month in 1..12) { "Invalid date: month must be a number between 1 and 12, got $_month" }
3848
require(day in 1..31) { "Invalid date: day of month must be a number between 1 and 31, got $day" }
3949
if (day > 28 && day > _month.monthLength(isLeapYear(year))) {
4050
if (day == 29) {
4151
throw IllegalArgumentException("Invalid date 'February 29' as '$year' is not a leap year")
4252
} else {
43-
throw IllegalArgumentException("Invalid date '${Month(month)} $day'")
53+
throw IllegalArgumentException("Invalid date '$month $day'")
4454
}
4555
}
4656
}
4757

48-
public actual constructor(year: Int, month: Month, day: Int) : this(year, month.number, day)
49-
5058
public actual companion object {
5159
public actual fun parse(input: CharSequence, format: DateTimeFormat<LocalDate>): LocalDate = format.parse(input)
5260

@@ -58,7 +66,7 @@ public actual class LocalDate actual constructor(public actual val year: Int, mo
5866
(day > 28 && day > month.monthLength(isLeapYear(year)))) {
5967
null
6068
} else {
61-
LocalDate(year, month, day)
69+
LocalDate(year, month, day, Unit)
6270
}
6371

6472
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
98106
val dom = marchDoy0 - (marchMonth0 * 306 + 5) / 10 + 1
99107
yearEst += marchMonth0 / 10
100108

101-
return LocalDate(yearEst.toInt(), month, dom)
109+
// The range of valid values was checked in the beginning
110+
return LocalDate(yearEst.toInt(), month, dom, Unit)
102111
}
103112

104113
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
171180

172181
// org.threeten.bp.LocalDate#resolvePreviousValid
173182
/**
183+
* May only be called if the year and month are valid.
184+
*
174185
* @throws IllegalArgumentException if the result exceeds the boundaries
175186
*/
176187
private fun resolvePreviousValid(year: Int, month: Int, day: Int): LocalDate {
177188
val newDay = min(day, month.monthLength(isLeapYear(year)))
178-
return LocalDate(year, month, newDay)
189+
return LocalDate(year, month, newDay, Unit)
179190
}
180191

181192
// org.threeten.bp.LocalDate#plusMonths

‎core/commonKotlin/src/LocalDateTime.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ public actual constructor(public actual val date: LocalDate, public actual val t
6565
}
6666

6767
public actual constructor(year: Int, month: Int, day: Int, hour: Int, minute: Int, second: Int, nanosecond: Int) :
68-
this(LocalDate(year, month, day), LocalTime.of(hour, minute, second, nanosecond))
68+
this(LocalDate(year, month, day), LocalTime(hour, minute, second, nanosecond))
6969

7070
public actual constructor(year: Int, month: Month, day: Int, hour: Int, minute: Int, second: Int, nanosecond: Int) :
71-
this(LocalDate(year, month, day), LocalTime.of(hour, minute, second, nanosecond))
71+
this(LocalDate(year, month, day), LocalTime(hour, minute, second, nanosecond))
7272

7373
public actual val year: Int get() = date.year
7474
@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
106106

107107
// org.threeten.bp.chrono.ChronoLocalDateTime#toEpochSecond
108108
internal fun toEpochSecond(offset: UtcOffset): Long {
109-
val epochDay = date.toEpochDays().toLong()
109+
val epochDay = date.toEpochDays()
110110
var secs: Long = epochDay * 86400 + time.toSecondOfDay()
111111
secs -= offset.totalSeconds
112112
return secs

‎core/commonKotlin/src/LocalTime.kt‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,17 @@ import kotlinx.datetime.serializers.*
1414
import kotlinx.serialization.Serializable
1515

1616
@Serializable(LocalTimeSerializer::class)
17-
public actual class LocalTime actual constructor(
17+
public actual class LocalTime private constructor(
1818
public actual val hour: Int,
1919
public actual val minute: Int,
2020
public actual val second: Int,
21-
public actual val nanosecond: Int
22-
) : Comparable<LocalTime> {
21+
public actual val nanosecond: Int,
22+
unit: Unit,
23+
) : Comparable<LocalTime> {
2324

24-
init {
25+
public actual constructor(
26+
hour: Int, minute: Int, second: Int, nanosecond: Int
27+
) : this(hour, minute, second, nanosecond, Unit) {
2528
fun check(value: Int, lower: Int, upper: Int, str: String) =
2629
require(value in lower..upper) {
2730
"Invalid time: $str must be a number between $lower and $upper, got $value"
@@ -37,7 +40,7 @@ public actual class LocalTime actual constructor(
3740
if (hour !in 0..23 || minute !in 0..59 || second !in 0..59 || nanosecond !in 0 until NANOS_PER_ONE) {
3841
null
3942
} else {
40-
LocalTime(hour, minute, second, nanosecond)
43+
LocalTime(hour, minute, second, nanosecond, Unit)
4144
}
4245

4346
public actual fun parse(input: CharSequence, format: DateTimeFormat<LocalTime>): LocalTime = format.parse(input)
@@ -56,30 +59,34 @@ public actual class LocalTime actual constructor(
5659

5760
// org.threeten.bp.LocalTime#ofSecondOfDay(long, int)
5861
internal fun ofSecondOfDay(secondOfDay: Int, nanoOfSecond: Int): LocalTime {
59-
require(secondOfDay in 0 until SECONDS_PER_DAY)
60-
require(nanoOfSecond in 0 until NANOS_PER_ONE)
62+
require(secondOfDay in 0 until SECONDS_PER_DAY) {
63+
"Invalid time: secondOfDay must be between 0 and $SECONDS_PER_DAY, got $secondOfDay"
64+
}
65+
require(nanoOfSecond in 0 until NANOS_PER_ONE) {
66+
"Invalid time: nanosecondOfSecond must be between 0 and $NANOS_PER_ONE, got $nanoOfSecond"
67+
}
6168
val hours = (secondOfDay / SECONDS_PER_HOUR)
6269
val secondWithoutHours = secondOfDay - hours * SECONDS_PER_HOUR
6370
val minutes = (secondWithoutHours / SECONDS_PER_MINUTE)
6471
val second = secondWithoutHours - minutes * SECONDS_PER_MINUTE
65-
return LocalTime(hours, minutes, second, nanoOfSecond)
66-
}
67-
68-
internal fun of(hour: Int, minute: Int, second: Int, nanosecond: Int): LocalTime {
69-
return LocalTime(hour, minute, second, nanosecond)
72+
// The range of valid values was checked in the require statements above
73+
return LocalTime(hours, minutes, second, nanoOfSecond, Unit)
7074
}
7175

7276
// org.threeten.bp.LocalTime#ofNanoOfDay
7377
internal fun ofNanoOfDay(nanoOfDay: Long): LocalTime {
74-
require(nanoOfDay >= 0 && nanoOfDay < SECONDS_PER_DAY.toLong() * NANOS_PER_ONE)
78+
require(nanoOfDay >= 0 && nanoOfDay < SECONDS_PER_DAY.toLong() * NANOS_PER_ONE) {
79+
"Invalid time: nanosecondOfDay must be between 0 and 86_400_000_000_000, got $nanoOfDay"
80+
}
7581
var newNanoOfDay = nanoOfDay
7682
val hours = (newNanoOfDay / NANOS_PER_HOUR).toInt()
7783
newNanoOfDay -= hours * NANOS_PER_HOUR
7884
val minutes = (newNanoOfDay / NANOS_PER_MINUTE).toInt()
7985
newNanoOfDay -= minutes * NANOS_PER_MINUTE
8086
val seconds = (newNanoOfDay / NANOS_PER_ONE).toInt()
8187
newNanoOfDay -= seconds * NANOS_PER_ONE
82-
return LocalTime(hours, minutes, seconds, newNanoOfDay.toInt())
88+
// The range of valid values was checked in the require statement
89+
return LocalTime(hours, minutes, seconds, newNanoOfDay.toInt(), Unit)
8390
}
8491

8592
internal actual val MIN: LocalTime = LocalTime(0, 0, 0, 0)

‎core/commonKotlin/src/UtcOffset.kt‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,25 @@ public actual class UtcOffset private constructor(public actual val totalSeconds
2424

2525
public actual val ZERO: UtcOffset = UtcOffset(totalSeconds = 0)
2626

27-
public actual fun orNull(hours: Int?, minutes: Int?, seconds: Int?): UtcOffset? = try {
28-
UtcOffset(hours, minutes, seconds)
29-
} catch (_: IllegalArgumentException) {
30-
null
27+
public actual fun orNull(hours: Int?, minutes: Int?, seconds: Int?): UtcOffset? = when {
28+
hours != null ->
29+
ofHoursMinutesSecondsOrNull(hours, minutes ?: 0, seconds ?: 0)
30+
minutes != null ->
31+
ofHoursMinutesSecondsOrNull(minutes / MINUTES_PER_HOUR, minutes % MINUTES_PER_HOUR, seconds ?: 0)
32+
else ->
33+
ofSecondsOrNull(seconds ?: 0)
3134
}
3235

3336
public actual fun parse(input: CharSequence, format: DateTimeFormat<UtcOffset>): UtcOffset = format.parse(input)
3437

3538
@Deprecated("This overload is only kept for binary compatibility", level = DeprecationLevel.HIDDEN)
3639
public fun parse(offsetString: String): UtcOffset = parse(input = offsetString)
3740

41+
private fun totalSecondsValid(totalSeconds: Int): Boolean =
42+
totalSeconds in -18 * SECONDS_PER_HOUR..18 * SECONDS_PER_HOUR
43+
3844
private fun validateTotal(totalSeconds: Int) {
39-
if (totalSeconds !in -18 * SECONDS_PER_HOUR .. 18 * SECONDS_PER_HOUR) {
40-
throw IllegalArgumentException("Total seconds value is out of range: $totalSeconds")
41-
}
45+
require(totalSecondsValid(totalSeconds)) { "Total seconds value is out of range: $totalSeconds" }
4246
}
4347

4448
// org.threeten.bp.ZoneOffset#validate
@@ -71,23 +75,48 @@ public actual class UtcOffset private constructor(public actual val totalSeconds
7175
}
7276
}
7377

78+
private fun hoursMinutesSecondsValid(hours: Int, minutes: Int, seconds: Int): Boolean =
79+
// valid range for hours, minutes and seconds
80+
hours in -18..18 && minutes in -59..59 && seconds in -59..59
81+
// same sign for all components
82+
&& !(hours > 0 && (minutes < 0 || seconds < 0))
83+
&& !(hours < 0 && (minutes > 0 || seconds > 0))
84+
// same sign for minutes and seconds
85+
&& !(minutes > 0 && seconds < 0 || minutes < 0 && seconds > 0)
86+
// valid range for total seconds
87+
&& !(abs(hours) == 18 && (abs(minutes) > 0 || abs(seconds) > 0))
88+
7489
// org.threeten.bp.ZoneOffset#ofHoursMinutesSeconds
90+
internal fun ofHoursMinutesSecondsUnsafe(hours: Int, minutes: Int, seconds: Int): UtcOffset =
91+
if (hours == 0 && minutes == 0 && seconds == 0) ZERO
92+
else ofSeconds(hours * SECONDS_PER_HOUR + minutes * SECONDS_PER_MINUTE + seconds)
93+
7594
internal fun ofHoursMinutesSeconds(hours: Int, minutes: Int, seconds: Int): UtcOffset {
7695
validate(hours, minutes, seconds)
77-
return if (hours == 0 && minutes == 0 && seconds == 0) ZERO
78-
else ofSeconds(hours * SECONDS_PER_HOUR + minutes * SECONDS_PER_MINUTE + seconds)
96+
return ofHoursMinutesSecondsUnsafe(hours, minutes, seconds)
7997
}
8098

81-
// org.threeten.bp.ZoneOffset#ofTotalSeconds
82-
internal fun ofSeconds(seconds: Int): UtcOffset {
83-
validateTotal(seconds)
84-
return if (seconds % (15 * SECONDS_PER_MINUTE) == 0) {
85-
utcOffsetCache[seconds] ?: UtcOffset(totalSeconds = seconds).also { utcOffsetCache[seconds] = it }
99+
internal fun ofHoursMinutesSecondsOrNull(hours: Int, minutes: Int, seconds: Int): UtcOffset? =
100+
if (hoursMinutesSecondsValid(hours, minutes, seconds)) {
101+
ofHoursMinutesSecondsUnsafe(hours, minutes, seconds)
86102
} else {
87-
UtcOffset(totalSeconds = seconds)
103+
null
88104
}
105+
106+
// org.threeten.bp.ZoneOffset#ofTotalSeconds
107+
private fun ofSecondsUnsafe(seconds: Int): UtcOffset = if (seconds % (15 * SECONDS_PER_MINUTE) == 0) {
108+
utcOffsetCache[seconds] ?: UtcOffset(totalSeconds = seconds).also { utcOffsetCache[seconds] = it }
109+
} else {
110+
UtcOffset(totalSeconds = seconds)
89111
}
90112

113+
internal fun ofSeconds(seconds: Int): UtcOffset = validateTotal(seconds).let {
114+
ofSecondsUnsafe(seconds)
115+
}
116+
117+
internal fun ofSecondsOrNull(seconds: Int): UtcOffset? =
118+
if (totalSecondsValid(seconds)) ofSecondsUnsafe(seconds) else null
119+
91120
@Suppress("FunctionName")
92121
public actual fun Format(block: DateTimeFormatBuilder.WithUtcOffset.() -> Unit): DateTimeFormat<UtcOffset> =
93122
UtcOffsetFormat.build(block)

0 commit comments

Comments
 (0)