Repository: BurntSushi/jiff (language: rust)

Issue:
`ZonedWith::time` unexpected behaviour
I got an unexpected behaviour of `ZonedWith::time`, I am not sure if I don't understand how it works or if is a bug.

In the time zone `Atlantic/Azores` on `2024/03/31` midnight doesn't exists because of DST.

This is correctly handled when I construct a `Zoned` like:
```rust
date(2024, 3, 31).to_datetime(Time::midnight()).intz("Atlantic/Azores").unwrap()
```
that returns `2024-03-31T01:00:00+00:00[Atlantic/Azores]` but when I try to change the time of a `Zoned` 
```rust
let zdt1 = date(2024, 3, 31).at(12, 00, 0, 0).intz("Atlantic/Azores").unwrap();
let zdt2 = zdt1.with().time(Time::midnight()).build()?;
```
I get `2024-03-30T23:00:00-01:00[Atlantic/Azores]`. In the documentation of `Zoned::with` or `ZonedWith::time` I don't see anything that could hint that is the correct behaviour.

I solve by using `start_of_day()` because I am interested in the midnight and it was probably what should I have reached for from the beginning but I think it is worth to raise this.


Discussion / hints:
This does indeed look like a bug! It is intended to match Temporal's semantics here, which presumably are what you'd expect:

```
>> zdt1 = Temporal.ZonedDateTime.from('2024-03-31T12:00+00[Atlantic/Azores]');
>> zdt1.toString()
"2024-03-31T12:00:00+00:00[Atlantic/Azores]"
>> zdt2 = zdt1.with({hour: 0, minute: 0, second: 0})
>> zdt2.toString()
"2024-03-31T01:00:00+00:00[Atlantic/Azores]"
```

