From e13571c199e5e5b9b44e08b03638e3a7fdc86e4a Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Wed, 17 Oct 2018 09:45:11 +0200
Subject: [PATCH] util,console: handle symbols as defined in the spec
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/pull/23708
Refs: https://console.spec.whatwg.org/#formatter
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
---
 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 27c88547f3e720..98becbc9258706 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/REPLACEME
     description: The `%o` specifier's `depth` has default depth of 4 again.
diff --git a/lib/util.js b/lib/util.js
index 07bde61a118067..1e11440ff3fd42 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -106,6 +106,8 @@ function formatWithOptions(inspectOptions, f) {
             // eslint-disable-next-line valid-typeof
             if (typeof tempNum === 'bigint') {
               tempStr = `${tempNum}n`;
+            } else if (typeof tempNum === 'symbol') {
+              tempStr = 'NaN';
             } else {
               tempStr = `${Number(tempNum)}`;
             }
@@ -128,12 +130,19 @@ function formatWithOptions(inspectOptions, f) {
             // 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(arguments[a++])}`;
+            const tempFloat = arguments[a++];
+            if (typeof tempFloat === 'symbol') {
+              tempStr = 'NaN';
+            } else {
+              tempStr = `${parseFloat(tempFloat)}`;
+            }
             break;
           case 37: // '%'
             str += f.slice(lastPos, i);
diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js
index 0c4ba82fec8359..480aba8d56b580 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');