Apply review feedback and various fixes

boa_ast/src/temporal/mod.rs

@@ -2,7 +2,7 @@
 
 /// An ISO Date Node consisting of non-validated date fields and calendar value.
 #[derive(Default, Debug)]
-pub struct ISODate {
+pub struct IsoDate {
     /// Date Year
     pub year: i32,
     /// Date Month
@@ -13,9 +13,9 @@ pub struct ISODate {
     pub calendar: Option<String>,
 }
 
-/// The `ISOTime` node consists of non-validated time fields.
+/// The `IsoTime` node consists of non-validated time fields.
 #[derive(Default, Debug, Clone, Copy)]
-pub struct ISOTime {
+pub struct IsoTime {
     /// An hour value between 0-23
     pub hour: u8,
     /// A minute value between 0-59
@@ -30,7 +30,7 @@ pub struct ISOTime {
     pub nanosecond: u16,
 }
 
-impl ISOTime {
+impl IsoTime {
     #[must_use]
     #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
     /// A utility initialization function to create `ISOTime` from the `TimeSpec` components.
@@ -52,13 +52,13 @@ impl ISOTime {
     }
 }
 
-/// The `ISODateTime` node output by the ISO parser
+/// The `IsoDateTime` node output by the ISO parser
 #[derive(Default, Debug)]
-pub struct ISODateTime {
+pub struct IsoDateTime {
     /// The `ISODate` record
-    pub date: ISODate,
+    pub date: IsoDate,
     /// The `ISOTime` record
-    pub time: ISOTime,
+    pub time: IsoTime,
     /// The `TimeZone` value for this `ISODateTime`
     pub tz: Option<TimeZone>,
 }
@@ -87,9 +87,9 @@ pub struct UTCOffset {
     pub fraction: f64,
 }
 
-/// An `ISODuration` Node output by the ISO parser.
+/// An `IsoDuration` Node output by the ISO parser.
 #[derive(Debug, Default, Clone, Copy)]
-pub struct ISODuration {
+pub struct IsoDuration {
     /// Years value.
     pub years: i32,
     /// Months value.

boa_engine/src/bigint.rs

@@ -227,8 +227,8 @@ impl JsBigInt {
 
     /// Utility function for performing `+` operation on more than two values.
     #[inline]
-    #[must_use]
-    pub fn add_n(values: &[Self]) -> Self {
+    #[cfg(feature = "experimental")]
+    pub(crate) fn add_n(values: &[Self]) -> Self {
         let mut result = Self::zero();
         for big_int in values {
             result = Self::add(&result, big_int);

boa_engine/src/builtins/temporal/calendar/mod.rs

@@ -616,11 +616,18 @@ impl Calendar {
             &options,
             utf16!("largestUnit"),
             TemporalUnitGroup::Date,
+            false,
             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());
+        };
+
         this_calendar.date_until(&one, &two, largest_unit, context)
     }
 

boa_engine/src/builtins/temporal/calendar/utils.rs

@@ -1,6 +1,6 @@
 //! Calendar utility calculations
 
-// TODO: determine if which are needed.
+// TODO: determine if any of the below are needed.
 
 use crate::builtins::temporal::{self, date_equations, plain_date::iso::IsoDateRecord};
 use crate::JsString;

boa_engine/src/builtins/temporal/date_equations.rs

@@ -5,7 +5,7 @@
 use std::ops::Mul;
 
 pub(crate) fn epoch_time_to_day_number(t: f64) -> i32 {
-    (t / super::NS_PER_DAY as f64).floor() as i32
+    (t / f64::from(super::MS_PER_DAY)).floor() as i32
 }
 
 pub(crate) fn mathematical_days_in_year(y: i32) -> i32 {
@@ -22,15 +22,17 @@ pub(crate) fn mathematical_days_in_year(y: i32) -> i32 {
     }
 }
 
-pub(crate) const fn epoch_day_number_for_year(y: i32) -> i32 {
-    365 * (y - 1970) + ((y - 1970) / 4) - ((y - 1901) / 100) + ((y - 1601) / 400)
+pub(crate) fn epoch_day_number_for_year(y: f64) -> f64 {
+    365.0f64.mul_add(y - 1970.0, ((y - 1969.0) / 4.0).floor()) - ((y - 1901.0) / 100.0).floor()
+        + ((y - 1601.0) / 400.0).floor()
 }
 
-// TODO: potentially inaccurate -> Need to further test this and epoch_day_number_for_year
 pub(crate) fn epoch_time_for_year(y: i32) -> f64 {
-    super::NS_PER_DAY as f64 * f64::from(epoch_day_number_for_year(y))
+    f64::from(super::MS_PER_DAY) * epoch_day_number_for_year(f64::from(y))
 }
 
+// NOTE: The below returns the epoch years (years since 1970). The spec
+// appears to assume the below returns with the epoch applied.
 pub(crate) fn epoch_time_to_epoch_year(t: f64) -> i32 {
     // roughly calculate the largest possible year given the time t,
     // then check and refine the year.
@@ -43,7 +45,7 @@ pub(crate) fn epoch_time_to_epoch_year(t: f64) -> i32 {
         year -= 1;
     }
 
-    year
+    year + 1970
 }
 
 /// Returns either 1 (true) or 0 (false)
@@ -52,39 +54,18 @@ pub(crate) fn mathematical_in_leap_year(t: f64) -> i32 {
 }
 
 pub(crate) fn epoch_time_to_month_in_year(t: f64) -> i32 {
-    let days = epoch_time_to_day_in_year(t);
-    let in_leap_year = mathematical_in_leap_year(t) == 1;
-
-    match days {
-        0..=30 => 0,
-        31..=59 if in_leap_year => 1,
-        31..=58 => 1,
-        60..=90 if in_leap_year => 2,
-        59..=89 => 2,
-        91..=121 if in_leap_year => 3,
-        90..=120 => 3,
-        122..=151 if in_leap_year => 4,
-        121..=150 => 4,
-        152..=182 if in_leap_year => 5,
-        151..=181 => 5,
-        183..=213 if in_leap_year => 6,
-        182..=212 => 6,
-        214..=243 if in_leap_year => 7,
-        213..=242 => 7,
-        244..=273 if in_leap_year => 8,
-        243..=272 => 8,
-        274..=304 if in_leap_year => 9,
-        273..=303 => 9,
-        305..=334 if in_leap_year => 10,
-        304..=333 => 10,
-        335..=366 if in_leap_year => 11,
-        334..=365 => 11,
-        _ => unreachable!(),
+    const DAYS: [i32; 11] = [30, 58, 89, 120, 150, 181, 212, 242, 272, 303, 333];
+    let day = epoch_time_to_day_in_year(t);
+
+    let result = DAYS.binary_search(&day);
+
+    match result {
+        Ok(i) | Err(i) => i as i32,
     }
 }
 
 pub(crate) fn epoch_time_for_month_given_year(m: i32, y: i32) -> f64 {
-    let leap_day = i32::from(mathematical_days_in_year(y) == 366);
+    let leap_day = mathematical_days_in_year(y) - 365;
 
     let days = match m {
         0 => 1,
@@ -128,7 +109,8 @@ pub(crate) fn epoch_time_to_date(t: f64) -> i32 {
 }
 
 pub(crate) fn epoch_time_to_day_in_year(t: f64) -> i32 {
-    epoch_time_to_day_number(t) - epoch_day_number_for_year(epoch_time_to_epoch_year(t))
+    epoch_time_to_day_number(t)
+        - (epoch_day_number_for_year(f64::from(epoch_time_to_epoch_year(t))) as i32)
 }
 
 pub(crate) fn epoch_time_to_week_day(t: f64) -> i32 {

boa_engine/src/builtins/temporal/duration/mod.rs

@@ -618,11 +618,12 @@ impl Duration {
         //   (ToRelativeTemporalObject reads "relativeTo", ToTemporalRoundingIncrement reads "roundingIncrement" and ToTemporalRoundingMode reads "roundingMode").
 
         // 9. Let largestUnit be ? GetTemporalUnit(roundTo, "largestUnit", datetime, undefined, « "auto" »).
-        let mut largest_unit = get_temporal_unit(
+        let largest_unit = get_temporal_unit(
             &round_to,
             utf16!("largestUnit"),
             TemporalUnitGroup::DateTime,
-            Some(TemporalUnit::Undefined),
+            false,
+            None,
             Some([TemporalUnit::Auto].into()),
             context,
         )?;
@@ -639,22 +640,25 @@ impl Duration {
                 .unwrap_or(RoundingMode::HalfExpand);
 
         // 13. Let smallestUnit be ? GetTemporalUnit(roundTo, "smallestUnit", datetime, undefined).
-        let mut smallest_unit = get_temporal_unit(
+        let smallest_unit = get_temporal_unit(
             &round_to,
             utf16!("smallestUnit"),
             TemporalUnitGroup::DateTime,
-            Some(TemporalUnit::Undefined),
+            false,
+            None,
             None,
             context,
         )?;
 
         // 14. If smallestUnit is undefined, then
-        if smallest_unit.is_undefined() {
+        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".
-            smallest_unit = TemporalUnit::Nanosecond;
-        }
+            TemporalUnit::Nanosecond
+        };
 
         // 15. Let defaultLargestUnit be ! DefaultTemporalLargestUnit(duration.[[Years]], duration.[[Months]], duration.[[Weeks]], duration.[[Days]], duration.[[Hours]], duration.[[Minutes]], duration.[[Seconds]], duration.[[Milliseconds]], duration.[[Microseconds]]).
         let mut default_largest_unit = duration.inner.default_temporal_largest_unit();
@@ -663,15 +667,16 @@ impl Duration {
         default_largest_unit = core::cmp::max(default_largest_unit, smallest_unit);
 
         // 17. If largestUnit is undefined, then
-        if largest_unit.is_undefined() {
-            // a. Set largestUnitPresent to false.
-            largest_unit_present = false;
-            // b. Set largestUnit to defaultLargestUnit.
-            largest_unit = default_largest_unit;
-        } else if largest_unit.is_auto() {
-            // a. Set largestUnit to defaultLargestUnit.
-            largest_unit = default_largest_unit;
-        }
+        let largest_unit = match largest_unit {
+            Some(u) if u == TemporalUnit::Auto => default_largest_unit,
+            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 {
@@ -693,7 +698,7 @@ impl Duration {
 
         // 22. If maximum is not undefined, perform ? ValidateTemporalRoundingIncrement(roundingIncrement, maximum, false).
         if let Some(max) = maximum {
-            validate_temporal_rounding_increment(rounding_increment, max, false)?;
+            validate_temporal_rounding_increment(rounding_increment, f64::from(max), false)?;
         }
 
         let mut unbalance_duration = DurationRecord::from_date_duration(duration.inner.date());
@@ -794,11 +799,18 @@ impl Duration {
             &total_of,
             utf16!("unit"),
             TemporalUnitGroup::DateTime,
+            true,
             None,
             None,
             context,
         )?;
 
+        let Some(unit) = unit else {
+            return Err(JsNativeError::range()
+                .with_message("TemporalUnit cannot be undefined in this context.")
+                .into());
+        };
+
         let mut unbalance_duration = DurationRecord::from_date_duration(duration.inner.date());
 
         // 9. Let unbalanceResult be ? UnbalanceDurationRelative(duration.[[Years]], duration.[[Months]], duration.[[Weeks]], duration.[[Days]], unit, relativeTo).
@@ -894,7 +906,7 @@ impl Duration {
             // b. Let whole be roundResult.[[Nanoseconds]].
             TemporalUnit::Nanosecond => round_record.nanoseconds(),
             // a. Assert: unit is "nanosecond".
-            _=> unreachable!("Unit must be a valid temporal unit. Any other value would be an implementation error."),
+            TemporalUnit::Auto=> unreachable!("Unit must be a valid temporal unit. Any other value would be an implementation error."),
         };
 
         // 28. Return 𝔽(whole + roundRecord.[[Remainder]]).