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

Years type implementation #900

Closed
wants to merge 3 commits into from
Closed
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
12 changes: 12 additions & 0 deletions src/date.rs
Expand Up @@ -23,6 +23,8 @@ use crate::oldtime::Duration as OldDuration;
use crate::DateTime;
use crate::{Datelike, Weekday};

use crate::Years;

Copy link
Collaborator

Choose a reason for hiding this comment

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

The Date type is deprecated, better to not add new methods to it.

/// ISO 8601 calendar date with time zone.
///
/// You almost certainly want to be using a [`NaiveDate`] instead of this type.
Expand Down Expand Up @@ -368,6 +370,16 @@ where
) -> DelayedFormat<StrftimeItems<'a>> {
self.format_localized_with_items(StrftimeItems::new_with_locale(fmt, locale), locale)
}

pub fn checked_add_years(mut self, num_years: Years) -> Option<Date<Tz>> {
self.date = self.date.checked_add_years(num_years)?;
Some(self)
}

pub fn checked_sub_years(mut self, num_years: Years) -> Option<Date<Tz>> {
self.date = self.date.checked_sub_years(num_years)?;
Some(self)
}
}

impl<Tz: TimeZone> Datelike for Date<Tz> {
Expand Down
12 changes: 12 additions & 0 deletions src/datetime/mod.rs
Expand Up @@ -35,6 +35,8 @@ use crate::Date;
use crate::Months;
use crate::{Datelike, Timelike, Weekday};

use crate::Years;

#[cfg(feature = "rkyv")]
use rkyv::{Archive, Deserialize, Serialize};

Expand Down Expand Up @@ -743,6 +745,16 @@ where
) -> DelayedFormat<StrftimeItems<'a>> {
self.format_localized_with_items(StrftimeItems::new_with_locale(fmt, locale), locale)
}

pub fn checked_add_years(mut self, num_years: Years) -> Option<DateTime<Tz>> {
self.datetime = self.datetime.checked_add_years(num_years)?;
Some(self)
}

pub fn checked_sub_years(mut self, num_years: Years) -> Option<DateTime<Tz>> {
self.datetime = self.datetime.checked_sub_years(num_years)?;
Some(self)
}
}

impl<Tz: TimeZone> Datelike for DateTime<Tz> {
Expand Down
6 changes: 6 additions & 0 deletions src/lib.rs
Expand Up @@ -492,6 +492,12 @@ pub use weekday::{ParseWeekdayError, Weekday};
mod month;
pub use month::{Month, Months, ParseMonthError};

// I'm not entirely sure too this should be here, because months
// was implemented at this level and Days at naive level
// I believe here might be better since it should help avoid coupling
pub use years::Years;
mod years; // add pub uses

mod traits;
pub use traits::{Datelike, Timelike};

Expand Down
35 changes: 35 additions & 0 deletions src/naive/date.rs
Expand Up @@ -27,6 +27,8 @@ use crate::naive::{IsoWeek, NaiveDateTime, NaiveTime};
use crate::oldtime::Duration as OldDuration;
use crate::{Datelike, Duration, Weekday};

use crate::Years;

use super::internals::{self, DateImpl, Mdf, Of, YearFlags};
use super::isoweek;

Expand Down Expand Up @@ -1248,6 +1250,23 @@ impl NaiveDate {
NaiveWeek { date: *self, start }
}

// I think self here is a copy, but I still have to make sure it's
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is a copy, you only want to take self by reference for DateTime<Tz>.

pub fn checked_add_years(self, years: Years) -> Option<NaiveDate> {
if years.0 == 0 {
return Some(self);
}
let num_years = i32::try_from(years.0).ok()?;
self.with_year(self.year() + num_years)
Copy link
Collaborator

@pitdicker pitdicker Jul 6, 2023

Choose a reason for hiding this comment

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

This will fail if you add 1 year to February 29th. This is different from checked_add_months, which will fall back to the last day of the month. Shouldn't they work the same?

Copy link
Collaborator

@pitdicker pitdicker Jul 6, 2023

Choose a reason for hiding this comment

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

How do you handle integer overflow in for example self.year() + i32::MAX?

}

pub fn checked_sub_years(self, years: Years) -> Option<NaiveDate> {
if years.0 == 0 {
return Some(self);
}
let num_years = i32::try_from(years.0).ok()?;
self.with_year(self.year() - num_years)
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above.

}

/// The minimum possible `NaiveDate` (January 1, 262145 BCE).
pub const MIN: NaiveDate = NaiveDate { ymdf: (MIN_YEAR << 13) | (1 << 4) | 0o07 /*FE*/ };
/// The maximum possible `NaiveDate` (December 31, 262143 CE).
Expand Down Expand Up @@ -1687,6 +1706,22 @@ impl Sub<Months> for NaiveDate {
}
}

