Temporal duration compare (#186)

Fixes #154

Wrote this along with @lockels, @Neelzee, @sebastianjacmatt,
@Magnus-Fjeldstad, @HenrikTennebekk.

Please see question regarding the return type of DateDurationDays.

---------

Co-authored-by: SebastianJM <[email protected]>
Co-authored-by: Henrik Tennebekk <[email protected]>
Co-authored-by: Mafje8943 <[email protected]>
Co-authored-by: Idris Elmi <[email protected]>
Co-authored-by: lockels <[email protected]>

Author: 5 people

src/builtins/compiled/duration.rs

@@ -4,6 +4,8 @@ use crate::{
     Duration, TemporalError, TemporalResult,
 };
 
+use core::cmp::Ordering;
+
 #[cfg(test)]
 mod tests;
 
@@ -23,4 +25,20 @@ impl Duration {
         self.round_with_provider(options, relative_to, &*provider)
             .map(Into::into)
     }
+
+    /// Returns the ordering between two [`Duration`], takes an optional
+    /// [`RelativeTo`]
+    ///
+    /// Enable with the `compiled_data` feature flag.
+    pub fn compare(
+        &self,
+        two: &Duration,
+        relative_to: Option<RelativeTo>,
+    ) -> TemporalResult<Ordering> {
+        let provider = TZ_PROVIDER
+            .lock()
+            .map_err(|_| TemporalError::general("Unable to acquire lock"))?;
+        self.compare_with_provider(two, relative_to, &*provider)
+            .map(Into::into)
+    }
 }

src/builtins/compiled/duration/tests.rs

@@ -1,8 +1,15 @@
+use std::string::ToString;
+
 use crate::{
-    options::{RelativeTo, RoundingIncrement, RoundingOptions, TemporalRoundingMode, TemporalUnit},
+    options::{
+        OffsetDisambiguation, RelativeTo, RoundingIncrement, RoundingOptions, TemporalRoundingMode,
+        TemporalUnit,
+    },
+    partial::PartialDuration,
     primitive::FiniteF64,
     Calendar, DateDuration, PlainDate, TimeDuration, TimeZone, ZonedDateTime,
 };
+
 use alloc::vec::Vec;
 use core::str::FromStr;
 
@@ -629,3 +636,52 @@ fn round_relative_to_zoned_datetime() {
     assert_eq!(result.days(), 1.0);
     assert_eq!(result.hours(), 1.0);
 }
+
+#[test]
+fn test_duration_compare() {
+    // TODO(#199): fix this on Windows
+    if cfg!(not(windows)) {
+        let one = Duration::from_partial_duration(PartialDuration {
+            hours: Some(FiniteF64::from(79)),
+            minutes: Some(FiniteF64::from(10)),
+            ..Default::default()
+        })
+        .unwrap();
+        let two = Duration::from_partial_duration(PartialDuration {
+            days: Some(FiniteF64::from(3)),
+            hours: Some(FiniteF64::from(7)),
+            seconds: Some(FiniteF64::from(630)),
+            ..Default::default()
+        })
+        .unwrap();
+        let three = Duration::from_partial_duration(PartialDuration {
+            days: Some(FiniteF64::from(3)),
+            hours: Some(FiniteF64::from(6)),
+            minutes: Some(FiniteF64::from(50)),
+            ..Default::default()
+        })
+        .unwrap();
+
+        let mut arr = [&one, &two, &three];
+        arr.sort_by(|a, b| Duration::compare(a, b, None).unwrap());
+        assert_eq!(
+            arr.map(ToString::to_string),
+            [&three, &one, &two].map(ToString::to_string)
+        );
+
+        // Sorting relative to a date, taking DST changes into account:
+        let zdt = ZonedDateTime::from_str(
+            "2020-11-01T00:00-07:00[America/Los_Angeles]",
+            Default::default(),
+            OffsetDisambiguation::Reject,
+        )
+        .unwrap();
+        arr.sort_by(|a, b| {
+            Duration::compare(a, b, Some(RelativeTo::ZonedDateTime(zdt.clone()))).unwrap()
+        });
+        assert_eq!(
+            arr.map(ToString::to_string),
+            [&one, &three, &two].map(ToString::to_string)
+        )
+    }
+}

