From 32d9d681548fc1b092bb10ece3ffa343e9e7bfd7 Mon Sep 17 00:00:00 2001 From: Joseph Frazier <1212jtraceur@gmail.com> Date: Tue, 9 May 2017 17:17:29 -0400 Subject: [PATCH] Print directive literals verbatim (#1560) * Print directive literals verbatim This addresses https://github.com/prettier/prettier/issues/1555, but doesn't seem to pass the AST_COMPARE=1 tests: AST_COMPARE=1 npm test -- tests/quotes -t strings However, running `prettier --debug-check` on the relevant file *does* work: prettier tests/quotes/strings.js --debug-check * Change directive literal quotes if it doesn't contain quotes This addresses https://github.com/prettier/prettier/pull/1560#discussion_r115396257 From https://github.com/prettier/prettier/issues/1555#issue-227206837: > It's okay to change the type of quotation marks used, but only if doing so does not require changing any characters within the directive. * Don't change directive literal quotes if it contains a backslash This passes the `--debug-check` tests again: prettier tests/quotes/strings.js --debug-check * Try to add regression test for escaped directive literals This seems not to work, despite the following command having the correct output: echo "'\''" | prettier You can use the following to get an idea of how flow/typescript parse this: node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'flow'}), null, 2)" node -p "JSON.stringify(require('./src/parser').parse('\\'\\\\\'\\'', {parser: 'typescript'}), null, 2)" * WIP Disable Flow/Typescript for ./tests/directives We don't yet handle escaped directives for them, but Babylon works. (similar to https://github.com/prettier/prettier/pull/602/commits/90bf93713c78a6a6b3f55e52d7be172ece9b56df#diff-0de18284f37da79ab8af4e4690919abaR1) * Revert "WIP Disable Flow/Typescript for ./tests/directives" This reverts commit 2aba6231271f6985a395c31e3df9323e8f3da115. * Prevent test strings from being parsed as directives See https://github.com/prettier/prettier/pull/1560#issue-227225960 * Add more escaped directive tests * Infer DirectiveLiterals from Flow parser * Don't test TypeScript on directives See https://github.com/prettier/prettier/pull/1560#issuecomment-300296221 * fixup! Infer DirectiveLiterals from Flow parser --- src/printer.js | 26 +++++++++++++++++++ .../__snapshots__/jsfmt.spec.js.snap | 13 ++++++++++ tests/directives/escaped.js | 4 +++ tests/directives/jsfmt.spec.js | 2 +- tests/quotes/__snapshots__/jsfmt.spec.js.snap | 16 ++++++++++++ tests/quotes/strings.js | 4 +++ 6 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 tests/directives/escaped.js diff --git a/src/printer.js b/src/printer.js index 52228264..ac5a25fc 100644 --- a/src/printer.js +++ b/src/printer.js @@ -181,6 +181,20 @@ function genericPrintNoParens(path, options, print, args) { case "EmptyStatement": return ""; case "ExpressionStatement": + // Detect Flow-parsed directives + if (n.directive) { + return concat([ + nodeStr( + { + type: 'DirectiveLiteral', + value: n.expression.value, + raw: n.expression.raw + }, + options + ), + semi + ]); + } return concat([path.call(print, "expression"), semi]); // Babel extension. case "ParenthesizedExpression": return concat(["(", path.call(print, "expression"), ")"]); @@ -3831,6 +3845,7 @@ function nodeStr(node, options) { const alternate = preferred === single ? double : single; let shouldUseAlternateQuote = false; + let canChangeDirectiveQuotes = !rawContent.includes('\\'); // If `rawContent` contains at least one of the quote preferred for enclosing // the string, we might want to enclose with the alternate quote instead, to @@ -3840,12 +3855,23 @@ function nodeStr(node, options) { const numAlternateQuotes = (rawContent.match(alternate.regex) || []).length; shouldUseAlternateQuote = numPreferredQuotes > numAlternateQuotes; + canChangeDirectiveQuotes = canChangeDirectiveQuotes && + numPreferredQuotes === 0 && + numAlternateQuotes === 0; } const enclosingQuote = shouldUseAlternateQuote ? alternate.quote : preferred.quote; + // Directives are exact code unit sequences, which means that you can't + // change the escape sequences they use. + // See https://github.com/prettier/prettier/issues/1555 + // and https://tc39.github.io/ecma262/#directive-prologue + if (node.type === 'DirectiveLiteral' && !canChangeDirectiveQuotes) { + return raw; + } + // It might sound unnecessary to use `makeString` even if `node.raw` already // is enclosed with `enclosingQuote`, but it isn't. `node.raw` could contain // unnecessary escapes (such as in `"\'"`). Always using `makeString` makes diff --git a/tests/directives/__snapshots__/jsfmt.spec.js.snap b/tests/directives/__snapshots__/jsfmt.spec.js.snap index c5df6ee7..0e737efb 100644 --- a/tests/directives/__snapshots__/jsfmt.spec.js.snap +++ b/tests/directives/__snapshots__/jsfmt.spec.js.snap @@ -1,5 +1,18 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`escaped.js 1`] = ` +'\\''; +'\\"'; +"\\'"; +"\\""; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +'\\''; +'\\"'; +"\\'"; +"\\""; + +`; + exports[`last-line-0.js 1`] = ` 'use strict';~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ "use strict"; diff --git a/tests/directives/escaped.js b/tests/directives/escaped.js new file mode 100644 index 00000000..097acc04 --- /dev/null +++ b/tests/directives/escaped.js @@ -0,0 +1,4 @@ +'\''; +'\"'; +"\'"; +"\""; diff --git a/tests/directives/jsfmt.spec.js b/tests/directives/jsfmt.spec.js index 320b4cfe..1840e9d3 100644 --- a/tests/directives/jsfmt.spec.js +++ b/tests/directives/jsfmt.spec.js @@ -1 +1 @@ -run_spec(__dirname, null, ["typescript", "babylon"]); +run_spec(__dirname, null, ["babylon"]); diff --git a/tests/quotes/__snapshots__/jsfmt.spec.js.snap b/tests/quotes/__snapshots__/jsfmt.spec.js.snap index 063e65e3..f5ead896 100644 --- a/tests/quotes/__snapshots__/jsfmt.spec.js.snap +++ b/tests/quotes/__snapshots__/jsfmt.spec.js.snap @@ -31,6 +31,10 @@ function b(object, key) { `; exports[`strings.js 1`] = ` +// Prevent strings from being parsed as directives +// See https://github.com/prettier/prettier/pull/1560#issue-227225960 +0; + // Every string will be changed to double quotes, unless we end up with fewer // escaped quotes by using single quotes. (Vice versa if the "singleQuote" // option is true). @@ -101,6 +105,10 @@ exports[`strings.js 1`] = ` "var backslash = \\"\\\\\\", doubleQuote = '\\"';" 'var backslash = "\\\\", doubleQuote = \\'"\\';' ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// Prevent strings from being parsed as directives +// See https://github.com/prettier/prettier/pull/1560#issue-227225960 +0; + // Every string will be changed to double quotes, unless we end up with fewer // escaped quotes by using single quotes. (Vice versa if the "singleQuote" // option is true). @@ -161,6 +169,10 @@ exports[`strings.js 1`] = ` `; exports[`strings.js 2`] = ` +// Prevent strings from being parsed as directives +// See https://github.com/prettier/prettier/pull/1560#issue-227225960 +0; + // Every string will be changed to double quotes, unless we end up with fewer // escaped quotes by using single quotes. (Vice versa if the "singleQuote" // option is true). @@ -231,6 +243,10 @@ exports[`strings.js 2`] = ` "var backslash = \\"\\\\\\", doubleQuote = '\\"';" 'var backslash = "\\\\", doubleQuote = \\'"\\';' ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// Prevent strings from being parsed as directives +// See https://github.com/prettier/prettier/pull/1560#issue-227225960 +0; + // Every string will be changed to double quotes, unless we end up with fewer // escaped quotes by using single quotes. (Vice versa if the "singleQuote" // option is true). diff --git a/tests/quotes/strings.js b/tests/quotes/strings.js index 11ff656f..16372423 100644 --- a/tests/quotes/strings.js +++ b/tests/quotes/strings.js @@ -1,3 +1,7 @@ +// Prevent strings from being parsed as directives +// See https://github.com/prettier/prettier/pull/1560#issue-227225960 +0; + // Every string will be changed to double quotes, unless we end up with fewer // escaped quotes by using single quotes. (Vice versa if the "singleQuote" // option is true).