util: treat format arguments equally

Two changes here which bring us closer to the console standard:

- Arguments to `util.format` are no longer formatted differently
  depending on their order, with format strings being an exception.
- Format specifier formatting is now only triggered if the string
  actually contains a format string.

Under the hood, we now use a single shared function to format the given
arguments which will make the code easier to read and modify.

PR-URL: https://github.com/nodejs/node/pull/23162
Fixes: https://github.com/nodejs/node/issues/23137
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>

1 files changed, 98 insertions, 83 deletions

diff --git a/lib/util.js b/lib/util.js
index 2e42beb58b..22c2b260da 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -72,98 +72,113 @@ function format(...args) {
   return formatWithOptions(emptyOptions, ...args);
 }
 
-function formatWithOptions(inspectOptions, f) {
-  let i, tempStr;
-  if (typeof f !== 'string') {
-    if (arguments.length === 1) return '';
-    let res = '';
-    for (i = 1; i < arguments.length - 1; i++) {
-      res += inspect(arguments[i], inspectOptions);
-      res += ' ';
-    }
-    res += inspect(arguments[i], inspectOptions);
-    return res;
+function formatValue(val, inspectOptions) {
+  const inspectTypes = ['object', 'symbol', 'function', 'number'];
+
+  if (inspectTypes.includes(typeof val)) {
+    return inspect(val, inspectOptions);
+  } else {
+    return String(val);
+  }
+}
+
+function formatWithOptions(inspectOptions, ...args) {
+  const first = args[0];
+  const parts = [];
+
+  const firstIsString = typeof first === 'string';
+
+  if (firstIsString && args.length === 1) {
+    return first;
   }
 
-  if (arguments.length === 2) return f;
-
-  let str = '';
-  let a = 2;
-  let lastPos = 0;
-  for (i = 0; i < f.length - 1; i++) {
-    if (f.charCodeAt(i) === 37) { // '%'
-      const nextChar = f.charCodeAt(++i);
-      if (a !== arguments.length) {
-        switch (nextChar) {
-          case 115: // 's'
-            tempStr = String(arguments[a++]);
-            break;
-          case 106: // 'j'
-            tempStr = tryStringify(arguments[a++]);
-            break;
-          case 100: // 'd'
-            const tempNum = arguments[a++];
-            // eslint-disable-next-line valid-typeof
-            if (typeof tempNum === 'bigint') {
-              tempStr = `${tempNum}n`;
-            } else {
-              tempStr = `${Number(tempNum)}`;
+  if (firstIsString && /%[sjdOoif%]/.test(first)) {
+    let i, tempStr;
+    let str = '';
+    let a = 1;
+    let lastPos = 0;
+
+    for (i = 0; i < first.length - 1; i++) {
+      if (first.charCodeAt(i) === 37) { // '%'
+        const nextChar = first.charCodeAt(++i);
+        if (a !== args.length) {
+          switch (nextChar) {
+            case 115: // 's'
+              tempStr = String(args[a++]);
+              break;
+            case 106: // 'j'
+              tempStr = tryStringify(args[a++]);
+              break;
+            case 100: // 'd'
+              const tempNum = args[a++];
+              // eslint-disable-next-line valid-typeof
+              if (typeof tempNum === 'bigint') {
+                tempStr = `${tempNum}n`;
+              } else {
+                tempStr = `${Number(tempNum)}`;
+              }
+              break;
+            case 79: // 'O'
+              tempStr = inspect(args[a++], inspectOptions);
+              break;
+            case 111: // 'o'
+            {
+              const opts = Object.assign({}, inspectOptions, {
+                showHidden: true,
+                showProxy: true,
+                depth: 4
+              });
+              tempStr = inspect(args[a++], opts);
+              break;
             }
-            break;
-          case 79: // 'O'
-            tempStr = inspect(arguments[a++], inspectOptions);
-            break;
-          case 111: // 'o'
-          {
-            const opts = Object.assign({}, inspectOptions, {
-              showHidden: true,
-              showProxy: true
-            });
-            tempStr = inspect(arguments[a++], opts);
-            break;
+            case 105: // 'i'
+              const tempInteger = args[a++];
+              // eslint-disable-next-line valid-typeof
+              if (typeof tempInteger === 'bigint') {
+                tempStr = `${tempInteger}n`;
+              } else {
+                tempStr = `${parseInt(tempInteger)}`;
+              }
+              break;
+            case 102: // 'f'
+              tempStr = `${parseFloat(args[a++])}`;
+              break;
+            case 37: // '%'
+              str += first.slice(lastPos, i);
+              lastPos = i + 1;
+              continue;
+            default: // any other character is not a correct placeholder
+              continue;
           }
-          case 105: // 'i'
-            const tempInteger = arguments[a++];
-            // eslint-disable-next-line valid-typeof
-            if (typeof tempInteger === 'bigint') {
-              tempStr = `${tempInteger}n`;
-            } else {
-              tempStr = `${parseInt(tempInteger)}`;
-            }
-            break;
-          case 102: // 'f'
-            tempStr = `${parseFloat(arguments[a++])}`;
-            break;
-          case 37: // '%'
-            str += f.slice(lastPos, i);
-            lastPos = i + 1;
-            continue;
-          default: // any other character is not a correct placeholder
-            continue;
+          if (lastPos !== i - 1) {
+            str += first.slice(lastPos, i - 1);
+          }
+          str += tempStr;
+          lastPos = i + 1;
+        } else if (nextChar === 37) {
+          str += first.slice(lastPos, i);
+          lastPos = i + 1;
         }
-        if (lastPos !== i - 1)
-          str += f.slice(lastPos, i - 1);
-        str += tempStr;
-        lastPos = i + 1;
-      } else if (nextChar === 37) {
-        str += f.slice(lastPos, i);
-        lastPos = i + 1;
       }
     }
-  }
-  if (lastPos === 0)
-    str = f;
-  else if (lastPos < f.length)
-    str += f.slice(lastPos);
-  while (a < arguments.length) {
-    const x = arguments[a++];
-    if ((typeof x !== 'object' && typeof x !== 'symbol') || x === null) {
-      str += ` ${x}`;
-    } else {
-      str += ` ${inspect(x, inspectOptions)}`;
+    if (lastPos === 0) {
+      str = first;
+    } else if (lastPos < first.length) {
+      str += first.slice(lastPos);
+    }
+
+    parts.push(str);
+    while (a < args.length) {
+      parts.push(formatValue(args[a], inspectOptions));
+      a++;
+    }
+  } else {
+    for (const arg of args) {
+      parts.push(formatValue(arg, inspectOptions));
     }
   }
-  return str;
+
+  return parts.join(' ');
 }
 
 const debugs = {};