Improve timezone APIs (#5747)

* Fewer overlapping constructors for `TimeZoneInfo` * Private fields for `TimeZoneInfo` * Return type for `ZoneOffsetCalculator` * docs
unicode-org · Oct 30, 2024 · 7feeacd · 7feeacd
1 parent a763d9f
commit 7feeacd
Show file tree

Hide file tree

Showing 20 changed files with 319 additions and 344 deletions.
diff --git a/components/datetime/src/fieldset.rs b/components/datetime/src/fieldset.rs
@@ -565,12 +565,10 @@ macro_rules! impl_zone_marker {
             /// .unwrap();
             ///
             /// // Time zone info for America/Chicago in the summer
-            /// let zone = TimeZoneInfo::from_id_and_offset(
-            ///     TimeZoneBcp47Id(tinystr!(8, "uschi")),
-            ///     UtcOffset::from_eighths_of_hour(-5 * 8),
-            /// )
-            /// .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()))
-            /// .with_zone_variant(ZoneVariant::daylight());
+            /// let zone = TimeZoneBcp47Id(tinystr!(8, "uschi"))
+            ///     .with_offset("-05".parse().ok())
+            ///     .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()))
+            ///     .with_zone_variant(ZoneVariant::daylight());
             ///
             /// assert_try_writeable_eq!(
             ///     fmt.convert_and_format(&zone),
@@ -597,12 +595,10 @@ macro_rules! impl_zone_marker {
             /// .unwrap();
             ///
             /// // Time zone info for America/Chicago in the summer
-            /// let zone = TimeZoneInfo::from_id_and_offset(
-            ///     TimeZoneBcp47Id(tinystr!(8, "uschi")),
-            ///     UtcOffset::from_eighths_of_hour(-5 * 8),
-            /// )
-            /// .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()))
-            /// .with_zone_variant(ZoneVariant::daylight());
+            /// let zone = TimeZoneBcp47Id(tinystr!(8, "uschi"))
+            ///     .with_offset("-05".parse().ok())
+            ///     .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()))
+            ///     .with_zone_variant(ZoneVariant::daylight());
             ///
             /// assert_try_writeable_eq!(
             ///     fmt.format(&zone),
@@ -996,12 +992,10 @@ impl_zone_marker!(
     /// .unwrap();
     ///
     /// // Time zone info for America/Sao_Paulo in the winter
-    /// let zone = TimeZoneInfo::from_id_and_offset(
-    ///     TimeZoneBcp47Id(tinystr!(8, "brsao")),
-    ///     UtcOffset::from_eighths_of_hour(-3 * 8),
-    /// )
-    /// .at_time((Date::try_new_iso(2022, 1, 29).unwrap(), Time::midnight()))
-    /// .with_zone_variant(ZoneVariant::standard());
+    /// let zone = TimeZoneBcp47Id(tinystr!(8, "brsao"))
+    ///     .with_offset("-03".parse().ok())
+    ///     .at_time((Date::try_new_iso(2022, 1, 29).unwrap(), Time::midnight()))
+    ///     .with_zone_variant(ZoneVariant::standard());
     ///
     /// assert_try_writeable_eq!(
     ///     fmt.format(&zone),
@@ -1013,7 +1007,7 @@ impl_zone_marker!(
     /// only a full time zone info can be formatted with this style.
     /// For example, [`TimeZoneInfo<AtTime>`] cannot be formatted.
     ///
-    /// ```compile_fail
+    /// ```compile_fail,E0271
     /// use icu::calendar::{DateTime, Iso};
     /// use icu::datetime::FixedCalendarDateTimeFormatter;
     /// use icu::datetime::fieldset::Z;
@@ -1023,8 +1017,7 @@ impl_zone_marker!(
     /// use writeable::assert_try_writeable_eq;
     ///
     /// let datetime = DateTime::try_new_gregorian(2024, 10, 18, 0, 0, 0).unwrap();
-    /// let utc_offset = UtcOffset::from_eighths_of_hour(-6 * 8);
-    /// let time_zone_basic = utc_offset.with_id(TimeZoneBcp47Id(tinystr!(8, "uschi")));
+    /// let time_zone_basic = TimeZoneBcp47Id(tinystr!(8, "uschi")).with_offset("-06".parse().ok());
     /// let time_zone_at_time = time_zone_basic.at_time((datetime.date.to_iso(), datetime.time));
     ///
     /// let formatter = FixedCalendarDateTimeFormatter::try_new(
@@ -1034,7 +1027,6 @@ impl_zone_marker!(
     /// .unwrap();
     ///
     /// // error[E0271]: type mismatch resolving `<AtTime as TimeZoneModel>::ZoneVariant == ZoneVariant`
-    /// // note: required by a bound in `FixedCalendarDateTimeFormatter::<C, FSet>::format`
     /// formatter.format(&time_zone_at_time);
     /// ```
     Z,
@@ -1108,7 +1100,7 @@ impl_zone_marker!(
     /// only a full time zone info can be formatted with this style.
     /// For example, [`TimeZoneInfo<AtTime>`] cannot be formatted.
     ///
-    /// ```compile_fail
+    /// ```compile_fail,E0271
     /// use icu::calendar::{DateTime, Iso};
     /// use icu::datetime::FixedCalendarDateTimeFormatter;
     /// use icu::datetime::fieldset::Zs;
@@ -1118,8 +1110,7 @@ impl_zone_marker!(
     /// use writeable::assert_try_writeable_eq;
     ///
     /// let datetime = DateTime::try_new_gregorian(2024, 10, 18, 0, 0, 0).unwrap();
-    /// let utc_offset = UtcOffset::from_eighths_of_hour(-6 * 8);
-    /// let time_zone_basic = utc_offset.with_id(TimeZoneBcp47Id(tinystr!(8, "uschi")));
+    /// let time_zone_basic = TimeZoneBcp47Id(tinystr!(8, "uschi")).with_offset("-06".parse().ok());
     /// let time_zone_at_time = time_zone_basic.at_time((datetime.date.to_iso(), datetime.time));
     ///
     /// let formatter = FixedCalendarDateTimeFormatter::try_new(
@@ -1157,9 +1148,8 @@ impl_zone_marker!(
     /// use icu::locale::locale;
     /// use writeable::assert_try_writeable_eq;
     ///
-    /// let utc_offset = UtcOffset::from_eighths_of_hour(-6 * 8);
-    ///
-    /// let time_zone_basic = utc_offset.with_id(TimeZoneBcp47Id(tinystr!(8, "uschi")));
+    /// let utc_offset = "-06".parse().unwrap();
+    /// let time_zone_basic = TimeZoneBcp47Id(tinystr!(8, "uschi")).with_offset(Some(utc_offset));
     ///
     /// let date = Date::try_new_iso(2024, 10, 18).unwrap();
     /// let time = Time::midnight();
@@ -1221,12 +1211,10 @@ impl_zone_marker!(
     /// .unwrap();
     ///
     /// // Time zone info for America/Sao_Paulo in the winter
-    /// let zone = TimeZoneInfo::from_id_and_offset(
-    ///     TimeZoneBcp47Id(tinystr!(8, "brsao")),
-    ///     UtcOffset::from_eighths_of_hour(-3 * 8),
-    /// )
-    /// .at_time((Date::try_new_iso(2022, 1, 29).unwrap(), Time::midnight()))
-    /// .with_zone_variant(ZoneVariant::standard());
+    /// let zone = TimeZoneBcp47Id(tinystr!(8, "brsao"))
+    ///     .with_offset("-03".parse().ok())
+    ///     .at_time((Date::try_new_iso(2022, 1, 29).unwrap(), Time::midnight()))
+    ///     .with_zone_variant(ZoneVariant::standard());
     ///
     /// assert_try_writeable_eq!(
     ///     fmt.format(&zone),
@@ -1237,7 +1225,7 @@ impl_zone_marker!(
     /// Since non-location names might change over time,
     /// this time zone style requires a reference time.
     ///
-    /// ```compile_fail
+    /// ```compile_fail,E0271
     /// use icu::calendar::{DateTime, Iso};
     /// use icu::datetime::FixedCalendarDateTimeFormatter;
     /// use icu::datetime::fieldset::V;
@@ -1246,8 +1234,7 @@ impl_zone_marker!(
     /// use icu::locale::locale;
     /// use writeable::assert_try_writeable_eq;
     ///
-    /// let utc_offset = UtcOffset::from_eighths_of_hour(-6 * 8);
-    /// let time_zone_basic = utc_offset.with_id(TimeZoneBcp47Id(tinystr!(8, "uschi")));
+    /// let time_zone_basic = TimeZoneBcp47Id(tinystr!(8, "uschi")).with_offset("-06".parse().ok());
     ///
     /// let formatter = FixedCalendarDateTimeFormatter::try_new(
     ///     &locale!("en-US").into(),
@@ -1329,7 +1316,7 @@ impl_zone_marker!(
     /// Since non-location names might change over time,
     /// this time zone style requires a reference time.
     ///
-    /// ```compile_fail
+    /// ```compile_fail,E0271
     /// use icu::calendar::{DateTime, Iso};
     /// use icu::datetime::FixedCalendarDateTimeFormatter;
     /// use icu::datetime::fieldset::Vs;
@@ -1338,8 +1325,7 @@ impl_zone_marker!(
     /// use icu::locale::locale;
     /// use writeable::assert_try_writeable_eq;
     ///
-    /// let utc_offset = UtcOffset::from_eighths_of_hour(-6 * 8);
-    /// let time_zone_basic = utc_offset.with_id(TimeZoneBcp47Id(tinystr!(8, "uschi")));
+    /// let time_zone_basic = TimeZoneBcp47Id(tinystr!(8, "uschi")).with_offset("-06".parse().ok());1
     ///
     /// let formatter = FixedCalendarDateTimeFormatter::try_new(
     ///     &locale!("en-US").into(),
@@ -1368,7 +1354,7 @@ impl_zone_marker!(
     /// A time zone ID is required to format with this style.
     /// For example, a raw [`UtcOffset`] cannot be used here.
     ///
-    /// ```compile_fail
+    /// ```compile_fail,E0277
     /// use icu::calendar::{DateTime, Iso};
     /// use icu::datetime::FixedCalendarDateTimeFormatter;
     /// use icu::datetime::fieldset::L;
@@ -1377,7 +1363,7 @@ impl_zone_marker!(
     /// use icu::locale::locale;
     /// use writeable::assert_try_writeable_eq;
     ///
-    /// let utc_offset = UtcOffset::from_eighths_of_hour(-6 * 8);
+    /// let utc_offset = UtcOffset::try_from_str("-06").unwrap();
     ///
     /// let formatter = FixedCalendarDateTimeFormatter::try_new(
     ///     &locale!("en-US").into(),

diff --git a/components/datetime/src/neo_marker.rs b/components/datetime/src/neo_marker.rs
@@ -265,47 +265,38 @@
 //! use tinystr::tinystr;
 //! use writeable::assert_try_writeable_eq;
 //!
-//! // Set up the time zone. Note: the inputs here are
-//! //   1. The offset
-//! //   2. The IANA time zone ID
-//! //   3. A date and time (for non-location name resolution)
-//! //   4. Note: we do not need the zone variant because of `load_generic_*()`
-//!
 //! // Set up the formatter
 //! let mut tzf = FixedCalendarDateTimeFormatter::<(), _>::try_new(
 //!     &locale!("en").into(),
 //!     V::short(),
 //! )
 //! .unwrap();
 //!
-//! // "uschi" - has symbol data for generic_non_location_short
-//! let time_zone = TimeZoneInfo::from_id_and_offset(
-//!     TimeZoneIdMapper::new().iana_to_bcp47("America/Chicago"),
-//!     UtcOffset::from_eighths_of_hour(-5 * 8),
-//! )
-//! .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()));
+//! // "uschi" - has symbol data for short generic non-location
+//! let time_zone = TimeZoneIdMapper::new()
+//!     .iana_to_bcp47("America/Chicago")
+//!     .with_offset("-05".parse().ok())
+//!     .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()));
 //! assert_try_writeable_eq!(
 //!     tzf.format(&time_zone),
 //!     "CT"
 //! );
 //!
-//! // "ushnl" - has time zone override symbol data for generic_non_location_short
-//! let time_zone = TimeZoneInfo::from_id_and_offset(
-//!     TimeZoneIdMapper::new().iana_to_bcp47("Pacific/Honolulu"),
-//!     UtcOffset::from_eighths_of_hour(-10 * 8),
-//! )
-//! .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()));
+//! // "ushnl" - has time zone override symbol data for short generic non-location
+//! let time_zone = TimeZoneIdMapper::new()
+//!     .iana_to_bcp47("Pacific/Honolulu")
+//!     .with_offset("-10".parse().ok())
+//!     .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()));
 //! assert_try_writeable_eq!(
 //!     tzf.format(&time_zone),
 //!     "HST"
 //! );
 //!
-//! // Mis-spelling of "America/Chicago" results in a fallback to GMT format
-//! let time_zone = TimeZoneInfo::from_id_and_offset(
-//!     TimeZoneIdMapper::new().iana_to_bcp47("America/Chigagou"),
-//!     UtcOffset::from_eighths_of_hour(-5 * 8),
-//! )
-//! .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()));
+//! // Mis-spelling of "America/Chicago" results in a fallback to offset format
+//! let time_zone = TimeZoneIdMapper::new()
+//!     .iana_to_bcp47("America/Chigagou")
+//!     .with_offset("-05".parse().ok())
+//!     .at_time((Date::try_new_iso(2022, 8, 29).unwrap(), Time::midnight()));
 //! assert_try_writeable_eq!(
 //!     tzf.format(&time_zone),
 //!     "GMT-5"

diff --git a/components/datetime/src/scaffold/get_field.rs b/components/datetime/src/scaffold/get_field.rs
@@ -352,7 +352,7 @@ where
 {
     #[inline]
     fn get_field(&self) -> TimeZoneBcp47Id {
-        self.time_zone_id
+        self.time_zone_id()
     }
 }
 
@@ -362,7 +362,7 @@ where
 {
     #[inline]
     fn get_field(&self) -> Option<UtcOffset> {
-        self.offset
+        self.offset()
     }
 }
 
@@ -372,7 +372,7 @@ where
 {
     #[inline]
     fn get_field(&self) -> ZoneVariant {
-        self.zone_variant
+        self.zone_variant()
     }
 }
 
@@ -382,7 +382,7 @@ where
 {
     #[inline]
     fn get_field(&self) -> (Date<Iso>, Time) {
-        self.local_time
+        self.local_time()
     }
 }
 

diff --git a/components/datetime/src/time_zone.rs b/components/datetime/src/time_zone.rs
@@ -727,7 +727,7 @@ impl Iso8601Format {
 
         let extended_format = matches!(self.format, IsoFormat::Extended | IsoFormat::UtcExtended);
 
-        sink.write_char(if offset.is_positive() { '+' } else { '-' })?;
+        sink.write_char(if offset.is_non_negative() { '+' } else { '-' })?;
 
         format_time_segment(sink, offset.hours_part().unsigned_abs())?;
 

diff --git a/components/datetime/tests/datetime.rs b/components/datetime/tests/datetime.rs
@@ -477,23 +477,28 @@ fn test_time_zone_format_configs() {
 fn test_time_zone_format_offset_seconds() {
     use icu_datetime::fieldset::O;
 
-    let time_zone = TimeZoneInfo::from(UtcOffset::try_from_seconds(12).unwrap());
     let tzf = FixedCalendarDateTimeFormatter::<(), _>::try_new(&locale!("en").into(), O::medium())
         .unwrap();
-    assert_try_writeable_eq!(tzf.format(&time_zone), "GMT+0:00:12",);
+    assert_try_writeable_eq!(
+        tzf.format(&UtcOffset::try_from_seconds(12).unwrap()),
+        "GMT+0:00:12",
+    );
 }
 
 #[test]
-fn test_time_zone_format_offset_not_set_debug_assert_panic() {
+fn test_time_zone_format_offset_fallback() {
     use icu_datetime::fieldset::O;
 
-    let time_zone = TimeZoneInfo {
-        time_zone_id: TimeZoneIdMapper::new().iana_to_bcp47("America/Los_Angeles"),
-        ..TimeZoneInfo::unknown()
-    };
     let tzf = FixedCalendarDateTimeFormatter::<(), _>::try_new(&locale!("en").into(), O::medium())
         .unwrap();
-    assert_try_writeable_eq!(tzf.format(&time_zone), "GMT+?",);
+    assert_try_writeable_eq!(
+        tzf.format(
+            &TimeZoneIdMapper::new()
+                .iana_to_bcp47("America/Los_Angeles")
+                .with_offset(None)
+        ),
+        "GMT+?",
+    );
 }
 
 #[test]