From baae49c61b3a93d529842ba61447427297a145db Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater <ruben@bridgewater.de> Date: Wed, 17 Oct 2018 09:45:11 +0200 Subject: [PATCH 1/2] 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. --- doc/api/util.md | 4 ++++ lib/util.js | 11 ++++++++++- test/parallel/test-util-format.js | 16 ++++------------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 8b2e7d2e41141e..9c4c6cca40d784 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -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 diff --git a/lib/util.js b/lib/util.js index 22c2b260daf319..63b34f48bfa896 100644 --- a/lib/util.js +++ b/lib/util.js @@ -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); diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js index 2ca8e0857f3aad..189343c6e19713 100644 --- a/test/parallel/test-util-format.js +++ b/test/parallel/test-util-format.js @@ -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'); From dcdf5c28f09887ef8ff59e8e5075713f9ac72ec1 Mon Sep 17 00:00:00 2001 From: Richard Lau <riclau@uk.ibm.com> Date: Sun, 2 Dec 2018 15:34:27 +0100 Subject: [PATCH 2/2] fixup: fix typo Co-Authored-By: BridgeAR <ruben@bridgewater.de> --- test/parallel/test-util-format.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js index 189343c6e19713..066b4de58c1399 100644 --- a/test/parallel/test-util-format.js +++ b/test/parallel/test-util-format.js @@ -116,7 +116,7 @@ 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', 5n), '5'); assert.strictEqual(util.format('%f %f', 42, 43), '42 43'); assert.strictEqual(util.format('%f %f', 42), '42 %f');