Skip to content

Commit 25d45b2

Browse files
authored
PlainYearMonth parsing and Calendar field resolution cleanup/fixes (#205)
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.
1 parent 7484b25 commit 25d45b2

File tree

5 files changed

+165
-32
lines changed

5 files changed

+165
-32
lines changed

src/builtins/core/calendar.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use alloc::string::String;
99
use alloc::vec::Vec;
1010
use core::str::FromStr;
1111
use icu_calendar::types::{Era as IcuEra, MonthCode as IcuMonthCode, MonthInfo, YearInfo};
12+
use types::ResolveType;
1213

1314
use crate::{
1415
builtins::core::{
@@ -226,7 +227,8 @@ impl Calendar {
226227
partial: &PartialDate,
227228
overflow: ArithmeticOverflow,
228229
) -> TemporalResult<PlainDate> {
229-
let resolved_fields = ResolvedCalendarFields::try_from_partial(partial, overflow)?;
230+
let resolved_fields =
231+
ResolvedCalendarFields::try_from_partial(partial, overflow, ResolveType::Date)?;
230232

231233
if self.is_iso() {
232234
// Resolve month and monthCode;
@@ -264,7 +266,8 @@ impl Calendar {
264266
partial: &PartialDate,
265267
overflow: ArithmeticOverflow,
266268
) -> TemporalResult<PlainMonthDay> {
267-
let resolved_fields = ResolvedCalendarFields::try_from_partial(partial, overflow)?;
269+
let resolved_fields =
270+
ResolvedCalendarFields::try_from_partial(partial, overflow, ResolveType::MonthDay)?;
268271
if self.is_iso() {
269272
return PlainMonthDay::new_with_overflow(
270273
resolved_fields.month_code.as_iso_month_integer()?,
@@ -286,7 +289,8 @@ impl Calendar {
286289
partial: &PartialDate,
287290
overflow: ArithmeticOverflow,
288291
) -> TemporalResult<PlainYearMonth> {
289-
let resolved_fields = ResolvedCalendarFields::try_from_partial(partial, overflow)?;
292+
let resolved_fields =
293+
ResolvedCalendarFields::try_from_partial(partial, overflow, ResolveType::YearMonth)?;
290294
if self.is_iso() {
291295
return PlainYearMonth::new_with_overflow(
292296
resolved_fields.era_year.year,

src/builtins/core/calendar/types.rs

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ use crate::{TemporalError, TemporalResult};
1111

1212
use crate::builtins::core::{calendar::Calendar, PartialDate};
1313

14+
#[derive(Debug, Clone, Copy, PartialEq)]
15+
pub enum ResolveType {
16+
Date,
17+
YearMonth,
18+
MonthDay,
19+
}
20+
1421
/// `ResolvedCalendarFields` represents the resolved field values necessary for
1522
/// creating a Date from potentially partial values.
1623
#[derive(Debug)]
@@ -25,6 +32,7 @@ impl ResolvedCalendarFields {
2532
pub fn try_from_partial(
2633
partial_date: &PartialDate,
2734
overflow: ArithmeticOverflow,
35+
resolve_type: ResolveType,
2836
) -> TemporalResult<Self> {
2937
let era_year = EraYear::try_from_partial_values_and_calendar(
3038
partial_date.year,
@@ -35,9 +43,13 @@ impl ResolvedCalendarFields {
3543
if partial_date.calendar.is_iso() {
3644
let month_code =
3745
resolve_iso_month(partial_date.month_code, partial_date.month, overflow)?;
38-
let day = partial_date
39-
.day
40-
.ok_or(TemporalError::r#type().with_message("Required day field is empty."))?;
46+
let day = if resolve_type != ResolveType::YearMonth {
47+
partial_date
48+
.day
49+
.ok_or(TemporalError::r#type().with_message("Required day field is empty."))?
50+
} else {
51+
partial_date.day.unwrap_or(1)
52+
};
4153

4254
let day = if overflow == ArithmeticOverflow::Constrain {
4355
constrain_iso_day(era_year.year, ascii_four_to_integer(month_code)?, day)
@@ -57,9 +69,13 @@ impl ResolvedCalendarFields {
5769
}
5870

5971
let month_code = MonthCode::try_from_partial_date(partial_date, &partial_date.calendar)?;
60-
let day = partial_date
61-
.day
62-
.ok_or(TemporalError::r#type().with_message("Required day field is empty."))?;
72+
let day = if resolve_type != ResolveType::YearMonth {
73+
partial_date
74+
.day
75+
.ok_or(TemporalError::r#type().with_message("Required day field is empty."))?
76+
} else {
77+
partial_date.day.unwrap_or(1)
78+
};
6379

6480
Ok(Self {
6581
era_year,
@@ -322,7 +338,10 @@ mod tests {
322338
use tinystr::tinystr;
323339

324340
use crate::{
325-
builtins::core::{calendar::Calendar, PartialDate},
341+
builtins::{
342+
calendar::types::ResolveType,
343+
core::{calendar::Calendar, PartialDate},
344+
},
326345
options::ArithmeticOverflow,
327346
};
328347

@@ -355,7 +374,11 @@ mod tests {
355374
..Default::default()
356375
};
357376

358-
let err = ResolvedCalendarFields::try_from_partial(&bad_fields, ArithmeticOverflow::Reject);
377+
let err = ResolvedCalendarFields::try_from_partial(
378+
&bad_fields,
379+
ArithmeticOverflow::Reject,
380+
ResolveType::Date,
381+
);
359382
assert!(err.is_err());
360383
}
361384

@@ -367,11 +390,19 @@ mod tests {
367390
..Default::default()
368391
};
369392

370-
let err = ResolvedCalendarFields::try_from_partial(&bad_fields, ArithmeticOverflow::Reject);
393+
let err = ResolvedCalendarFields::try_from_partial(
394+
&bad_fields,
395+
ArithmeticOverflow::Reject,
396+
ResolveType::Date,
397+
);
371398
assert!(err.is_err());
372399

373400
let bad_fields = PartialDate::default();
374-
let err = ResolvedCalendarFields::try_from_partial(&bad_fields, ArithmeticOverflow::Reject);
401+
let err = ResolvedCalendarFields::try_from_partial(
402+
&bad_fields,
403+
ArithmeticOverflow::Reject,
404+
ResolveType::Date,
405+
);
375406
assert!(err.is_err());
376407
}
377408
}

src/builtins/core/year_month.rs

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use core::{cmp::Ordering, str::FromStr};
66
use tinystr::TinyAsciiStr;
77

88
use crate::{
9-
iso::IsoDate,
9+
iso::{year_month_within_limits, IsoDate},
1010
options::{ArithmeticOverflow, DifferenceOperation, DifferenceSettings, DisplayCalendar},
1111
parsers::{FormattableCalendar, FormattableDate, FormattableYearMonth},
1212
utils::pad_iso_year,
@@ -81,10 +81,21 @@ impl PlainYearMonth {
8181
overflow: ArithmeticOverflow,
8282
) -> TemporalResult<Self> {
8383
let day = reference_day.unwrap_or(1);
84-
let iso = IsoDate::new_with_overflow(year, month, day, overflow)?;
84+
let iso = IsoDate::regulate(year, month, day, overflow)?;
85+
if !year_month_within_limits(iso.year, iso.month) {
86+
return Err(TemporalError::range().with_message("Exceeded valid range."));
87+
}
8588
Ok(Self::new_unchecked(iso, calendar))
8689
}
8790

91+
/// Create a `PlainYearMonth` from a `PartialDate`
92+
pub fn from_partial(
93+
partial: PartialDate,
94+
overflow: ArithmeticOverflow,
95+
) -> TemporalResult<Self> {
96+
partial.calendar.year_month_from_partial(&partial, overflow)
97+
}
98+
8899
/// Returns the iso year value for this `YearMonth`.
89100
#[inline]
90101
#[must_use]
@@ -245,7 +256,6 @@ impl FromStr for PlainYearMonth {
245256

246257
fn from_str(s: &str) -> Result<Self, Self::Err> {
247258
let record = crate::parsers::parse_year_month(s)?;
248-
249259
let calendar = record
250260
.calendar
251261
.map(Calendar::from_utf8)
@@ -254,12 +264,70 @@ impl FromStr for PlainYearMonth {
254264

255265
let date = record.date.temporal_unwrap()?;
256266

257-
Self::new_with_overflow(
258-
date.year,
259-
date.month,
260-
None,
261-
calendar,
262-
ArithmeticOverflow::Reject,
263-
)
267+
// The below steps are from `ToTemporalYearMonth`
268+
// 10. Let isoDate be CreateISODateRecord(result.[[Year]], result.[[Month]], result.[[Day]]).
269+
let iso = IsoDate::new_unchecked(date.year, date.month, date.day);
270+
271+
// 11. If ISOYearMonthWithinLimits(isoDate) is false, throw a RangeError exception.
272+
if !year_month_within_limits(iso.year, iso.month) {
273+
return Err(TemporalError::range().with_message("Exceeded valid range."));
274+
}
275+
276+
let intermediate = Self::new_unchecked(iso, calendar);
277+
// 12. Set result to ISODateToFields(calendar, isoDate, year-month).
278+
let partial = PartialDate::try_from_year_month(&intermediate)?;
279+
// 13. NOTE: The following operation is called with constrain regardless of the
280+
// value of overflow, in order for the calendar to store a canonical value in the
281+
// [[Day]] field of the [[ISODate]] internal slot of the result.
282+
// 14. Set isoDate to ? CalendarYearMonthFromFields(calendar, result, constrain).
283+
// 15. Return ! CreateTemporalYearMonth(isoDate, calendar).
284+
PlainYearMonth::from_partial(partial, ArithmeticOverflow::Constrain)
285+
}
286+
}
287+
288+
#[cfg(test)]
289+
mod tests {
290+
use core::str::FromStr;
291+
292+
use super::PlainYearMonth;
293+
294+
#[test]
295+
fn basic_from_str() {
296+
let valid_strings = [
297+
"-271821-04",
298+
"-271821-04-01",
299+
"-271821-04-01T00:00",
300+
"+275760-09",
301+
"+275760-09-30",
302+
"+275760-09-30T23:59:59.999999999",
303+
];
304+
305+
for valid_case in valid_strings {
306+
let ym = PlainYearMonth::from_str(valid_case);
307+
assert!(ym.is_ok());
308+
}
309+
}
310+
311+
#[test]
312+
fn invalid_from_str() {
313+
let invalid_strings = [
314+
"-271821-03-31",
315+
"-271821-03-31T23:59:59.999999999",
316+
"+275760-10",
317+
"+275760-10-01",
318+
"+275760-10-01T00:00",
319+
];
320+
321+
for invalid_case in invalid_strings {
322+
let err = PlainYearMonth::from_str(invalid_case);
323+
assert!(err.is_err());
324+
}
325+
326+
let invalid_strings = ["2019-10-01T09:00:00Z", "2019-10-01T09:00:00Z[UTC]"];
327+
328+
for invalid_case in invalid_strings {
329+
let err = PlainYearMonth::from_str(invalid_case);
330+
assert!(err.is_err());
331+
}
264332
}
265333
}

src/iso.rs

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -301,35 +301,42 @@ impl IsoDate {
301301
Self { year, month, day }
302302
}
303303

304-
pub(crate) fn new_with_overflow(
304+
pub(crate) fn regulate(
305305
year: i32,
306306
month: u8,
307307
day: u8,
308308
overflow: ArithmeticOverflow,
309309
) -> TemporalResult<Self> {
310-
let id = match overflow {
310+
match overflow {
311311
ArithmeticOverflow::Constrain => {
312312
let month = month.clamp(1, 12);
313313
let day = constrain_iso_day(year, month, day);
314314
// NOTE: Values are clamped in a u8 range.
315-
Self::new_unchecked(year, month, day)
315+
Ok(Self::new_unchecked(year, month, day))
316316
}
317317
ArithmeticOverflow::Reject => {
318318
if !is_valid_date(year, month, day) {
319319
return Err(TemporalError::range().with_message("not a valid ISO date."));
320320
}
321321
// NOTE: Values have been verified to be in a u8 range.
322-
Self::new_unchecked(year, month, day)
322+
Ok(Self::new_unchecked(year, month, day))
323323
}
324-
};
324+
}
325+
}
325326

326-
if !iso_dt_within_valid_limits(id, &IsoTime::noon()) {
327+
pub(crate) fn new_with_overflow(
328+
year: i32,
329+
month: u8,
330+
day: u8,
331+
overflow: ArithmeticOverflow,
332+
) -> TemporalResult<Self> {
333+
let date = Self::regulate(year, month, day, overflow)?;
334+
if !iso_dt_within_valid_limits(date, &IsoTime::noon()) {
327335
return Err(
328336
TemporalError::range().with_message("Date is not within ISO date time limits.")
329337
);
330338
}
331-
332-
Ok(id)
339+
Ok(date)
333340
}
334341

335342
/// Create a balanced `IsoDate`
@@ -957,6 +964,25 @@ fn iso_date_surpasses(this: &IsoDate, other: &IsoDate, sign: i8) -> bool {
957964
this.cmp(other) as i8 * sign == 1
958965
}
959966

967+
#[inline]
968+
pub(crate) fn year_month_within_limits(year: i32, month: u8) -> bool {
969+
// 1. If isoDate.[[Year]] < -271821 or isoDate.[[Year]] > 275760, then
970+
if !(-271821..=275760).contains(&year) {
971+
// a. Return false.
972+
return false;
973+
// 2. If isoDate.[[Year]] = -271821 and isoDate.[[Month]] < 4, then
974+
} else if year == -271821 && month < 4 {
975+
// a. Return false.
976+
return false;
977+
// 3. If isoDate.[[Year]] = 275760 and isoDate.[[Month]] > 9, then
978+
} else if year == 275760 && month > 9 {
979+
// a. Return false.
980+
return false;
981+
}
982+
// 4. Return true.
983+
true
984+
}
985+
960986
#[inline]
961987
fn balance_iso_year_month(year: i32, month: i32) -> (i32, u8) {
962988
// 1. Assert: year and month are integers.

src/parsers.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,10 +755,14 @@ pub(crate) fn parse_year_month(source: &str) -> TemporalResult<IxdtfParseRecord>
755755
let ym_record = parse_ixdtf(source, ParseVariant::YearMonth);
756756

757757
if let Ok(ym) = ym_record {
758+
if ym.offset == Some(UtcOffsetRecordOrZ::Z) {
759+
return Err(TemporalError::range()
760+
.with_message("UTC designator is not valid for DateTime parsing."));
761+
}
758762
return Ok(ym);
759763
}
760764

761-
let dt_parse = parse_ixdtf(source, ParseVariant::DateTime);
765+
let dt_parse = parse_date_time(source);
762766

763767
match dt_parse {
764768
Ok(dt) => Ok(dt),

0 commit comments

Comments
 (0)