From b3e93a91eb4e2c38c07550d61be899bd32137a0a Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Mon, 2 Jul 2018 20:52:34 +0200
Subject: [PATCH] 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                        | 50 ++++++++++++++++++++++++++----
 test/parallel/test-repl-colors.js  |  2 +-
 test/parallel/test-util-inspect.js |  8 ++++-
 3 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/lib/util.js b/lib/util.js
index 5413cc0a337abd..c56b8429f47350 100644
--- a/lib/util.js
+++ b/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) {
diff --git a/test/parallel/test-repl-colors.js b/test/parallel/test-repl-colors.js
index dfcc020d899ee6..f484f57945a16b 100644
--- a/test/parallel/test-repl-colors.js
+++ b/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);
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index f97ab95cdea524..e0dceb60f794a9 100644
--- a/test/parallel/test-util-inspect.js
+++ b/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}'");