From d4217f5508f7b7efbeacfc4f5efbe2794ddbb176 Mon Sep 17 00:00:00 2001 From: Joseph Frazier <1212jtraceur@gmail.com> Date: Wed, 10 May 2017 19:02:49 -0400 Subject: [PATCH] Unescape unnecessarily escaped characters in strings (#1575) * Add test with unnecessarily escaped non-quote character In https://github.com/prettier/prettier/pull/1571/commits/0b6d19db18cd64f5d191f8c01dea3ea5525503b3, I noticed that `makeString` doesn't unescape unnecessarily escaped non-quote characters. This change simply adds a test for that. * Fix test with unnecessarily escaped non-quote character Unfortunately, this breaks a couple of other tests... * Revert "Fix test with unnecessarily escaped non-quote character" See https://github.com/prettier/prettier/pull/1575#issuecomment-300490172 This reverts commit d05652518fe7d4e2fb82ce48ffc922b153de5593. * Unescape unnecessarily escaped characters in strings * Add test for unnecessarily escaped character at not-beginning of string * Fix test for unnecessarily escaped character at not-beginning of string * Add test for multiple unnecessary escapes in strings * Pass test for multiple unnecessary escapes in strings * Add test for octal escapes in strings See https://github.com/prettier/prettier/pull/1575#discussion_r115804065 * Pass test for octal escapes in strings See https://github.com/prettier/prettier/pull/1575#discussion_r115804065 * Add test for unnecessarily escaped character preced by escaped backslash See https://github.com/prettier/prettier/pull/1575#discussion_r115804065 * Pass test for unnecessarily escaped character preced by escaped backslash This just allows an even number of backslashes to precede the unnecessary one. See https://github.com/prettier/prettier/pull/1575#discussion_r115804065 * Add test for unescaped character after escaped backslash in strings * Add test for unescaped character preceded by two escaped backslashes in string See https://github.com/prettier/prettier/pull/1575#discussion_r115808571 * Pass test for unescaped character preceded by two escaped backslashes in string This breaks another test though... See https://github.com/prettier/prettier/pull/1575#discussion_r115808571 * Update snapshot It turns out the test wasn't broken, I had just flubbed the escaping in the snapshot. The easiest way to see that this actually works is ```bash $ cat | prettier --stdin "hol\\a (the a is not escaped)" // press Control-D after the newline "hol\\a (the a is not escaped)"; // press Control-D after the newline ``` * Prevent test strings from being parsed as directives See https://github.com/prettier/prettier/pull/1575#discussion_r115820336 (cherry picked from commit 126e56ab2c79801cbf7fee22189212d8c93581df) * Add test for consecutive unnecessarily escaped characters in strings See https://github.com/prettier/prettier/pull/1575#discussion_r115822286 * Pass test for consecutive unnecessarily escaped characters in strings See https://github.com/prettier/prettier/pull/1575#discussion_r115822286 This looping is hacky. We might be able to emulate lookbehind instead. * Optimize (maybe?) string unescaping loop Not sure how expensive string comparison is here... See https://github.com/prettier/prettier/commit/2323c8c025e5302995b2846ed45fd0bd67e66f1b#commitcomment-22092267 * Safeguard against string unescaping loop hanging See https://github.com/prettier/prettier/pull/1575#discussion_r115827531 * Add more comprehensive tests for unnecessary string escapes See https://github.com/prettier/prettier/pull/1575#discussion_r115798155 * Remove superfluous variables from makeString() See https://github.com/prettier/prettier/pull/1575#discussion_r115834468 * Unescape unnecessary strings escapes without looping * Unescape unnecessary string escapes while handling quotes Kudos to @lydell for figuring this out! See https://github.com/prettier/prettier/pull/1575/files#r115860741 * Test that unnecessary escapes remain in directive literals See: * https://github.com/prettier/prettier/pull/1575#discussion_r115820336 * https://github.com/prettier/prettier/pull/1575#issuecomment-300633277 --- src/printer.js | 11 +- .../__snapshots__/jsfmt.spec.js.snap | 58 ++++++++++ tests/directives/escaped.js | 29 +++++ tests/quotes/__snapshots__/jsfmt.spec.js.snap | 106 ++++++++++++++++++ tests/quotes/strings.js | 27 +++++ 5 files changed, 229 insertions(+), 2 deletions(-) diff --git a/src/printer.js b/src/printer.js index de92d7510..f62728040 100644 --- a/src/printer.js +++ b/src/printer.js @@ -3917,8 +3917,15 @@ function makeString(rawContent, enclosingQuote) { return "\\" + quote; } - // Otherwise return the escape or unescaped quote as-is. - return match; + if (quote) { + return quote; + } + + // Unescape any unnecessarily escaped character. + // Adapted from https://github.com/eslint/eslint/blob/de0b4ad7bd820ade41b1f606008bea68683dc11a/lib/rules/no-useless-escape.js#L27 + return /^[^\\nrvtbfux\r\n\u2028\u2029"'0-7]$/.test(escaped) + ? escaped + : "\\" + escaped; }); return enclosingQuote + newContent + enclosingQuote; diff --git a/tests/directives/__snapshots__/jsfmt.spec.js.snap b/tests/directives/__snapshots__/jsfmt.spec.js.snap index b7e1f0373..342de6894 100644 --- a/tests/directives/__snapshots__/jsfmt.spec.js.snap +++ b/tests/directives/__snapshots__/jsfmt.spec.js.snap @@ -1,19 +1,77 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`escaped.js 1`] = ` +// Unnecessary escapes. (adapted from tests/quotes/strings.js) +// Note that in directives, unnecessary escapes should be preserved. +// See https://github.com/prettier/prettier/issues/1555 '\\''; '\\"'; "\\'"; "\\""; '\\\\'; '\\a'; +"hol\\a" +'hol\\a' +"hol\\\\a (the a is not escaped)" +'hol\\\\a (the a is not escaped)' +"multiple \\a unnecessary \\a escapes" +'multiple \\a unnecessary \\a escapes' +"unnecessarily escaped character preceded by escaped backslash \\\\\\a" +'unnecessarily escaped character preceded by escaped backslash \\\\\\a' +"unescaped character preceded by two escaped backslashes \\\\\\\\a" +'unescaped character preceded by two escaped backslashes \\\\\\\\a' +"\\a\\a" // consecutive unnecessarily escaped characters +'\\a\\a' // consecutive unnecessarily escaped characters +'escaped \\u2030 \\‰ (should still stay escaped)' + +// Meaningful escapes +// Commented out to avoid \`SyntaxError: Octal literals are not allowed in strict mode.\` +// "octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7" +// 'octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7' +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61" +'meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61' +'escaped newline \\ +' +'escaped carriage return \\ +' +'escaped \\u2028 \\
' +'escaped \\u2029 \\
' ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// Unnecessary escapes. (adapted from tests/quotes/strings.js) +// Note that in directives, unnecessary escapes should be preserved. +// See https://github.com/prettier/prettier/issues/1555 '\\''; '\\"'; "\\'"; "\\""; "\\\\"; "\\a"; +"hol\\a"; +"hol\\a"; +"hol\\\\a (the a is not escaped)"; +"hol\\\\a (the a is not escaped)"; +"multiple \\a unnecessary \\a escapes"; +"multiple \\a unnecessary \\a escapes"; +"unnecessarily escaped character preceded by escaped backslash \\\\\\a"; +"unnecessarily escaped character preceded by escaped backslash \\\\\\a"; +"unescaped character preceded by two escaped backslashes \\\\\\\\a"; +"unescaped character preceded by two escaped backslashes \\\\\\\\a"; +"\\a\\a"; // consecutive unnecessarily escaped characters +"\\a\\a"; // consecutive unnecessarily escaped characters +"escaped \\u2030 \\‰ (should still stay escaped)"; + +// Meaningful escapes +// Commented out to avoid \`SyntaxError: Octal literals are not allowed in strict mode.\` +// "octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7" +// 'octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7' +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61"; +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61"; +"escaped newline \\ +"; +"escaped carriage return \\ +"; +"escaped \\u2028 \\
"; +"escaped \\u2029 \\
"; `; diff --git a/tests/directives/escaped.js b/tests/directives/escaped.js index 67fb803fb..8894f70a6 100644 --- a/tests/directives/escaped.js +++ b/tests/directives/escaped.js @@ -1,6 +1,35 @@ +// Unnecessary escapes. (adapted from tests/quotes/strings.js) +// Note that in directives, unnecessary escapes should be preserved. +// See https://github.com/prettier/prettier/issues/1555 '\''; '\"'; "\'"; "\""; '\\'; '\a'; +"hol\a" +'hol\a' +"hol\\a (the a is not escaped)" +'hol\\a (the a is not escaped)' +"multiple \a unnecessary \a escapes" +'multiple \a unnecessary \a escapes' +"unnecessarily escaped character preceded by escaped backslash \\\a" +'unnecessarily escaped character preceded by escaped backslash \\\a' +"unescaped character preceded by two escaped backslashes \\\\a" +'unescaped character preceded by two escaped backslashes \\\\a' +"\a\a" // consecutive unnecessarily escaped characters +'\a\a' // consecutive unnecessarily escaped characters +'escaped \u2030 \‰ (should still stay escaped)' + +// Meaningful escapes +// Commented out to avoid `SyntaxError: Octal literals are not allowed in strict mode.` +// "octal escapes \0 \1 \2 \3 \4 \5 \6 \7" +// 'octal escapes \0 \1 \2 \3 \4 \5 \6 \7' +"meaningfully escaped alphabetical characters \n \r \v \t \b \f \u2713 \x61" +'meaningfully escaped alphabetical characters \n \r \v \t \b \f \u2713 \x61' +'escaped newline \ +' +'escaped carriage return \ +' +'escaped \u2028 \
' +'escaped \u2029 \
' diff --git a/tests/quotes/__snapshots__/jsfmt.spec.js.snap b/tests/quotes/__snapshots__/jsfmt.spec.js.snap index f5ead8960..558c9e1e4 100644 --- a/tests/quotes/__snapshots__/jsfmt.spec.js.snap +++ b/tests/quotes/__snapshots__/jsfmt.spec.js.snap @@ -68,6 +68,33 @@ exports[`strings.js 1`] = ` // Unnecessary escapes. "\\'" '\\"' +"\\a" +'\\a' +"hol\\a" +'hol\\a' +"hol\\\\a (the a is not escaped)" +'hol\\\\a (the a is not escaped)' +"multiple \\a unnecessary \\a escapes" +'multiple \\a unnecessary \\a escapes' +"unnecessarily escaped character preceded by escaped backslash \\\\\\a" +'unnecessarily escaped character preceded by escaped backslash \\\\\\a' +"unescaped character preceded by two escaped backslashes \\\\\\\\a" +'unescaped character preceded by two escaped backslashes \\\\\\\\a' +"\\a\\a" // consecutive unnecessarily escaped characters +'\\a\\a' // consecutive unnecessarily escaped characters +'escaped \\u2030 \\‰ (should not stay escaped)' + +// Meaningful escapes +"octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7" +'octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7' +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61" +'meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61' +'escaped newline \\ +' +'escaped carriage return \\ +' +'escaped \\u2028 \\
' +'escaped \\u2029 \\
' // One of each. "\\"'" @@ -138,6 +165,32 @@ exports[`strings.js 1`] = ` // Unnecessary escapes. "'"; '"'; +"a"; +"a"; +"hola"; +"hola"; +"hol\\\\a (the a is not escaped)"; +"hol\\\\a (the a is not escaped)"; +"multiple a unnecessary a escapes"; +"multiple a unnecessary a escapes"; +"unnecessarily escaped character preceded by escaped backslash \\\\a"; +"unnecessarily escaped character preceded by escaped backslash \\\\a"; +"unescaped character preceded by two escaped backslashes \\\\\\\\a"; +"unescaped character preceded by two escaped backslashes \\\\\\\\a"; +"aa"; // consecutive unnecessarily escaped characters +"aa"; // consecutive unnecessarily escaped characters +"escaped \\u2030 ‰ (should not stay escaped)"; +// Meaningful escapes +"octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7"; +"octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7"; +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61"; +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61"; +"escaped newline \\ +"; +"escaped carriage return \\ +"; +"escaped \\u2028 \\
"; +"escaped \\u2029 \\
"; // One of each. "\\"'"; "\\"'"; @@ -206,6 +259,33 @@ exports[`strings.js 2`] = ` // Unnecessary escapes. "\\'" '\\"' +"\\a" +'\\a' +"hol\\a" +'hol\\a' +"hol\\\\a (the a is not escaped)" +'hol\\\\a (the a is not escaped)' +"multiple \\a unnecessary \\a escapes" +'multiple \\a unnecessary \\a escapes' +"unnecessarily escaped character preceded by escaped backslash \\\\\\a" +'unnecessarily escaped character preceded by escaped backslash \\\\\\a' +"unescaped character preceded by two escaped backslashes \\\\\\\\a" +'unescaped character preceded by two escaped backslashes \\\\\\\\a' +"\\a\\a" // consecutive unnecessarily escaped characters +'\\a\\a' // consecutive unnecessarily escaped characters +'escaped \\u2030 \\‰ (should not stay escaped)' + +// Meaningful escapes +"octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7" +'octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7' +"meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61" +'meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61' +'escaped newline \\ +' +'escaped carriage return \\ +' +'escaped \\u2028 \\
' +'escaped \\u2029 \\
' // One of each. "\\"'" @@ -276,6 +356,32 @@ exports[`strings.js 2`] = ` // Unnecessary escapes. "'"; '"'; +'a'; +'a'; +'hola'; +'hola'; +'hol\\\\a (the a is not escaped)'; +'hol\\\\a (the a is not escaped)'; +'multiple a unnecessary a escapes'; +'multiple a unnecessary a escapes'; +'unnecessarily escaped character preceded by escaped backslash \\\\a'; +'unnecessarily escaped character preceded by escaped backslash \\\\a'; +'unescaped character preceded by two escaped backslashes \\\\\\\\a'; +'unescaped character preceded by two escaped backslashes \\\\\\\\a'; +'aa'; // consecutive unnecessarily escaped characters +'aa'; // consecutive unnecessarily escaped characters +'escaped \\u2030 ‰ (should not stay escaped)'; +// Meaningful escapes +'octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7'; +'octal escapes \\0 \\1 \\2 \\3 \\4 \\5 \\6 \\7'; +'meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61'; +'meaningfully escaped alphabetical characters \\n \\r \\v \\t \\b \\f \\u2713 \\x61'; +'escaped newline \\ +'; +'escaped carriage return \\ +'; +'escaped \\u2028 \\
'; +'escaped \\u2029 \\
'; // One of each. '"\\''; '"\\''; diff --git a/tests/quotes/strings.js b/tests/quotes/strings.js index 163724232..34c5b921e 100644 --- a/tests/quotes/strings.js +++ b/tests/quotes/strings.js @@ -35,6 +35,33 @@ // Unnecessary escapes. "\'" '\"' +"\a" +'\a' +"hol\a" +'hol\a' +"hol\\a (the a is not escaped)" +'hol\\a (the a is not escaped)' +"multiple \a unnecessary \a escapes" +'multiple \a unnecessary \a escapes' +"unnecessarily escaped character preceded by escaped backslash \\\a" +'unnecessarily escaped character preceded by escaped backslash \\\a' +"unescaped character preceded by two escaped backslashes \\\\a" +'unescaped character preceded by two escaped backslashes \\\\a' +"\a\a" // consecutive unnecessarily escaped characters +'\a\a' // consecutive unnecessarily escaped characters +'escaped \u2030 \‰ (should not stay escaped)' + +// Meaningful escapes +"octal escapes \0 \1 \2 \3 \4 \5 \6 \7" +'octal escapes \0 \1 \2 \3 \4 \5 \6 \7' +"meaningfully escaped alphabetical characters \n \r \v \t \b \f \u2713 \x61" +'meaningfully escaped alphabetical characters \n \r \v \t \b \f \u2713 \x61' +'escaped newline \ +' +'escaped carriage return \ +' +'escaped \u2028 \
' +'escaped \u2029 \
' // One of each. "\"'"