Skip to content
Permalink
Browse files
[JSC] Update Intl.NumberFormat's useGrouping handling
https://bugs.webkit.org/show_bug.cgi?id=246196
rdar://100877842

Reviewed by Ross Kirsling.

Update the implementation with the spec update[1], handling "true" and "false"
specially and throwing a range error for the other invalid cases.

[1]: tc39/proposal-intl-numberformat-v3@4751da5

* JSTests/stress/intl-numberformat-usegrouping-v3.js:
(nonListedUseGrouping.forEach):
(specialUseGrouping.forEach):
* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:
(JSC::IntlNumberFormat::initializeNumberFormat):
* Source/JavaScriptCore/runtime/IntlObjectInlines.h:
(JSC::intlStringOrBooleanOption):

Canonical link: https://commits.webkit.org/255275@main
  • Loading branch information
Constellation committed Oct 7, 2022
1 parent 8ed46d3 commit 4a7ae871cccb5324e996699daf4c5d9e7a8f768e
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 8 deletions.
@@ -54,18 +54,28 @@ let validUseGrouping = [

let nonListedUseGrouping = [
"min-2",
];

let specialUseGrouping = [
"true",
"false",
];
]

validUseGrouping.forEach(function(useGrouping) {
let nf = new Intl.NumberFormat(undefined, {useGrouping});
shouldBe(useGrouping, nf.resolvedOptions().useGrouping);
});

nonListedUseGrouping.forEach(function(useGrouping) {
shouldThrow(() => {
let nf = new Intl.NumberFormat(undefined, {useGrouping});
nf.resolvedOptions().useGrouping
}, `RangeError: useGrouping must be either true, false, "min2", "auto", or "always"`);
});

specialUseGrouping.forEach(function(useGrouping) {
let nf = new Intl.NumberFormat(undefined, {useGrouping});
shouldBe(nf.resolvedOptions().useGrouping, `auto`);
shouldBe("auto", nf.resolvedOptions().useGrouping);
});

// useGrouping: undefined get "auto"
@@ -1233,9 +1233,6 @@ test/intl402/NumberFormat/prototype/formatRangeToParts/x-greater-than-y-not-thro
test/intl402/NumberFormat/test-option-roundingPriority-mixed-options.js:
default: 'Test262Error: Formatted value for 1, en-US-u-nu-arab and options {"minimumSignificantDigits":2,"minimumFractionDigits":2,"roundingPriority":"morePrecision","userGrouping":false} is ١٫٠٠; expected ١٫٠.'
strict mode: 'Test262Error: Formatted value for 1, en-US-u-nu-arab and options {"minimumSignificantDigits":2,"minimumFractionDigits":2,"roundingPriority":"morePrecision","userGrouping":false} is ١٫٠٠; expected ١٫٠.'
test/intl402/NumberFormat/test-option-useGrouping.js:
default: 'Test262Error: Throws RangeError when useGrouping value is not supported Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: Throws RangeError when useGrouping value is not supported Expected a RangeError to be thrown but no exception was thrown at all'
test/intl402/PluralRules/prototype/selectRange/default-en-us.js:
default: "TypeError: pr.selectRange is not a function. (In 'pr.selectRange(102, 201)', 'pr.selectRange' is undefined)"
strict mode: "TypeError: pr.selectRange is not a function. (In 'pr.selectRange(102, 201)', 'pr.selectRange' is undefined)"
@@ -414,7 +414,7 @@ void IntlNumberFormat::initializeNumberFormat(JSGlobalObject* globalObject, JSVa
if (m_notation == IntlNotation::Compact)
defaultUseGrouping = UseGrouping::Min2;

m_useGrouping = intlStringOrBooleanOption<UseGrouping>(globalObject, options, Identifier::fromString(vm, "useGrouping"_s), UseGrouping::Always, UseGrouping::False, { { "min2"_s, UseGrouping::Min2 }, { "auto"_s, UseGrouping::Auto }, { "always"_s, UseGrouping::Always } }, defaultUseGrouping);
m_useGrouping = intlStringOrBooleanOption<UseGrouping>(globalObject, options, Identifier::fromString(vm, "useGrouping"_s), UseGrouping::Always, UseGrouping::False, { { "min2"_s, UseGrouping::Min2 }, { "auto"_s, UseGrouping::Auto }, { "always"_s, UseGrouping::Always } }, "useGrouping must be either true, false, \"min2\", \"auto\", or \"always\""_s, defaultUseGrouping);
RETURN_IF_EXCEPTION(scope, void());

m_signDisplay = intlOption<SignDisplay>(globalObject, options, Identifier::fromString(vm, "signDisplay"_s), { { "auto"_s, SignDisplay::Auto }, { "never"_s, SignDisplay::Never }, { "always"_s, SignDisplay::Always }, { "exceptZero"_s, SignDisplay::ExceptZero }, { "negative"_s, SignDisplay::Negative } }, "signDisplay must be either \"auto\", \"never\", \"always\", \"exceptZero\", or \"negative\""_s, SignDisplay::Auto);
@@ -152,7 +152,7 @@ ResultType intlOption(JSGlobalObject* globalObject, JSObject* options, PropertyN
}

template<typename ResultType>
ResultType intlStringOrBooleanOption(JSGlobalObject* globalObject, JSObject* options, PropertyName property, ResultType trueValue, ResultType falsyValue, std::initializer_list<std::pair<ASCIILiteral, ResultType>> values, ResultType fallback)
ResultType intlStringOrBooleanOption(JSGlobalObject* globalObject, JSObject* options, PropertyName property, ResultType trueValue, ResultType falsyValue, std::initializer_list<std::pair<ASCIILiteral, ResultType>> values, ASCIILiteral notFoundMessage, ResultType fallback)
{
// https://tc39.es/proposal-intl-numberformat-v3/out/negotiation/diff.html#sec-getstringorbooleanoption

@@ -182,12 +182,18 @@ ResultType intlStringOrBooleanOption(JSGlobalObject* globalObject, JSObject* opt
String stringValue = value.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

// FIXME: We need to know whether this fallback is actually correct.
// https://github.com/tc39/proposal-intl-numberformat-v3/issues/111
if (stringValue == "true"_s || stringValue == "false"_s)
return fallback;

for (const auto& entry : values) {
if (entry.first == stringValue)
return entry.second;
}

return fallback;
throwRangeError(globalObject, scope, notFoundMessage);
return { };
}

ALWAYS_INLINE bool canUseASCIIUCADUCETComparison(UChar character)

0 comments on commit 4a7ae87

Please sign in to comment.