Fix bugs in several calendars with new continuity test (#4904)

Fixes #2703
Fixes #4914
Mostly fixes #2713

components/calendar/Cargo.toml

@@ -39,6 +39,7 @@ icu = { path = "../../components/icu", default-features = false }
 icu_benchmark_macros = { path = "../../tools/benchmark/macros" }
 serde = { workspace = true, features = ["derive", "alloc"] }
 serde_json = { workspace = true }
+simple_logger = { workspace = true }
 
 
 [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
@@ -47,6 +48,7 @@ criterion = { workspace = true }
 
 [features]
 default = ["compiled_data"]
+logging = ["calendrical_calculations/logging"]
 std = ["icu_provider/std", "icu_locid/std", "calendrical_calculations/std"]
 serde = ["dep:serde", "zerovec/serde", "tinystr/serde", "icu_provider/serde"]
 datagen = ["serde", "dep:databake", "zerovec/databake", "tinystr/databake"]

components/calendar/src/chinese_based.rs

@@ -106,7 +106,10 @@ fn compute_packed_with_yb<CB: ChineseBased>(
     let ny_offset = if let Ok(ny_offset) = u8::try_from(ny_offset) {
         ny_offset
     } else {
-        debug_assert!(false, "Expected small new years offset, got {ny_offset}");
+        debug_assert!(
+            false,
+            "Expected small new years offset, got {ny_offset} in ISO year {related_iso}"
+        );
         0
     };
     PackedChineseBasedYearInfo::new(month_lengths, leap_month, ny_offset)

components/calendar/src/coptic.rs

@@ -87,7 +87,7 @@ impl CalendarArithmetic for Coptic {
     }
 
     fn is_leap_year(year: i32, _data: ()) -> bool {
-        year % 4 == 3
+        year.rem_euclid(4) == 3
     }
 
     fn last_month_day_in_year(year: i32, _data: ()) -> (u8, u8) {

components/calendar/src/duration.rs

@@ -64,7 +64,7 @@ use core::marker::PhantomData;
 /// ```
 ///
 /// Currently unstable for ICU4X 1.0
-#[derive(Copy, Clone, Eq, PartialEq)]
+#[derive(Eq, PartialEq)]
 #[allow(clippy::exhaustive_structs)] // this type should be stable (and is intended to be constructed manually)
 #[doc(hidden)]
 pub struct DateDuration<C: Calendar + ?Sized> {
@@ -80,6 +80,16 @@ pub struct DateDuration<C: Calendar + ?Sized> {
     pub marker: PhantomData<C>,
 }
 
+// Custom impl so that C need not be bound on Copy/Clone
+impl<C: Calendar + ?Sized> Clone for DateDuration<C> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+// Custom impl so that C need not be bound on Copy/Clone
+impl<C: Calendar + ?Sized> Copy for DateDuration<C> {}
+
 /// A "duration unit" used to specify the minimum or maximum duration of time to
 /// care about
 ///

components/calendar/src/ethiopian.rs

@@ -107,7 +107,7 @@ impl CalendarArithmetic for Ethiopian {
     }
 
     fn is_leap_year(year: i32, _data: ()) -> bool {
-        year % 4 == 3
+        year.rem_euclid(4) == 3
     }
 
     fn last_month_day_in_year(year: i32, _data: ()) -> (u8, u8) {

components/calendar/src/islamic.rs

@@ -218,34 +218,89 @@ impl IslamicTabular {
     }
 }
 
+/// Compact representation of the length of an Islamic year.
+#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
+enum IslamicYearLength {
+    /// Long (355-day) Islamic year
+    L355,
+    /// Short (354-day) Islamic year
+    L354,
+    /// Unexpectedly Short (353-day) Islamic year
+    ///
+    /// It is probably a bug when this year length is returned. See:
+    /// <https://github.com/unicode-org/icu4x/issues/4930>
+    L353,
+}
+
+impl Default for IslamicYearLength {
+    fn default() -> Self {
+        Self::L354
+    }
+}
+
+impl IslamicYearLength {
+    fn try_from_int(value: i64) -> Option<Self> {
+        match value {
+            355 => Some(Self::L355),
+            354 => Some(Self::L354),
+            353 => Some(Self::L353),
+            _ => None,
+        }
+    }
+    fn to_int(self) -> u16 {
+        match self {
+            Self::L355 => 355,
+            Self::L354 => 354,
+            Self::L353 => 353,
+        }
+    }
+}
+
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub(crate) struct IslamicYearInfo {
     packed_data: PackedIslamicYearInfo,
-    /// Is the previous year 355 days (short = 354)
-    prev_year_long: bool,
+    prev_year_length: IslamicYearLength,
 }
 
 impl IslamicYearInfo {
     pub(crate) const LONG_YEAR_LEN: u16 = 355;
     const SHORT_YEAR_LEN: u16 = 354;
-    pub(crate) fn new(prev_year_long: bool, packed_data: PackedIslamicYearInfo) -> Self {
-        Self {
-            prev_year_long,
-            packed_data,
-        }
+    pub(crate) fn new(
+        prev_packed: PackedIslamicYearInfo,
+        this_packed: PackedIslamicYearInfo,
+        extended_year: i32,
+    ) -> (Self, i32) {
+        let days_in_year = prev_packed.days_in_year();
+        let days_in_year = match IslamicYearLength::try_from_int(days_in_year as i64) {
+            Some(x) => x,
+            None => {
+                debug_assert!(false, "Found wrong year length for Islamic year {extended_year}: Expected 355, 354, or 353, got {days_in_year}");
+                Default::default()
+            }
+        };
+        let year_info = Self {
+            prev_year_length: days_in_year,
+            packed_data: this_packed,
+        };
+        (year_info, extended_year)
     }
 
     fn compute<IB: IslamicBasedMarker>(extended_year: i32) -> Self {
         let ny = IB::fixed_from_islamic(extended_year, 1, 1);
         let packed_data = PackedIslamicYearInfo::compute_with_ny::<IB>(extended_year, ny);
         let prev_ny = IB::fixed_from_islamic(extended_year - 1, 1, 1);
-        let diff = u16::try_from(ny - prev_ny).unwrap_or(0);
-        debug_assert!(
-            diff == Self::SHORT_YEAR_LEN || diff == Self::LONG_YEAR_LEN,
-            "Found wrong year length for Islamic year {}: Expected 355 or 354, got {diff}",
-            extended_year - 1
-        );
-        Self::new(diff == Self::LONG_YEAR_LEN, packed_data)
+        let rd_diff = ny - prev_ny;
+        let rd_diff = match IslamicYearLength::try_from_int(rd_diff) {
+            Some(x) => x,
+            None => {
+                debug_assert!(false, "({}) Found wrong year length for Islamic year {extended_year}: Expected 355, 354, or 353, got {rd_diff}", IB::DEBUG_NAME);
+                Default::default()
+            }
+        };
+        Self {
+            prev_year_length: rd_diff,
+            packed_data,
+        }
     }
     /// Get the new year R.D. given the extended year that this yearinfo is for    
     fn new_year<IB: IslamicBasedMarker>(self, extended_year: i32) -> RataDie {
@@ -266,11 +321,7 @@ impl IslamicYearInfo {
 
     #[inline]
     fn days_in_prev_year(self) -> u16 {
-        if self.prev_year_long {
-            Self::LONG_YEAR_LEN
-        } else {
-            Self::SHORT_YEAR_LEN
-        }
+        self.prev_year_length.to_int()
     }
 }
 
@@ -848,9 +899,9 @@ impl CalendarArithmetic for IslamicCivil {
 
     fn days_in_provided_year(year: i32, _data: ()) -> u16 {
         if Self::is_leap_year(year, ()) {
-            355
+            IslamicYearInfo::LONG_YEAR_LEN
         } else {
-            354
+            IslamicYearInfo::SHORT_YEAR_LEN
         }
     }
 
@@ -1092,9 +1143,9 @@ impl CalendarArithmetic for IslamicTabular {
 
     fn days_in_provided_year(year: i32, _data: ()) -> u16 {
         if Self::is_leap_year(year, ()) {
-            355
+            IslamicYearInfo::LONG_YEAR_LEN
         } else {
-            354
+            IslamicYearInfo::SHORT_YEAR_LEN
         }
     }
 
@@ -2176,4 +2227,17 @@ mod test {
         assert_eq!(islamic.month().ordinal, 4);
         assert_eq!(islamic.year().number, 1432);
     }
+
+    #[test]
+    fn test_regression_4914() {
+        // https://github.com/unicode-org/icu4x/issues/4914
+        let cal = IslamicUmmAlQura::new_always_calculating();
+        let era = "ah".parse().unwrap();
+        let year = -6823;
+        let month_code = "M01".parse().unwrap();
+        let dt = cal.date_from_codes(era, year, month_code, 1).unwrap();
+        assert_eq!(dt.0.day, 1);
+        assert_eq!(dt.0.month, 1);
+        assert_eq!(dt.0.year, -6823);
+    }
 }

components/calendar/src/iso.rs

@@ -834,49 +834,4 @@ mod test {
         check(-1439, 1969, 12, 31, 0, 1);
         check(-2879, 1969, 12, 30, 0, 1);
     }
-
-    #[test]
-    fn test_continuity_near_year_zero() {
-        // https://github.com/unicode-org/icu4x/issues/4893
-        const ONE_DAY_DURATION: DateDuration<Iso> = DateDuration {
-            years: 0,
-            months: 0,
-            weeks: 0,
-            days: 1,
-            marker: core::marker::PhantomData,
-        };
-
-        let mut iso_date = Date::try_new_iso_date(-10, 1, 1).unwrap();
-        let mut rata_die = iso_date.to_fixed();
-        let mut weekday = iso_date.day_of_week();
-
-        for _ in 0..(366 * 20) {
-            let next_iso_date = iso_date.added(ONE_DAY_DURATION);
-            let next_rata_die = next_iso_date.to_fixed();
-            assert_eq!(
-                next_rata_die,
-                rata_die + 1,
-                "{iso_date:?}..{next_iso_date:?}"
-            );
-            let next_weekday = next_iso_date.day_of_week();
-            assert_eq!(
-                (next_weekday as usize) % 7,
-                (weekday as usize + 1) % 7,
-                "{iso_date:?}..{next_iso_date:?}"
-            );
-            if iso_date.month().code.parsed().unwrap().0 == 2 && iso_date.day_of_month().0 == 28 {
-                assert_eq!(next_iso_date.is_in_leap_year(), iso_date.is_in_leap_year());
-                if iso_date.is_in_leap_year() {
-                    assert_eq!(next_iso_date.month().code.parsed().unwrap().0, 2);
-                    assert_eq!(next_iso_date.day_of_month().0, 29);
-                } else {
-                    assert_eq!(next_iso_date.month().code.parsed().unwrap().0, 3);
-                    assert_eq!(next_iso_date.day_of_month().0, 1);
-                }
-            }
-            iso_date = next_iso_date;
-            rata_die = next_rata_die;
-            weekday = next_weekday;
-        }
-    }
 }

components/calendar/src/lib.rs

@@ -136,6 +136,8 @@ pub mod julian;
 pub mod persian;
 pub mod provider;
 pub mod roc;
+#[cfg(test)]
+mod tests;
 pub mod types;
 mod week_of;
 

components/calendar/src/provider/chinese_based.rs

@@ -151,8 +151,11 @@ impl PackedChineseBasedYearInfo {
     ///
     /// According to Reingold & Dershowitz, ch 19.6, Chinese New Year occurs on Jan 21 - Feb 21 inclusive.
     ///
-    /// Chinese New Year in the year 30 AD is January 20 (30-01-20)
-    pub(crate) const FIRST_NY: u8 = 20;
+    /// Chinese New Year in the year 30 AD is January 20 (30-01-20).
+    ///
+    /// We allow it to occur as early as January 19 which is the earliest the second new moon
+    /// could occur after the Winter Solstice if the solstice is pinned to December 20.
+    pub(crate) const FIRST_NY: u8 = 19;
 
     pub(crate) fn new(
         month_lengths: [bool; 13],
@@ -163,7 +166,7 @@ impl PackedChineseBasedYearInfo {
             !month_lengths[12] || leap_month_idx.is_some(),
             "Last month length should not be set for non-leap years"
         );
-        debug_assert!(ny_offset < 33, "Year offset too big to store");
+        debug_assert!(ny_offset < 34, "Year offset too big to store");
         debug_assert!(
             leap_month_idx.map(|l| l.get() <= 13).unwrap_or(true),
             "Leap month indices must be 1 <= i <= 13"