util: do not escape single quotes if not necessary

Right now util.inspect will always escape single quotes. That is not
necessary though in case the string that will be escaped does not
contain double quotes. In that case the string can simply be wrapped
in double quotes instead.
If the string contains single and double quotes and it does not
contain `${` as part of the string, backticks will be used instead.
That makes sure only very few strings have to escape quotes at all.
Thus it increases the readability of these strings.

PR-URL: https://github.com/nodejs/node/pull/21624
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

lib/util.js

@@ -90,6 +90,9 @@ let internalDeepEqual;
 /* eslint-disable no-control-regex */
 const strEscapeSequencesRegExp = /[\x00-\x1f\x27\x5c]/;
 const strEscapeSequencesReplacer = /[\x00-\x1f\x27\x5c]/g;
+const strEscapeSequencesRegExpSingle = /[\x00-\x1f\x5c]/;
+const strEscapeSequencesReplacerSingle = /[\x00-\x1f\x5c]/g;
+
 /* eslint-enable no-control-regex */
 
 const keyStrRegExp = /^[a-zA-Z_][a-zA-Z_0-9]*$/;
@@ -116,21 +119,56 @@ const meta = [
   '', '', '', '', '', '', '', '\\\\'
 ];
 
+function addQuotes(str, quotes) {
+  if (quotes === -1) {
+    return `"${str}"`;
+  }
+  if (quotes === -2) {
+    return `\`${str}\``;
+  }
+  return `'${str}'`;
+}
+
 const escapeFn = (str) => meta[str.charCodeAt(0)];
 
 // Escape control characters, single quotes and the backslash.
 // This is similar to JSON stringify escaping.
 function strEscape(str) {
+  let escapeTest = strEscapeSequencesRegExp;
+  let escapeReplace = strEscapeSequencesReplacer;
+  let singleQuote = 39;
+
+  // Check for double quotes. If not present, do not escape single quotes and
+  // instead wrap the text in double quotes. If double quotes exist, check for
+  // backticks. If they do not exist, use those as fallback instead of the
+  // double quotes.
+  if (str.indexOf("'") !== -1) {
+    // This invalidates the charCode and therefore can not be matched for
+    // anymore.
+    if (str.indexOf('"') === -1) {
+      singleQuote = -1;
+    } else if (str.indexOf('`') === -1 && str.indexOf('${') === -1) {
+      singleQuote = -2;
+    }
+    if (singleQuote !== 39) {
+      escapeTest = strEscapeSequencesRegExpSingle;
+      escapeReplace = strEscapeSequencesReplacerSingle;
+    }
+  }
+
   // Some magic numbers that worked out fine while benchmarking with v8 6.0
-  if (str.length < 5000 && !strEscapeSequencesRegExp.test(str))
-    return `'${str}'`;
-  if (str.length > 100)
-    return `'${str.replace(strEscapeSequencesReplacer, escapeFn)}'`;
+  if (str.length < 5000 && !escapeTest.test(str))
+    return addQuotes(str, singleQuote);
+  if (str.length > 100) {
+    str = str.replace(escapeReplace, escapeFn);
+    return addQuotes(str, singleQuote);
+  }
+
   let result = '';
   let last = 0;
   for (var i = 0; i < str.length; i++) {
     const point = str.charCodeAt(i);
-    if (point === 39 || point === 92 || point < 32) {
+    if (point === singleQuote || point === 92 || point < 32) {
       if (last === i) {
         result += meta[point];
       } else {
@@ -144,7 +182,7 @@ function strEscape(str) {
   } else if (last !== i) {
     result += str.slice(last);
   }
-  return `'${result}'`;
+  return addQuotes(result, singleQuote);
 }
 
 function tryStringify(arg) {

test/parallel/test-repl-colors.js

@@ -24,7 +24,7 @@ process.on('exit', function() {
   // https://github.com/nodejs/node/pull/16485#issuecomment-350428638
   // The color setting of the REPL should not have leaked over into
   // the color setting of `util.inspect.defaultOptions`.
-  strictEqual(output.includes(`'\\'string\\''`), true);
+  strictEqual(output.includes(`"'string'"`), true);
   strictEqual(output.includes(`'\u001b[32m\\'string\\'\u001b[39m'`), false);
   strictEqual(inspect.defaultOptions.colors, false);
   strictEqual(repl.writer.options.colors, true);

test/parallel/test-util-inspect.js

@@ -47,7 +47,7 @@ assert.strictEqual(util.inspect(new Date('')), (new Date('')).toString());
 assert.strictEqual(util.inspect('\n\u0001'), "'\\n\\u0001'");
 assert.strictEqual(
   util.inspect(`${Array(75).fill(1)}'\n\u001d\n\u0003`),
-  `'${Array(75).fill(1)}\\'\\n\\u001d\\n\\u0003'`
+  `"${Array(75).fill(1)}'\\n\\u001d\\n\\u0003"`
 );
 assert.strictEqual(util.inspect([]), '[]');
 assert.strictEqual(util.inspect(Object.create([])), 'Array {}');
@@ -1424,3 +1424,9 @@ util.inspect(process);
   assert(longList.includes('[Object: Inspection interrupted ' +
     'prematurely. Maximum call stack size exceeded.]'));
 }
+
+// Do not escape single quotes if no double quote or backtick is present.
+assert.strictEqual(util.inspect("'"), '"\'"');
+assert.strictEqual(util.inspect('"\''), '`"\'`');
+// eslint-disable-next-line no-template-curly-in-string
+assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");