chronotope / chrono-tz

TimeZone implementations for rust-chrono from the IANA database
Other
228 stars 55 forks source link

Daylight savings time doesn't appear to be taken into account after the year 2099 #155

Closed ethan-readyset closed 3 months ago

ethan-readyset commented 3 months ago

Background When testing with the America/New_York time zone, I'm seeing some surprising results when it comes to daylight savings times. Since 2007 in the United States, daylight savings time begins on the second Sunday in March at 02:00 local time. My testing shows that, for the years between 2007 and 2099 (inclusive), the following passes:

let date = chrono::NaiveDate::from_ymd_opt(YEAR, 3, SECOND_SUNDAY).unwrap();
let naive_dt = NaiveDateTime::new(date, time);
assert!(TIMEZONE.from_local_datetime(&naive_dt).single().is_none());

Adding one day to the original naive_dt produces a timestamp with timezone EDT, which is the America/New_York timezone when daylight savings is active.

However, beginning with year 2100 for datetimes on the second Sunday in March at 02:00, TIMEZONE.from_local_datetime(&naive_dt).single() returns Some and produces a datetime with the timezone EST, which is the America/New_York timezone when daylight savings is not active. Timestamps after this one but before the end of daylight savings within the same year continue to have EST timezones, even though I'd expect them to be EDT.

Is this a known limitation of chrono-tz, or am I just misunderstanding how daylight savings time should work?

Minimal Reproducible Example

use chrono::{DateTime, Days, Months, NaiveDateTime, NaiveTime, TimeZone};
use chrono_tz::{OffsetComponents, Tz};

// Daylight savings always starts exactly at 02:00 local time in most time zones in the United
// States
const TIMEZONE: Tz = Tz::America__New_York;

fn main() {
    let time = NaiveTime::from_hms_opt(2, 0, 0).unwrap();

    // 2099-03-08 is the second Sunday of the month, so this time is not representable
    let date = chrono::NaiveDate::from_ymd_opt(2099, 3, 8).unwrap();
    let naive_dt = NaiveDateTime::new(date, time);
    assert!(TIMEZONE.from_local_datetime(&naive_dt).single().is_none());

    // Adding a day produces a timestamp with EDT and DST offset of 1 hour, which is correct
    let naive_dt = NaiveDateTime::new(date.succ_opt().unwrap(), time);
    let next_day = TIMEZONE.from_local_datetime(&naive_dt).single().unwrap();
    assert_timezone_and_offset(&next_day, "EDT", 3600);

    // 2100-03-14 is the second Sunday and thus the start of DST, so this should be None as it is
    // above. However, it's not, and we can unwrap() it
    let date = chrono::NaiveDate::from_ymd_opt(2100, 3, 14).unwrap();
    let naive_dt = NaiveDateTime::new(date, time);
    let dt = TIMEZONE.from_local_datetime(&naive_dt).single().unwrap();
    // The timezone is EST and the DST offset is zero
    assert_timezone_and_offset(&dt, "EST", 0);

    // Adding a day to the date still produces a timestamp with EST and no DST offset
    let next_day = dt.checked_add_days(Days::new(1)).unwrap();
    assert_timezone_and_offset(&next_day, "EST", 0);

    // Adding a month to the date still produces a timestamp with EST and no DST offset, even
    // though we should be well into EDT by now
    let next_month = dt.checked_add_months(Months::new(1)).unwrap();
    assert_timezone_and_offset(&next_month, "EST", 0);
}

fn assert_timezone_and_offset(dt: &DateTime<Tz>, tz: &str, offset: i64) {
    assert_eq!(dt.format("%Z").to_string(), tz);
    assert_eq!(dt.offset().dst_offset().num_seconds(), offset);
}

Versions rustc: 1.70.0-nightly chrono: 0.4.37 chrono-tz: 0.8.6

ethan-readyset commented 3 months ago

Upon further digging, it appears this is due to zoneinfo_parse: https://github.com/chronotope/parse-zoneinfo/blob/95f488543b3a093890c004c615c74717a76bc8a1/src/transitions.rs#L15-L20

djc commented 3 months ago

Ugh, sorry for that -- we should probably reconsider that choice.

westy92 commented 3 months ago

This is likely the cause of: https://github.com/chronotope/chrono-tz/issues/135.

djc commented 3 months ago

Maybe we should expose some API from chrono (which has a parser internally) so we can use that?

pitdicker commented 3 months ago

This are two things that I hope to work on, but there are plenty of things to do first for the coming months. Chrono already has most of the code to use the system time zone database, we just don't expose it yet.

And I've got the basics down for a rewrite of this crate that uses rules instead of a large table of transition dates. That would reduce the size added to the binary by ca. 90% and allow us to handle dates without an arbitrary cutoff point such as 2099.

pitdicker commented 3 months ago

Closing as a duplicate of #135. Thank you @westy92 for investigating!