impl Add<Years> for NaiveDate {
type Output = NaiveDate;

fn add(self, years: Years) -> Self::Output {
self.checked_add_years(years).unwrap()
}
}

impl Sub<Years> for NaiveDate {
type Output = NaiveDate;

fn sub(self, years: Years) -> Self::Output {
self.checked_sub_years(years).unwrap()
}
}

impl Add<Days> for NaiveDate {
type Output = NaiveDate;

Expand Down
12 changes: 12 additions & 0 deletions src/naive/datetime/mod.rs
Expand Up @@ -24,6 +24,8 @@ use crate::oldtime::Duration as OldDuration;
use crate::{DateTime, Datelike, LocalResult, Months, TimeZone, Timelike, Weekday};
use core::cmp::Ordering;

use crate::Years;

#[cfg(feature = "rustc-serialize")]
pub(super) mod rustc_serialize;

Expand Down Expand Up @@ -927,6 +929,16 @@ impl NaiveDateTime {
}
}
}

pub fn checked_add_years(mut self, num_years: Years) -> Option<NaiveDateTime> {
self.date = self.date.checked_add_years(num_years)?;
Some(self)
}

pub fn checked_sub_years(mut self, num_years: Years) -> Option<NaiveDateTime> {
self.date = self.date.checked_sub_years(num_years)?;
Some(self)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

NaiveDateTime could also use Add and Sub implementations.


impl Datelike for NaiveDateTime {
Expand Down
58 changes: 58 additions & 0 deletions src/years.rs
@@ -0,0 +1,58 @@
// I usually develop with this clippy flags
// but I can remove once the this module is done
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 when done.

#![warn(clippy::all,
/*#![warn(*/clippy::pedantic,
clippy::perf,
clippy::nursery,
// clippy::cargo,
clippy::unwrap_used,
clippy::expect_used)]

/// TODO
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs documentation.

#[derive(Debug)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make this match the derives and attributes of Month?

pub struct Years(pub(crate) u32); // I believe the fields of the struct should be private

impl Years {
/// TODO
pub fn new(num_years: u32) -> Years {
Self(num_years)
}
}

#[cfg(test)]
mod tests {
use crate::NaiveDate;
use crate::Years;

#[test]
fn checked_adding_years() {
assert_eq!(
NaiveDate::from_ymd_opt(2023, 2, 28).unwrap(),
NaiveDate::from_ymd_opt(2020, 2, 28).unwrap().checked_add_years(Years::new(3)).unwrap()
);
}

#[test]
fn using_add_operator() {
assert_eq!(
NaiveDate::from_ymd_opt(2023, 2, 28).unwrap(),
NaiveDate::from_ymd_opt(2020, 2, 28).unwrap() + Years::new(3)
);
}

#[test]
fn checked_subtracting_years() {
assert_eq!(
NaiveDate::from_ymd_opt(2017, 2, 28).unwrap(),
NaiveDate::from_ymd_opt(2020, 2, 28).unwrap().checked_sub_years(Years::new(3)).unwrap()
);
}

#[test]
fn using_sub_operator() {
assert_eq!(
NaiveDate::from_ymd_opt(2017, 2, 28).unwrap(),
NaiveDate::from_ymd_opt(2020, 2, 28).unwrap() - Years::new(3)
);
}
}