Unify day-of-week calculation (#6368)

Weekdays are not calender-specific, and the current code is overly
complex.

Author: robertbastian

components/calendar/src/cal/coptic.rs

@@ -131,10 +131,6 @@ impl Calendar for Coptic {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(Coptic.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &());
     }

components/calendar/src/cal/dangi.rs

@@ -242,10 +242,6 @@ impl Calendar for Dangi {
         date.0 .0.day_of_year()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> crate::types::Weekday {
-        self.date_to_iso(date).day_of_week()
-    }
-
     fn any_calendar_kind(&self) -> Option<crate::AnyCalendarKind> {
         Some(crate::any_calendar::IntoAnyCalendar::kind(self))
     }

components/calendar/src/cal/ethiopian.rs

@@ -158,10 +158,6 @@ impl Calendar for Ethiopian {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(self.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &());
     }

components/calendar/src/cal/hebrew.rs

@@ -497,10 +497,10 @@ mod tests {
         let cal = Hebrew::new();
         let era = "am";
         let month_code = MonthCode(tinystr!(4, "M01"));
-        let dt = cal.date_from_codes(Some(era), 3760, month_code, 1).unwrap();
+        let dt = Date::try_new_from_codes(Some(era), 3760, month_code, 1, cal).unwrap();
 
         // Should be Saturday per:
         // https://www.hebcal.com/converter?hd=1&hm=Tishrei&hy=3760&h2g=1
-        assert_eq!(6, cal.day_of_week(&dt) as usize);
+        assert_eq!(6, dt.day_of_week() as usize);
     }
 }

components/calendar/src/cal/hijri.rs

@@ -484,10 +484,6 @@ impl Calendar for HijriObservational {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(self.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &self.precomputed_data())
     }
@@ -839,10 +835,6 @@ impl Calendar for HijriCivil {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(self.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &())
     }
@@ -1020,10 +1012,6 @@ impl Calendar for HijriTabular {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(self.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &())
     }

components/calendar/src/cal/indian.rs

@@ -156,10 +156,6 @@ impl Calendar for Indian {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(Indian.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &());
     }

components/calendar/src/cal/iso.rs

@@ -114,54 +114,6 @@ impl Calendar for Iso {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        // For the purposes of the calculation here, Monday is 0, Sunday is 6
-        // ISO has Monday=1, Sunday=7, which we transform in the last step
-
-        // The days of the week are the same every 400 years
-        // so we normalize to the nearest multiple of 400
-        let years_since_400 = date.0.year.rem_euclid(400);
-        debug_assert!(years_since_400 >= 0); // rem_euclid returns positive numbers
-        let years_since_400 = years_since_400 as u32;
-        let leap_years_since_400 = years_since_400 / 4 - years_since_400 / 100;
-        // The number of days to the current year
-        // Can never cause an overflow because years_since_400 has a maximum value of 399.
-        let days_to_current_year = 365 * years_since_400 + leap_years_since_400;
-        // The weekday offset from January 1 this year and January 1 2000
-        let year_offset = days_to_current_year % 7;
-
-        // Corresponding months from
-        // https://en.wikipedia.org/wiki/Determination_of_the_day_of_the_week#Corresponding_months
-        let month_offset = if Self::is_leap_year(date.0.year, ()) {
-            match date.0.month {
-                10 => 0,
-                5 => 1,
-                2 | 8 => 2,
-                3 | 11 => 3,
-                6 => 4,
-                9 | 12 => 5,
-                1 | 4 | 7 => 6,
-                _ => unreachable!(),
-            }
-        } else {
-            match date.0.month {
-                1 | 10 => 0,
-                5 => 1,
-                8 => 2,
-                2 | 3 | 11 => 3,
-                6 => 4,
-                9 | 12 => 5,
-                4 | 7 => 6,
-                _ => unreachable!(),
-            }
-        };
-        let january_1_2000 = 5; // Saturday
-        let day_offset = (january_1_2000 + year_offset + month_offset + date.0.day as u32) % 7;
-
-        // We calculated in a zero-indexed fashion, but ISO specifies one-indexed
-        types::Weekday::from((day_offset + 1) as usize)
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &());
     }

components/calendar/src/cal/julian.rs

@@ -122,10 +122,6 @@ impl Calendar for Julian {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(Julian.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &());
     }

components/calendar/src/cal/persian.rs

@@ -123,10 +123,6 @@ impl Calendar for Persian {
         date.0.days_in_month()
     }
 
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        Iso.day_of_week(self.date_to_iso(date).inner())
-    }
-
     fn offset_date(&self, date: &mut Self::DateInner, offset: DateDuration<Self>) {
         date.0.offset_date(offset, &())
     }

components/calendar/src/calendar.rs

