Skip to content

Commit

Permalink
zoned: treat all offsets attached to gaps in civil time as invalid
Browse files Browse the repository at this point in the history 
This will cause Jiff to reject some datetime strings that it
previously accepted. This also brings Jiff into line with how
Temporal behaves in these case. It is overall safe because
it avoids misinterpreting datetimes in the future that are
serialized before a change to DST. For example, imagine serializing
`2006-04-02T02:30-05[America/Indiana/Vevay]` before you knew that
Indiana/Vevay was going to switch to DST (they hadn't previously used
DST since 1972). Before that switch, it was perfectly reasonable to
think that the datetime was valid. But after the switch, it corresponded
to a gap. Jiff will now reject this.

Users can still of course change the offset conflict resolution strategy
depending on what they want to do (reject, keep exact time or keep civil
time).

Fixes #212
  • Loading branch information
@BurntSushi
BurntSushi committed Feb 2, 2025
1 parent 36d80d4 commit a7216bc
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 87 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@ Change the behavior of the deprecated `%V` conversion specifier in
`jiff::fmt::strtime` from formatting an IANA time zone identifier to formatting
an ISO 8601 week number. To format an IANA time zone identifier, use `%Q` or
`%:Q` (which were introduced in `jiff 0.1`).
* [#212](https://github.com/BurntSushi/jiff/issues/212):
When parsing into a `Zoned` with a civil time corresponding to a gap, we treat
all offsets as invalid and return an error. Previously, we would accept the
offset as given. This brings us into line with Temporal's behavior. For
example, previously Jiff accepted `2006-04-02T02:30-05[America/Indiana/Vevay]`
but will now return an error. This is desirable for cases where a datetime in
the future is serialized before a change in the daylight saving time rules.
For more examples, see `jiff::fmt::temporal::DateTimeParser::offset_conflict`
for details on how to change Jiff's default behavior. This behavior change also
applies to `tz::OffsetConflict::PreferOffset`.
* [#218](https://github.com/BurntSushi/jiff/issues/218):
In order to make naming a little more consistent between `Zoned`
and `civil::Date`, the `civil::Date::to_iso_week_date` and
Expand Down
75 changes: 74 additions & 1 deletion src/fmt/temporal/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl DateTimeParser {
/// whenever parsing a datetime with an offset that is inconsistent with
/// the time zone.
///
/// # Example
/// # Example: respecting offsets even when they're invalid
///
/// ```
/// use jiff::{civil::date, fmt::temporal::DateTimeParser, tz};
Expand All @@ -297,6 +297,79 @@ impl DateTimeParser {
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
///
/// # Example: all offsets are invalid for gaps in civil time by default
///
/// When parsing a datetime with an offset for a gap in civil time, the
/// offset is treated as invalid. This results in parsing failing. For
/// example, some parts of Indiana in the US didn't start using daylight
/// saving time until 2006. If a datetime for 2006 were serialized before
/// the updated daylight saving time rules were known, then this parse
/// error will prevent you from silently changing the originally intended
/// time:
///
/// ```
/// use jiff::{fmt::temporal::DateTimeParser};
///
/// static PARSER: DateTimeParser = DateTimeParser::new();
///
/// // DST in Indiana/Vevay began at 2006-04-02T02:00 local time.
/// // The last time Indiana/Vevay observed DST was in 1972.
/// let result = PARSER.parse_zoned(
/// "2006-04-02T02:30-05[America/Indiana/Vevay]",
/// );
/// assert_eq!(
/// result.unwrap_err().to_string(),
/// "parsing \"2006-04-02T02:30-05[America/Indiana/Vevay]\" failed: \
/// datetime 2006-04-02T02:30:00 could not resolve to timestamp \
/// since 'reject' conflict resolution was chosen, and because \
/// datetime has offset -05, but the time zone America/Indiana/Vevay \
/// for the given datetime falls in a gap \
/// (between offsets -05 and -04), \
/// and all offsets for a gap are regarded as invalid",
/// );
/// ```
///
/// If one doesn't want an error here, then you can either prioritize the
/// instant in time by respecting the offset:
///
/// ```
/// use jiff::{fmt::temporal::DateTimeParser, tz};
///
/// static PARSER: DateTimeParser = DateTimeParser::new()
/// .offset_conflict(tz::OffsetConflict::AlwaysOffset);
///
/// let zdt = PARSER.parse_zoned(
/// "2006-04-02T02:30-05[America/Indiana/Vevay]",
/// )?;
/// assert_eq!(
/// zdt.to_string(),
/// "2006-04-02T03:30:00-04:00[America/Indiana/Vevay]",
/// );
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
///
/// or you can force your own disambiguation rules, e.g., by taking the
/// earlier time:
///
/// ```
/// use jiff::{fmt::temporal::DateTimeParser, tz};
///
/// static PARSER: DateTimeParser = DateTimeParser::new()
/// .disambiguation(tz::Disambiguation::Earlier)
/// .offset_conflict(tz::OffsetConflict::AlwaysTimeZone);
///
/// let zdt = PARSER.parse_zoned(
/// "2006-04-02T02:30-05[America/Indiana/Vevay]",
/// )?;
/// assert_eq!(
/// zdt.to_string(),
/// "2006-04-02T01:30:00-05:00[America/Indiana/Vevay]",
/// );
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
#[inline]
pub const fn offset_conflict(
self,
Expand Down
86 changes: 16 additions & 70 deletions src/tz/offset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1934,45 +1934,10 @@ impl OffsetConflict {

let amb = tz.to_ambiguous_timestamp(dt);
match amb.offset() {
// Basically, if the offset parsed matches one of our ambiguous
// offsets, then the offset is used to resolve the ambiguity and
// yields an unambiguous result. But in every other case, the
// offset parsed is completely ignored. (And this may result in
// returning an ambiguous instant.)
//
// Note that gaps are a little weird here. We always pick the
// "before" offset even when it doesn't match what's `given`.
// This matches what Temporal does, and I think makes more
// intuitive sense. For example, if we have
//
// 2024-03-10T01:30:00-05:00[US/Eastern]
//
// And set the hour to `2`, then it makes sense to result in a
// clock time of `03:30` in offset `-04`. Moreover, if we always
// took what was given, then starting with
//
// 2024-03-31T12:00:00+00:00[Atlantic/Azores]
//
// And setting the hour to `0` would result in
//
// 2024-03-30T23:00:00-01:00[Atlantic/Azores]
//
// which is highly unintuitive. In the other direction, we might
// start with
//
// 2024-03-10T03:30:00-04:00[US/Eastern]
//
// And setting the hour to `2` still results in the clock time
// being set to `03:30`, i.e., not changing in this instance.
//
// See also: https://github.com/tc39/proposal-temporal/issues/3078
// See also: https://github.com/BurntSushi/jiff/issues/211
Gap { before, after }
if is_equal(given, before) || is_equal(given, after) =>
{
let kind = Unambiguous { offset: before };
AmbiguousTimestamp::new(dt, kind)
}
// We only look for folds because we consider all offsets for gaps
// to be invalid. Which is consistent with how they're treated as
// `OffsetConflict::Reject`. Thus, like any other invalid offset,
// we fallback to disambiguation (which is handled by the caller).
Fold { before, after }
if is_equal(given, before) || is_equal(given, after) =>
{
Expand Down Expand Up @@ -2014,43 +1979,24 @@ impl OffsetConflict {
tzname = tz.diagnostic_name(),
)),
Unambiguous { .. } => Ok(amb.into_ambiguous_zoned(tz)),
Gap { before, after }
if !is_equal(given, before) && !is_equal(given, after) =>
{
// Temporal actually seems to report an error whenever a gap
// is found, even if the parsed offset matches one of the two
// offsets in the gap. I think the reasoning is because the
// parsed offset and offsets from the gap will flip-flop, thus
// resulting in a datetime with an offset distinct from the one
// given. Here's an example. Consider this datetime:
//
// 2024-03-10T02:30-04:00[America/New_York]
//
// This occurs in the EST->EDT transition gap, such that
// `02:30` never actually appears on a clock for folks in
// `America/New_York`. The `-04` offset means that the
// timestamp this corresponds to is unambiguous. Namely, it is:
//
// 2024-03-10T06:30Z
//
// This instant, when converted to `America/New_York`, is:
//
// 2024-03-10T01:30-05:00[America/New_York]
//
// As you can see, the offset flip-flopped. The same flip-flop
// happens the other way if you use `02:30-05:00`. Presumably,
// Temporal errors here because the offset changes. But the
// instant in time is the same *and* it is unambiguous. So it's
// not clear to me why we ought to error in this case.
Gap { before, after } => {
// In `jiff 0.1`, we reported an error when we found a gap
// where neither offset matched what was given. But now we
// report an error whenever we find a gap, as we consider
// all offsets to be invalid for the gap. This now matches
// Temporal's behavior which I think is more consistent. And in
// particular, this makes it more consistent with the behavior
// of `PreferOffset` when a gap is found (which was also
// changed to treat all offsets in a gap as invalid).
//
// Ref: https://github.com/tc39/proposal-temporal/issues/2892
Err(err!(
"datetime {dt} could not resolve to timestamp \
since 'reject' conflict resolution was chosen, and \
because datetime has offset {given}, but the time \
zone {tzname} for the given datetime falls in a gap \
between offsets {before} and {after}, neither of which \
match the offset",
(between offsets {before} and {after}), and all \
offsets for a gap are regarded as invalid",
tzname = tz.diagnostic_name(),
))
}
Expand All @@ -2067,7 +2013,7 @@ impl OffsetConflict {
tzname = tz.diagnostic_name(),
))
}
Gap { .. } | Fold { .. } => {
Fold { .. } => {
let kind = Unambiguous { offset: given };
Ok(AmbiguousTimestamp::new(dt, kind).into_ambiguous_zoned(tz))
}
Expand Down
70 changes: 54 additions & 16 deletions src/zoned.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5166,10 +5166,9 @@ impl ZonedWith {
///
/// # Example: offset conflict resolution and disambiguation
///
/// This example shows how to set the disambiguation configuration can
/// interact with the default offset conflict resolution strategy in
/// unintuitive ways. For example, here, we request the "earlier" datetime
/// whenever there's an ambiguity:
/// This example shows how the disambiguation configuration can
/// interact with the default offset conflict resolution strategy of
/// [`OffsetConflict::PreferOffset`]:
///
/// ```
/// use jiff::{civil::date, tz, Zoned};
Expand All @@ -5183,19 +5182,42 @@ impl ZonedWith {
/// .disambiguation(tz::Disambiguation::Earlier)
/// .day(10)
/// .build()?;
/// assert_eq!(zdt2.datetime(), date(2024, 3, 10).at(1, 5, 0, 0));
/// assert_eq!(zdt2.offset(), tz::offset(-5));
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
///
/// Namely, while we started with an offset of `-04`, it (along with all
/// other offsets) are considered invalid during civil time gaps due to
/// time zone transitions (such as the beginning of daylight saving time in
/// most locations).
///
/// The default disambiguation strategy is
/// [`Disambiguation::Compatible`], which in the case of gaps, chooses the
/// time after the gap:
///
/// ```
/// use jiff::{civil::date, tz, Zoned};
///
/// // This datetime is unambiguous.
/// let zdt1 = "2024-03-11T02:05[America/New_York]".parse::<Zoned>()?;
/// assert_eq!(zdt1.offset(), tz::offset(-4));
/// // But the same time on March 10 is ambiguous because there is a gap!
/// let zdt2 = zdt1
/// .with()
/// .day(10)
/// .build()?;
/// assert_eq!(zdt2.datetime(), date(2024, 3, 10).at(3, 5, 0, 0));
/// assert_eq!(zdt2.offset(), tz::offset(-4));
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
///
/// Yet instead, we get the later time, after the gap. This is because
/// the default offset conflict resolution strategy is
/// [`OffsetConflict::PreferOffset`], and therefore, the offset in the
/// current zoned datetime is prioritized. In the example above, the
/// offset of `zdt1` is `-04`, since it comes after DST takes effect in
/// `America/New_York`. One can override the offset conflict resolution
/// to force disambiguation in cases like this:
/// Alternatively, one can choose to always respect the offset, and thus
/// civil time for the provided time zone will be adjusted to match the
/// instant prescribed by the offset. In this case, no disambiguation is
/// performed:
///
/// ```
/// use jiff::{civil::date, tz, Zoned};
Expand All @@ -5206,11 +5228,12 @@ impl ZonedWith {
/// // But the same time on March 10 is ambiguous because there is a gap!
/// let zdt2 = zdt1
/// .with()
/// .disambiguation(tz::Disambiguation::Earlier)
/// // ignore any offset given
/// .offset_conflict(tz::OffsetConflict::AlwaysTimeZone)
/// .offset_conflict(tz::OffsetConflict::AlwaysOffset)
/// .day(10)
/// .build()?;
/// // Why do we get this result? Because `2024-03-10T02:05-04` is
/// // `2024-03-10T06:05Z`. And in `America/New_York`, the civil time
/// // for that timestamp is `2024-03-10T01:05-05`.
/// assert_eq!(zdt2.datetime(), date(2024, 3, 10).at(1, 5, 0, 0));
/// assert_eq!(zdt2.offset(), tz::offset(-5));
///
Expand Down Expand Up @@ -5454,8 +5477,9 @@ mod tests {
let zdt2 = zdt1.with().hour(2).build().unwrap();
assert_eq!(zdt2.to_string(), "2024-03-10T03:30:00-04:00[US/Eastern]");

// This is a current difference from Temporal. Temporal ignores the
// disambiguation setting (and the bad offset).
// I originally thought that this was difference from Temporal. Namely,
// I thought that Temporal ignored the disambiguation setting (and the
// bad offset). But it doesn't. I was holding it wrong.
//
// See: https://github.com/tc39/proposal-temporal/issues/3078
let zdt1: Zoned = "2024-03-10T01:30[US/Eastern]".parse().unwrap();
Expand All @@ -5468,6 +5492,20 @@ mod tests {
.build()
.unwrap();
assert_eq!(zdt2.to_string(), "2024-03-10T01:30:00-05:00[US/Eastern]");

// This should also respect the disambiguation setting even without
// explicitly specifying an invalid offset. This is becaue `02:30-05`
// is regarded as invalid since `02:30` isn't a valid civil time on
// this date in this time zone.
let zdt1: Zoned = "2024-03-10T01:30[US/Eastern]".parse().unwrap();
assert_eq!(zdt1.to_string(), "2024-03-10T01:30:00-05:00[US/Eastern]");
let zdt2 = zdt1
.with()
.hour(2)
.disambiguation(Disambiguation::Earlier)
.build()
.unwrap();
assert_eq!(zdt2.to_string(), "2024-03-10T01:30:00-05:00[US/Eastern]");
}

#[test]
Expand Down

0 comments on commit a7216bc

Please sign in to comment.