PlainYearMonth parsing and Calendar field resolution cleanup/fixes (#205)

nekevss · web-flow · commit 25d45b2d1ad0 · 2025-02-23T09:11:29.000-06:00
This PR fixes some issues with handling `PlainYearMonth`'s calendar
field resolution as well as aligning `PlainYearMonth::new_with_overflow`
closer to the specification behavior.

This PR does introduce a new internal API `IsoDate::regulate`, which
does the regulation work without any date/time limit validations (year
month has its own `year_month_within_limits` validation).

Tested on Boa locally, this does increase `Temporal.PlainYearMonth.from`
conformance to near 100%. The remaining tests are related to other known
issues: minus sign parsing and MonthCode validation.
diff --git a/src/builtins/core/calendar.rs b/src/builtins/core/calendar.rs
@@ -9,6 +9,7 @@ use alloc::string::String;
 use alloc::vec::Vec;
 use core::str::FromStr;
 use icu_calendar::types::{Era as IcuEra, MonthCode as IcuMonthCode, MonthInfo, YearInfo};
+use types::ResolveType;
 
 use crate::{
     builtins::core::{
@@ -226,7 +227,8 @@ impl Calendar {
         partial: &PartialDate,
         overflow: ArithmeticOverflow,
     ) -> TemporalResult<PlainDate> {
-        let resolved_fields = ResolvedCalendarFields::try_from_partial(partial, overflow)?;
+        let resolved_fields =
+            ResolvedCalendarFields::try_from_partial(partial, overflow, ResolveType::Date)?;
 
         if self.is_iso() {
             // Resolve month and monthCode;
@@ -264,7 +266,8 @@ impl Calendar {
         partial: &PartialDate,
         overflow: ArithmeticOverflow,
     ) -> TemporalResult<PlainMonthDay> {
-        let resolved_fields = ResolvedCalendarFields::try_from_partial(partial, overflow)?;
+        let resolved_fields =
+            ResolvedCalendarFields::try_from_partial(partial, overflow, ResolveType::MonthDay)?;
         if self.is_iso() {
             return PlainMonthDay::new_with_overflow(
                 resolved_fields.month_code.as_iso_month_integer()?,
@@ -286,7 +289,8 @@ impl Calendar {
         partial: &PartialDate,
         overflow: ArithmeticOverflow,
     ) -> TemporalResult<PlainYearMonth> {
-        let resolved_fields = ResolvedCalendarFields::try_from_partial(partial, overflow)?;
+        let resolved_fields =
+            ResolvedCalendarFields::try_from_partial(partial, overflow, ResolveType::YearMonth)?;
         if self.is_iso() {
             return PlainYearMonth::new_with_overflow(
                 resolved_fields.era_year.year,
diff --git a/src/builtins/core/calendar/types.rs b/src/builtins/core/calendar/types.rs
@@ -11,6 +11,13 @@ use crate::{TemporalError, TemporalResult};
 
 use crate::builtins::core::{calendar::Calendar, PartialDate};
 
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum ResolveType {
+    Date,
+    YearMonth,
+    MonthDay,
+}
+
 /// `ResolvedCalendarFields` represents the resolved field values necessary for
 /// creating a Date from potentially partial values.
 #[derive(Debug)]
@@ -25,6 +32,7 @@ impl ResolvedCalendarFields {
     pub fn try_from_partial(
         partial_date: &PartialDate,
         overflow: ArithmeticOverflow,
+        resolve_type: ResolveType,
     ) -> TemporalResult<Self> {
         let era_year = EraYear::try_from_partial_values_and_calendar(
             partial_date.year,
@@ -35,9 +43,13 @@ impl ResolvedCalendarFields {
         if partial_date.calendar.is_iso() {
             let month_code =
                 resolve_iso_month(partial_date.month_code, partial_date.month, overflow)?;
-            let day = partial_date
-                .day
-                .ok_or(TemporalError::r#type().with_message("Required day field is empty."))?;
+            let day = if resolve_type != ResolveType::YearMonth {
+                partial_date
+                    .day
+                    .ok_or(TemporalError::r#type().with_message("Required day field is empty."))?
+            } else {
+                partial_date.day.unwrap_or(1)
+            };
 
             let day = if overflow == ArithmeticOverflow::Constrain {
                 constrain_iso_day(era_year.year, ascii_four_to_integer(month_code)?, day)
@@ -57,9 +69,13 @@ impl ResolvedCalendarFields {
         }
 
         let month_code = MonthCode::try_from_partial_date(partial_date, &partial_date.calendar)?;
-        let day = partial_date
-            .day
-            .ok_or(TemporalError::r#type().with_message("Required day field is empty."))?;
+        let day = if resolve_type != ResolveType::YearMonth {
+            partial_date
+                .day
+                .ok_or(TemporalError::r#type().with_message("Required day field is empty."))?
+        } else {
+            partial_date.day.unwrap_or(1)
+        };
 
         Ok(Self {
             era_year,
@@ -322,7 +338,10 @@ mod tests {
     use tinystr::tinystr;
 
     use crate::{
-        builtins::core::{calendar::Calendar, PartialDate},
+        builtins::{
+            calendar::types::ResolveType,
+            core::{calendar::Calendar, PartialDate},
+        },
         options::ArithmeticOverflow,
     };
 
@@ -355,7 +374,11 @@ mod tests {
             ..Default::default()
         };
 
-        let err = ResolvedCalendarFields::try_from_partial(&bad_fields, ArithmeticOverflow::Reject);
+        let err = ResolvedCalendarFields::try_from_partial(
+            &bad_fields,
+            ArithmeticOverflow::Reject,
+            ResolveType::Date,
+        );
         assert!(err.is_err());
     }
 
@@ -367,11 +390,19 @@ mod tests {
             ..Default::default()
         };
 
-        let err = ResolvedCalendarFields::try_from_partial(&bad_fields, ArithmeticOverflow::Reject);
+        let err = ResolvedCalendarFields::try_from_partial(
+            &bad_fields,
+            ArithmeticOverflow::Reject,
+            ResolveType::Date,
+        );
         assert!(err.is_err());
 
         let bad_fields = PartialDate::default();
-        let err = ResolvedCalendarFields::try_from_partial(&bad_fields, ArithmeticOverflow::Reject);
+        let err = ResolvedCalendarFields::try_from_partial(
+            &bad_fields,
+            ArithmeticOverflow::Reject,
+            ResolveType::Date,
+        );
         assert!(err.is_err());
     }
 }
diff --git a/src/builtins/core/year_month.rs b/src/builtins/core/year_month.rs
@@ -6,7 +6,7 @@ use core::{cmp::Ordering, str::FromStr};
 use tinystr::TinyAsciiStr;
 
 use crate::{
-    iso::IsoDate,
+    iso::{year_month_within_limits, IsoDate},
     options::{ArithmeticOverflow, DifferenceOperation, DifferenceSettings, DisplayCalendar},
     parsers::{FormattableCalendar, FormattableDate, FormattableYearMonth},
     utils::pad_iso_year,
@@ -81,10 +81,21 @@ impl PlainYearMonth {
         overflow: ArithmeticOverflow,
     ) -> TemporalResult<Self> {
         let day = reference_day.unwrap_or(1);
-        let iso = IsoDate::new_with_overflow(year, month, day, overflow)?;
+        let iso = IsoDate::regulate(year, month, day, overflow)?;
+        if !year_month_within_limits(iso.year, iso.month) {
+            return Err(TemporalError::range().with_message("Exceeded valid range."));
+        }
         Ok(Self::new_unchecked(iso, calendar))
     }
 
+    /// Create a `PlainYearMonth` from a `PartialDate`
+    pub fn from_partial(
+        partial: PartialDate,
+        overflow: ArithmeticOverflow,
+    ) -> TemporalResult<Self> {
+        partial.calendar.year_month_from_partial(&partial, overflow)
+    }
+
     /// Returns the iso year value for this `YearMonth`.
     #[inline]
     #[must_use]
@@ -245,7 +256,6 @@ impl FromStr for PlainYearMonth {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let record = crate::parsers::parse_year_month(s)?;
-
         let calendar = record
             .calendar
             .map(Calendar::from_utf8)
@@ -254,12 +264,70 @@ impl FromStr for PlainYearMonth {
 
         let date = record.date.temporal_unwrap()?;
 
-        Self::new_with_overflow(
-            date.year,
-            date.month,
-            None,
-            calendar,
-            ArithmeticOverflow::Reject,
-        )
+        // The below steps are from `ToTemporalYearMonth`
+        // 10. Let isoDate be CreateISODateRecord(result.[[Year]], result.[[Month]], result.[[Day]]).
+        let iso = IsoDate::new_unchecked(date.year, date.month, date.day);
+
+        // 11. If ISOYearMonthWithinLimits(isoDate) is false, throw a RangeError exception.
+        if !year_month_within_limits(iso.year, iso.month) {
+            return Err(TemporalError::range().with_message("Exceeded valid range."));
+        }
+
+        let intermediate = Self::new_unchecked(iso, calendar);
+        // 12. Set result to ISODateToFields(calendar, isoDate, year-month).
+        let partial = PartialDate::try_from_year_month(&intermediate)?;
+        // 13. NOTE: The following operation is called with constrain regardless of the
+        // value of overflow, in order for the calendar to store a canonical value in the
+        // [[Day]] field of the [[ISODate]] internal slot of the result.
+        // 14. Set isoDate to ? CalendarYearMonthFromFields(calendar, result, constrain).
+        // 15. Return ! CreateTemporalYearMonth(isoDate, calendar).
+        PlainYearMonth::from_partial(partial, ArithmeticOverflow::Constrain)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use core::str::FromStr;
+
+    use super::PlainYearMonth;
+
+    #[test]
+    fn basic_from_str() {
+        let valid_strings = [
+            "-271821-04",
+            "-271821-04-01",
+            "-271821-04-01T00:00",
+            "+275760-09",
+            "+275760-09-30",
+            "+275760-09-30T23:59:59.999999999",
+        ];
+
+        for valid_case in valid_strings {
+            let ym = PlainYearMonth::from_str(valid_case);
+            assert!(ym.is_ok());
+        }
+    }
+
+    #[test]
+    fn invalid_from_str() {
+        let invalid_strings = [
+            "-271821-03-31",
+            "-271821-03-31T23:59:59.999999999",
+            "+275760-10",
+            "+275760-10-01",
+            "+275760-10-01T00:00",
+        ];
+
+        for invalid_case in invalid_strings {
+            let err = PlainYearMonth::from_str(invalid_case);
+            assert!(err.is_err());
+        }
+
+        let invalid_strings = ["2019-10-01T09:00:00Z", "2019-10-01T09:00:00Z[UTC]"];
+
+        for invalid_case in invalid_strings {
+            let err = PlainYearMonth::from_str(invalid_case);
+            assert!(err.is_err());
+        }
     }
 }
diff --git a/src/iso.rs b/src/iso.rs
@@ -301,35 +301,42 @@ impl IsoDate {
         Self { year, month, day }
     }
 
-    pub(crate) fn new_with_overflow(
+    pub(crate) fn regulate(
         year: i32,
         month: u8,
         day: u8,
         overflow: ArithmeticOverflow,
     ) -> TemporalResult<Self> {
-        let id = match overflow {
+        match overflow {
             ArithmeticOverflow::Constrain => {
                 let month = month.clamp(1, 12);
                 let day = constrain_iso_day(year, month, day);
                 // NOTE: Values are clamped in a u8 range.
-                Self::new_unchecked(year, month, day)
+                Ok(Self::new_unchecked(year, month, day))
             }
             ArithmeticOverflow::Reject => {
                 if !is_valid_date(year, month, day) {
                     return Err(TemporalError::range().with_message("not a valid ISO date."));
                 }
                 // NOTE: Values have been verified to be in a u8 range.
-                Self::new_unchecked(year, month, day)
+                Ok(Self::new_unchecked(year, month, day))
             }
-        };
+        }
+    }
 
-        if !iso_dt_within_valid_limits(id, &IsoTime::noon()) {
+    pub(crate) fn new_with_overflow(
+        year: i32,
+        month: u8,
+        day: u8,
+        overflow: ArithmeticOverflow,
+    ) -> TemporalResult<Self> {
+        let date = Self::regulate(year, month, day, overflow)?;
+        if !iso_dt_within_valid_limits(date, &IsoTime::noon()) {
             return Err(
                 TemporalError::range().with_message("Date is not within ISO date time limits.")
             );
         }
-
-        Ok(id)
+        Ok(date)
     }
 
     /// Create a balanced `IsoDate`
@@ -957,6 +964,25 @@ fn iso_date_surpasses(this: &IsoDate, other: &IsoDate, sign: i8) -> bool {
     this.cmp(other) as i8 * sign == 1
 }
 
+#[inline]
+pub(crate) fn year_month_within_limits(year: i32, month: u8) -> bool {
+    // 1. If isoDate.[[Year]] < -271821 or isoDate.[[Year]] > 275760, then
+    if !(-271821..=275760).contains(&year) {
+        // a. Return false.
+        return false;
+    // 2. If isoDate.[[Year]] = -271821 and isoDate.[[Month]] < 4, then
+    } else if year == -271821 && month < 4 {
+        // a. Return false.
+        return false;
+    // 3. If isoDate.[[Year]] = 275760 and isoDate.[[Month]] > 9, then
+    } else if year == 275760 && month > 9 {
+        // a. Return false.
+        return false;
+    }
+    // 4. Return true.
+    true
+}
+
 #[inline]
 fn balance_iso_year_month(year: i32, month: i32) -> (i32, u8) {
     // 1. Assert: year and month are integers.
diff --git a/src/parsers.rs b/src/parsers.rs
@@ -755,10 +755,14 @@ pub(crate) fn parse_year_month(source: &str) -> TemporalResult<IxdtfParseRecord>
     let ym_record = parse_ixdtf(source, ParseVariant::YearMonth);
 
     if let Ok(ym) = ym_record {
+        if ym.offset == Some(UtcOffsetRecordOrZ::Z) {
+            return Err(TemporalError::range()
+                .with_message("UTC designator is not valid for DateTime parsing."));
+        }
         return Ok(ym);
     }
 
-    let dt_parse = parse_ixdtf(source, ParseVariant::DateTime);
+    let dt_parse = parse_date_time(source);
 
     match dt_parse {
         Ok(dt) => Ok(dt),
