Improve docs and code hygiene in icu_datetime (#6498)

Toward #5634

Author: sffc

components/calendar/Cargo.toml

@@ -4,7 +4,7 @@
 
 [package]
 name = "icu_calendar"
-description = "API for supporting various types of calendars"
+description = "Date APIs for Gregorian and non-Gregorian calendars"
 
 authors.workspace = true
 categories.workspace = true

components/datetime/Cargo.toml

@@ -4,7 +4,7 @@
 
 [package]
 name = "icu_datetime"
-description = "API for formatting date and time to user readable textual representation"
+description = "Human-readable formatting of dates, times, and time zones in hundreds of locales"
 
 authors.workspace = true
 categories.workspace = true

components/datetime/src/builder.rs

@@ -223,7 +223,8 @@ impl ZoneStyle {
 }
 
 /// An error that occurs when creating a [field set](crate::fieldsets) from a builder.
-#[derive(Debug, displaydoc::Display)]
+// Not Copy: one of the variants contains a non-Copy type
+#[derive(Debug, Clone, displaydoc::Display)]
 #[ignore_extra_doc_attributes] // lines after the first won't go into `impl Display`
 #[non_exhaustive]
 pub enum BuilderError {
@@ -247,6 +248,30 @@ pub enum BuilderError {
     /// Superfluous options were specified.
     ///
     /// For example, you cannot set a [`YearStyle`] unless the field set contains years.
+    ///
+    /// The options that were _not_ read are returned back to the user.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use icu::datetime::fieldsets;
+    /// use icu::datetime::fieldsets::builder::*;
+    /// use icu::datetime::options::*;
+    ///
+    /// let mut builder = FieldSetBuilder::new();
+    /// builder.length = Some(Length::Short);
+    /// builder.time_precision = Some(TimePrecision::Minute);
+    /// builder.year_style = Some(YearStyle::WithEra);
+    ///
+    /// let err = builder.build_composite().unwrap_err();
+    ///
+    /// let BuilderError::SuperfluousOptions(superfluous_options) = err else {
+    ///     panic!("error type should be SuperfluousOptions");
+    /// };
+    ///
+    /// assert!(superfluous_options.year_style.is_some());
+    /// assert!(superfluous_options.time_precision.is_none());
+    /// ```
     SuperfluousOptions(FieldSetBuilder),
 }
 

components/datetime/src/dynamic.rs

@@ -112,7 +112,7 @@ pub enum CalendarPeriodFieldSet {
     /// A year, as in
     /// “2000”.
     Y(fieldsets::Y),
-    // TODO: Add support for week-of-year
+    // TODO(#5643): Add support for week-of-year
     // /// The year and week of the year, as in
     // /// “52nd week of 1999”.
     // YW(fieldsets::YW),

components/datetime/src/fieldsets.rs

@@ -711,7 +711,7 @@ macro_rules! impl_date_marker {
             type WeekdayNamesV1 = datetime_marker_helper!(@weekdays, $($weekdays_yes)?);
         }
         impl TimeMarkers for $type_time {
-            // TODO: Consider making dayperiods optional again
+            // TODO(#6497): Consider making dayperiods optional
             type DayPeriodNamesV1 = datetime_marker_helper!(@dayperiods, yes);
             type TimeSkeletonPatternsV1 = datetime_marker_helper!(@times, yes);
             type HourInput = datetime_marker_helper!(@input/hour, yes);
@@ -1465,8 +1465,6 @@ pub mod zone {
         zone_essentials = yes,
     );
 
-    // TODO: Add short/long UTC offset?
-
     impl_zone_marker!(
         /// When a display name is unavailable, falls back to the location format:
         ///

components/datetime/src/neo.rs

@@ -219,6 +219,10 @@ where
     ///
     /// This ignores the `calendar_kind` preference and instead uses the static calendar type,
     /// and supports calendars that are not expressible as preferences, such as [`JapaneseExtended`](icu_calendar::cal::JapaneseExtended).
+    ///
+    /// ✨ *Enabled with the `compiled_data` Cargo feature.*
+    ///
+    /// [📚 Help choosing a constructor](icu_provider::constructors)
     #[cfg(feature = "compiled_data")]
     pub fn try_new(
         prefs: DateTimeFormatterPreferences,
@@ -471,6 +475,28 @@ size_test!(
 /// a calendar selected at runtime.
 ///
 /// For more details, please read the [crate root docs][crate].
+///
+/// # Examples
+///
+/// Basic usage:
+///
+/// ```
+/// use icu::datetime::fieldsets::YMD;
+/// use icu::datetime::input::Date;
+/// use icu::datetime::DateTimeFormatter;
+/// use icu::locale::locale;
+/// use writeable::assert_writeable_eq;
+///
+/// let formatter = DateTimeFormatter::try_new(
+///     locale!("en-u-ca-hebrew").into(),
+///     YMD::medium(),
+/// )
+/// .unwrap();
+///
+/// let date = Date::try_new_iso(2024, 5, 8).unwrap();
+///
+/// assert_writeable_eq!(formatter.format(&date), "30 Nisan 5784");
+/// ```
 #[doc = neo_year_month_day_formatter_size!()]
 #[derive(Debug, Clone)]
 pub struct DateTimeFormatter<FSet: DateTimeNamesMarker> {
@@ -495,30 +521,6 @@ where
     /// ✨ *Enabled with the `compiled_data` Cargo feature.*
     ///
     /// [📚 Help choosing a constructor](icu_provider::constructors)
-    ///
-    /// # Examples
-    ///
-    /// Basic usage:
-    ///
-    /// ```
-    /// use icu::datetime::fieldsets::YMD;
-    /// use icu::datetime::input::Date;
-    /// use icu::datetime::DateTimeFormatter;
-    /// use icu::locale::locale;
-    /// use writeable::assert_writeable_eq;
-    ///
-    /// let formatter = DateTimeFormatter::try_new(
-    ///     locale!("en-u-ca-hebrew").into(),
-    ///     YMD::medium(),
-    /// )
-    /// .unwrap();
-    ///
-    /// let date = Date::try_new_iso(2024, 5, 8).unwrap();
-    ///
-    /// assert_writeable_eq!(formatter.format(&date), "30 Nisan 5784");
-    /// ```
-    ///
-    /// [`AnyCalendarKind`]: icu_calendar::AnyCalendarKind
     #[inline(never)]
     #[cfg(feature = "compiled_data")]
     pub fn try_new(

components/datetime/src/pattern/names.rs

@@ -952,11 +952,15 @@ where
     FSet::Z: ZoneMarkers,
     FSet: GetField<CompositeFieldSet>,
 {
-    /// Loads a pattern for the given field set and returns a [`FixedCalendarDateTimeFormatter`].
+    /// Loads a pattern for the given field set with compiled data and returns a [`FixedCalendarDateTimeFormatter`].
     ///
     /// The names in the current [`FixedCalendarDateTimeNames`] _must_ be sufficient for the field set.
     /// If not, the input object will be returned with an error.
     ///
+    /// ✨ *Enabled with the `compiled_data` Cargo feature.*
+    ///
+    /// [📚 Help choosing a constructor](icu_provider::constructors)
+    ///
     /// # Examples
     ///
     /// ```
@@ -1154,11 +1158,15 @@ where
     FSet::Z: ZoneMarkers,
     FSet: GetField<CompositeFieldSet>,
 {
-    /// Loads a pattern for the given field set and returns a [`DateTimeFormatter`].
+    /// Loads a pattern for the given field set with compiled data and returns a [`DateTimeFormatter`].
     ///
     /// The names in the current [`DateTimeNames`] _must_ be sufficient for the field set.
     /// If not, the input object will be returned with an error.
     ///
+    /// ✨ *Enabled with the `compiled_data` Cargo feature.*
+    ///
+    /// [📚 Help choosing a constructor](icu_provider::constructors)
+    ///
     /// # Examples
     ///
     /// ```
@@ -1888,7 +1896,11 @@ impl<C, FSet: DateTimeNamesMarker> FixedCalendarDateTimeNames<C, FSet> {
     ///     names
     ///         .with_pattern_unchecked(&pattern)
     ///         .format(&zone_london_summer),
-    ///     "Your time zone is: Greenwich Mean Time", // TODO
+    ///     // Note: The year-round generic name of this zone is Greenwich
+    ///     // Mean Time, which may be confusing since the zone observes
+    ///     // daylight savings time. See:
+    ///     // <https://unicode-org.atlassian.net/issues/CLDR-18378>
+    ///     "Your time zone is: Greenwich Mean Time",
     /// );
     /// ```
     #[cfg(feature = "compiled_data")]
@@ -1975,7 +1987,11 @@ impl<C, FSet: DateTimeNamesMarker> FixedCalendarDateTimeNames<C, FSet> {
     ///     names
     ///         .with_pattern_unchecked(&pattern)
     ///         .format(&zone_london_summer),
-    ///     "Your time zone is: GMT", // TODO
+    ///     // Note: The year-round generic name of this zone is Greenwich
+    ///     // Mean Time, which may be confusing since the zone observes
+    ///     // daylight savings time. See:
+    ///     // <https://unicode-org.atlassian.net/issues/CLDR-18378>
+    ///     "Your time zone is: GMT",
     /// );
     /// ```
     #[cfg(feature = "compiled_data")]
@@ -2540,7 +2556,6 @@ impl<C: CldrCalendar, FSet: DateTimeNamesMarker> FixedCalendarDateTimeNames<C, F
             &C::MonthNamesV1::bind(provider),
             &WeekdayNamesV1::bind(provider),
             &DayPeriodNamesV1::bind(provider),
-            // TODO: Consider making time zone name loading optional here (lots of data)
             &tz::EssentialsV1::bind(provider),
             &tz::LocationsRootV1::bind(provider),
             &tz::LocationsOverrideV1::bind(provider),
@@ -3705,7 +3720,7 @@ impl RawDateTimeNamesBorrowed<'_> {
         weekday_names
             .names
             .get((day as usize) % 7)
-            // TODO: make weekday_names length 7 in the type system
+            // Note: LinearNames does not guarantee a length of 7.
             .ok_or(GetNameForWeekdayError::NotLoaded)
     }
 

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

@@ -142,7 +142,7 @@ impl Bag {
                 symbol: FieldSymbol::Year(match year {
                     Year::Numeric | Year::TwoDigit => fields::Year::Calendar,
                     Year::NumericWeekOf | Year::TwoDigitWeekOf => {
-                        unimplemented!("fields::Year::WeekOf")
+                        unimplemented!("#5643 fields::Year::WeekOf")
                     }
                 }),
                 length: match year {

components/datetime/src/provider/fields/symbols.rs

@@ -396,8 +396,7 @@ macro_rules! field_type {
     );
     ($(#[$enum_attr:meta])* $i:ident; { $( $(#[$variant_attr:meta])* $key:literal => $val:ident = $idx:expr,)* }; $($ule_name:ident)?) => (
         #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Copy, yoke::Yokeable, zerofrom::ZeroFrom)]
-        // FIXME: This should be replaced with a custom derive.
-        // See: https://github.com/unicode-org/icu4x/issues/1044
+        // TODO(#1044): This should be replaced with a custom derive.
         #[cfg_attr(feature = "datagen", derive(serde::Serialize, databake::Bake))]
         #[cfg_attr(feature = "datagen", databake(path = icu_datetime::fields))]
         #[cfg_attr(feature = "serde", derive(serde::Deserialize))]

components/datetime/src/provider/skeleton/helpers.rs

@@ -142,7 +142,7 @@ fn naively_apply_time_zone_name(
     }
 }
 
-// TODO - This could return a Cow<'a, Pattern>, but it affects every other part of the API to
+// Note: This could return a Cow<'a, Pattern>, but it affects every other part of the API to
 // add a lifetime here. The pattern returned here could be one that we've already constructed in
 // the CLDR as an exotic type, or it could be one that was modified to meet the requirements of
 // the components bag.