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: #27499 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · May 4, 2019 · caab7d4 · caab7d4
1 parent 99e4a57
commit caab7d4
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 11 deletions.
diff --git 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,15 +1555,15 @@ function formatWithOptions(inspectOptions, ...args) {
               } else if (typeof tempInteger === 'symbol') {
                 tempStr = 'NaN';
               } else {
-                tempStr = `${parseInt(tempInteger)}`;
+                tempStr = formatNumber(stylizeNoColor, parseInt(tempInteger));
               }
               break;
             case 102: // 'f'
               const tempFloat = args[++a];
               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
@@ -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');