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
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/api/util.md
Expand Up @@ -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
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
pr-url: https://github.com/nodejs/node/pull/23162
description: The `format` argument is now only taken as such if it actually
Expand Down
11 changes: 10 additions & 1 deletion lib/util.js
Expand Up @@ -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)}`;
}
Expand All @@ -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);
Expand Down
16 changes: 4 additions & 12 deletions test/parallel/test-util-format.js
Expand Up @@ -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');
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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'));
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
assert.strictEqual(util.format('%f %f', 42, 43), '42 43');
assert.strictEqual(util.format('%f %f', 42), '42 %f');

Expand Down