Skip to content

Commit aeae459

Browse files
BridgeARrefack
authored andcommitted
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: nodejs#23708 Refs: https://console.spec.whatwg.org/#formatter Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent dd6cd9c commit aeae459

File tree

3 files changed

+18
-13
lines changed

3 files changed

+18
-13
lines changed

Diff for: doc/api/util.md

+4
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,10 @@ property take precedence over `--trace-deprecation` and
183183
<!-- YAML
184184
added: v0.5.3
185185
changes:
186+
- version: REPLACEME
187+
pr-url: https://github.com/nodejs/node/pull/23708
188+
description: The `%d`, `%f` and `%i` specifiers now support Symbols
189+
properly.
186190
- version: REPLACEME
187191
pr-url: https://github.com/nodejs/node/pull/23162
188192
description: The `format` argument is now only taken as such if it actually

Diff for: lib/util.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ function formatWithOptions(inspectOptions, ...args) {
114114
// eslint-disable-next-line valid-typeof
115115
if (typeof tempNum === 'bigint') {
116116
tempStr = `${tempNum}n`;
117+
} else if (typeof tempNum === 'symbol') {
118+
tempStr = 'NaN';
117119
} else {
118120
tempStr = `${Number(tempNum)}`;
119121
}
@@ -136,12 +138,19 @@ function formatWithOptions(inspectOptions, ...args) {
136138
// eslint-disable-next-line valid-typeof
137139
if (typeof tempInteger === 'bigint') {
138140
tempStr = `${tempInteger}n`;
141+
} else if (typeof tempInteger === 'symbol') {
142+
tempStr = 'NaN';
139143
} else {
140144
tempStr = `${parseInt(tempInteger)}`;
141145
}
142146
break;
143147
case 102: // 'f'
144-
tempStr = `${parseFloat(args[a++])}`;
148+
const tempFloat = args[a++];
149+
if (typeof tempFloat === 'symbol') {
150+
tempStr = 'NaN';
151+
} else {
152+
tempStr = `${parseFloat(tempFloat)}`;
153+
}
145154
break;
146155
case 37: // '%'
147156
str += first.slice(lastPos, i);

Diff for: test/parallel/test-util-format.js

+4-12
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,6 @@ assert.strictEqual(util.format(symbol), 'Symbol(foo)');
4444
assert.strictEqual(util.format('foo', symbol), 'foo Symbol(foo)');
4545
assert.strictEqual(util.format('%s', symbol), 'Symbol(foo)');
4646
assert.strictEqual(util.format('%j', symbol), 'undefined');
47-
assert.throws(
48-
() => { util.format('%d', symbol); },
49-
(e) => {
50-
// The error should be a TypeError.
51-
if (!(e instanceof TypeError))
52-
return false;
53-
54-
// The error should be from the JS engine and not from Node.js.
55-
// JS engine errors do not have the `code` property.
56-
return e.code === undefined;
57-
}
58-
);
5947

6048
// Number format specifier
6149
assert.strictEqual(util.format('%d'), '%d');
@@ -66,6 +54,7 @@ assert.strictEqual(util.format('%d', '42.0'), '42');
6654
assert.strictEqual(util.format('%d', 1.5), '1.5');
6755
assert.strictEqual(util.format('%d', -0.5), '-0.5');
6856
assert.strictEqual(util.format('%d', ''), '0');
57+
assert.strictEqual(util.format('%d', Symbol()), 'NaN');
6958
assert.strictEqual(util.format('%d %d', 42, 43), '42 43');
7059
assert.strictEqual(util.format('%d %d', 42), '42 %d');
7160
assert.strictEqual(
@@ -90,6 +79,7 @@ assert.strictEqual(util.format('%i', '42.0'), '42');
9079
assert.strictEqual(util.format('%i', 1.5), '1');
9180
assert.strictEqual(util.format('%i', -0.5), '0');
9281
assert.strictEqual(util.format('%i', ''), 'NaN');
82+
assert.strictEqual(util.format('%i', Symbol()), 'NaN');
9383
assert.strictEqual(util.format('%i %i', 42, 43), '42 43');
9484
assert.strictEqual(util.format('%i %i', 42), '42 %i');
9585
assert.strictEqual(
@@ -125,6 +115,8 @@ assert.strictEqual(util.format('%f', 1.5), '1.5');
125115
assert.strictEqual(util.format('%f', -0.5), '-0.5');
126116
assert.strictEqual(util.format('%f', Math.PI), '3.141592653589793');
127117
assert.strictEqual(util.format('%f', ''), 'NaN');
118+
assert.strictEqual(util.format('%f', Symbol('foo')), 'NaN');
119+
assert.strictEqual(util.format('%f', 5n), '5');
128120
assert.strictEqual(util.format('%f %f', 42, 43), '42 43');
129121
assert.strictEqual(util.format('%f %f', 42), '42 %f');
130122

0 commit comments

Comments
 (0)