There are some hijinks going on here with respect to [offset conflict resolution](https://docs.rs/jiff/latest/jiff/tz/enum.OffsetConflict.html).
There _might_ be a bug in Temporal related to this as well: https://github.com/tc39/proposal-temporal/issues/3078

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 85d7511..df8fecc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,8 @@ Deprecations:
 
 * [#210](https://github.com/BurntSushi/jiff/pull/210):
   Deprecate `ISOWeekDate::to_date` in favor of `ISOWeekDate::date`.
+* [#211](https://github.com/BurntSushi/jiff/issues/211):
+  Fix unintuitive behavior of `Zoned::with` when time falls in a DST gap.
 
 
 0.1.25 (2025-01-21)
diff --git a/src/tz/offset.rs b/src/tz/offset.rs
index 82058de..b359e10 100644
--- a/src/tz/offset.rs
+++ b/src/tz/offset.rs
@@ -1519,9 +1519,39 @@ impl OffsetConflict {
             // yields an unambiguous result. But in every other case, the
             // offset parsed is completely ignored. (And this may result in
             // returning an ambiguous instant.)
-            Gap { before, after } | Fold { before, after }
-                if given == before || given == after =>
-            {
+            //
+            // 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 given == before || given == after => {
+                let kind = Unambiguous { offset: before };
+                AmbiguousTimestamp::new(dt, kind)
+            }
+            Fold { before, after } if given == before || given == after => {
                 let kind = Unambiguous { offset: given };
                 AmbiguousTimestamp::new(dt, kind)
             }
diff --git a/src/zoned.rs b/src/zoned.rs
index e7aa358..85a81cf 100644
--- a/src/zoned.rs
+++ b/src/zoned.rs
@@ -5088,7 +5088,7 @@ impl ZonedWith {
     /// # Example
     ///
     /// ```
-    /// use jiff::{civil::date, tz, Zoned};
+    /// use jiff::Zoned;
     ///
     /// // Set to the "second" time 1:30 is on the clocks in New York on
     /// // 2024-11-03. The offset in the datetime string makes this
@@ -5144,29 +5144,6 @@ impl ZonedWith {
     /// because of a change in the offset of the time zone itself. (See the
     /// examples below.)
     ///
-    /// # Example
-    ///
-    /// This example shows how to set the disambiguation configuration while
-    /// creating a [`Zoned`] datetime. In this example, we always prefer the
-    /// earlier time.
-    ///
-    /// ```
-    /// use jiff::{civil::date, tz, Zoned};
-    ///
-    /// // This datetime is unambiguous.
-    /// let zdt1 = "2024-03-11T02:05[America/New_York]".parse::<Zoned>()?;
-    /// // But the same time on March 10 is ambiguous because there is a gap!
-    /// let zdt2 = zdt1
-    ///     .with()
-    ///     .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>>(())
-    /// ```
-    ///
     /// # Example: time zone offset change
     ///
     /// In this example, we explore a time zone offset change in Hawaii that
@@ -5200,6 +5177,59 @@ impl ZonedWith {
     ///
     /// # Ok::<(), Box<dyn std::error::Error>>(())
     /// ```
+    ///
+    /// # 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:
+    ///
+    /// ```
+    /// 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()
+    ///     .disambiguation(tz::Disambiguation::Earlier)
+    ///     .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:
+    ///
+    /// ```
+    /// 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()
+    ///     .disambiguation(tz::Disambiguation::Earlier)
+    ///     // ignore any offset given
+    ///     .offset_conflict(tz::OffsetConflict::AlwaysTimeZone)
+    ///     .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>>(())
+    /// ```
     #[inline]
     pub fn disambiguation(self, strategy: Disambiguation) -> ZonedWith {
         ZonedWith { disambiguation: strategy, ..self }
@@ -5245,10 +5275,12 @@ fn day_length(
 mod tests {
     use std::io::Cursor;
 
+    use alloc::string::ToString;
+
     use crate::{
         civil::{date, datetime},
         span::span_eq,
-        ToSpan,
+        tz, ToSpan,
     };
 
     use super::*;
@@ -5391,4 +5423,64 @@ mod tests {
 
         assert_eq!(deserialized, expected);
     }
+
+    /// This is a regression test for a case where changing a zoned datetime
+    /// to have a time of midnight ends up producing a counter-intuitive
+    /// result.
+    ///
+    /// See: <https://github.com/BurntSushi/jiff/issues/211>
+    #[test]
+    fn zoned_with_time_dst_after_gap() {
+        if crate::tz::db().is_definitively_empty() {
+            return;
+        }
+
+        let zdt1: Zoned = "2024-03-31T12:00[Atlantic/Azores]".parse().unwrap();
+        assert_eq!(
+            zdt1.to_string(),
+            "2024-03-31T12:00:00+00:00[Atlantic/Azores]"
+        );
+
+        let zdt2 = zdt1.with().time(Time::midnight()).build().unwrap();
+        assert_eq!(
+            zdt2.to_string(),
+            "2024-03-31T01:00:00+00:00[Atlantic/Azores]"
+        );
+    }
+
+    /// Similar to `zoned_with_time_dst_after_gap`, but tests what happens
+    /// when moving from/to both sides of the gap.
+    ///
+    /// See: <https://github.com/BurntSushi/jiff/issues/211>
+    #[test]
+    fn zoned_with_time_dst_us_eastern() {
+        if crate::tz::db().is_definitively_empty() {
+            return;
+        }
+
+        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).build().unwrap();
+        assert_eq!(zdt2.to_string(), "2024-03-10T03:30:00-04:00[US/Eastern]");
+
+        let zdt1: Zoned = "2024-03-10T03:30[US/Eastern]".parse().unwrap();
+        assert_eq!(zdt1.to_string(), "2024-03-10T03:30:00-04:00[US/Eastern]");
+        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).
+        //
+        // See: https://github.com/tc39/proposal-temporal/issues/3078
+        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()
+            .offset(tz::offset(10))
+            .hour(2)
+            .disambiguation(Disambiguation::Earlier)
+            .build()
+            .unwrap();
+        assert_eq!(zdt2.to_string(), "2024-03-10T01:30:00-05:00[US/Eastern]");
+    }
 }


Test patch: