Skip to content

Deprecate plus and minus operations taking DateTimeUnit without amount #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions core/common/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public fun Instant.minus(period: DateTimePeriod, timeZone: TimeZone): Instant =
plus(negatedPeriod, timeZone)
} else {
val negatedPeriod = with(period) { buildDateTimePeriod(-totalMonths, -days, -(totalNanoseconds+1)) }
plus(negatedPeriod, timeZone).plus(DateTimeUnit.NANOSECOND)
plus(negatedPeriod, timeZone).plus(1, DateTimeUnit.NANOSECOND)
}

/**
Expand Down Expand Up @@ -351,6 +351,7 @@ public fun Instant.minus(other: Instant, timeZone: TimeZone): DateTimePeriod =
*
* @throws DateTimeArithmeticException if this value or the result is too large to fit in [LocalDateTime].
*/
@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit, timeZone)"))
public expect fun Instant.plus(unit: DateTimeUnit, timeZone: TimeZone): Instant

/**
Expand All @@ -361,6 +362,7 @@ public expect fun Instant.plus(unit: DateTimeUnit, timeZone: TimeZone): Instant
*
* @throws DateTimeArithmeticException if this value or the result is too large to fit in [LocalDateTime].
*/
@Deprecated("Use the minus overload with explicit amount of units", ReplaceWith("this.minus(1, unit, timeZone)"))
public fun Instant.minus(unit: DateTimeUnit, timeZone: TimeZone): Instant =
plus(-1, unit, timeZone)

Expand All @@ -371,6 +373,7 @@ public fun Instant.minus(unit: DateTimeUnit, timeZone: TimeZone): Instant =
*
* The return value is clamped to the platform-specific boundaries for [Instant] if the result exceeds them.
*/
@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit)"))
public fun Instant.plus(unit: DateTimeUnit.TimeBased): Instant =
plus(1L, unit)

Expand All @@ -381,6 +384,7 @@ public fun Instant.plus(unit: DateTimeUnit.TimeBased): Instant =
*
* The return value is clamped to the platform-specific boundaries for [Instant] if the result exceeds them.
*/
@Deprecated("Use the minus overload with explicit amount of units", ReplaceWith("this.minus(1, unit)"))
public fun Instant.minus(unit: DateTimeUnit.TimeBased): Instant =
plus(-1L, unit)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the practice we have in coroutines: when deprecating something, we move it either to the bottom of the file or altogether to a different file designated for deprecated things. This way, the deprecated (and no longer all that relevant) things are out of sight. Maybe we could adopt it here as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll clean up these deprecations near the stabilization, so it's not relevant here. We can consider this practice after stabilization, however I'm not currently fond of it because it makes history digging with git blame harder.


Expand Down Expand Up @@ -452,7 +456,7 @@ public fun Instant.minus(value: Long, unit: DateTimeUnit, timeZone: TimeZone): I
if (value != Long.MIN_VALUE) {
plus(-value, unit, timeZone)
} else {
plus(-(value + 1), unit, timeZone).plus(unit, timeZone)
plus(-(value + 1), unit, timeZone).plus(1, unit, timeZone)
}

/**
Expand All @@ -477,7 +481,7 @@ public fun Instant.minus(value: Long, unit: DateTimeUnit.TimeBased): Instant =
if (value != Long.MIN_VALUE) {
plus(-value, unit)
} else {
plus(-(value + 1), unit).plus(unit)
plus(-(value + 1), unit).plus(1, unit)
}

/**
Expand Down
2 changes: 2 additions & 0 deletions core/common/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public expect fun LocalDate.yearsUntil(other: LocalDate): Int
*
* @throws DateTimeArithmeticException if the result exceeds the boundaries of [LocalDate].
*/
@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit)"))
public expect fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate

/**
Expand All @@ -259,6 +260,7 @@ public expect fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate
*
* @throws DateTimeArithmeticException if the result exceeds the boundaries of [LocalDate].
*/
@Deprecated("Use the minus overload with explicit amount of units", ReplaceWith("this.minus(1, unit)"))
public fun LocalDate.minus(unit: DateTimeUnit.DateBased): LocalDate = plus(-1, unit)

