fix(intl): #5582 — Temporal/DTF no-overlap TypeError + formatRange respects DTF options#5782
Conversation
…options
ECMA-402 HandleDateTimeValue requires a TypeError when the options explicitly
set on an Intl.DateTimeFormat have no field in common with the Temporal type's
data model (e.g. {timeStyle: "short"} + PlainDate, or {dateStyle: "long"} +
PlainTime). DTFs constructed with no options (where ToDateTimeOptions applies
the year/month/day defaults) must NOT raise this error.
Changes:
- intl.rs: add KEY_DT_IS_DEFAULT flag written when the constructor falls back
to the default year/month/day components (no explicit style or component).
- date_collator.rs: add dtf_primary_mask() / temporal_primary_mask() / validate_
temporal_dtf_overlap() helpers; call the overlap check in all four
format/formatToParts thunks before date_arg_to_clipped_ms.
- date_collator.rs: date_time_format_range_value and range_parts_value now
accept an obj parameter and format via format_ms_with_dtf_obj (respecting
dateStyle, timeStyle, component options) instead of the fixed M/D/YYYY
date_short_utc_from_ms path. The same-date collapse now compares formatted
strings rather than raw milliseconds. All four range thunks pass obj through.
Fixes temporal-plain{date,time,yearmonth,monthday}-formatting-datetime-style.js
TypeError branches and improves formatRange DTF-option fidelity (#5582).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013gZGFwJdk83JqXRt8KYWyX
📝 WalkthroughWalkthroughAdds a ChangesTemporal DTF overlap validation and range formatting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry-runtime/src/intl/date_collator.rs (1)
810-831: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
formatRangeToPartspart values still ignore DTF options.
sx/syare formatted viaformat_ms_with_dtf_objand used only for thesx == sycollapse decision, but the emitted parts still come fromdate_range_parts_from_ms, which is the fixedmonth/day/year(M/D/YYYY) representation. So whiledate_time_format_range_valuenow honorsdateStyle/timeStyle/component options, the parts path does not — e.g. with{ timeStyle: "short" },formatRangereturns time strings butformatRangeToPartsreturns date parts. Worse, two distinct times on the same date take the non-collapsed branch (sx != sy) yet emit identicalstartRange/endRangedate parts.Derive the emitted parts from the DTF object's resolved options so both APIs stay consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/intl/date_collator.rs` around lines 810 - 831, The `date_time_format_range_parts_value` path still emits fixed `date_range_parts_from_ms` date parts instead of using the `DateTimeFormat` options, so `formatRangeToParts` diverges from `formatRange`. Update the part generation in `date_time_format_range_parts_value` to derive tokens from the same DTF configuration used by `format_ms_with_dtf_obj`/`date_time_format_range_value`, and keep the `sx == sy` collapse logic while ensuring both the shared and start/end branches emit option-resolved parts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/perry-runtime/src/intl/date_collator.rs`:
- Around line 810-831: The `date_time_format_range_parts_value` path still emits
fixed `date_range_parts_from_ms` date parts instead of using the
`DateTimeFormat` options, so `formatRangeToParts` diverges from `formatRange`.
Update the part generation in `date_time_format_range_parts_value` to derive
tokens from the same DTF configuration used by
`format_ms_with_dtf_obj`/`date_time_format_range_value`, and keep the `sx == sy`
collapse logic while ensuring both the shared and start/end branches emit
option-resolved parts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10101fa7-bf2f-4816-b43d-c54f9659f3b6
📒 Files selected for processing (2)
crates/perry-runtime/src/intl.rscrates/perry-runtime/src/intl/date_collator.rs
Fixes test262 failures in
intl402/DateTimeFormattracked in #5582.Root cause
Two distinct bugs in
date_collator.rs:Missing no-overlap TypeError. ECMA-402 §11.5.5 HandleDateTimeValue requires a
TypeErrorwhen the options explicitly configured on anIntl.DateTimeFormathave no field in common with the Temporal type's data model — e.g.{timeStyle: "short"}with aTemporal.PlainDate(which carries no time fields), or{dateStyle: "long"}with aTemporal.PlainTime. DTFs constructed with no options (whereToDateTimeOptionssilently appliesyear/month/daydefaults) must not raise this error. Perry had no such check at all, so all combinations silently succeeded or hitdate_arg_to_clipped_ms's ZonedDateTime guard.formatRangeignored DTF options.date_time_format_range_value/date_time_format_range_parts_valuecalleddate_short_utc_from_ms(hardcodedM/D/YYYY) and compared raw milliseconds for the same-date collapse, both ignoringdateStyle,timeStyle, and component options entirely. The four range thunks retrieved the DTF object but discarded it (_obj).Changes
intl.rs: addKEY_DT_IS_DEFAULTconstant; write the flag inmake_instancewhenToDateTimeOptionsfalls back to the numericyear/month/daydefaults (no explicit style or component requested).date_collator.rs:dtf_primary_mask(obj)— bitmask of year/month/day/time dimensions present in the DTF's explicit options.temporal_primary_mask(kind)— bitmask of dimensions a Temporal type actually carries.validate_temporal_dtf_overlap(kind, obj)— skips whenKEY_DT_IS_DEFAULTis set; otherwise throwsTypeErrorwhen the two masks have no bits in common.format/formatToPartsthunks callvalidate_temporal_dtf_overlapbeforedate_arg_to_clipped_ms.date_time_format_range_valueanddate_time_format_range_parts_valuenow accept anobj: *const ObjectHeaderand format viaformat_ms_with_dtf_obj(respectingdateStyle,timeStyle, component options). Same-date collapse compares formatted strings rather than raw milliseconds._objsuppression and passobjthrough to the value functions.Before / after (test262
intl402/DateTimeFormat)The remaining failures are split between tests that require real IANA timezone support (e.g.
temporal-objects-not-overlapping-options.jsusesAmerica/Vancouverfor local-noon vs UTC-noon comparison) and other unrelated gaps tracked separately in #5582.Checklist
cargo fmt --all -- --checkpassesbash scripts/check_file_size.shpasses (no file exceeds 2000 lines)cargo build --release -p perry-runtimesucceeds (warnings only, no errors)Cargo.tomlversion bump, noCLAUDE.mdorCHANGELOG.mdedits (external contributor PR)Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
formatRangeandformatRangeToPartsso single-value results use the same formatting style as regular date-time output.