More datetime cleanups for 2.0 Beta (#5823)

#1317

- Move docs tests from neo_marker to better places
- Making Serde impls experimental

components/datetime/Cargo.toml

@@ -114,16 +114,16 @@ unexpected_cfgs = { level = "warn", check-cfg = ['cfg(icu4x_run_size_tests)'] }
 [[bench]]
 name = "datetime"
 harness = false
-required-features = ["compiled_data"]
+required-features = ["compiled_data", "serde", "experimental"]
 
 [[bench]]
 name = "pattern"
 harness = false
-required-features = ["bench"]
+required-features = ["bench", "serde", "experimental"]
 
 [[test]]
 name = "datetime"
-required-features = ["experimental", "compiled_data"]
+required-features = ["experimental", "compiled_data", "serde"]
 
 [[test]]
 name = "resolved_components"

components/datetime/benches/datetime.rs

@@ -5,7 +5,7 @@
 mod fixtures;
 
 use criterion::{criterion_group, criterion_main, Criterion};
-use icu_datetime::FixedCalendarDateTimeFormatter;
+use icu_datetime::{fieldset::dynamic::CompositeFieldSet, FixedCalendarDateTimeFormatter};
 
 use icu_calendar::{Date, DateTime, Gregorian, Time};
 use icu_locale_core::Locale;
@@ -47,7 +47,8 @@ fn datetime_benches(c: &mut Criterion) {
                         .collect();
                     for setup in &fx.setups {
                         let locale: Locale = setup.locale.parse().expect("Failed to parse locale.");
-                        let skeleton = setup.options.semantic.unwrap();
+                        let skeleton =
+                            CompositeFieldSet::try_from(setup.options.semantic.unwrap()).unwrap();
 
                         let dtf = {
                             FixedCalendarDateTimeFormatter::<Gregorian, _>::try_new(

components/datetime/benches/fixtures/mod.rs

@@ -2,7 +2,7 @@
 // called LICENSE at the top level of the ICU4X source tree
 // (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
 
-use icu_datetime::{options, provider::skeleton::components};
+use icu_datetime::{fields::components, fieldset::serde::CompositeFieldSetSerde, options};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -24,7 +24,7 @@ pub struct TestInput {
 pub struct TestOptions {
     pub length: Option<TestOptionsLength>,
     pub components: Option<TestComponentsBag>,
-    pub semantic: Option<icu_datetime::fieldset::dynamic::CompositeFieldSet>,
+    pub semantic: Option<CompositeFieldSetSerde>,
     #[serde(rename = "hourCycle")]
     pub hour_cycle: Option<TestHourCycle>,
 }

components/datetime/src/dynamic.rs

@@ -2,8 +2,6 @@
 // called LICENSE at the top level of the ICU4X source tree
 // (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
 
-#[cfg(feature = "serde")]
-use crate::neo_serde::*;
 use crate::raw::neo::RawNeoOptions;
 use crate::scaffold::GetField;
 use crate::{fields, fieldset, NeoSkeletonLength};
@@ -211,11 +209,6 @@ impl GetField<CompositeFieldSet> for CompositeDateTimeFieldSet {
 
 /// An enum supporting all possible field sets and options.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-#[cfg_attr(
-    feature = "serde",
-    serde(try_from = "SemanticSkeletonSerde", into = "SemanticSkeletonSerde")
-)]
 #[non_exhaustive]
 pub enum CompositeFieldSet {
     /// Field set for a date.
@@ -365,7 +358,7 @@ macro_rules! impl_attrs {
                     alignment,
                 })
             }
-            #[cfg(feature = "serde")]
+            #[cfg(all(feature = "serde", feature = "experimental"))]
             pub(crate) fn from_date_field_set_with_raw_options(date_field_set: DateFieldSet, options: RawNeoOptions) -> Self {
                 match date_field_set {
                     $(

components/datetime/src/provider/skeleton/components.rs → components/datetime/src/fields/components.rs

@@ -2,7 +2,7 @@
 // called LICENSE at the top level of the ICU4X source tree
 // (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
 
-//! Types for specifying fields in a classical datetime skeleton.
+//! 🚧 \[Experimental\] Types for specifying fields in a classical datetime skeleton.
 //!
 //! <div class="stab unstable">
 //! 🚧 This code is experimental; it may change at any time, in breaking or non-breaking ways,
@@ -14,7 +14,7 @@
 //! # Examples
 //!
 //! ```
-//! use icu::datetime::provider::skeleton::components;
+//! use icu::datetime::fields::components;
 //!
 //! let mut bag = components::Bag::default();
 //! bag.year = Some(components::Year::Numeric);
@@ -33,7 +33,6 @@ use crate::{
     fields::{self, Field, FieldLength, FieldSymbol},
     options::FractionalSecondDigits,
     provider::pattern::{runtime::Pattern, PatternItem},
-    provider::skeleton::PatternPlurals,
 };
 
 use crate::neo_pattern::DateTimePattern;
@@ -116,7 +115,6 @@ impl Bag {
     ///
     /// - `default_hour_cycle` specifies the hour cycle to use for the hour field if not in the Bag.
     ///   `preferences::Bag::hour_cycle` takes precedence over this argument.
-    #[cfg(feature = "datagen")]
     pub fn to_vec_fields(&self, default_hour_cycle: HourCycle) -> alloc::vec::Vec<Field> {
         let mut fields = alloc::vec::Vec::new();
         if let Some(era) = self.era {
@@ -579,18 +577,6 @@ impl From<TimeZoneName> for Field {
     }
 }
 
-/// Get the resolved components for a FixedCalendarDateTimeFormatter, via the PatternPlurals. In the case of
-/// plurals resolve off of the required `other` pattern.
-impl From<&PatternPlurals<'_>> for Bag {
-    fn from(other: &PatternPlurals) -> Self {
-        let pattern = match other {
-            PatternPlurals::SinglePattern(pattern) => pattern,
-            PatternPlurals::MultipleVariants(plural_pattern) => &plural_pattern.other,
-        };
-        Self::from(pattern)
-    }
-}
-
 impl From<&DateTimePattern> for Bag {
     fn from(value: &DateTimePattern) -> Self {
         Self::from(value.as_borrowed().0)

components/datetime/src/fields/mod.rs

@@ -13,6 +13,9 @@ use displaydoc::Display;
 pub use length::{FieldLength, FieldNumericOverrides, LengthError};
 pub use symbols::*;
 
+#[cfg(any(feature = "experimental", feature = "datagen"))]
+pub mod components;
+
 use core::{
     cmp::{Ord, PartialOrd},
     convert::TryFrom,

components/datetime/src/fieldset.rs

@@ -30,6 +30,19 @@ pub mod dynamic {
     pub use crate::dynamic::*;
 }
 
+/// 🚧 \[Experimental\] Types for dealing with serialization of semantic skeletons.
+///
+/// <div class="stab unstable">
+/// 🚧 This code is experimental; it may change at any time, in breaking or non-breaking ways,
+/// including in SemVer minor releases. Use with caution.
+/// <a href="https://github.com/unicode-org/icu4x/issues/5825">#5825</a>
+/// </div>
+#[cfg(all(feature = "experimental", feature = "serde"))]
+pub mod serde {
+    pub use crate::neo_serde::CompositeFieldSetSerde;
+    pub use crate::neo_serde::CompositeFieldSetSerdeError;
+}
+
 #[cfg(doc)]
 use icu_timezone::TimeZoneInfo;
 
@@ -1147,6 +1160,88 @@ impl_calendar_period_marker!(
 );
 
 impl_time_marker!(
+    /// Hours can be switched between 12-hour and 24-hour time via the `u-hc` locale keyword.
+    ///
+    /// ```
+    /// use icu::calendar::Time;
+    /// use icu::datetime::fieldset::T;
+    /// use icu::datetime::FixedCalendarDateTimeFormatter;
+    /// use icu::locale::locale;
+    /// use writeable::assert_try_writeable_eq;
+    ///
+    /// // By default, en-US uses 12-hour time and fr-FR uses 24-hour time,
+    /// // but we can set overrides.
+    ///
+    /// let formatter = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("en-US-u-hc-h12").into(),
+    ///     T::short().hm(),
+    /// )
+    /// .unwrap();
+    /// assert_try_writeable_eq!(
+    ///     formatter.format(&Time::try_new(16, 12, 20, 0).unwrap()),
+    ///     "4:12 PM"
+    /// );
+    ///
+    /// let formatter = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("en-US-u-hc-h23").into(),
+    ///     T::short().hm(),
+    /// )
+    /// .unwrap();
+    /// assert_try_writeable_eq!(
+    ///     formatter.format(&Time::try_new(16, 12, 20, 0).unwrap()),
+    ///     "16:12"
+    /// );
+    ///
+    /// let formatter = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("fr-FR-u-hc-h12").into(),
+    ///     T::short().hm(),
+    /// )
+    /// .unwrap();
+    /// assert_try_writeable_eq!(
+    ///     formatter.format(&Time::try_new(16, 12, 20, 0).unwrap()),
+    ///     "4:12 PM"
+    /// );
+    ///
+    /// let formatter = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("fr-FR-u-hc-h23").into(),
+    ///     T::short().hm(),
+    /// )
+    /// .unwrap();
+    /// assert_try_writeable_eq!(
+    ///     formatter.format(&Time::try_new(16, 12, 20, 0).unwrap()),
+    ///     "16:12"
+    /// );
+    /// ```
+    ///
+    /// Hour cycles `h11` and `h24` are supported, too:
+    ///
+    /// ```
+    /// use icu::calendar::Time;
+    /// use icu::datetime::fieldset::T;
+    /// use icu::datetime::FixedCalendarDateTimeFormatter;
+    /// use icu::locale::locale;
+    /// use writeable::assert_try_writeable_eq;
+    ///
+    /// let formatter = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("und-u-hc-h11").into(),
+    ///     T::short().hm(),
+    /// )
+    /// .unwrap();
+    /// assert_try_writeable_eq!(
+    ///     formatter.format(&Time::try_new(0, 0, 0, 0).unwrap()),
+    ///     "0:00 AM"
+    /// );
+    ///
+    /// let formatter = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("und-u-hc-h24").into(),
+    ///     T::short().hm(),
+    /// )
+    /// .unwrap();
+    /// assert_try_writeable_eq!(
+    ///     formatter.format(&Time::try_new(0, 0, 0, 0).unwrap()),
+    ///     "24:00"
+    /// );
+    /// ```
     T,
     NeoTimeComponents::Time,
     description = "time (locale-dependent hour cycle)",
@@ -1381,6 +1476,56 @@ impl_zone_marker!(
     /// );
     /// ```
     ///
+    /// Can also fall back to the UTC offset:
+    ///
+    /// ```
+    /// use icu::calendar::{Date, Time};
+    /// use icu::timezone::{TimeZoneInfo, UtcOffset, TimeZoneIdMapper, TimeZoneBcp47Id};
+    /// use icu::datetime::FixedCalendarDateTimeFormatter;
+    /// use icu::datetime::fieldset::V;
+    /// use icu::datetime::DateTimeWriteError;
+    /// use icu::locale::locale;
+    /// use tinystr::tinystr;
+    /// use writeable::assert_try_writeable_eq;
+    ///
+    /// // Set up the formatter
+    /// let mut tzf = FixedCalendarDateTimeFormatter::<(), _>::try_new(
+    ///     &locale!("en").into(),
+    ///     V::short(),
+    /// )
+    /// .unwrap();
+    ///
+    /// // "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 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 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"
+    /// );
+    /// ```
+    ///
     /// Since non-location names might change over time,
     /// this time zone style requires a reference time.
     ///

components/datetime/src/lib.rs

@@ -96,9 +96,8 @@ pub mod fieldset;
 mod format;
 pub mod input;
 mod neo;
-mod neo_marker;
 pub mod neo_pattern;
-#[cfg(feature = "serde")]
+#[cfg(all(feature = "experimental", feature = "serde"))]
 mod neo_serde;
 pub mod options;
 pub mod provider;