Unescape unnecessarily escaped characters in strings (#1575)

* Add test with unnecessarily escaped non-quote character In 0b6d19d, 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 #1575 (comment) This reverts commit d056525. * 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 #1575 (comment) * Pass test for octal escapes in strings See #1575 (comment) * Add test for unnecessarily escaped character preced by escaped backslash See #1575 (comment) * Pass test for unnecessarily escaped character preced by escaped backslash This just allows an even number of backslashes to precede the unnecessary one. See #1575 (comment) * Add test for unescaped character after escaped backslash in strings * Add test for unescaped character preceded by two escaped backslashes in string See #1575 (comment) * Pass test for unescaped character preceded by two escaped backslashes in string This breaks another test though... See #1575 (comment) * 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 #1575 (comment) (cherry picked from commit 126e56a) * Add test for consecutive unnecessarily escaped characters in strings See #1575 (comment) * Pass test for consecutive unnecessarily escaped characters in strings See #1575 (comment) 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 2323c8c#commitcomment-22092267 * Safeguard against string unescaping loop hanging See #1575 (comment) * Add more comprehensive tests for unnecessary string escapes See #1575 (comment) * Remove superfluous variables from makeString() See #1575 (comment) * 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: * #1575 (comment) * #1575 (comment)
prettier · May 10, 2017 · d4217f5 · d4217f5
1 parent ef9a93d
commit d4217f5
Show file tree

Hide file tree

Showing 5 changed files with 229 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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.
 "\"'"