Skip to content
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

Use overflowing_naive_local in DateTime::checked_add* #1333

Merged
merged 4 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 43 additions & 21 deletions src/datetime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,13 +436,17 @@ impl<Tz: TimeZone> DateTime<Tz> {
/// # Errors
///
/// Returns `None` if:
/// - The resulting date would be out of range.
/// - The local time at the resulting date does not exist or is ambiguous, for example during a
/// daylight saving time transition.
/// - The resulting UTC datetime would be out of range.
/// - The resulting local datetime would be out of range (unless `months` is zero).
#[must_use]
pub fn checked_add_months(self, rhs: Months) -> Option<DateTime<Tz>> {
self.naive_local()
.checked_add_months(rhs)?
pub fn checked_add_months(self, months: Months) -> Option<DateTime<Tz>> {
// `NaiveDate::checked_add_months` has a fast path for `Months(0)` that does not validate
// the resulting date, with which we can return `Some` even for an out of range local
// datetime.
self.overflowing_naive_local()
.checked_add_months(months)?
.and_local_timezone(Tz::from_offset(&self.offset))
.single()
}
Expand All @@ -469,13 +473,17 @@ impl<Tz: TimeZone> DateTime<Tz> {
/// # Errors
///
/// Returns `None` if:
/// - The resulting date would be out of range.
/// - The local time at the resulting date does not exist or is ambiguous, for example during a
/// daylight saving time transition.
/// - The resulting UTC datetime would be out of range.
/// - The resulting local datetime would be out of range (unless `months` is zero).
#[must_use]
pub fn checked_sub_months(self, rhs: Months) -> Option<DateTime<Tz>> {
self.naive_local()
.checked_sub_months(rhs)?
pub fn checked_sub_months(self, months: Months) -> Option<DateTime<Tz>> {
// `NaiveDate::checked_sub_months` has a fast path for `Months(0)` that does not validate
// the resulting date, with which we can return `Some` even for an out of range local
// datetime.
self.overflowing_naive_local()
.checked_sub_months(months)?
.and_local_timezone(Tz::from_offset(&self.offset))
.single()
}
Expand All @@ -485,31 +493,42 @@ impl<Tz: TimeZone> DateTime<Tz> {
/// # Errors
///
/// Returns `None` if:
/// - The resulting date would be out of range.
/// - The local time at the resulting date does not exist or is ambiguous, for example during a
/// daylight saving time transition.
/// - The resulting UTC datetime would be out of range.
/// - The resulting local datetime would be out of range (unless `days` is zero).
#[must_use]
pub fn checked_add_days(self, days: Days) -> Option<Self> {
self.naive_local()
.checked_add_days(days)?
.and_local_timezone(TimeZone::from_offset(&self.offset))
.single()
if days == Days::new(0) {
return Some(self);
}
// `NaiveDate::add_days` has a fast path if the result remains within the same year, that
// does not validate the resulting date. This allows us to return `Some` even for an out of
// range local datetime when adding `Days(0)`.
self.overflowing_naive_local()
.checked_add_days(days)
.and_then(|dt| self.timezone().from_local_datetime(&dt).single())
.filter(|dt| dt <= &DateTime::<Utc>::MAX_UTC)
}

/// Subtract a duration in [`Days`] from the date part of the `DateTime`.
///
/// # Errors
///
/// Returns `None` if:
/// - The resulting date would be out of range.
/// - The local time at the resulting date does not exist or is ambiguous, for example during a
/// daylight saving time transition.
/// - The resulting UTC datetime would be out of range.
/// - The resulting local datetime would be out of range (unless `days` is zero).
#[must_use]
pub fn checked_sub_days(self, days: Days) -> Option<Self> {
self.naive_local()
.checked_sub_days(days)?
.and_local_timezone(TimeZone::from_offset(&self.offset))
.single()
// `NaiveDate::add_days` has a fast path if the result remains within the same year, that
// does not validate the resulting date. This allows us to return `Some` even for an out of
// range local datetime when adding `Days(0)`.
self.overflowing_naive_local()
.checked_sub_days(days)
.and_then(|dt| self.timezone().from_local_datetime(&dt).single())
.filter(|dt| dt >= &DateTime::<Utc>::MIN_UTC)
}

/// Subtracts another `DateTime` from the current date and time.
Expand Down Expand Up @@ -1080,12 +1099,15 @@ impl<Tz: TimeZone> Datelike for DateTime<Tz> {
/// # Errors
///
/// Returns `None` if:
/// - The resulting date does not exist.
/// - When the `NaiveDateTime` would be out of range.
/// - The local time at the resulting date does not exist or is ambiguous, for example during a
/// daylight saving time transition.
/// - The resulting UTC datetime would be out of range.
/// - The resulting local datetime would be out of range (unless the year remains the same).
fn with_year(&self, year: i32) -> Option<DateTime<Tz>> {
map_local(self, |datetime| datetime.with_year(year))
map_local(self, |dt| match dt.year() == year {
true => Some(dt),
false => dt.with_year(year),
})
}

/// Makes a new `DateTime` with the month number (starting from 1) changed.
Expand Down
52 changes: 52 additions & 0 deletions src/datetime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1421,6 +1421,7 @@ fn test_min_max_setters() {
let beyond_max = offset_max.from_utc_datetime(&NaiveDateTime::MAX);

assert_eq!(beyond_min.with_year(2020).unwrap().year(), 2020);
assert_eq!(beyond_min.with_year(beyond_min.year()), Some(beyond_min));
assert_eq!(beyond_min.with_month(beyond_min.month()), Some(beyond_min));
assert_eq!(beyond_min.with_month(3), None);
assert_eq!(beyond_min.with_month0(beyond_min.month0()), Some(beyond_min));
Expand All @@ -1441,6 +1442,7 @@ fn test_min_max_setters() {
assert_eq!(beyond_min.with_nanosecond(0), Some(beyond_min));

assert_eq!(beyond_max.with_year(2020).unwrap().year(), 2020);
assert_eq!(beyond_max.with_year(beyond_max.year()), Some(beyond_max));
assert_eq!(beyond_max.with_month(beyond_max.month()), Some(beyond_max));
assert_eq!(beyond_max.with_month(3), None);
assert_eq!(beyond_max.with_month0(beyond_max.month0()), Some(beyond_max));
Expand All @@ -1461,6 +1463,56 @@ fn test_min_max_setters() {
assert_eq!(beyond_max.with_nanosecond(beyond_max.nanosecond()), Some(beyond_max));
}

#[test]
fn test_min_max_add_days() {
let offset_min = FixedOffset::west_opt(2 * 60 * 60).unwrap();
let beyond_min = offset_min.from_utc_datetime(&NaiveDateTime::MIN);
let offset_max = FixedOffset::east_opt(2 * 60 * 60).unwrap();
let beyond_max = offset_max.from_utc_datetime(&NaiveDateTime::MAX);
let max_time = NaiveTime::from_hms_nano_opt(23, 59, 59, 999_999_999).unwrap();

assert_eq!(beyond_min.checked_add_days(Days::new(0)), Some(beyond_min));
assert_eq!(
beyond_min.checked_add_days(Days::new(1)),
Some(offset_min.from_utc_datetime(&(NaiveDate::MIN + Days(1)).and_time(NaiveTime::MIN)))
);
assert_eq!(beyond_min.checked_sub_days(Days::new(0)), Some(beyond_min));
assert_eq!(beyond_min.checked_sub_days(Days::new(1)), None);

assert_eq!(beyond_max.checked_add_days(Days::new(0)), Some(beyond_max));
assert_eq!(beyond_max.checked_add_days(Days::new(1)), None);
assert_eq!(beyond_max.checked_sub_days(Days::new(0)), Some(beyond_max));
assert_eq!(
beyond_max.checked_sub_days(Days::new(1)),
Some(offset_max.from_utc_datetime(&(NaiveDate::MAX - Days(1)).and_time(max_time)))
);
}

#[test]
fn test_min_max_add_months() {
let offset_min = FixedOffset::west_opt(2 * 60 * 60).unwrap();
let beyond_min = offset_min.from_utc_datetime(&NaiveDateTime::MIN);
let offset_max = FixedOffset::east_opt(2 * 60 * 60).unwrap();
let beyond_max = offset_max.from_utc_datetime(&NaiveDateTime::MAX);
let max_time = NaiveTime::from_hms_nano_opt(23, 59, 59, 999_999_999).unwrap();

assert_eq!(beyond_min.checked_add_months(Months::new(0)), Some(beyond_min));
assert_eq!(
beyond_min.checked_add_months(Months::new(1)),
Some(offset_min.from_utc_datetime(&(NaiveDate::MIN + Months(1)).and_time(NaiveTime::MIN)))
);
assert_eq!(beyond_min.checked_sub_months(Months::new(0)), Some(beyond_min));
assert_eq!(beyond_min.checked_sub_months(Months::new(1)), None);

assert_eq!(beyond_max.checked_add_months(Months::new(0)), Some(beyond_max));
assert_eq!(beyond_max.checked_add_months(Months::new(1)), None);
assert_eq!(beyond_max.checked_sub_months(Months::new(0)), Some(beyond_max));
assert_eq!(
beyond_max.checked_sub_months(Months::new(1)),
Some(offset_max.from_utc_datetime(&(NaiveDate::MAX - Months(1)).and_time(max_time)))
);
}

#[test]
#[should_panic]
fn test_local_beyond_min_datetime() {
Expand Down
8 changes: 6 additions & 2 deletions src/naive/date/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,10 +699,14 @@ impl NaiveDate {

/// Add a duration of `i32` days to the date.
pub(crate) const fn add_days(self, days: i32) -> Option<Self> {
// fast path if the result is within the same year
// Fast path if the result is within the same year.
// Also `DateTime::checked_(add|sub)_days` relies on this path, because if the value remains
// within the year it doesn't do a check if the year is in range.
// This way `DateTime:checked_(add|sub)_days(Days::new(0))` can be a no-op on dates were the
// local datetime is beyond `NaiveDate::{MIN, MAX}.
const ORDINAL_MASK: i32 = 0b1_1111_1111_0000;
if let Some(ordinal) = ((self.yof() & ORDINAL_MASK) >> 4).checked_add(days) {
if ordinal > 0 && ordinal <= 365 {
if ordinal > 0 && ordinal <= (365 + self.leap_year() as i32) {
let year_and_flags = self.yof() & !ORDINAL_MASK;
return Some(NaiveDate::from_yof(year_and_flags | (ordinal << 4)));
}
Expand Down