Panic when shifting to a DST transition

[ebegumisa]

Firstly, thanks for the library.

Problem

I'm in a timezone which regrettably uses daylight saving tIme.

When taking a chrono DST timezone datetime and attempting to apply chronoutils shift utility functions (either directly or indirectly via a relative addition), chronoutils panics if the computed datetime lands on a DST transition.

Examples

    use chrono::{TimeZone, LocalResult};

    #[test] 
    fn test_shift_months_datetime_to_dst_backward_transition() {

        let dst_tz = &chrono_tz::Australia::Melbourne;

        // On Apr 5th 2020 after 02:59:59, clocks were wound back to 02:00:00 making 02:00::00 to
        // 02:59:59 ambiguous.
        // <https://www.timeanddate.com/time/change/australia/melbourne?year=2020>

        base = dst_tz.with_ymd_and_hms(2020, 3, 5, 2, 00, 0).single().unwrap();
        shift_months(base, 1); 
     // ^^^^^^^^^^^^^^^^^^^^^ panics due to shift to DST ambiguous datetime
    }

    #[test]
    fn test_shift_months_datetime_to_dst_forward_transition() {

        let dst_tz = &chrono_tz::Australia::Melbourne;

        // On Oct 4th 2020 after 01:59:59, clocks were advanced to 03:00:00 making 02:00:00 to 
        // 02:59:59 non-existent.
        // <https://www.timeanddate.com/time/change/australia/melbourne?year=2020>   

        let base = dst_tz.with_ymd_and_hms(2020, 9, 4, 2, 00, 0).single().unwrap();
        shift_months(base, 1); 
     // ^^^^^^^^^^^^^^^^^^^^^ panics due to shift to DST non-existent datetime
    }

Cause

These panics are due to the shift utils unwrapping calls to the chrono::Datelike::with_* functions, with the latter's doc currently stating:

Returns None when: - The resulting date does not exist (for example day(31) in April). - _In case of [`DateTime`](https://docs.rs/chrono/latest/chrono/struct.DateTime.html) if the resulting date and time **fall within a timezone transition** such as from DST to standard time._ - The value for day is out of range.

pub fn shift_months<D: Datelike>(date: D, months: i32) -> D {

...

   if day <= 28 {
        date.with_day(day)
            .unwrap() 
          // ^^^^^^^^ could panic due to DST
            .with_month(month as u32)
            .unwrap() // <<< Ditto
            .with_year(year)
            .unwrap() // <<< Ditto
    } else {
        date.with_day(1)
            .unwrap() // <<< Ditto
            .with_month(month as u32)
            .unwrap() // <<< Ditto
            .with_year(year)
            .unwrap() // <<< Ditto
            .with_day(day)
            .unwrap() // <<< Ditto
    }
...

Proposed Solution

It would be helpful to have *_opt versions of the shift utils which return an Option rather than a Datelike, for example

pub fn shift_months_opt( .. ) -> Option<Datelike>

This way, the caller can choose how to handle DST issues. A returned None won't help the caller distinguish nonsensical input args from args which result in a DST transition, but callers should be able to determine that for themselves.

[ebegumisa]

I am working an a pull request to resolve this issue.

[olliemath]

Hey, thanks for the comprehensive issue.

Ah, that's annoying that the fold is not handled by chrono (usually the way you resolve this with datetime types is by having a "fold" attribute which is either 0 or 1, depending on if it's the first or second 2am).

A shift_months_opt sounds like a good start. The larger solution will probably be to emulate the way that chrono handles the addition of regular duration to a datetime around the fold. If you can get shift_months_opt looking good I'm happy to add them and will investigate investigate improving the RelativeDuration for the 0.3 release

[ebegumisa]

Thanks @olliemath,

I've sent the PR #12 adding the shift_months_opt and related functions.

[olliemath]

shift_months_opt is now published under 0.2.6 - I'm going to keep this issue open to track progress on RelativeDelta - which I'll plan for 0.3.0

[ebegumisa]

shift_months_opt is now published under 0.2.6 - I'm going to keep this issue open to track progress on RelativeDelta - which I'll plan for 0.3.0

Great @olliemath . Much appreciated.