@@ -47,10 +47,6 @@ pub trait Calendar {
     /// Count the number of days in a given month, specified by providing a date
     /// from that year/month
     fn days_in_month(&self, date: &Self::DateInner) -> u8;
-    /// Calculate the day of the week and return it
-    fn day_of_week(&self, date: &Self::DateInner) -> types::Weekday {
-        self.date_to_iso(date).day_of_week()
-    }
     // fn week_of_year(&self, date: &Self::DateInner) -> u8;
 
     #[doc(hidden)] // unstable

components/calendar/src/date.rs

@@ -168,11 +168,9 @@ impl<A: AsCalendar> Date<A> {
     }
 
     /// The day of the week for this date
-    ///
-    /// Monday is 1, Sunday is 7, according to ISO
     #[inline]
     pub fn day_of_week(&self) -> types::Weekday {
-        self.calendar.as_calendar().day_of_week(self.inner())
+        Iso::to_fixed(self.to_iso()).into()
     }
 
     /// Add a `duration` to this date, mutating it
@@ -449,6 +447,7 @@ impl<A> Copy for Date<A> where A: AsCalendar + Copy {}
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::types::Weekday;
 
     #[test]
     fn test_ord() {
@@ -478,4 +477,23 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_day_of_week() {
+        // June 23, 2021 is a Wednesday
+        assert_eq!(
+            Date::try_new_iso(2021, 6, 23).unwrap().day_of_week(),
+            Weekday::Wednesday,
+        );
+        // Feb 2, 1983 was a Wednesday
+        assert_eq!(
+            Date::try_new_iso(1983, 2, 2).unwrap().day_of_week(),
+            Weekday::Wednesday,
+        );
+        // Jan 21, 2021 was a Tuesday
+        assert_eq!(
+            Date::try_new_iso(2020, 1, 21).unwrap().day_of_week(),
+            Weekday::Tuesday,
+        );
+    }
 }

components/calendar/src/types.rs

@@ -4,6 +4,7 @@
 
 //! This module contains various types used by `icu_calendar` and `icu::datetime`
 
+use calendrical_calculations::rata_die::RataDie;
 use core::fmt;
 use core::num::NonZeroU8;
 use tinystr::TinyAsciiStr;
@@ -420,23 +421,13 @@ pub enum Weekday {
     Sunday,
 }
 
-impl From<usize> for Weekday {
-    /// Convert from an ISO-8601 weekday number to an [`Weekday`] enum. 0 is automatically converted
-    /// to 7 (Sunday). If the number is out of range, it is interpreted modulo 7.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use icu::calendar::types::Weekday;
-    ///
-    /// assert_eq!(Weekday::Sunday, Weekday::from(0));
-    /// assert_eq!(Weekday::Monday, Weekday::from(1));
-    /// assert_eq!(Weekday::Sunday, Weekday::from(7));
-    /// assert_eq!(Weekday::Monday, Weekday::from(8));
-    /// ```
-    fn from(input: usize) -> Self {
+// RD 0 is a Sunday
+const SUNDAY: RataDie = RataDie::new(0);
+
+impl From<RataDie> for Weekday {
+    fn from(value: RataDie) -> Self {
         use Weekday::*;
-        match input % 7 {
+        match (value - SUNDAY).rem_euclid(7) {
             0 => Sunday,
             1 => Monday,
             2 => Tuesday,
@@ -450,6 +441,23 @@ impl From<usize> for Weekday {
 }
 
 impl Weekday {
+    /// Convert from an ISO-8601 weekday number to an [`Weekday`] enum. 0 is automatically converted
+    /// to 7 (Sunday). If the number is out of range, it is interpreted modulo 7.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use icu::calendar::types::Weekday;
+    ///
+    /// assert_eq!(Weekday::Sunday, Weekday::from_days_since_sunday(0));
+    /// assert_eq!(Weekday::Monday, Weekday::from_days_since_sunday(1));
+    /// assert_eq!(Weekday::Sunday, Weekday::from_days_since_sunday(7));
+    /// assert_eq!(Weekday::Monday, Weekday::from_days_since_sunday(8));
+    /// ```
+    pub fn from_days_since_sunday(input: isize) -> Self {
+        (SUNDAY + input as i64).into()
+    }
+
     /// Returns the day after the current day.
     pub(crate) fn next_day(self) -> Weekday {
         use Weekday::*;

components/calendar/src/week.rs

@@ -126,7 +126,7 @@ impl WeekCalculator {
 /// Returns the weekday that's `num_days` after `weekday`.
 fn add_to_weekday(weekday: Weekday, num_days: i32) -> Weekday {
     let new_weekday = (7 + (weekday as i32) + (num_days % 7)) % 7;
-    Weekday::from(new_weekday as usize)
+    Weekday::from_days_since_sunday(new_weekday as isize)
 }
 
 /// Which year or month that a calendar assigns a week to relative to the year/month
@@ -409,13 +409,16 @@ mod tests {
         for min_week_days in 1..7 {
             for start_of_week in 1..7 {
                 let calendar = WeekCalculator {
-                    first_weekday: Weekday::from(start_of_week),
+                    first_weekday: Weekday::from_days_since_sunday(start_of_week),
                     min_week_days,
                 };
                 for unit_duration in super::MIN_UNIT_DAYS..400 {
                     for start_of_unit in 1..7 {
-                        let unit =
-                            UnitInfo::new(Weekday::from(start_of_unit), unit_duration).unwrap();
+                        let unit = UnitInfo::new(
+                            Weekday::from_days_since_sunday(start_of_unit),
+                            unit_duration,
+                        )
+                        .unwrap();
                         let expected = classify_days_of_unit(calendar, &unit);
                         for (index, expected_week_of) in expected.iter().enumerate() {
                             let day = index + 1;