/**
Expand Down
8 changes: 4 additions & 4 deletions core/common/test/InstantTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ class InstantTest {
expectBetween(instant1, instant2, 24, DateTimeUnit.HOUR)
assertEquals(instant1, instant2.minus(DateTimePeriod(hours = 24), zone))

val instant3 = instant1.plus(DateTimeUnit.DAY, zone)
val instant3 = instant1.plus(1, DateTimeUnit.DAY, zone)
checkComponents(instant3.toLocalDateTime(zone), 2019, 10, 28, 2, 59)
expectBetween(instant1, instant3, 25, DateTimeUnit.HOUR)
expectBetween(instant1, instant3, 1, DateTimeUnit.DAY)
assertEquals(1, instant1.daysUntil(instant3, zone))
assertEquals(instant1.minus(DateTimeUnit.HOUR), instant2.minus(DateTimeUnit.DAY, zone))
assertEquals(instant1.minus(1, DateTimeUnit.HOUR), instant2.minus(1, DateTimeUnit.DAY, zone))

val instant4 = instant1.plus(14, DateTimeUnit.MONTH, zone)
checkComponents(instant4.toLocalDateTime(zone), 2020, 12, 27, 2, 59)
Expand All @@ -178,15 +178,15 @@ class InstantTest {
expectBetween(instant1, instant4, 61, DateTimeUnit.WEEK)
expectBetween(instant1, instant4, 366 + 31 + 30, DateTimeUnit.DAY)
expectBetween(instant1, instant4, (366 + 31 + 30) * 24 + 1, DateTimeUnit.HOUR)
assertEquals(instant1.plus(DateTimeUnit.HOUR), instant4.minus(14, DateTimeUnit.MONTH, zone))
assertEquals(instant1.plus(1, DateTimeUnit.HOUR), instant4.minus(14, DateTimeUnit.MONTH, zone))

val period = DateTimePeriod(days = 1, hours = 1)
val instant5 = instant1.plus(period, zone)
checkComponents(instant5.toLocalDateTime(zone), 2019, 10, 28, 3, 59)
assertEquals(period, instant1.periodUntil(instant5, zone))
assertEquals(period, instant5.minus(instant1, zone))
assertEquals(26.hours, instant5.minus(instant1))
assertEquals(instant1.plus(DateTimeUnit.HOUR), instant5.minus(period, zone))
assertEquals(instant1.plus(1, DateTimeUnit.HOUR), instant5.minus(period, zone))

val instant6 = instant1.plus(23, DateTimeUnit.HOUR, zone)
checkComponents(instant6.toLocalDateTime(zone), 2019, 10, 28, 0, 59)
Expand Down
8 changes: 4 additions & 4 deletions core/common/test/LocalDateTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ class LocalDateTest {
fun addComponents() {
val startDate = LocalDate(2016, 2, 29)
checkComponents(startDate.plus(1, DateTimeUnit.DAY), 2016, 3, 1)
checkComponents(startDate.plus(DateTimeUnit.YEAR), 2017, 2, 28)
checkComponents(startDate.plus(1, DateTimeUnit.YEAR), 2017, 2, 28)
checkComponents(startDate + DatePeriod(years = 4), 2020, 2, 29)
assertEquals(startDate, startDate.plus(DateTimeUnit.DAY).minus(DateTimeUnit.DAY))
assertEquals(startDate, startDate.plus(1, DateTimeUnit.DAY).minus(1, DateTimeUnit.DAY))
assertEquals(startDate, startDate.plus(3, DateTimeUnit.DAY).minus(3, DateTimeUnit.DAY))
assertEquals(startDate, startDate + DatePeriod(years = 4) - DatePeriod(years = 4))

Expand All @@ -121,9 +121,9 @@ class LocalDateTest {
fun tomorrow() {
val today = Clock.System.todayIn(TimeZone.currentSystemDefault())

val nextMonthPlusDay1 = today.plus(DateTimeUnit.MONTH).plus(1, DateTimeUnit.DAY)
val nextMonthPlusDay1 = today.plus(1, DateTimeUnit.MONTH).plus(1, DateTimeUnit.DAY)
val nextMonthPlusDay2 = today + DatePeriod(months = 1, days = 1)
val nextMonthPlusDay3 = today.plus(DateTimeUnit.DAY).plus(1, DateTimeUnit.MONTH)
val nextMonthPlusDay3 = today.plus(1, DateTimeUnit.DAY).plus(1, DateTimeUnit.MONTH)
}

@Test
Expand Down
1 change: 1 addition & 0 deletions core/js/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst
private fun Instant.atZone(zone: TimeZone): ZonedDateTime = value.atZone(zone.zoneId)
private fun jtInstant.checkZone(zone: TimeZone): jtInstant = apply { atZone(zone.zoneId) }

@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit, timeZone)"))
public actual fun Instant.plus(unit: DateTimeUnit, timeZone: TimeZone): Instant =
plus(1, unit, timeZone)

Expand Down
1 change: 1 addition & 0 deletions core/js/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa
public actual fun toEpochDays(): Int = value.toEpochDay().toInt()
}

@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit)"))
public actual fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate = plusNumber(1, unit)
public actual fun LocalDate.plus(value: Int, unit: DateTimeUnit.DateBased): LocalDate = plusNumber(value, unit)
public actual fun LocalDate.minus(value: Int, unit: DateTimeUnit.DateBased): LocalDate = plusNumber(-value, unit)
Expand Down
1 change: 1 addition & 0 deletions core/jvm/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst
}
}

@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit, timeZone)"))
public actual fun Instant.plus(unit: DateTimeUnit, timeZone: TimeZone): Instant =
plus(1L, unit, timeZone)

Expand Down
1 change: 1 addition & 0 deletions core/jvm/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa
public actual fun toEpochDays(): Int = value.toEpochDay().clampToInt()
}

@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit)"))
public actual fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate =
plus(1L, unit)

Expand Down
1 change: 1 addition & 0 deletions core/native/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ public actual fun Instant.plus(period: DateTimePeriod, timeZone: TimeZone): Inst
throw DateTimeArithmeticException("Boundaries of Instant exceeded when adding CalendarPeriod", e)
}

@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit, timeZone)"))
public actual fun Instant.plus(unit: DateTimeUnit, timeZone: TimeZone): Instant =
plus(1L, unit, timeZone)
public actual fun Instant.plus(value: Int, unit: DateTimeUnit, timeZone: TimeZone): Instant =
Expand Down
1 change: 1 addition & 0 deletions core/native/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ public actual class LocalDate actual constructor(public actual val year: Int, pu
}
}

@Deprecated("Use the plus overload with explicit amount of units", ReplaceWith("this.plus(1, unit)"))
public actual fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate =
plus(1, unit)

Expand Down