diff --git a/Cargo.lock b/Cargo.lock index f02b6d6e1e..df85e88428 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6333,8 +6333,10 @@ dependencies = [ "hostname", "icu_calendar", "icu_datetime", + "icu_decimal", "icu_locale", "icu_locale_core", + "icu_provider", "icu_time", "idna", "itoa", diff --git a/changelog.d/9450-intl-locale-grouping.md b/changelog.d/9450-intl-locale-grouping.md new file mode 100644 index 0000000000..6076e5fccc --- /dev/null +++ b/changelog.d/9450-intl-locale-grouping.md @@ -0,0 +1,6 @@ +### Fixed + +- `Intl.NumberFormat` and Number/BigInt locale formatting now use CLDR primary + and secondary grouping widths instead of fixed three-digit groups. Indian + locales therefore render `123456789` as `12,34,56,789`, while western + grouping and `useGrouping: false` remain unchanged. (#9450) diff --git a/crates/perry-runtime/Cargo.toml b/crates/perry-runtime/Cargo.toml index f2546c6af5..d7822cf070 100644 --- a/crates/perry-runtime/Cargo.toml +++ b/crates/perry-runtime/Cargo.toml @@ -162,7 +162,7 @@ intl-segmenter = ["dep:unicode-segmentation"] # reachable), so only the `Intl.` namespace members depend on it. The # compiler enables it on any `Intl`/locale-formatting token, erring toward # enabling (same over-approximation contract as `temporal`). -intl-namespace = [] +intl-namespace = ["dep:icu_decimal", "dep:icu_provider", "dep:icu_locale_core"] # Per-namespace `globalThis` member tables. Each installs the reflectable # members of one built-in namespace (`Math.max`, `JSON.stringify`, # `Reflect.get`, `Atomics.add`, …) as real properties. Call sites in user code @@ -343,6 +343,11 @@ unicode-segmentation = { version = "1", optional = true } # their compiled data are already in the default lock graph via icu_datetime. icu_locale = { version = "2", optional = true } icu_locale_core = { version = "2", optional = true } +# CLDR decimal grouping metadata (primary/secondary group widths and the +# locale's minimum grouping threshold), shared by Intl.NumberFormat and +# Number/BigInt.prototype.toLocaleString. +icu_decimal = { version = "2", default-features = false, features = ["compiled_data"], optional = true } +icu_provider = { version = "2", default-features = false, optional = true } # CLDR date/time formatting for Intl.DateTimeFormat / Date.prototype.toLocale* # (icu4x 2.x, matching the icu_calendar/icu_locale_core already in the graph). # `compiled_data` vendors the CLDR dataset, so byte-for-byte-with-Node locale diff --git a/crates/perry-runtime/src/intl.rs b/crates/perry-runtime/src/intl.rs index 06656f6ed2..ae6c46176d 100644 --- a/crates/perry-runtime/src/intl.rs +++ b/crates/perry-runtime/src/intl.rs @@ -737,19 +737,6 @@ fn rest_arg(rest: f64, index: u32) -> f64 { } } -fn group_integer_digits(digits: &str, separator: char) -> String { - let mut grouped = String::with_capacity(digits.len() + digits.len() / 3); - let len = digits.len(); - for (i, ch) in digits.chars().enumerate() { - let from_end = len - i; - grouped.push(ch); - if from_end > 1 && from_end % 3 == 1 { - grouped.push(separator); - } - } - grouped -} - fn format_number_parts( value: f64, locale: &str, @@ -793,7 +780,9 @@ fn format_number_parts( if negative { out.push('-'); } - out.push_str(&group_integer_digits(int_part, group_sep)); + out.push_str(&number_format::group_integer_digits_for_locale( + int_part, group_sep, locale, + )); if !frac_part.is_empty() { out.push(decimal_sep); out.push_str(frac_part); diff --git a/crates/perry-runtime/src/intl/number_format.rs b/crates/perry-runtime/src/intl/number_format.rs index 1779acca07..b20e3bfa5e 100644 --- a/crates/perry-runtime/src/intl/number_format.rs +++ b/crates/perry-runtime/src/intl/number_format.rs @@ -495,40 +495,108 @@ pub(crate) fn decimal_msd_exponent(int_part: &str, frac_part: &str) -> i32 { } } -/// Group an integer digit string into locale parts. Pushes `integer`/`group` -/// segments. Grouping is applied when `grouping` is true and the integer has >3 -/// digits. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct DecimalGrouping { + primary: usize, + secondary: usize, + min_grouping: usize, +} + +const WESTERN_GROUPING: DecimalGrouping = DecimalGrouping { + primary: 3, + secondary: 3, + min_grouping: 1, +}; + +#[cfg(feature = "intl-namespace")] +fn icu_locale_grouping(locale: &str) -> Option { + use icu_decimal::provider::{Baked, DecimalSymbolsV1}; + use icu_provider::{DataIdentifierBorrowed, DataProvider, DataRequest, DataResponse}; + + let locale: icu_locale_core::Locale = locale.parse().ok()?; + let data_locale = icu_provider::DataLocale::from(&locale); + let response: DataResponse = Baked + .load(DataRequest { + id: DataIdentifierBorrowed::for_locale(&data_locale), + ..Default::default() + }) + .ok()?; + let sizes = response.payload.get().grouping_sizes; + Some(DecimalGrouping { + primary: sizes.primary as usize, + secondary: if sizes.secondary == 0 { + sizes.primary as usize + } else { + sizes.secondary as usize + }, + min_grouping: sizes.min_grouping as usize, + }) +} + +/// CLDR grouping widths for the resolved locale. +pub(crate) fn locale_grouping(locale: &str) -> DecimalGrouping { + #[cfg(feature = "intl-namespace")] + if let Some(grouping) = icu_locale_grouping(locale) { + return grouping; + } + let _ = locale; + WESTERN_GROUPING +} + +/// Whether grouping separators should be emitted for an integer of `int_len` +/// digits under the resolved `useGrouping` value. +pub(crate) fn grouping_enabled(use_grouping: &str, int_len: usize, sizes: DecimalGrouping) -> bool { + if sizes.primary == 0 { + return false; + } + match use_grouping { + "false" => false, + "always" => int_len > sizes.primary, + "min2" => int_len >= sizes.primary + sizes.min_grouping.max(2), + _ => int_len >= sizes.primary + sizes.min_grouping.max(1), + } +} + +/// Group an ASCII integer digit string into locale-aware `integer`/`group` +/// segments. The rightmost group uses `primary`; all preceding groups use +/// `secondary` (for example, en-IN uses 3 then repeated 2). pub(crate) fn push_grouped_integer( parts: &mut Vec<(&'static str, String)>, int_digits: &str, group_sep: char, grouping: bool, + sizes: DecimalGrouping, ) { - if !grouping || int_digits.len() <= 3 { + let primary = sizes.primary; + if !grouping || primary == 0 || int_digits.len() <= primary { parts.push(("integer", int_digits.to_string())); return; } - let chars: Vec = int_digits.chars().collect(); - let n = chars.len(); - let head = if n % 3 == 0 { 3 } else { n % 3 }; - parts.push(("integer", chars[..head].iter().collect())); - let mut i = head; - while i < n { + let secondary = sizes.secondary.max(1); + let primary_start = int_digits.len() - primary; + let remainder = primary_start % secondary; + let head = if remainder == 0 { secondary } else { remainder }; + parts.push(("integer", int_digits[..head].to_string())); + let mut index = head; + while index < primary_start { parts.push(("group", group_sep.to_string())); - parts.push(("integer", chars[i..i + 3].iter().collect())); - i += 3; + parts.push(("integer", int_digits[index..index + secondary].to_string())); + index += secondary; } + parts.push(("group", group_sep.to_string())); + parts.push(("integer", int_digits[primary_start..].to_string())); } -/// Whether grouping separators should be emitted for an integer of `int_len` -/// digits under the resolved `useGrouping` value. -pub(crate) fn grouping_enabled(use_grouping: &str, int_len: usize) -> bool { - match use_grouping { - "false" => false, - "min2" => int_len >= 5, - // "auto" / "always" both group for the locales we render (Latin/de). - _ => int_len > 3, - } +pub(crate) fn group_integer_digits_for_locale( + digits: &str, + separator: char, + locale: &str, +) -> String { + let sizes = locale_grouping(locale); + let enabled = grouping_enabled("auto", digits.len(), sizes); + let mut parts = Vec::new(); + push_grouped_integer(&mut parts, digits, separator, enabled, sizes); + parts.into_iter().map(|(_, value)| value).collect() } /// Locale-specific display symbol for USD. Most locales use "$"; Korean and @@ -666,6 +734,7 @@ fn number_parts_core(r: &NfResolved, value: f64) -> Vec<(&'static str, String)> // #7429: CLDR separators for the resolved locale, not a de-vs-rest guess. let (group_sep, decimal_sep) = locale_separators(&r.locale); + let grouping_sizes = locale_grouping(&r.locale); let mut parts: Vec<(&'static str, String)> = Vec::new(); let is_zero = value == 0.0; @@ -758,7 +827,7 @@ fn number_parts_core(r: &NfResolved, value: f64) -> Vec<(&'static str, String)> while (i_out.len() as u32) < r.min_int { i_out.insert(0, '0'); } - push_grouped_integer(&mut parts, &i_out, group_sep, false); + push_grouped_integer(&mut parts, &i_out, group_sep, false, grouping_sizes); if !f_out.is_empty() { parts.push(("decimal", decimal_sep.to_string())); parts.push(("fraction", f_out)); @@ -804,8 +873,8 @@ fn number_parts_core(r: &NfResolved, value: f64) -> Vec<(&'static str, String)> while (i_out.len() as u32) < r.min_int { i_out.insert(0, '0'); } - let grouping = grouping_enabled(&r.use_grouping, i_out.len()); - push_grouped_integer(&mut parts, &i_out, group_sep, grouping); + let grouping = grouping_enabled(&r.use_grouping, i_out.len(), grouping_sizes); + push_grouped_integer(&mut parts, &i_out, group_sep, grouping, grouping_sizes); if !f_out.is_empty() { parts.push(("decimal", decimal_sep.to_string())); parts.push(("fraction", f_out)); @@ -827,8 +896,8 @@ fn number_parts_core(r: &NfResolved, value: f64) -> Vec<(&'static str, String)> while (i_out.len() as u32) < r.min_int { i_out.insert(0, '0'); } - let grouping = grouping_enabled(&r.use_grouping, i_out.len()); - push_grouped_integer(&mut parts, &i_out, group_sep, grouping); + let grouping = grouping_enabled(&r.use_grouping, i_out.len(), grouping_sizes); + push_grouped_integer(&mut parts, &i_out, group_sep, grouping, grouping_sizes); if !f_out.is_empty() { parts.push(("decimal", decimal_sep.to_string())); parts.push(("fraction", f_out)); @@ -1145,6 +1214,7 @@ fn bigint_number_parts_exact( ) -> Vec<(&'static str, String)> { // #7429: CLDR separators for the resolved locale, not a de-vs-rest guess. let (group_sep, decimal_sep) = locale_separators(&r.locale); + let grouping_sizes = locale_grouping(&r.locale); set_round_ctx(&r.rounding_mode, negative); let mut parts: Vec<(&'static str, String)> = Vec::new(); @@ -1155,8 +1225,8 @@ fn bigint_number_parts_exact( while (i_out.len() as u32) < r.min_int { i_out.insert(0, '0'); } - let grouping = grouping_enabled(&r.use_grouping, i_out.len()); - push_grouped_integer(&mut parts, &i_out, group_sep, grouping); + let grouping = grouping_enabled(&r.use_grouping, i_out.len(), grouping_sizes); + push_grouped_integer(&mut parts, &i_out, group_sep, grouping, grouping_sizes); if !f_out.is_empty() { parts.push(("decimal", decimal_sep.to_string())); parts.push(("fraction", f_out)); @@ -1532,3 +1602,33 @@ pub(crate) fn number_format_resolved_options_object(obj: *const ObjectHeader) -> set_field(out, "trailingZeroDisplay", string_value(&r.trailing_zero)); js_nanbox_pointer(out as i64) } + +#[cfg(test)] +mod grouping_tests { + use super::*; + + fn render(digits: &str, sizes: DecimalGrouping) -> String { + let mut parts = Vec::new(); + push_grouped_integer(&mut parts, digits, ',', true, sizes); + parts.into_iter().map(|(_, value)| value).collect() + } + + #[cfg(feature = "intl-namespace")] + #[test] + fn uses_cldr_secondary_group_widths() { + let sizes = locale_grouping("en-IN"); + assert_eq!( + (sizes.primary, sizes.secondary, sizes.min_grouping), + (3, 2, 1) + ); + assert_eq!(render("123456789", sizes), "12,34,56,789"); + } + + #[test] + fn preserves_western_grouping() { + let sizes = locale_grouping("en-US"); + assert_eq!(render("123456789", sizes), "123,456,789"); + assert!(grouping_enabled("auto", 4, sizes)); + assert!(!grouping_enabled("false", 9, sizes)); + } +} diff --git a/test-files/test_gap_9440_error_name_own_enumerable.ts b/test-files/test_gap_9440_error_name_own_enumerable.ts new file mode 100644 index 0000000000..c03fc848a6 --- /dev/null +++ b/test-files/test_gap_9440_error_name_own_enumerable.ts @@ -0,0 +1,66 @@ +// #9440: Error-subclass construction stamped `name` as an own enumerable +// property. Node inherits the non-enumerable value from Error.prototype, so +// the wrong shape leaked through JSON, every own-key API, for-in and spread. +// Compared byte-for-byte against `node --experimental-strip-types`. + +import { inspect } from "node:util"; + +class MyErr extends Error {} +class MyTypeErr extends TypeError {} + +function ownSnapshot(label: string, error: Error): void { + const forIn: string[] = []; + for (const key in error) { + forIn.push(key); + } + + console.log(label + " value: " + error.name); + console.log(label + " json: " + JSON.stringify(error)); + console.log( + label + " own names: " + JSON.stringify(Object.getOwnPropertyNames(error)), + ); + console.log(label + " keys: " + JSON.stringify(Object.keys(error))); + console.log(label + " for-in: " + JSON.stringify(forIn)); + console.log(label + " spread: " + JSON.stringify({ ...error })); + // Stack paths and frames are host-specific. The first line is the stable + // Error headline that util.inspect derives from the effective name. + console.log(label + " inspect: " + inspect(error).split("\n")[0]); +} + +ownSnapshot("Error", new Error("boom")); +ownSnapshot("subclass", new MyErr("boom")); +ownSnapshot("type-subclass", new MyTypeErr("boom")); + +// Assignment must still use ordinary [[Set]] semantics: it creates an own, +// writable/enumerable/configurable data property and all enumeration paths see +// exactly that one additional key. +const assigned = new MyErr("boom"); +assigned.name = "Custom"; +ownSnapshot("assigned", assigned); +const descriptor = Object.getOwnPropertyDescriptor(assigned, "name"); +console.log( + "assigned descriptor: " + + JSON.stringify({ + value: descriptor?.value, + writable: descriptor?.writable, + enumerable: descriptor?.enumerable, + configurable: descriptor?.configurable, + }), +); + +// The native ErrorHeader path must agree with the ordinary ObjectHeader used +// by a subclass. This also pins assignment after construction, rather than a +// class-body field initializer. +const assignedBase = new Error("base"); +assignedBase.name = "CustomBase"; +ownSnapshot("assigned-base", assignedBase); + +// Controls for the actual prototype placement. +console.log( + "Error.prototype.name: " + + JSON.stringify(Object.getOwnPropertyDescriptor(Error.prototype, "name")), +); +console.log( + "TypeError.prototype.name: " + + JSON.stringify(Object.getOwnPropertyDescriptor(TypeError.prototype, "name")), +); diff --git a/test-files/test_gap_9450_intl_indian_grouping.ts b/test-files/test_gap_9450_intl_indian_grouping.ts new file mode 100644 index 0000000000..51bbe298a4 --- /dev/null +++ b/test-files/test_gap_9450_intl_indian_grouping.ts @@ -0,0 +1,40 @@ +// #9450: Intl.NumberFormat grouped every locale in fixed 3-digit runs. +// CLDR's en-IN pattern has a 3-digit primary group and 2-digit secondary +// groups. Compared byte-for-byte against `node --experimental-strip-types`. + +const indianValues = [1234, 12345, 123456, 1234567, 123456789]; +const indian = new Intl.NumberFormat("en-IN"); + +for (const value of indianValues) { + console.log("Intl en-IN " + value + ": " + indian.format(value)); +} + +// Pin typed group/integer parts, not only their concatenated spelling. +console.log( + "parts en-IN: " + + JSON.stringify(indian.formatToParts(123456789).map(({ type, value }) => [type, value])), +); + +// Three-digit controls must remain unchanged, including distinct separators. +for (const locale of ["en-US", "de-DE", "fr-FR"]) { + console.log(locale + ": " + new Intl.NumberFormat(locale).format(123456789)); +} + +console.log( + "en-IN ungrouped: " + + new Intl.NumberFormat("en-IN", { useGrouping: false }).format(123456789), +); + +// Number.prototype.toLocaleString delegates to the same formatter and must +// preserve the locale's primary/secondary pattern at every divergent width. +for (const value of indianValues) { + console.log("toLocaleString en-IN " + value + ": " + value.toLocaleString("en-IN")); +} + +// BigInt has a separate exact-precision rendering path; it consumes the same +// grouping metadata and must not quietly retain fixed 3-digit runs. +console.log( + "BigInt en-IN: " + + (12345678901234567890n as any).toLocaleString("en-IN"), +); +