Skip to content

Commit c3309bc

Browse files
committed
Update NumberFormat to treat -0 as a negative number and PluralRules to not define sigfig properties on the resolved options if the user did not request them
1 parent 9fdb174 commit c3309bc

File tree

3 files changed

+43
-24
lines changed

3 files changed

+43
-24
lines changed

lib/Runtime/Library/InJavascript/Intl.js

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -929,14 +929,18 @@
929929
*
930930
* @param {String[]} props The list of properties to extract from hiddenObject and add to the final resolved options
931931
* @param {Object} hiddenObject The hiddenObject of the calling constructor that contains values for each prop in props
932-
* @param {Function} func An optional custom function(prop, resolved) run for each prop; if omitted, default logic will be used
932+
* @param {Function} func An optional custom function(prop, resolved) run for each prop; it should return true when
933+
* it handles a property itself. If it does not return true, the default logic will be used.
933934
*/
934935
const createResolvedOptions = function (props, hiddenObject, func = null) {
935936
const resolved = _.create();
936937
_.forEach(props, function (prop) {
937-
if (func !== null) {
938-
func(prop, resolved);
939-
} else if (typeof hiddenObject[prop] !== "undefined") {
938+
if (func !== null && func(prop, resolved) === true) {
939+
// the callback returned true, which means this property was handled and we can go to the next one
940+
return;
941+
}
942+
943+
if (typeof hiddenObject[prop] !== "undefined") {
940944
resolved[prop] = hiddenObject[prop];
941945
}
942946
});
@@ -1220,8 +1224,7 @@
12201224
InitializeNumberFormat(stateObject, arguments[0], arguments[1]);
12211225

12221226
const n = Internal.ToNumber(this);
1223-
// Need to special case the "-0" case to format as 0 instead of -0.
1224-
return platform.formatNumber(n === -0 ? 0 : n, stateObject, /* toParts */ false, /* forNumberPrototypeToLocaleString */ true);
1227+
return platform.formatNumber(n, stateObject, /* toParts */ false, /* forNumberPrototypeToLocaleString */ true);
12251228
}), IntlBuiltInFunctionID.NumberToLocaleString);
12261229

12271230
if (InitType === "Number") {
@@ -1266,8 +1269,7 @@
12661269
platform.raiseNeedObjectOfType("NumberFormat.prototype.format", "NumberFormat");
12671270
}
12681271

1269-
// Need to special case the '-0' case to format as 0 instead of -0.
1270-
return platform.formatNumber(n === -0 ? 0 : n, hiddenObject, /* toParts */ false, /* forNumberPrototypeToLocaleString */ false);
1272+
return platform.formatNumber(n, hiddenObject, /* toParts */ false, /* forNumberPrototypeToLocaleString */ false);
12711273
});
12721274

12731275
const formatToParts = tagPublicFunction("Intl.NumberFormat.prototype.formatToParts", function formatToParts(n) {
@@ -1282,8 +1284,7 @@
12821284
platform.raiseNeedObjectOfType("NumberFormat.prototype.formatToParts", "NumberFormat");
12831285
}
12841286

1285-
// Need to special case the '-0' case to format as 0 instead of -0.
1286-
return platform.formatNumber(n === -0 ? 0 : n, hiddenObject, /* toParts */ true, /* forNumberPrototypeToLocaleString */ false);
1287+
return platform.formatNumber(n, hiddenObject, /* toParts */ true, /* forNumberPrototypeToLocaleString */ false);
12871288
});
12881289

12891290
// See explanation of `getCanonicalLocales`
@@ -1933,8 +1934,8 @@
19331934
resolved.hourCycle = hc;
19341935
resolved.hour12 = hc === "h11" || hc === "h12";
19351936
}
1936-
} else if (hiddenObject[prop] !== undefined && hiddenObject[prop] !== null) {
1937-
resolved[prop] = hiddenObject[prop];
1937+
1938+
return true;
19381939
}
19391940
});
19401941
},
@@ -2096,8 +2097,7 @@
20962097
if (prop === "pluralCategories") {
20972098
// https://github.com/tc39/ecma402/issues/224: create a copy of the pluralCategories array
20982099
resolved.pluralCategories = _.slice(pr.pluralCategories, 0);
2099-
} else {
2100-
resolved[prop] = pr[prop];
2100+
return true;
21012101
}
21022102
});
21032103
};
@@ -3181,8 +3181,7 @@
31813181
InitializeNumberFormat(stateObject, arguments[0], arguments[1]);
31823182

