Skip to content

Commit b3e93a9

Browse files
committed
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: #21624 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 49681e7 commit b3e93a9

File tree

3 files changed

+52
-8
lines changed

3 files changed

+52
-8
lines changed

lib/util.js

+44-6
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ let internalDeepEqual;
9090
/* eslint-disable no-control-regex */
9191
const strEscapeSequencesRegExp = /[\x00-\x1f\x27\x5c]/;
9292
const strEscapeSequencesReplacer = /[\x00-\x1f\x27\x5c]/g;
93+
const strEscapeSequencesRegExpSingle = /[\x00-\x1f\x5c]/;
94+
const strEscapeSequencesReplacerSingle = /[\x00-\x1f\x5c]/g;
95+
9396
/* eslint-enable no-control-regex */
9497

9598
const keyStrRegExp = /^[a-zA-Z_][a-zA-Z_0-9]*$/;
@@ -116,21 +119,56 @@ const meta = [
116119
'', '', '', '', '', '', '', '\\\\'
117120
];
118121

122+
function addQuotes(str, quotes) {
123+
if (quotes === -1) {
124+
return `"${str}"`;
125+
}
126+
if (quotes === -2) {
127+
return `\`${str}\``;
128+
}
129+
return `'${str}'`;
130+
}
131+
119132
const escapeFn = (str) => meta[str.charCodeAt(0)];
120133

121134
// Escape control characters, single quotes and the backslash.
122135
// This is similar to JSON stringify escaping.
123136
function strEscape(str) {
137+
let escapeTest = strEscapeSequencesRegExp;
138+
let escapeReplace = strEscapeSequencesReplacer;
139+
let singleQuote = 39;
140+
141+
// Check for double quotes. If not present, do not escape single quotes and
142+
// instead wrap the text in double quotes. If double quotes exist, check for
143+
// backticks. If they do not exist, use those as fallback instead of the
144+
// double quotes.
145+
if (str.indexOf("'") !== -1) {
146+
// This invalidates the charCode and therefore can not be matched for
147+
// anymore.
148+
if (str.indexOf('"') === -1) {
149+
singleQuote = -1;
150+
} else if (str.indexOf('`') === -1 && str.indexOf('${') === -1) {
151+
singleQuote = -2;
152+
}
153+
if (singleQuote !== 39) {
154+
escapeTest = strEscapeSequencesRegExpSingle;
155+
escapeReplace = strEscapeSequencesReplacerSingle;
156+
}
157+
}
158+
124159
// Some magic numbers that worked out fine while benchmarking with v8 6.0
125-
if (str.length < 5000 && !strEscapeSequencesRegExp.test(str))
126-
return `'${str}'`;
127-
if (str.length > 100)
128-
return `'${str.replace(strEscapeSequencesReplacer, escapeFn)}'`;
160+
if (str.length < 5000 && !escapeTest.test(str))
161+
return addQuotes(str, singleQuote);
162+
if (str.length > 100) {
163+
str = str.replace(escapeReplace, escapeFn);
164+
return addQuotes(str, singleQuote);
165+
}
166+
129167
let result = '';
130168
let last = 0;
131169
for (var i = 0; i < str.length; i++) {
132170
const point = str.charCodeAt(i);
133-
if (point === 39 || point === 92 || point < 32) {
171+
if (point === singleQuote || point === 92 || point < 32) {
134172
if (last === i) {
135173
result += meta[point];
136174
} else {
@@ -144,7 +182,7 @@ function strEscape(str) {
144182
} else if (last !== i) {
145183
result += str.slice(last);
146184
}
147-
return `'${result}'`;
185+
return addQuotes(result, singleQuote);
148186
}
149187

150188
function tryStringify(arg) {

test/parallel/test-repl-colors.js

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

test/parallel/test-util-inspect.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ assert.strictEqual(util.inspect(new Date('')), (new Date('')).toString());
4747
assert.strictEqual(util.inspect('\n\u0001'), "'\\n\\u0001'");
4848
assert.strictEqual(
4949
util.inspect(`${Array(75).fill(1)}'\n\u001d\n\u0003`),
50-
`'${Array(75).fill(1)}\\'\\n\\u001d\\n\\u0003'`
50+
`"${Array(75).fill(1)}'\\n\\u001d\\n\\u0003"`
5151
);
5252
assert.strictEqual(util.inspect([]), '[]');
5353
assert.strictEqual(util.inspect(Object.create([])), 'Array {}');
@@ -1424,3 +1424,9 @@ util.inspect(process);
14241424
assert(longList.includes('[Object: Inspection interrupted ' +
14251425
'prematurely. Maximum call stack size exceeded.]'));
14261426
}
1427+
1428+
// Do not escape single quotes if no double quote or backtick is present.
1429+
assert.strictEqual(util.inspect("'"), '"\'"');
1430+
assert.strictEqual(util.inspect('"\''), '`"\'`');
1431+
// eslint-disable-next-line no-template-curly-in-string
1432+
assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");

0 commit comments

Comments
 (0)