util,console: handle symbols as defined in the spec

The `console` functions rely on the `util.format()` behavior. It
did not follow the whatwg spec when it comes to symbols in combination
with the %d, %i and %f format specifiers. Using a symbol argument in
combination with one of these specifiers resulted in an error instead
of returning `'NaN'`. This is now fixed by this patch.

PR-URL: https://github.com/nodejs/node/pull/23708
Refs: https://console.spec.whatwg.org/#formatter
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This commit is contained in:
Ruben Bridgewater 2018-10-17 09:45:11 +02:00
parent f8f96017e8
commit 1fe824bcbb
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
3 changed files with 18 additions and 13 deletions

View File

@ -183,6 +183,10 @@ property take precedence over `--trace-deprecation` and
<!-- YAML <!-- YAML
added: v0.5.3 added: v0.5.3
changes: changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23708
description: The `%d`, `%f` and `%i` specifiers now support Symbols
properly.
- version: REPLACEME - version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23162 pr-url: https://github.com/nodejs/node/pull/23162
description: The `format` argument is now only taken as such if it actually description: The `format` argument is now only taken as such if it actually

View File

@ -114,6 +114,8 @@ function formatWithOptions(inspectOptions, ...args) {
// eslint-disable-next-line valid-typeof // eslint-disable-next-line valid-typeof
if (typeof tempNum === 'bigint') { if (typeof tempNum === 'bigint') {
tempStr = `${tempNum}n`; tempStr = `${tempNum}n`;
} else if (typeof tempNum === 'symbol') {
tempStr = 'NaN';
} else { } else {
tempStr = `${Number(tempNum)}`; tempStr = `${Number(tempNum)}`;
} }
@ -136,12 +138,19 @@ function formatWithOptions(inspectOptions, ...args) {
// eslint-disable-next-line valid-typeof // eslint-disable-next-line valid-typeof
if (typeof tempInteger === 'bigint') { if (typeof tempInteger === 'bigint') {
tempStr = `${tempInteger}n`; tempStr = `${tempInteger}n`;
} else if (typeof tempInteger === 'symbol') {
tempStr = 'NaN';
} else { } else {
tempStr = `${parseInt(tempInteger)}`; tempStr = `${parseInt(tempInteger)}`;
} }
break; break;
case 102: // 'f' case 102: // 'f'
tempStr = `${parseFloat(args[a++])}`; const tempFloat = args[a++];
if (typeof tempFloat === 'symbol') {
tempStr = 'NaN';
} else {
tempStr = `${parseFloat(tempFloat)}`;
}
break; break;
case 37: // '%' case 37: // '%'
str += first.slice(lastPos, i); str += first.slice(lastPos, i);

View File

@ -44,18 +44,6 @@ assert.strictEqual(util.format(symbol), 'Symbol(foo)');
assert.strictEqual(util.format('foo', symbol), 'foo Symbol(foo)'); assert.strictEqual(util.format('foo', symbol), 'foo Symbol(foo)');
assert.strictEqual(util.format('%s', symbol), 'Symbol(foo)'); assert.strictEqual(util.format('%s', symbol), 'Symbol(foo)');
assert.strictEqual(util.format('%j', symbol), 'undefined'); assert.strictEqual(util.format('%j', symbol), 'undefined');
assert.throws(
() => { util.format('%d', symbol); },
(e) => {
// The error should be a TypeError.
if (!(e instanceof TypeError))
return false;
// The error should be from the JS engine and not from Node.js.
// JS engine errors do not have the `code` property.
return e.code === undefined;
}
);
// Number format specifier // Number format specifier
assert.strictEqual(util.format('%d'), '%d'); assert.strictEqual(util.format('%d'), '%d');
@ -66,6 +54,7 @@ assert.strictEqual(util.format('%d', '42.0'), '42');
assert.strictEqual(util.format('%d', 1.5), '1.5'); assert.strictEqual(util.format('%d', 1.5), '1.5');
assert.strictEqual(util.format('%d', -0.5), '-0.5'); assert.strictEqual(util.format('%d', -0.5), '-0.5');
assert.strictEqual(util.format('%d', ''), '0'); assert.strictEqual(util.format('%d', ''), '0');
assert.strictEqual(util.format('%d', Symbol()), 'NaN');
assert.strictEqual(util.format('%d %d', 42, 43), '42 43'); assert.strictEqual(util.format('%d %d', 42, 43), '42 43');
assert.strictEqual(util.format('%d %d', 42), '42 %d'); assert.strictEqual(util.format('%d %d', 42), '42 %d');
assert.strictEqual( assert.strictEqual(
@ -90,6 +79,7 @@ assert.strictEqual(util.format('%i', '42.0'), '42');
assert.strictEqual(util.format('%i', 1.5), '1'); assert.strictEqual(util.format('%i', 1.5), '1');
assert.strictEqual(util.format('%i', -0.5), '0'); assert.strictEqual(util.format('%i', -0.5), '0');
assert.strictEqual(util.format('%i', ''), 'NaN'); assert.strictEqual(util.format('%i', ''), 'NaN');
assert.strictEqual(util.format('%i', Symbol()), 'NaN');
assert.strictEqual(util.format('%i %i', 42, 43), '42 43'); assert.strictEqual(util.format('%i %i', 42, 43), '42 43');
assert.strictEqual(util.format('%i %i', 42), '42 %i'); assert.strictEqual(util.format('%i %i', 42), '42 %i');
assert.strictEqual( assert.strictEqual(
@ -125,6 +115,8 @@ assert.strictEqual(util.format('%f', 1.5), '1.5');
assert.strictEqual(util.format('%f', -0.5), '-0.5'); assert.strictEqual(util.format('%f', -0.5), '-0.5');
assert.strictEqual(util.format('%f', Math.PI), '3.141592653589793'); assert.strictEqual(util.format('%f', Math.PI), '3.141592653589793');
assert.strictEqual(util.format('%f', ''), 'NaN'); assert.strictEqual(util.format('%f', ''), 'NaN');
assert.strictEqual(util.format('%f', Symbol('foo')), 'NaN');
assert.strictEqual(util.format('%f', 5n), '5');
assert.strictEqual(util.format('%f %f', 42, 43), '42 43'); assert.strictEqual(util.format('%f %f', 42, 43), '42 43');
assert.strictEqual(util.format('%f %f', 42), '42 %f'); assert.strictEqual(util.format('%f %f', 42), '42 %f');