From 624157700a424e3a062d841a710024ce3a1f4d70 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Mon, 27 Jan 2025 06:33:46 -0800 Subject: [PATCH 1/5] Initial implementation of Temporal.Duration.compare --- src/builtins/core/duration.rs | 64 +++++++++++++++++++++++- src/builtins/core/duration/date.rs | 63 ++++++++++++++++++++++- src/builtins/core/duration/normalized.rs | 3 +- src/builtins/core/duration/tests.rs | 45 ++++++++++++++++- src/iso.rs | 2 +- src/options.rs | 12 ++++- 6 files changed, 181 insertions(+), 8 deletions(-) diff --git a/src/builtins/core/duration.rs b/src/builtins/core/duration.rs index 973f93b4..857053b9 100644 --- a/src/builtins/core/duration.rs +++ b/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, @@ -77,7 +77,7 @@ impl PartialDuration { /// `Duration` is made up of a `DateDuration` and `TimeDuration` as primarily /// defined by Abtract Operation 7.5.1-5. #[non_exhaustive] -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, Clone, Copy, Default, PartialEq, PartialOrd)] pub struct Duration { date: DateDuration, time: TimeDuration, @@ -277,6 +277,65 @@ impl Duration { pub fn is_time_within_range(&self) -> bool { self.time.is_within_range() } + + /// Returns the ordering between the two durations. + /// + /// `7.2.3 Temporal.Duration.compare ( one, two [ , options ] )` + #[must_use] + pub fn compare( + one: &Duration, + two: &Duration, + relative_to: Option, + ) -> TemporalResult { + if one == two { + return Ok(Ordering::Equal); + } + // 8. Let largestUnit1 be DefaultTemporalLargestUnit(one). + // 9. Let largestUnit2 be DefaultTemporalLargestUnit(two). + let largest_unit_1 = one.default_largest_unit(); + let largest_unit_2 = two.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(one, Some(ArithmeticOverflow::Constrain))?; + let after2 = zdt.add(two, Some(ArithmeticOverflow::Constrain))?; + // 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 = one.date.days(pdt)?; + let days2 = two.date.days(pdt)?; + (days1, days2) + } else { + ( + one.date.days.as_integer_if_integral()?, + two.date.days.as_integer_if_integral()?, + ) + }; + // 15. Let timeDuration1 be ? Add24HourDaysToTimeDuration(duration1.[[Time]], days1). + let time_duration_1 = one.time.to_normalized().add_days(days1)?; + // 16. Let timeDuration2 be ? Add24HourDaysToTimeDuration(duration2.[[Time]], days2). + let time_duration_2 = two.time.to_normalized().add_days(days2)?; + // 17. Return 𝔽(CompareTimeDuration(timeDuration1, timeDuration2)). + Ok(time_duration_1.cmp(&time_duration_2)) + } } // ==== Public `Duration` Getters/Setters ==== @@ -572,6 +631,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)?; diff --git a/src/builtins/core/duration/date.rs b/src/builtins/core/duration/date.rs index ef85ea8f..46eeb1b4 100644 --- a/src/builtins/core/duration/date.rs +++ b/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,60 @@ impl DateDuration { pub fn sign(&self) -> Sign { super::duration_sign(&self.fields()) } + + /// DateDurationDays + /// TODO(review): Question: what the correct return type? + pub fn days(&self, relative_to: &PlainDate) -> TemporalResult { + // 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()) - 1, + 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()) - 1, + 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::try_from(ymd_in_days).map_err(|_| TemporalError::range())?) + } + + /// AdjustDateDurationRecord + pub fn adjust( + &self, + days: FiniteF64, + weeks: Option, + months: Option, + ) -> TemporalResult { + // 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, + }) + } } diff --git a/src/builtins/core/duration/normalized.rs b/src/builtins/core/duration/normalized.rs index 3055bc23..d718b489 100644 --- a/src/builtins/core/duration/normalized.rs +++ b/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 { let result = self.0 + i128::from(days) * i128::from(NS_PER_DAY); if result.abs() > MAX_TIME_DURATION { diff --git a/src/builtins/core/duration/tests.rs b/src/builtins/core/duration/tests.rs index a3c5e692..7e0d1f56 100644 --- a/src/builtins/core/duration/tests.rs +++ b/src/builtins/core/duration/tests.rs @@ -1,6 +1,9 @@ use crate::{ - options::ToStringRoundingOptions, parsers::Precision, partial::PartialDuration, + options::{OffsetDisambiguation, RelativeTo, ToStringRoundingOptions}, + parsers::Precision, + partial::PartialDuration, primitive::FiniteF64, + ZonedDateTime, }; use super::Duration; @@ -186,3 +189,43 @@ fn preserve_precision_loss() { assert_eq!(&result, "PT9016206453995.731991S"); } + +#[test] +fn test_duration_compare() { + let one = Duration::from_partial_duration(PartialDuration { + hours: Some(FiniteF64::try_from(79).unwrap()), + minutes: Some(FiniteF64::try_from(10).unwrap()), + ..Default::default() + }) + .unwrap(); + let two = Duration::from_partial_duration(PartialDuration { + days: Some(FiniteF64::try_from(3).unwrap()), + hours: Some(FiniteF64::try_from(7).unwrap()), + seconds: Some(FiniteF64::try_from(630).unwrap()), + ..Default::default() + }) + .unwrap(); + let three = Duration::from_partial_duration(PartialDuration { + days: Some(FiniteF64::try_from(3).unwrap()), + hours: Some(FiniteF64::try_from(6).unwrap()), + minutes: Some(FiniteF64::try_from(50).unwrap()), + ..Default::default() + }) + .unwrap(); + + let mut arr = [&one, &two, &three]; + arr.sort_by(|a, b| Duration::compare(a, b, None).unwrap()); + assert_eq!(arr, [&three, &one, &two]); + + // 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, [&one, &three, &two]) +} diff --git a/src/iso.rs b/src/iso.rs index e4e53cee..66425e4a 100644 --- a/src/iso.rs +++ b/src/iso.rs @@ -931,7 +931,7 @@ fn to_unchecked_epoch_nanoseconds(date: IsoDate, time: &IsoTime) -> i128 { /// Returns the Epoch days based off the given year, month, and day. #[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. diff --git a/src/options.rs b/src/options.rs index 6b21468e..b6a89182 100644 --- a/src/options.rs +++ b/src/options.rs @@ -424,6 +424,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 { @@ -597,9 +604,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, From 749504c6461a23cd34333f55a796fb17eb838365 Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Mon, 27 Jan 2025 06:34:13 -0800 Subject: [PATCH 2/5] Fix pre-existing docs comment --- src/builtins/core/duration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builtins/core/duration.rs b/src/builtins/core/duration.rs index 857053b9..6cb192d8 100644 --- a/src/builtins/core/duration.rs +++ b/src/builtins/core/duration.rs @@ -382,7 +382,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 { From e845af648331dbf37ed22c0f3f9d48a63e2b5e72 Mon Sep 17 00:00:00 2001 From: SebastianJM Date: Tue, 28 Jan 2025 13:34:51 +0100 Subject: [PATCH 3/5] changed back to 1 indexed iso_date_to_epoch_days --- src/builtins/core/duration/date.rs | 4 ++-- src/iso.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/builtins/core/duration/date.rs b/src/builtins/core/duration/date.rs index 46eeb1b4..32fb73e3 100644 --- a/src/builtins/core/duration/date.rs +++ b/src/builtins/core/duration/date.rs @@ -129,13 +129,13 @@ impl DateDuration { // 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()) - 1, + 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()) - 1, + i32::from(later.iso_month()), // this function takes 1 based month number i32::from(later.iso_day()), ); // 6. Let yearsMonthsWeeksInDays be epochDays2 - epochDays1. diff --git a/src/iso.rs b/src/iso.rs index 66425e4a..c1e97a38 100644 --- a/src/iso.rs +++ b/src/iso.rs @@ -930,6 +930,7 @@ 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] pub(crate) fn iso_date_to_epoch_days(year: i32, month: i32, day: i32) -> i32 { // 1. Let resolvedYear be year + floor(month / 12). From 3ead5d18a533c290dd947d46fe6c266da3f87262 Mon Sep 17 00:00:00 2001 From: Henrik Tennebekk Date: Tue, 28 Jan 2025 14:18:40 +0100 Subject: [PATCH 4/5] Remove PartialEq and PartialOrd from Duration --- src/builtins/core/duration.rs | 14 ++++++++++++-- src/builtins/core/duration/tests.rs | 6 ++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/builtins/core/duration.rs b/src/builtins/core/duration.rs index 6cb192d8..0f5fe58c 100644 --- a/src/builtins/core/duration.rs +++ b/src/builtins/core/duration.rs @@ -77,7 +77,7 @@ impl PartialDuration { /// `Duration` is made up of a `DateDuration` and `TimeDuration` as primarily /// defined by Abtract Operation 7.5.1-5. #[non_exhaustive] -#[derive(Debug, Clone, Copy, Default, PartialEq, PartialOrd)] +#[derive(Debug, Clone, Copy, Default)] pub struct Duration { date: DateDuration, time: TimeDuration, @@ -278,6 +278,16 @@ impl Duration { self.time.is_within_range() } + /// Compares self with other on a field by field basis. + fn cmp_raw(&self, other: &Self) -> Option { + let date_ordering = self.date.partial_cmp(&other.date)?; + if date_ordering != Ordering::Equal { + return Some(date_ordering); + } + let time_ordering = self.time.partial_cmp(&other.time)?; + Some(time_ordering) + } + /// Returns the ordering between the two durations. /// /// `7.2.3 Temporal.Duration.compare ( one, two [ , options ] )` @@ -287,7 +297,7 @@ impl Duration { two: &Duration, relative_to: Option, ) -> TemporalResult { - if one == two { + if let Some(Ordering::Equal) = one.cmp_raw(two) { return Ok(Ordering::Equal); } // 8. Let largestUnit1 be DefaultTemporalLargestUnit(one). diff --git a/src/builtins/core/duration/tests.rs b/src/builtins/core/duration/tests.rs index 7e0d1f56..af529f2c 100644 --- a/src/builtins/core/duration/tests.rs +++ b/src/builtins/core/duration/tests.rs @@ -1,3 +1,5 @@ +use std::string::ToString; + use crate::{ options::{OffsetDisambiguation, RelativeTo, ToStringRoundingOptions}, parsers::Precision, @@ -215,7 +217,7 @@ fn test_duration_compare() { let mut arr = [&one, &two, &three]; arr.sort_by(|a, b| Duration::compare(a, b, None).unwrap()); - assert_eq!(arr, [&three, &one, &two]); + 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( @@ -227,5 +229,5 @@ fn test_duration_compare() { arr.sort_by(|a, b| { Duration::compare(a, b, Some(RelativeTo::ZonedDateTime(zdt.clone()))).unwrap() }); - assert_eq!(arr, [&one, &three, &two]) + assert_eq!(arr.map(ToString::to_string), [&one, &three, &two].map(ToString::to_string)) } From fd6feba111710ce993ed7d4c7677f49cdffc2f06 Mon Sep 17 00:00:00 2001 From: Mafje8943 Date: Tue, 28 Jan 2025 14:29:22 +0100 Subject: [PATCH 5/5] Changed methods on DateDuration to pub (crate) and ran cargo fmt --- src/builtins/core/duration.rs | 2 +- src/builtins/core/duration/date.rs | 6 +++--- src/builtins/core/duration/tests.rs | 10 ++++++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/builtins/core/duration.rs b/src/builtins/core/duration.rs index 0f5fe58c..1adc0126 100644 --- a/src/builtins/core/duration.rs +++ b/src/builtins/core/duration.rs @@ -289,7 +289,7 @@ impl Duration { } /// Returns the ordering between the two durations. - /// + /// /// `7.2.3 Temporal.Duration.compare ( one, two [ , options ] )` #[must_use] pub fn compare( diff --git a/src/builtins/core/duration/date.rs b/src/builtins/core/duration/date.rs index 32fb73e3..c062e2b2 100644 --- a/src/builtins/core/duration/date.rs +++ b/src/builtins/core/duration/date.rs @@ -111,7 +111,7 @@ impl DateDuration { /// DateDurationDays /// TODO(review): Question: what the correct return type? - pub fn days(&self, relative_to: &PlainDate) -> TemporalResult { + pub(crate) fn days(&self, relative_to: &PlainDate) -> TemporalResult { // 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]]. @@ -129,7 +129,7 @@ impl DateDuration { // 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_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]]). @@ -146,7 +146,7 @@ impl DateDuration { } /// AdjustDateDurationRecord - pub fn adjust( + pub(crate) fn adjust( &self, days: FiniteF64, weeks: Option, diff --git a/src/builtins/core/duration/tests.rs b/src/builtins/core/duration/tests.rs index af529f2c..f87bc158 100644 --- a/src/builtins/core/duration/tests.rs +++ b/src/builtins/core/duration/tests.rs @@ -217,7 +217,10 @@ fn test_duration_compare() { 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)); + 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( @@ -229,5 +232,8 @@ fn test_duration_compare() { 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)) + assert_eq!( + arr.map(ToString::to_string), + [&one, &three, &two].map(ToString::to_string) + ) }