Drop the `Datelike` and `Timelike` traits

[pitdicker]

Chrono would become more correct and convenient if the Datelike and Timelike traits were removed in 0.5.

I propose to implement their methods directly on the types that currently implement the traits.

• Is anyone even depending on these traits? Writing code that is generic over types implementing Datelike or Timelike is asking for incorrect results and errors. It kind of defeats the thoughtful API of chrono to write generic code that mixes types like that.
• When implemented on DateTime the with_ method should return LocalResult, so library users can choose how to deal with ambiguous or non-existing local datetimes. The current workaround to use them is cumbersome. See https://github.com/chronotope/chrono/issues/1049#issuecomment-1537157454 example 4.
• Trait methods can't be made const for the foreeseable future, which would be a gap in the const API of chrono (once it has one that is).
• The methods are currently less discoverable, but this can be considered a documentation issue.
• Library users would need to do one import less because the methods would just be available on the type.

[jtmoon79]

FWIW I also get hung up on the Datelike and Timelike traits; e.g. I spend tens of minutes searching for something that I know I've seen work elsewhere to then discover I need to use ...Datelike or Timelike. It's happened a few times and it's a little bit exasperating. At the same time, this could be fixed with better documentation.

The methods are currently less discoverable, but this can be considered a documentation issue.

[djc]

I'm not very convinced by your arguments so far to remove them. Abstracting over a Date and the date components of a Time seems conceptually reasonable to me, if maybe something that doesn't come up a lot.

However, if you can show that over some large body of Rust code there is negligible code that abstracts over Datelike or Timelike (with T: Datelike or impl Datelike bounds), we could consider it.

[olliemath]

My 2c: as the author of a library depending on chrono it's useful for me to offer a single function that takes a T: Datelike, rather than 3 boilerplate functions that take the various concrete date types

[pitdicker]

My thoughts have changed a bit on this issue. I now think it is best to shrink the Datelike and Timelike traits. They currently offer getter and setter methods.

The getter methods are fine as they are:

pub trait Datelike: Sized {
    // Required methods
    fn year(&self) -> i32;
    fn month(&self) -> u32;
    fn month0(&self) -> u32;
    fn day(&self) -> u32;
    fn day0(&self) -> u32;
    fn ordinal(&self) -> u32;
    fn ordinal0(&self) -> u32;
    fn weekday(&self) -> Weekday;
    fn iso_week(&self) -> IsoWeek;

    // Provided methods
    fn year_ce(&self) -> (bool, u32) { ... }
    fn num_days_from_ce(&self) -> i32 { ... }
}

pub trait Timelike: Sized {
    // Required methods
    fn hour(&self) -> u32;
    fn minute(&self) -> u32;
    fn second(&self) -> u32;
    fn nanosecond(&self) -> u32;

    // Provided methods
    fn hour12(&self) -> (bool, u32) { ... }
    fn num_seconds_from_midnight(&self) -> u32 { ... }
}

The setters are where the problem is. They are fallible, and depending on the type they are fallible for different reasons. I am pretty sure any code that is abstracting over Datelike or Timelike types and modifying them is hiding errors. If a goal of chrono is to help users write correct code involving dates/times it would be better to not have generic setters in my opinion.

This means we would move the following methods to types:

impl NaiveDate {
    pub fn with_year(&self, year: i32) -> Result<NaiveDate, Error>;
    pub fn with_month(&self, month: u32) -> Result<NaiveDate, Error>;
    pub fn with_month0(&self, month0: u32) -> Result<NaiveDate, Error>;
    pub fn with_day(&self, day: u32) -> Result<NaiveDate, Error>;
    pub fn with_day0(&self, day0: u32) -> Result<NaiveDate, Error>;
    pub fn with_ordinal(&self, ordinal: u32) -> Result<NaiveDate, Error>;
    pub fn with_ordinal0(&self, ordinal0: u32) -> Result<NaiveDate, Error>;
}

impl NaiveTime {
    pub fn with_hour(&self, hour: u32) -> Result<NaiveTime, Error>;
    pub fn with_minute(&self, min: u32) -> Result<NaiveTime, Error>;
    pub fn with_second(&self, sec: u32) -> Result<NaiveTime, Error>;
    pub fn with_nanosecond(&self, nano: u32) -> Result<NaiveTime, Error>;
}

impl NaiveDateTime {
    pub fn with_year(&self, year: i32) -> Result<NaiveDateTime, Error>;
    pub fn with_month(&self, month: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_month0(&self, month0: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_day(&self, day: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_day0(&self, day0: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_ordinal(&self, ordinal: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_ordinal0(&self, ordinal0: u32) -> Result<NaiveDateTime, Error>;

    pub fn with_hour(&self, hour: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_minute(&self, min: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_second(&self, sec: u32) -> Result<NaiveDateTime, Error>;
    pub fn with_nanosecond(&self, nano: u32) -> Result<NaiveDateTime, Error>;
}

impl<Tz: TimeZone> DateTime<Tz> {
    pub fn with_year(&self, year: i32) -> LocalResult<Self>;
    pub fn with_month(&self, month: u32) -> LocalResult<Self>;
    pub fn with_month0(&self, month0: u32) -> LocalResult<Self>;
    pub fn with_day(&self, day: u32) -> LocalResult<Self>;
    pub fn with_day0(&self, day0: u32) -> LocalResult<Self>;
    pub fn with_ordinal(&self, ordinal: u32) -> LocalResult<Self>;
    pub fn with_ordinal0(&self, ordinal0: u32) -> LocalResult<Self>;

    pub fn with_hour(&self, hour: u32) -> LocalResult<Self>;
    pub fn with_minute(&self, min: u32) -> LocalResult<Self>;
    pub fn with_second(&self, sec: u32) -> LocalResult<Self>;
    pub fn with_nanosecond(&self, nano: u32) -> LocalResult<Self>;
}

[Zomtir]

Sidenote about LocalResult as return type for DateTime<Tz>:

How about returning Result<LocalDatetime<T>> instead? This prevents LocalResult<T>::None from masking the error cause. LocalDatetime would be the same as LocalResult sans the None variant, ideally as a drop-in replacement.

pub enum LocalDatetime<T> {
    /// Given local time representation has a single unique result.
    Single(T),
    /// Given local time representation has multiple results and thus ambiguous.
    /// This can occur when, for example, the negative timezone transition.
    Ambiguous(T /*min*/, T /*max*/),
}

[djc]

The setters are where the problem is. They are fallible, and depending on the type they are fallible for different reasons. I am pretty sure any code that is abstracting over Datelike or Timelike types and modifying them is hiding errors. If a goal of chrono is to help users write correct code involving dates/times it would be better to not have generic setters in my opinion.

Okay, so the main issue here is that we different return type for DateTime<Tz> because it needs to yield LocalResult (or whatever), to cope with possible DST transitions?

I think I can live with moving the setters out of the traits on 0.5.x. I expect their usage is pretty rare anyway.