feat(intl): implement Intl.DurationFormat (#5344)#5405
Merged
Conversation
Adds the `Intl.DurationFormat` constructor — the other missing ECMA-402 constructor behind the intl402 "undefined is not a constructor" cluster (#5344). Lives in a new `intl/duration_format` submodule wired into the existing formatter machinery (`make_instance` + `install_constructor`). Implemented: - Constructor + option resolution in spec order (localeMatcher, numberingSystem, style, then the ten units + their Display slots, then fractionalDigits), via a faithful `GetDurationUnitOptions`: the `numeric`→`fractional` promotion for milliseconds/microseconds/nanoseconds, the `prevStyle`-driven `2-digit` propagation onto minutes/seconds, and the per-unit `displayDefault` rules. `resolvedOptions` reports an internal `fractional` style as `numeric`, matching the spec. - Structural validation: out-of-range style/display/numberingSystem/ fractionalDigits values and `null` (which `GetOption` coerces via ToString, unlike the lenient shared reader) throw `RangeError`. - `format` / `formatToParts` validate the duration argument per `ToDurationRecord` + `IsValidDuration` (non-object → TypeError; non-integral fields, mixed signs, years/months/weeks > 2^32-1, or normalized seconds >= 2^53 → RangeError), then render a deterministic English string. - `resolvedOptions` with the full key set + order. Localized output uses a data-free English rendering: full CLDR unit patterns and list grouping need `NumberFormat {style:"unit"}` + `ListFormat` + CLDR data Perry doesn't carry, so the exact-output `style-*-en` / `formatToParts-*` tests still fail, as do `Temporal.Duration`-argument cases, the `newTarget` (no-`new`) detection, and inherited `Symbol.toStringTag` (a pre-existing gap shared by the other Intl formatters). test262 intl402/DurationFormat: 0 -> 45/110 (41%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a full ChangesIntl.DurationFormat
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@crates/perry-runtime/src/intl/duration_format.rs`:
- Around line 204-215: The issue is that when `numberingSystem: null` is passed,
`df_get_option_string` converts it to the string "null", which then passes the
`valid_numbering_system` check because "null" is a valid alphanumeric subtag.
This bypasses proper null validation. Fix this by adding an explicit check after
retrieving the numbering system value but before calling
`valid_numbering_system`: if the returned string value is literally "null",
immediately throw a RangeError with an appropriate message before performing the
syntax validation. This ensures null is properly rejected as an invalid value
for the numberingSystem option.
- Around line 431-434: The saturating cast of n to i64 in both format! calls
within the style check (numeric/2-digit and the else branch) causes valid large
microseconds/nanoseconds values to be truncated when n exceeds i64::MAX. Replace
the as i64 casts with either the original type of n if it can represent the full
range of valid sub-second values, or cast to u64 if the values are guaranteed to
be non-negative. This applies to both instances where n is cast in the two
pieces.push() calls.
- Around line 240-248: The get_option_number function incorrectly treats both
undefined and null as absent, but per ECMA-402 spec only undefined should select
the default while null must be coerced through ToNumber and validated. Fix the
fractionalDigits option handling by either creating a numeric equivalent to
df_get_option_string that only treats undefined as absent and coerces all other
values (including null) through the same validation logic, or refactor
get_option_number to distinguish between undefined and null, ensuring that null
values are processed through the validation checks rather than silently
defaulting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a595b34-529c-4918-97b1-db1c5ad9cfb7
📒 Files selected for processing (2)
crates/perry-runtime/src/intl.rscrates/perry-runtime/src/intl/duration_format.rs
Comment on lines
+204
to
+215
| let numbering = match df_get_option_string(options, "numberingSystem") { | ||
| Some(ns) => { | ||
| if !valid_numbering_system(&ns) { | ||
| throw_range_error(&format!( | ||
| "Value {ns} out of range for Intl.DurationFormat options property numberingSystem" | ||
| )); | ||
| } | ||
| ns | ||
| } | ||
| None => "latn".to_string(), | ||
| }; | ||
| set_internal_field(obj, KEY_DF_NUMBERING, string_value(&numbering)); |
There was a problem hiding this comment.
Reject numberingSystem: null before the syntax check.
df_get_option_string stringifies null to "null", and valid_numbering_system("null") currently accepts it because it is a 4-letter alphanumeric subtag. That bypasses the PR’s null-as-RangeError validation contract for numberingSystem.
🐛 Proposed fix
let numbering = match df_get_option_string(options, "numberingSystem") {
Some(ns) => {
- if !valid_numbering_system(&ns) {
+ if ns == "null" || !valid_numbering_system(&ns) {
throw_range_error(&format!(
"Value {ns} out of range for Intl.DurationFormat options property numberingSystem"
));
}
ns📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let numbering = match df_get_option_string(options, "numberingSystem") { | |
| Some(ns) => { | |
| if !valid_numbering_system(&ns) { | |
| throw_range_error(&format!( | |
| "Value {ns} out of range for Intl.DurationFormat options property numberingSystem" | |
| )); | |
| } | |
| ns | |
| } | |
| None => "latn".to_string(), | |
| }; | |
| set_internal_field(obj, KEY_DF_NUMBERING, string_value(&numbering)); | |
| let numbering = match df_get_option_string(options, "numberingSystem") { | |
| Some(ns) => { | |
| if ns == "null" || !valid_numbering_system(&ns) { | |
| throw_range_error(&format!( | |
| "Value {ns} out of range for Intl.DurationFormat options property numberingSystem" | |
| )); | |
| } | |
| ns | |
| } | |
| None => "latn".to_string(), | |
| }; | |
| set_internal_field(obj, KEY_DF_NUMBERING, string_value(&numbering)); |
🤖 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/duration_format.rs` around lines 204 - 215, The
issue is that when `numberingSystem: null` is passed, `df_get_option_string`
converts it to the string "null", which then passes the `valid_numbering_system`
check because "null" is a valid alphanumeric subtag. This bypasses proper null
validation. Fix this by adding an explicit check after retrieving the numbering
system value but before calling `valid_numbering_system`: if the returned string
value is literally "null", immediately throw a RangeError with an appropriate
message before performing the syntax validation. This ensures null is properly
rejected as an invalid value for the numberingSystem option.
Comment on lines
+240
to
+248
| // fractionalDigits: integer in [0, 9], else RangeError. Read last. | ||
| if let Some(n) = get_option_number(options, "fractionalDigits") { | ||
| if !n.is_finite() || n.fract() != 0.0 || !(0.0..=9.0).contains(&n) { | ||
| throw_range_error( | ||
| "Value out of range for Intl.DurationFormat options property fractionalDigits", | ||
| ); | ||
| } | ||
| set_internal_field(obj, KEY_DF_FRACTIONAL, n); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect DurationFormat fractionalDigits parsing and the shared number option helper.
# Expectation: `fractionalDigits: null` is not treated as absent or accepted as 0.
rg -n -C5 'fn get_option_number|get_option_number\(|fractionalDigits' crates/perry-runtime/src/intl.rs crates/perry-runtime/src/intl/duration_format.rsRepository: PerryTS/perry
Length of output: 7314
🏁 Script executed:
#!/bin/bash
# Search for df_get_option_string implementation and how it handles null
rg -n -C8 'fn df_get_option_string|fn get_option_string' crates/perry-runtime/src/intl.rs crates/perry-runtime/src/intl/duration_format.rsRepository: PerryTS/perry
Length of output: 2872
🏁 Script executed:
#!/bin/bash
# Search for any PR, spec comments, or test files that mention fractionalDigits null behavior
rg -n 'fractionalDigits.*null|null.*fractionalDigits|fractionalDigits.*RangeError' crates/perry-runtime/src/intl/ --type rsRepository: PerryTS/perry
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Check for test files that might document expected behavior
find crates/perry-runtime -name '*test*' -o -name '*spec*' | head -20Repository: PerryTS/perry
Length of output: 856
Use a spec-compliant get_option_number implementation that coerces null through validation, not treat it as absent.
The string options correctly use df_get_option_string, which only treats undefined as absent and coerces all other values (including null) to trigger validation. However, fractionalDigits relies on the shared get_option_number, which treats both undefined and null as None, silently defaulting when null is passed. This violates the ECMA-402 GetOption specification: only undefined should select the default; null must be coerced through ToNumber and validated, raising a RangeError for non-integer or out-of-range values. Either provide a numeric equivalent to df_get_option_string or refactor get_option_number to match the spec-compliant behavior.
🤖 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/duration_format.rs` around lines 240 - 248, The
get_option_number function incorrectly treats both undefined and null as absent,
but per ECMA-402 spec only undefined should select the default while null must
be coerced through ToNumber and validated. Fix the fractionalDigits option
handling by either creating a numeric equivalent to df_get_option_string that
only treats undefined as absent and coerces all other values (including null)
through the same validation logic, or refactor get_option_number to distinguish
between undefined and null, ensuring that null values are processed through the
validation checks rather than silently defaulting.
Comment on lines
+431
to
+434
| if style == "numeric" || style == "2-digit" { | ||
| pieces.push(format!("{}", n as i64)); | ||
| } else { | ||
| pieces.push(format!("{} {}", n as i64, unit_label(unit, &style, n))); |
There was a problem hiding this comment.
Avoid saturating valid large sub-second values through as i64.
to_duration_record allows large microseconds/nanoseconds values when normalized seconds stay below 2^53, but n as i64 saturates once the raw field exceeds i64::MAX, producing the wrong formatted value for valid inputs.
🐛 Proposed fix
+fn format_integral_duration_number(n: f64) -> String {
+ if n == 0.0 {
+ "0".to_string()
+ } else {
+ format!("{n:.0}")
+ }
+}
+
/// Build the rendered segments for a validated duration. Best-effort English;
/// the concatenation is what `format` returns and `formatToParts` mirrors.
fn render(obj: *const ObjectHeader, vals: &[f64]) -> Vec<(&'static str, String)> {
@@
let style = get_string_field(obj, &style_key(unit)).unwrap_or_else(|| "short".to_string());
if style == "numeric" || style == "2-digit" {
- pieces.push(format!("{}", n as i64));
+ pieces.push(format_integral_duration_number(n));
} else {
- pieces.push(format!("{} {}", n as i64, unit_label(unit, &style, n)));
+ pieces.push(format!(
+ "{} {}",
+ format_integral_duration_number(n),
+ unit_label(unit, &style, n)
+ ));
}🤖 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/duration_format.rs` around lines 431 - 434, The
saturating cast of n to i64 in both format! calls within the style check
(numeric/2-digit and the else branch) causes valid large
microseconds/nanoseconds values to be truncated when n exceeds i64::MAX. Replace
the as i64 casts with either the original type of n if it can represent the full
range of valid sub-second values, or cast to u64 if the values are guaranteed to
be non-negative. This applies to both instances where n is cast in the two
pieces.push() calls.
This was referenced Jun 18, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the
Intl.DurationFormatconstructor — the other missing ECMA-402 constructor behind the intl402 "undefined is not a constructor" cluster (#5344), followingIntl.Locale(#5359). Lives in a newintl/duration_formatsubmodule wired into the existing formatter machinery (make_instance+install_constructor).What's implemented
localeMatcher,numberingSystem,style, then the ten units + their*Displayslots, thenfractionalDigits) via a faithfulGetDurationUnitOptions:numeric→fractionalpromotion formilliseconds/microseconds/nanoseconds,prevStyle-driven2-digitpropagation ontominutes/seconds,displayDefaultrules,resolvedOptionsreports an internalfractionalstyle asnumeric(per spec).style/*Display/numberingSystem/fractionalDigits, andnull(whichGetOptioncoerces viaToString, unlike the lenient shared reader) →RangeError.format/formatToPartsvalidate the duration argument perToDurationRecord+IsValidDuration(non-object →TypeError; non-integral fields / mixed signs /years·months·weeks> 2³²−1 / normalized seconds ≥ 2⁵³ →RangeError), then render output.resolvedOptionswith the full key set and order.Results
scripts/test262_subset.py --dir intl402/DurationFormat --all-features(node v26): 0 → 45/110 (41%).Remaining failures are out of scope or known gaps:
style-*-en,formatToParts-*-en,negative-*) — localized output uses a data-free English rendering; matching CLDR needsNumberFormat {style:"unit"}+ListFormat+ CLDR unit patterns Perry doesn't carry.Temporal.Duration-argument cases (reading fields off a Temporal cell / ISO duration-string parsing).newTarget(no-new) detection and inheritedSymbol.toStringTag— pre-existing runtime gaps shared by the other Intl formatters.🤖 Generated with Claude Code
Summary by CodeRabbit
Intl.DurationFormatAPI to format durations with locale-aware settings. Supportsformat,formatToParts, andresolvedOptionsmethods for flexible duration formatting and localization.