From caab7d4664ca8825c3332d9683d732c1500b19fd Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 30 Apr 2019 19:04:55 +0200 Subject: [PATCH] util: better number formatters This makes sure the `%d`, `%f`, `%i` and `%s` formatters properly visualize `-0`. On top, this also switches to using a safer symbol toString function by using the primordial function. PR-URL: https://github.com/nodejs/node/pull/27499 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott --- lib/internal/util/inspect.js | 17 +++++++---------- test/parallel/test-util-format.js | 7 ++++++- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index fccb46085d4ead..5a16c0fa99f558 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -1055,7 +1055,7 @@ function formatPrimitive(fn, value, ctx) { if (typeof value === 'undefined') return fn('undefined', 'undefined'); // es6 symbol primitive - return fn(value.toString(), 'symbol'); + return fn(SymbolPrototype.toString(value), 'symbol'); } function formatNamespaceObject(ctx, value, recurseTimes, keys) { @@ -1461,9 +1461,8 @@ function reduceToSingleString( return `${braces[0]}${ln}${join(output, `,\n${indentation} `)} ${braces[1]}`; } -const emptyOptions = {}; function format(...args) { - return formatWithOptions(emptyOptions, ...args); + return formatWithOptions(undefined, ...args); } @@ -1509,16 +1508,14 @@ function formatWithOptions(inspectOptions, ...args) { switch (nextChar) { case 115: // 's' const tempArg = args[++a]; - if (typeof tempArg === 'object' && tempArg !== null) { + if (typeof tempArg !== 'string' && + typeof tempArg !== 'function') { tempStr = inspect(tempArg, { ...inspectOptions, compact: 3, colors: false, depth: 0 }); - // eslint-disable-next-line valid-typeof - } else if (typeof tempArg === 'bigint') { - tempStr = `${tempArg}n`; } else { tempStr = String(tempArg); } @@ -1534,7 +1531,7 @@ function formatWithOptions(inspectOptions, ...args) { } else if (typeof tempNum === 'symbol') { tempStr = 'NaN'; } else { - tempStr = `${Number(tempNum)}`; + tempStr = formatNumber(stylizeNoColor, Number(tempNum)); } break; case 79: // 'O' @@ -1558,7 +1555,7 @@ function formatWithOptions(inspectOptions, ...args) { } else if (typeof tempInteger === 'symbol') { tempStr = 'NaN'; } else { - tempStr = `${parseInt(tempInteger)}`; + tempStr = formatNumber(stylizeNoColor, parseInt(tempInteger)); } break; case 102: // 'f' @@ -1566,7 +1563,7 @@ function formatWithOptions(inspectOptions, ...args) { if (typeof tempFloat === 'symbol') { tempStr = 'NaN'; } else { - tempStr = `${parseFloat(tempFloat)}`; + tempStr = formatNumber(stylizeNoColor, parseFloat(tempFloat)); } break; case 37: // '%' diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js index 206b66e17c3be0..6e5c50ab406b83 100644 --- a/test/parallel/test-util-format.js +++ b/test/parallel/test-util-format.js @@ -53,7 +53,9 @@ assert.strictEqual(util.format('%d', '42'), '42'); 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.0), '-0'); assert.strictEqual(util.format('%d', ''), '0'); +assert.strictEqual(util.format('%d', ' -0.000'), '-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'); @@ -77,7 +79,7 @@ assert.strictEqual(util.format('%i', 42), '42'); assert.strictEqual(util.format('%i', '42'), '42'); 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', -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'); @@ -110,6 +112,7 @@ assert.strictEqual(util.format('%f'), '%f'); assert.strictEqual(util.format('%f', 42.0), '42'); assert.strictEqual(util.format('%f', 42), '42'); assert.strictEqual(util.format('%f', '42'), '42'); +assert.strictEqual(util.format('%f', '-0.0'), '-0'); assert.strictEqual(util.format('%f', '42.0'), '42'); assert.strictEqual(util.format('%f', 1.5), '1.5'); assert.strictEqual(util.format('%f', -0.5), '-0.5'); @@ -127,6 +130,8 @@ assert.strictEqual(util.format('%s', null), 'null'); assert.strictEqual(util.format('%s', 'foo'), 'foo'); assert.strictEqual(util.format('%s', 42), '42'); assert.strictEqual(util.format('%s', '42'), '42'); +assert.strictEqual(util.format('%s', -0), '-0'); +assert.strictEqual(util.format('%s', '-0.0'), '-0.0'); assert.strictEqual(util.format('%s %s', 42, 43), '42 43'); assert.strictEqual(util.format('%s %s', 42), '42 %s'); assert.strictEqual(util.format('%s', 42n), '42n');