Arithmetic API review (unicode-org#7762)

## Changelog: N/A

---------

Co-authored-by: Manish Goregaokar <manishsmail@gmail.com>

Author: robertbastian

components/calendar/src/cal/hebrew.rs

@@ -45,8 +45,8 @@ use calendrical_calculations::rata_die::RataDie;
 /// Due to Rosh Hashanah postponement rules, Ḥešvan and Kislev vary in length.
 ///
 /// In leap years (years 3, 6, 8, 11, 17, 19 in a 19-year cycle), the leap month Adar I (`M05L`, 30 days)
-/// is inserted before Adar, and Adar is called Adar II (the `formatting_code` returned by [`MonthInfo`]
-/// will be `M06L` to mark this, while the `standard_code` remains `M06`).
+/// is inserted before Adar (`M06`), which is then called Adar II ([`MonthInfo::leap_status`] will be
+/// [`LeapStatus::LeapBase`] to mark this).
 ///
 /// Standard years thus have 353-355 days, and leap years 383-385.
 #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Default)]

components/calendar/src/date.rs

@@ -110,13 +110,13 @@ impl<C> Deref for Ref<'_, C> {
 /// by changing the era/calendar. Furthermore, this is not the case for [`Date::try_from_fields`] and
 /// date arithmetic APIs: these APIs let you construct dates outside of that range.
 ///
-/// `Date` types have a fundamental range invariant as well, and ICU4X will refuse to construct
+/// The `Date` type has a fundamental range invariant as well, and it's not possible to construct
 /// dates outside of that range, regardless of the calendar.
-/// ICU4X APIs will return an `Overflow` error (e.g. `DateAddError::Overflow`) in these cases, or clamp
+/// APIs will return an `Overflow` error (e.g. `DateAddError::Overflow`) in these cases, or clamp
 /// in the case of `Date::from_rata_die()`.
 ///
 /// This range is currently dates with an ISO year between `-999_999..=999_999`, but
-/// ICU4X reserves the right to change these bounds in the future.
+/// we reserve the right to change these bounds in the future.
 ///
 /// Since `icu_calendar` is intended to be usable by implementors of the ECMA Temporal specification,
 /// this range will never be smaller than Temporal's validity range, which roughly maps to ISO years
@@ -338,7 +338,7 @@ impl<A: AsCalendar> Date<A> {
         self.calendar.as_calendar().months_in_year(self.inner())
     }
 
-    /// Add a `duration` to this date, mutating it
+    /// Add a `duration` to this [`Date`], mutating it.
     ///
     /// This API will not construct dates outside of the fundamental range described on the [`Date`] type,
     /// instead returning [`DateAddError::Overflow`].
@@ -366,15 +366,11 @@ impl<A: AsCalendar> Date<A> {
         Ok(())
     }
 
-    /// Add a `duration` to this date, returning the new one.
+    /// Add a `duration` to this [`Date`].
     ///
     /// This API will not construct dates outside of the fundamental range described on the [`Date`] type,
     /// instead returning [`DateAddError::Overflow`].
     ///
-    /// This clones the calendar: the calendars in this crate are cheap to clone (and usually `Copy`), but
-    /// the `A` wrapper you are using may not be. Consider using a wrapper like [`Ref`] for `A` if the cloning
-    /// will be a problem.
-    ///
     /// <div class="stab unstable">
     /// 🚧 This code is considered unstable; it may change at any time, in breaking or non-breaking ways,
     /// including in SemVer minor releases. Do not use this type unless you are prepared for things to occasionally break.

components/calendar/src/duration.rs

@@ -118,10 +118,10 @@ pub struct DateDuration {
     /// Whether the duration is negative.
     ///
     /// A negative duration is an abstract concept that could result, for example, from
-    /// taking the difference between two [`Date`](crate::Date)s.
+    /// taking the difference between two [`Date`](crate::Date)s in ascending order.
     ///
     /// The fields of the duration are either all positive or all negative. Mixed signs
-    /// are not allowed.
+    /// are not possible.
     ///
     /// By convention, this field should be `false` if the duration is zero.
     pub is_negative: bool,

components/calendar/src/error.rs

@@ -451,7 +451,7 @@ mod unstable {
         /// fields.day = Some(31);
         ///
         /// let err = Date::try_from_fields(fields, Default::default(), Iso)
-        ///     .expect_err("no day 31 in November");
+        ///     .expect_err("date out of range");
         ///
         /// assert!(matches!(
         ///     err,

components/calendar/src/options.rs

@@ -175,6 +175,8 @@ mod unstable {
     pub struct DateDifferenceOptions {
         /// Which date field to allow as the largest in a duration when taking the difference.
         ///
+        /// This defaults to [`DateDurationUnit::Days`].
+        ///
         /// When choosing [`Months`] or [`Years`], the resulting [`DateDuration`] might not be
         /// associative or commutative in subsequent arithmetic operations, and it might require
         /// [`Overflow::Constrain`] in addition.
@@ -408,7 +410,7 @@ mod unstable {
         /// This strategy makes the following changes:
         ///
         /// 1. If the fields identify a year and a month, but not a day, then set `day` to 1.
-        /// 2. If `month_code` and `day` are set but nothing else, then set the year to a
+        /// 2. If month and day are set but nothing else, then set the year to a
         ///    _reference year_: some year in the calendar that contains the specified month
         ///    and day, according to the ECMAScript specification.
         ///

components/calendar/src/types.rs

@@ -460,23 +460,21 @@ impl fmt::Display for MonthCode {
 ///
 /// A month has a "number" and "leap flag". In calendars without leap months (non-lunisolar
 /// calendars), the month with number n is always the nth month of the year (_ordinal month_),
-/// for example the Gregorian September is `Month:new(9)` and the 9th month of the year.
+/// for example the Gregorian September is `Month::new(9)` and the 9th month of the year.
 /// However, in calendars with leap months (lunisolar calendars), such as the Hebrew calendar,
 /// a month might repeat (leap) without affecting the number of each subsequent month (but
 /// obviously affecting their _ordinal number_). For example, the Hebrew month Nisan
 /// (`Month::new(7)`) might be the 7th or 8th month of the year, depending if the month
 /// Adar was repeated or not.
 ///
-/// Check the docs for a particular calendar for details on what its months are.
+/// Check the docs for a particular calendar (e.g. [`Hebrew`](crate::cal::Hebrew)) for details on
+/// what its months are.
 ///
 /// This concept of months matches the "month code" in [Temporal], and borrows its string
 /// representation:
 /// * `Month::new(7)` = `M07`
 /// * `Month::leap(2)` = `M02L`
 ///
-/// This means that the Hebrew months "Adar" and "Adar II"
-/// ("Adar, but during a leap year") are considered the same month, `Month::new(6)`.
-///
 /// [Temporal]: https://tc39.es/proposal-intl-era-monthcode/
 #[derive(Copy, Clone, Debug, PartialEq, Hash, Eq, PartialOrd)]
 pub struct Month {