31833183
var n = Internal.ToNumber(this);
3184-
// Need to special case the '-0' case to format as 0 instead of -0.
3185-
return String(platform.formatNumber(n === -0 ? 0 : n, stateObject));
3184+
return String(platform.formatNumber(n, stateObject));
31863185
}), IntlBuiltInFunctionID.NumberToLocaleString);
31873186

31883187
if (InitType === 'Intl') {
@@ -3226,8 +3225,7 @@
32263225
platform.raiseNeedObjectOfType("NumberFormat.prototype.format", "NumberFormat");
32273226
}
32283227

3229-
// Need to special case the '-0' case to format as 0 instead of -0.
3230-
return String(platform.formatNumber(n === -0 ? 0 : n, hiddenObject));
3228+
return String(platform.formatNumber(n, hiddenObject));
32313229
}
32323230
tagPublicFunction("Intl.NumberFormat.prototype.format", format);
32333231

test/Intl/NumberFormat.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,19 @@ const tests = [
155155
}
156156
},
157157
{
158-
name: "Negative 0",
158+
name: "Negative 0 (https://github.com/tc39/ecma402/issues/219)",
159159
body() {
160160
assert.areEqual(
161161
0,
162162
new Intl.NumberFormat(undefined, { minimumFractionDigits: -0 }).resolvedOptions().minimumFractionDigits,
163163
"Passing -0 for minimumFractionDigits should get normalized to 0 by DefaultNumberOption"
164164
);
165-
// This assert may need to change in the future, pending any decision made on https://github.com/tc39/ecma402/issues/219
166-
assert.areEqual("0", (-0).toLocaleString(), "-0 should be formatted as 0");
165+
166+
assert.areEqual("-0", (-0).toLocaleString(), "-0 should be treated as a negative number (toLocaleString)");
167+
assert.areEqual("-0", new Intl.NumberFormat().format(-0), "-0 should be treated as a negative number (NumberFormat.prototype.format)");
168+
if (WScript.Platform.INTL_LIBRARY === "icu") {
169+
assert.areEqual("-0", new Intl.NumberFormat().formatToParts(-0).map(v => v.value).join(""), "-0 should be treated as a negative number (NumberFormat.prototype.formatToParts)");
170+
}
167171
}
168172
},
169173
{

test/Intl/PluralRules.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,26 @@ testRunner.runTests([
115115
body() {
116116
function test(options, n, expected) {
117117
const pr = new Intl.PluralRules("en", options);
118+
const res = pr.resolvedOptions();
118119
assert.areEqual(expected, pr.select(n), `Incorrect result using n = ${n} and options = ${JSON.stringify(options)}`);
120+
121+
// We should only report sigfigs in the resolved options if they were asked for https://github.com/tc39/ecma402/issues/244
122+
if (options.minimumSignificantDigits !== undefined || options.maximumSignificantDigits !== undefined) {
123+
if (options.minimumSignificantDigits !== undefined) {
124+
assert.areEqual(options.minimumSignificantDigits, res.minimumSignificantDigits, "Incorrect minimumSignificantDigits");
125+
}
126+
if (options.maximumSignificantDigits !== undefined) {
127+
assert.areEqual(options.maximumSignificantDigits, res.maximumSignificantDigits, "Incorrect maximumSignificantDigits");
128+
}
129+
} else {
130+
assert.isFalse(res.hasOwnProperty("minimumSignificantDigits"), "Reported minimumSignificantDigits when it shouldn't have been");
131+
assert.isFalse(res.hasOwnProperty("maximumSignificantDigits"), "Reported maximumSignificantDigits when it shouldn't have been");
132+
}
119133
}
120134

121-
test(undefined, 1.0, "one");
122-
test(undefined, 1.1, "other");
123-
test(undefined, 1.001, "other");
135+
test({}, 1.0, "one");
136+
test({}, 1.1, "other");
137+
test({}, 1.001, "other");
124138

125139
test({ minimumFractionDigits: 1 }, 1.0, "one");
126140
test({ minimumFractionDigits: 1 }, 1.1, "other");
@@ -145,6 +159,9 @@ testRunner.runTests([
145159
test({ maximumSignificantDigits: 1 }, 1.0, "one");
146160
test({ maximumSignificantDigits: 1 }, 1.1, "one");
147161
test({ maximumSignificantDigits: 1 }, 1.001, "one");
162+
163+
// significantDigits should override fractionDigits and integerDigits
164+
test({ maximumSignificantDigits: 2, maximumFractionDigits: 0 }, 1.1, "other");
148165
}
149166
},
150167
], { verbose: false });

0 commit comments

Comments
 (0)