From 1d6bf18c5cf238340e64ee2834d278940093212a Mon Sep 17 00:00:00 2001 From: nekevss Date: Thu, 5 Oct 2023 16:53:58 -0400 Subject: [PATCH] Review feedback and get_option changes --- .../src/builtins/temporal/calendar/mod.rs | 31 +++++++------------ .../src/builtins/temporal/date_equations.rs | 9 +++++- .../src/builtins/temporal/duration/mod.rs | 28 ++++++++--------- .../src/builtins/temporal/instant/mod.rs | 12 ++----- boa_engine/src/builtins/temporal/mod.rs | 4 +-- boa_engine/src/builtins/temporal/options.rs | 2 +- .../src/builtins/temporal/plain_date/mod.rs | 10 +++--- boa_engine/src/builtins/temporal/tests.rs | 18 +++++++---- 8 files changed, 55 insertions(+), 59 deletions(-) diff --git a/boa_engine/src/builtins/temporal/calendar/mod.rs b/boa_engine/src/builtins/temporal/calendar/mod.rs index c95a3605a3a..18e44baf484 100644 --- a/boa_engine/src/builtins/temporal/calendar/mod.rs +++ b/boa_engine/src/builtins/temporal/calendar/mod.rs @@ -371,9 +371,8 @@ impl Calendar { }; // 8. Let overflow be ? ToTemporalOverflow(options). - let overflow = - get_option::(&options, utf16!("overflow"), false, context)? - .unwrap_or(ArithmeticOverflow::Constrain); + let overflow = get_option(&options, utf16!("overflow"), context)? + .unwrap_or(ArithmeticOverflow::Constrain); // NOTE: implement the below on the calenar itself // 9. If calendar.[[Identifier]] is "iso8601", then @@ -453,9 +452,8 @@ impl Calendar { }; // 7. Let overflow be ? ToTemporalOverflow(options). - let overflow = - get_option::(&options, utf16!("overflow"), false, context)? - .unwrap_or(ArithmeticOverflow::Constrain); + let overflow = get_option::(&options, utf16!("overflow"), context)? + .unwrap_or(ArithmeticOverflow::Constrain); this_calendar.year_month_from_fields(&mut fields, overflow, context) } @@ -530,9 +528,8 @@ impl Calendar { }; // 8. Let overflow be ? ToTemporalOverflow(options). - let overflow = - get_option::(&options, utf16!("overflow"), false, context)? - .unwrap_or(ArithmeticOverflow::Constrain); + let overflow = get_option(&options, utf16!("overflow"), context)? + .unwrap_or(ArithmeticOverflow::Constrain); this_calendar.month_day_from_fields(&mut fields, overflow, context) } @@ -569,9 +566,8 @@ impl Calendar { let options_obj = get_options_object(options)?; // 7. Let overflow be ? ToTemporalOverflow(options). - let overflow = - get_option::(&options_obj, utf16!("overflow"), false, context)? - .unwrap_or(ArithmeticOverflow::Constrain); + let overflow = get_option(&options_obj, utf16!("overflow"), context)? + .unwrap_or(ArithmeticOverflow::Constrain); // 8. Let balanceResult be ? BalanceTimeDuration(duration.[[Days]], duration.[[Hours]], duration.[[Minutes]], duration.[[Seconds]], duration.[[Milliseconds]], duration.[[Microseconds]], duration.[[Nanoseconds]], "day"). duration.balance_time_duration(TemporalUnit::Day, None)?; @@ -620,13 +616,10 @@ impl Calendar { Some(TemporalUnit::Day), None, context, - )?; - - let Some(largest_unit) = largest_unit else { - return Err(JsNativeError::range() - .with_message("largestUnit cannot be undefined in this context.") - .into()); - }; + )? + .ok_or_else(|| { + JsNativeError::range().with_message("largestUnit cannot be undefined in this context.") + })?; this_calendar.date_until(&one, &two, largest_unit, context) } diff --git a/boa_engine/src/builtins/temporal/date_equations.rs b/boa_engine/src/builtins/temporal/date_equations.rs index 222e0139757..7c4fd855db2 100644 --- a/boa_engine/src/builtins/temporal/date_equations.rs +++ b/boa_engine/src/builtins/temporal/date_equations.rs @@ -55,9 +55,16 @@ pub(crate) fn mathematical_in_leap_year(t: f64) -> i32 { pub(crate) fn epoch_time_to_month_in_year(t: f64) -> i32 { const DAYS: [i32; 11] = [30, 58, 89, 120, 150, 181, 212, 242, 272, 303, 333]; + const LEAP_DAYS: [i32; 11] = [30, 59, 90, 121, 151, 182, 213, 242, 272, 303, 334]; + + let in_leap_year = mathematical_in_leap_year(t) == 1; let day = epoch_time_to_day_in_year(t); - let result = DAYS.binary_search(&day); + let result = if in_leap_year { + LEAP_DAYS.binary_search(&day) + } else { + DAYS.binary_search(&day) + }; match result { Ok(i) | Err(i) => i as i32, diff --git a/boa_engine/src/builtins/temporal/duration/mod.rs b/boa_engine/src/builtins/temporal/duration/mod.rs index a63f9bc2bdf..ea5215c4fe6 100644 --- a/boa_engine/src/builtins/temporal/duration/mod.rs +++ b/boa_engine/src/builtins/temporal/duration/mod.rs @@ -610,10 +610,10 @@ impl Duration { } }; + // NOTE: 6 & 7 unused in favor of `is_none()`. // 6. Let smallestUnitPresent be true. - let mut smallest_unit_present = true; // 7. Let largestUnitPresent be true. - let mut largest_unit_present = true; + // 8. NOTE: The following steps read options and perform independent validation in alphabetical order // (ToRelativeTemporalObject reads "relativeTo", ToTemporalRoundingIncrement reads "roundingIncrement" and ToTemporalRoundingMode reads "roundingMode"). @@ -635,9 +635,8 @@ impl Duration { let rounding_increment = get_temporal_rounding_increment(&round_to, context)?; // 12. Let roundingMode be ? ToTemporalRoundingMode(roundTo, "halfExpand"). - let rounding_mode = - get_option::(&round_to, utf16!("roundingMode"), false, context)? - .unwrap_or(RoundingMode::HalfExpand); + let rounding_mode = get_option(&round_to, utf16!("roundingMode"), context)? + .unwrap_or(RoundingMode::HalfExpand); // 13. Let smallestUnit be ? GetTemporalUnit(roundTo, "smallestUnit", datetime, undefined). let smallest_unit = get_temporal_unit( @@ -650,12 +649,20 @@ impl Duration { context, )?; + // NOTE: execute step 19 earlier before initial values are shadowed. + // 19. If smallestUnitPresent is false and largestUnitPresent is false, then + if smallest_unit.is_none() && largest_unit.is_none() { + // a. Throw a RangeError exception. + return Err(JsNativeError::range() + .with_message("smallestUnit or largestUnit must be present.") + .into()); + } + // 14. If smallestUnit is undefined, then let smallest_unit = if let Some(unit) = smallest_unit { unit } else { // a. Set smallestUnitPresent to false. - smallest_unit_present = false; // b. Set smallestUnit to "nanosecond". TemporalUnit::Nanosecond }; @@ -672,20 +679,11 @@ impl Duration { Some(u) => u, None => { // a. Set largestUnitPresent to false. - largest_unit_present = false; // b. Set largestUnit to defaultLargestUnit. default_largest_unit } }; - // 19. If smallestUnitPresent is false and largestUnitPresent is false, then - if !smallest_unit_present && !largest_unit_present { - // a. Throw a RangeError exception. - return Err(JsNativeError::range() - .with_message("smallestUnit or largestUnit must be present.") - .into()); - } - // 20. If LargerOfTwoTemporalUnits(largestUnit, smallestUnit) is not largestUnit, throw a RangeError exception. if core::cmp::max(largest_unit, smallest_unit) != largest_unit { return Err(JsNativeError::range() diff --git a/boa_engine/src/builtins/temporal/instant/mod.rs b/boa_engine/src/builtins/temporal/instant/mod.rs index dd3c7efcb02..a062ce93b27 100644 --- a/boa_engine/src/builtins/temporal/instant/mod.rs +++ b/boa_engine/src/builtins/temporal/instant/mod.rs @@ -366,8 +366,7 @@ impl Instant { // 8. Let roundingMode be ? ToTemporalRoundingMode(roundTo, "halfExpand"). let rounding_mode = - get_option::(&round_to, utf16!("roundingMode"), false, context)? - .unwrap_or_default(); + get_option(&round_to, utf16!("roundingMode"), context)?.unwrap_or_default(); // 9. Let smallestUnit be ? GetTemporalUnit(roundTo, "smallestUnit"), time, required). let smallest_unit = get_temporal_unit( @@ -378,13 +377,8 @@ impl Instant { None, None, context, - )?; - - let Some(smallest_unit) = smallest_unit else { - return Err(JsNativeError::range() - .with_message("smallestUnit cannot be undefined.") - .into()); - }; + )? + .ok_or_else(|| JsNativeError::range().with_message("smallestUnit cannot be undefined."))?; let maximum = match smallest_unit { // 10. If smallestUnit is "hour"), then diff --git a/boa_engine/src/builtins/temporal/mod.rs b/boa_engine/src/builtins/temporal/mod.rs index e73d80b431c..6168f8e9f02 100644 --- a/boa_engine/src/builtins/temporal/mod.rs +++ b/boa_engine/src/builtins/temporal/mod.rs @@ -525,10 +525,10 @@ pub(crate) fn get_diff_settings( // 4. Let roundingIncrement be ? ToTemporalRoundingIncrement(options). let rounding_increment = get_temporal_rounding_increment(options, context)?; + // 5. Let roundingMode be ? ToTemporalRoundingMode(options, "trunc"). let mut rounding_mode = - get_option::(options, utf16!("roundingMode"), false, context)? - .unwrap_or(RoundingMode::Trunc); + get_option(options, utf16!("roundingMode"), context)?.unwrap_or(RoundingMode::Trunc); // 6. If operation is since, then if !op { diff --git a/boa_engine/src/builtins/temporal/options.rs b/boa_engine/src/builtins/temporal/options.rs index 2b144d008db..e3fbc2d9b5b 100644 --- a/boa_engine/src/builtins/temporal/options.rs +++ b/boa_engine/src/builtins/temporal/options.rs @@ -66,7 +66,7 @@ pub(crate) fn get_temporal_unit( let mut unit_values = unit_group.group(); unit_values.extend(extra); - let unit = get_option::(options, key, required, context)?.map_or(default, Some); + let unit = get_option(options, key, context)?.map_or(default, Some); if let Some(u) = &unit { if !unit_values.contains(u) { diff --git a/boa_engine/src/builtins/temporal/plain_date/mod.rs b/boa_engine/src/builtins/temporal/plain_date/mod.rs index c7f20828244..5fe1de68473 100644 --- a/boa_engine/src/builtins/temporal/plain_date/mod.rs +++ b/boa_engine/src/builtins/temporal/plain_date/mod.rs @@ -478,9 +478,8 @@ pub(crate) fn to_temporal_date( // c. If item has an [[InitializedTemporalDateTime]] internal slot, then } else if object.is_plain_date_time() { // i. Perform ? ToTemporalOverflow(options). - let _o = - get_option::(&options_obj, utf16!("overflow"), false, context)? - .unwrap_or(ArithmeticOverflow::Constrain); + let _o = get_option(&options_obj, utf16!("overflow"), context)? + .unwrap_or(ArithmeticOverflow::Constrain); let obj = object.borrow(); let date_time = obj @@ -537,9 +536,8 @@ pub(crate) fn to_temporal_date( // 11. Set calendar to the ASCII-lowercase of calendar. // 12. Perform ? ToTemporalOverflow(options). - let _result = - get_option::(&options_obj, utf16!("overflow"), false, context)? - .unwrap_or(ArithmeticOverflow::Constrain); + let _result = get_option(&options_obj, utf16!("overflow"), context)? + .unwrap_or(ArithmeticOverflow::Constrain); // 13. Return ? CreateTemporalDate(result.[[Year]], result.[[Month]], result.[[Day]], calendar). Ok(PlainDate { diff --git a/boa_engine/src/builtins/temporal/tests.rs b/boa_engine/src/builtins/temporal/tests.rs index 154946d7c69..5cd24b57f4b 100644 --- a/boa_engine/src/builtins/temporal/tests.rs +++ b/boa_engine/src/builtins/temporal/tests.rs @@ -1,4 +1,4 @@ -use super::date_equations::epoch_time_to_month_in_year; +use super::date_equations::{epoch_time_to_month_in_year, mathematical_in_leap_year}; use crate::{js_string, run_test_actions, JsValue, TestAction}; // Temporal Object tests. @@ -37,10 +37,16 @@ fn now_object() { #[test] fn time_to_month() { - let milliseconds = [1696459917000_f64]; + let oct_2023 = 1_696_459_917_000_f64; + let mar_1_2020 = 1_583_020_800_000_f64; + let feb_29_2020 = 1_582_934_400_000_f64; + let mar_1_2021 = 1_614_556_800_000_f64; - for test_epochs in milliseconds { - println!("{}", epoch_time_to_month_in_year(test_epochs)); - assert_eq!(epoch_time_to_month_in_year(test_epochs), 9); - } + assert_eq!(epoch_time_to_month_in_year(oct_2023), 9); + assert_eq!(epoch_time_to_month_in_year(mar_1_2020), 2); + assert_eq!(mathematical_in_leap_year(mar_1_2020), 1); + assert_eq!(epoch_time_to_month_in_year(feb_29_2020), 1); + assert_eq!(mathematical_in_leap_year(feb_29_2020), 1); + assert_eq!(epoch_time_to_month_in_year(mar_1_2021), 2); + assert_eq!(mathematical_in_leap_year(mar_1_2021), 0); }