src/builtins/core/duration.rs

@@ -16,7 +16,7 @@ use alloc::format;
 use alloc::string::String;
 use alloc::vec;
 use alloc::vec::Vec;
-use core::str::FromStr;
+use core::{cmp::Ordering, str::FromStr};
 use ixdtf::parsers::{
     records::{DateDurationRecord, DurationParseRecord, Sign as IxdtfSign, TimeDurationRecord},
     IsoDurationParser,
@@ -277,6 +277,63 @@ impl Duration {
     pub fn is_time_within_range(&self) -> bool {
         self.time.is_within_range()
     }
+
+    #[inline]
+    pub fn compare_with_provider(
+        &self,
+        other: &Duration,
+        relative_to: Option<RelativeTo>,
+        provider: &impl TimeZoneProvider,
+    ) -> TemporalResult<Ordering> {
+        if self.date == other.date && self.time == other.time {
+            return Ok(Ordering::Equal);
+        }
+        // 8. Let largestUnit1 be DefaultTemporalLargestUnit(one).
+        // 9. Let largestUnit2 be DefaultTemporalLargestUnit(two).
+        let largest_unit_1 = self.default_largest_unit();
+        let largest_unit_2 = other.default_largest_unit();
+        // 10. Let duration1 be ToInternalDurationRecord(one).
+        // 11. Let duration2 be ToInternalDurationRecord(two).
+        // 12. If zonedRelativeTo is not undefined, and either TemporalUnitCategory(largestUnit1) or TemporalUnitCategory(largestUnit2) is date, then
+        if let Some(RelativeTo::ZonedDateTime(zdt)) = relative_to.as_ref() {
+            if largest_unit_1.is_date_unit() || largest_unit_2.is_date_unit() {
+                // a. Let timeZone be zonedRelativeTo.[[TimeZone]].
+                // b. Let calendar be zonedRelativeTo.[[Calendar]].
+                // c. Let after1 be ? AddZonedDateTime(zonedRelativeTo.[[EpochNanoseconds]], timeZone, calendar, duration1, constrain).
+                // d. Let after2 be ? AddZonedDateTime(zonedRelativeTo.[[EpochNanoseconds]], timeZone, calendar, duration2, constrain).
+                let after1 = zdt.add_as_instant(self, ArithmeticOverflow::Constrain, provider)?;
+                let after2 = zdt.add_as_instant(other, ArithmeticOverflow::Constrain, provider)?;
+                // e. If after1 > after2, return 1𝔽.
+                // f. If after1 < after2, return -1𝔽.
+                // g. Return +0𝔽.
+                return Ok(after1.cmp(&after2));
+            }
+        }
+        // 13. If IsCalendarUnit(largestUnit1) is true or IsCalendarUnit(largestUnit2) is true, then
+        let (days1, days2) =
+            if largest_unit_1.is_calendar_unit() || largest_unit_2.is_calendar_unit() {
+                // a. If plainRelativeTo is undefined, throw a RangeError exception.
+                // b. Let days1 be ? DateDurationDays(duration1.[[Date]], plainRelativeTo).
+                // c. Let days2 be ? DateDurationDays(duration2.[[Date]], plainRelativeTo).
+                let Some(RelativeTo::PlainDate(pdt)) = relative_to.as_ref() else {
+                    return Err(TemporalError::range());
+                };
+                let days1 = self.date.days(pdt)?;
+                let days2 = other.date.days(pdt)?;
+                (days1, days2)
+            } else {
+                (
+                    self.date.days.as_integer_if_integral()?,
+                    other.date.days.as_integer_if_integral()?,
+                )
+            };
+        // 15. Let timeDuration1 be ? Add24HourDaysToTimeDuration(duration1.[[Time]], days1).
+        let time_duration_1 = self.time.to_normalized().add_days(days1)?;
+        // 16. Let timeDuration2 be ? Add24HourDaysToTimeDuration(duration2.[[Time]], days2).
+        let time_duration_2 = other.time.to_normalized().add_days(days2)?;
+        // 17. Return 𝔽(CompareTimeDuration(timeDuration1, timeDuration2)).
+        Ok(time_duration_1.cmp(&time_duration_2))
+    }
 }
 
 // ==== Public `Duration` Getters/Setters ====
@@ -323,7 +380,7 @@ impl Duration {
         self.date.weeks
     }
 
-    /// Returns the `weeks` field of duration.
+    /// Returns the `days` field of duration.
     #[inline]
     #[must_use]
     pub const fn days(&self) -> FiniteF64 {
@@ -572,6 +629,7 @@ impl Duration {
                     self.weeks(),
                     self.days().checked_add(&FiniteF64::from(balanced_days))?,
                 )?;
+                // TODO: Should this be using AdjustDateDurationRecord?
 
                 // c. Let targetDate be ? AddDate(calendarRec, plainRelativeTo, dateDuration).
                 let target_date = plain_date.add_date(&Duration::from(date_duration), None)?;

src/builtins/core/duration/date.rs

@@ -1,6 +1,9 @@
 //! Implementation of a `DateDuration`
 
-use crate::{primitive::FiniteF64, Sign, TemporalError, TemporalResult};
+use crate::{
+    iso::iso_date_to_epoch_days, options::ArithmeticOverflow, primitive::FiniteF64, Duration,
+    PlainDate, Sign, TemporalError, TemporalResult,
+};
 use alloc::vec::Vec;
 
 /// `DateDuration` represents the [date duration record][spec] of the `Duration.`
@@ -10,7 +13,7 @@ use alloc::vec::Vec;
 /// [spec]: https://tc39.es/proposal-temporal/#sec-temporal-date-duration-records
 /// [field spec]: https://tc39.es/proposal-temporal/#sec-properties-of-temporal-duration-instances
 #[non_exhaustive]
-#[derive(Debug, Default, Clone, Copy)]
+#[derive(Debug, Default, Clone, Copy, PartialEq, PartialOrd)]
 pub struct DateDuration {
     /// `DateDuration`'s internal year value.
     pub years: FiniteF64,
@@ -105,4 +108,58 @@ impl DateDuration {
     pub fn sign(&self) -> Sign {
         super::duration_sign(&self.fields())
     }
+
+    /// DateDurationDays
+    pub(crate) fn days(&self, relative_to: &PlainDate) -> TemporalResult<i64> {
+        // 1. Let yearsMonthsWeeksDuration be ! AdjustDateDurationRecord(dateDuration, 0).
+        let ymw_duration = self.adjust(FiniteF64(0.0), None, None)?;
+        // 2. If DateDurationSign(yearsMonthsWeeksDuration) = 0, return dateDuration.[[Days]].
+        if ymw_duration.sign() == Sign::Zero {
+            return self.days.as_integer_if_integral();
+        }
+        // 3. Let later be ? CalendarDateAdd(plainRelativeTo.[[Calendar]], plainRelativeTo.[[ISODate]], yearsMonthsWeeksDuration, constrain).
+        let later = relative_to.add(
+            &Duration {
+                date: *self,
+                time: Default::default(),
+            },
+            Some(ArithmeticOverflow::Constrain),
+        )?;
+        // 4. Let epochDays1 be ISODateToEpochDays(plainRelativeTo.[[ISODate]].[[Year]], plainRelativeTo.[[ISODate]].[[Month]] - 1, plainRelativeTo.[[ISODate]].[[Day]]).
+        let epoch_days_1 = iso_date_to_epoch_days(
+            relative_to.iso_year(),
+            i32::from(relative_to.iso_month()), // this function takes 1 based month number
+            i32::from(relative_to.iso_day()),
+        );
+        // 5. Let epochDays2 be ISODateToEpochDays(later.[[Year]], later.[[Month]] - 1, later.[[Day]]).
+        let epoch_days_2 = iso_date_to_epoch_days(
+            later.iso_year(),
+            i32::from(later.iso_month()), // this function takes 1 based month number
+            i32::from(later.iso_day()),
+        );
+        // 6. Let yearsMonthsWeeksInDays be epochDays2 - epochDays1.
+        let ymd_in_days = epoch_days_2 - epoch_days_1;
+        // 7. Return dateDuration.[[Days]] + yearsMonthsWeeksInDays.
+        Ok(self.days.as_integer_if_integral::<i64>()? + i64::from(ymd_in_days))
+    }
+
+    /// AdjustDateDurationRecord
+    pub(crate) fn adjust(
+        &self,
+        days: FiniteF64,
+        weeks: Option<FiniteF64>,
+        months: Option<FiniteF64>,
+    ) -> TemporalResult<Self> {
+        // 1. If weeks is not present, set weeks to dateDuration.[[Weeks]].
+        // 2. If months is not present, set months to dateDuration.[[Months]].
+        // 3. Return ? CreateDateDurationRecord(dateDuration.[[Years]], months, weeks, days).
+        let weeks = weeks.unwrap_or(self.weeks);
+        let months = months.unwrap_or(self.months);
+        Ok(Self {
+            years: self.years,
+            months,
+            weeks,
+            days,
+        })
+    }
 }

src/builtins/core/duration/normalized.rs

@@ -36,7 +36,7 @@ const NANOSECONDS_PER_HOUR: i128 = 60 * NANOSECONDS_PER_MINUTE;
 // nanoseconds.abs() <= MAX_TIME_DURATION
 
 /// A Normalized `TimeDuration` that represents the current `TimeDuration` in nanoseconds.
-#[derive(Debug, Clone, Copy, Default, PartialEq, PartialOrd)]
+#[derive(Debug, Clone, Copy, Default, PartialEq, PartialOrd, Eq, Ord)]
 pub(crate) struct NormalizedTimeDuration(pub(crate) i128);
 
 impl NormalizedTimeDuration {
@@ -66,6 +66,7 @@ impl NormalizedTimeDuration {
 
     // NOTE: `days: f64` should be an integer -> `i64`.
     /// Equivalent: 7.5.23 Add24HourDaysToNormalizedTimeDuration ( d, days )
+    /// Add24HourDaysToTimeDuration??
     pub(crate) fn add_days(&self, days: i64) -> TemporalResult<Self> {
         let result = self.0 + i128::from(days) * i128::from(NS_PER_DAY);
         if result.abs() > MAX_TIME_DURATION {

src/iso.rs

@@ -927,8 +927,9 @@ fn to_unchecked_epoch_nanoseconds(date: IsoDate, time: &IsoTime) -> i128 {
 // ==== `IsoDate` specific utiltiy functions ====
 
 /// Returns the Epoch days based off the given year, month, and day.
+/// Note: Month should be 1 indexed
 #[inline]
-fn iso_date_to_epoch_days(year: i32, month: i32, day: i32) -> i32 {
+pub(crate) fn iso_date_to_epoch_days(year: i32, month: i32, day: i32) -> i32 {
     // 1. Let resolvedYear be year + floor(month / 12).
     let resolved_year = year + month.div_euclid(12);
     // 2. Let resolvedMonth be month modulo 12.

src/options.rs

@@ -471,6 +471,13 @@ impl TemporalUnit {
         matches!(self, Year | Month | Week)
     }
 
+    #[inline]
+    #[must_use]
+    pub fn is_date_unit(&self) -> bool {
+        use TemporalUnit::{Day, Month, Week, Year};
+        matches!(self, Day | Year | Month | Week)
+    }
+
     #[inline]
     #[must_use]
     pub fn is_time_unit(&self) -> bool {
@@ -644,9 +651,12 @@ impl fmt::Display for DurationOverflow {
 }
 
 /// The disambiguation options for an instant.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
 pub enum Disambiguation {
     /// Compatible option
+    ///
+    /// This is the default according to GetTemporalDisambiguationOption
+    #[default]
     Compatible,
     /// Earlier option
     Earlier,