Preserve code unit sequence of directive literals (#1571)

* 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 90bf93713c (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

* Don't fake objects that look like a DirectiveLiteral

Instead, add a flag to nodeStr() that deals with the Flow node
accordingly. See https://github.com/prettier/prettier/pull/1560#discussion_r115605758

* Print preferred quotes around escaped DirectiveLiteral when it doesn't contain quotes

See https://github.com/prettier/prettier/pull/1560#discussion_r115606122

* Simplify `canChangeDirectiveQuotes` logic

* Add directive test with unnecessarily escaped non-quote character

* Fix boolean logic error

I thought that this would result in the following if-block executing, as
needed to pass the test case in the previous commit. However, it appears
that it's not actually needed to pass the test case, since `makeString`
doesn't unescape unnecessarily escaped non-quote characters.
Nevertheless, I think we should leave that if-block (`if (canChangeDirectiveQuotes)`)
there, in case `makeString` is updated.

See https://github.com/prettier/prettier/pull/1571#discussion_r115658398

* Make isFlowDirectiveLiteral a separate argument to nodeStr()

See https://github.com/prettier/prettier/pull/1571#discussion_r115810988

* Simplify isFlowDirectiveLiteral logic by passing n.expression to nodeStr()

See https://github.com/prettier/prettier/pull/1571#discussion_r115811216
master
Joseph Frazier 2017-05-10 15:15:27 -04:00 committed by Christopher Chedeau
parent dfbea5d8ab
commit 8cc38ad524
6 changed files with 79 additions and 3 deletions

View File

@ -181,6 +181,17 @@ function genericPrintNoParens(path, options, print, args) {
case "EmptyStatement":
return "";
case "ExpressionStatement":
// Detect Flow-parsed directives
if (n.directive) {
return concat([
nodeStr(
n.expression,
options,
true
),
semi
]);
}
return concat([path.call(print, "expression"), semi]); // Babel extension.
case "ParenthesizedExpression":
return concat(["(", path.call(print, "expression"), ")"]);
@ -3815,7 +3826,7 @@ function adjustClause(node, clause, forceSpace) {
return indent(concat([line, clause]));
}
function nodeStr(node, options) {
function nodeStr(node, options, isFlowDirectiveLiteral) {
const str = node.value;
isString.assert(str);
@ -3831,21 +3842,43 @@ function nodeStr(node, options) {
const alternate = preferred === single ? double : single;
let shouldUseAlternateQuote = false;
const isDirectiveLiteral =
isFlowDirectiveLiteral || node.type === "DirectiveLiteral";
let canChangeDirectiveQuotes = false;
// 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
// minimize the number of escaped quotes.
if (rawContent.includes(preferred.quote)) {
// Also check for the alternate quote, to determine if we're allowed to swap
// the quotes on a DirectiveLiteral.
if (
rawContent.includes(preferred.quote) ||
rawContent.includes(alternate.quote)
) {
const numPreferredQuotes = (rawContent.match(preferred.regex) || []).length;
const numAlternateQuotes = (rawContent.match(alternate.regex) || []).length;
shouldUseAlternateQuote = numPreferredQuotes > numAlternateQuotes;
} else {
canChangeDirectiveQuotes = true;
}
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 (isDirectiveLiteral) {
if (canChangeDirectiveQuotes) {
return enclosingQuote + rawContent + enclosingQuote;
} else {
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

View File

@ -1,5 +1,22 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`escaped.js 1`] = `
'\\'';
'\\"';
"\\'";
"\\"";
'\\\\';
'\\a';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
'\\'';
'\\"';
"\\'";
"\\"";
"\\\\";
"\\a";
`;
exports[`last-line-0.js 1`] = `
'use strict';~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"use strict";

View File

@ -0,0 +1,6 @@
'\'';
'\"';
"\'";
"\"";
'\\';
'\a';

View File

@ -1 +1 @@
run_spec(__dirname, null, ["typescript", "babylon"]);
run_spec(__dirname, null, ["babylon"]);

View File

@ -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).

View File

@ -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).