Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util,console: more spec compliant util format #23708

Closed
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
BridgeAR committed Nov 30, 2018

Verified

This commit was signed with the committer’s verified signature. The key has expired.
addaleax Anna Henningsen
commit baae49c61b3a93d529842ba61447427297a145db
4 changes: 4 additions & 0 deletions doc/api/util.md
Original file line number Diff line number Diff line change
@@ -183,6 +183,10 @@ property take precedence over `--trace-deprecation` and
<!-- YAML
added: v0.5.3
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
pr-url: https://github.com/nodejs/node/pull/23162
description: The `format` argument is now only taken as such if it actually
11 changes: 10 additions & 1 deletion lib/util.js
Original file line number Diff line number Diff line change
@@ -114,6 +114,8 @@ function formatWithOptions(inspectOptions, ...args) {
// eslint-disable-next-line valid-typeof
if (typeof tempNum === 'bigint') {
tempStr = `${tempNum}n`;
} else if (typeof tempNum === 'symbol') {
tempStr = 'NaN';
} else {
tempStr = `${Number(tempNum)}`;
}
@@ -136,12 +138,19 @@ function formatWithOptions(inspectOptions, ...args) {
// eslint-disable-next-line valid-typeof
if (typeof tempInteger === 'bigint') {
tempStr = `${tempInteger}n`;
} else if (typeof tempInteger === 'symbol') {
tempStr = 'NaN';
} else {
tempStr = `${parseInt(tempInteger)}`;
}
break;
case 102: // 'f'
tempStr = `${parseFloat(args[a++])}`;
const tempFloat = args[a++];
if (typeof tempFloat === 'symbol') {
tempStr = 'NaN';
} else {
tempStr = `${parseFloat(tempFloat)}`;
}
break;
case 37: // '%'
str += first.slice(lastPos, i);
16 changes: 4 additions & 12 deletions test/parallel/test-util-format.js
Original file line number Diff line number Diff line change
@@ -44,18 +44,6 @@ assert.strictEqual(util.format(symbol), 'Symbol(foo)');
assert.strictEqual(util.format('foo', symbol), 'foo Symbol(foo)');
assert.strictEqual(util.format('%s', symbol), 'Symbol(foo)');
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
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', -0.5), '-0.5');
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), '42 %d');
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', -0.5), '0');
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), '42 %i');
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', Math.PI), '3.141592653589793');
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), '42 %f');