diff --git a/changelog_unreleased/typescript/pr-8422.md b/changelog_unreleased/typescript/pr-8422.md new file mode 100644 index 000000000..2f8d05a17 --- /dev/null +++ b/changelog_unreleased/typescript/pr-8422.md @@ -0,0 +1,23 @@ +#### Parenthesize string literals to avoid interpreting as directives ([#8422](https://github.com/prettier/prettier/pull/8422) by [@thorn0](https://github.com/thorn0)) + +Prettier wraps any string literal which is in statement position in parentheses because otherwise such strings would get interpreted as directives if they occurred at the top of a function or program, which can change the behavior of the program. It is not technically necessary to do this for strings which are not on the first line of a function or program. But doing so anyway is more consistent and can highlight bugs. See for example [this thread on twitter](https://twitter.com/dan_abramov/status/874601179334017024). + +Previously this didn't consistently work for the `typescript` parser. Only strings already wrapped in parentheses retained them. + + +```ts +// Input +f(); +'use foo'; +('use bar'); + +// Prettier stable +f(); +"use foo"; +("use bar"); + +// Prettier master +f(); +("use foo"); +("use bar"); +``` diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js index d886c9989..3757c74cd 100644 --- a/src/language-js/needs-parens.js +++ b/src/language-js/needs-parens.js @@ -475,11 +475,7 @@ function needsParens(path, options) { if ( typeof node.value === "string" && parent.type === "ExpressionStatement" && - // TypeScript workaround for https://github.com/JamesHenry/typescript-estree/issues/2 - // See corresponding workaround in printer.js case: "Literal" - ((options.parser !== "typescript" && !parent.directive) || - (options.parser === "typescript" && - options.originalText.charAt(options.locStart(node) - 1) === "(")) + !parent.directive ) { // To avoid becoming a directive const grandParent = path.getParentNode(1); diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 3d0a3548d..8e1c0bc91 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -487,7 +487,7 @@ function printPathNoParens(path, options, print, args) { case "EmptyStatement": return ""; case "ExpressionStatement": - // Detect Flow-parsed directives + // Detect Flow and TypeScript directives if (n.directive) { return concat([nodeStr(n.expression, options, true), semi]); } @@ -1617,7 +1617,7 @@ function printPathNoParens(path, options, print, args) { return (n.bigint || n.extra.raw).toLowerCase(); case "BooleanLiteral": // Babel 6 Literal split case "StringLiteral": // Babel 6 Literal split - case "Literal": { + case "Literal": if (n.regex) { return printRegex(n.regex); } @@ -1631,18 +1631,7 @@ function printPathNoParens(path, options, print, args) { if (typeof n.value !== "string") { return "" + n.value; } - // TypeScript workaround for https://github.com/JamesHenry/typescript-estree/issues/2 - // See corresponding workaround in needs-parens.js - const grandParent = path.getParentNode(1); - const isTypeScriptDirective = - options.parser === "typescript" && - typeof n.value === "string" && - grandParent && - (grandParent.type === "Program" || - grandParent.type === "BlockStatement"); - - return nodeStr(n, options, isTypeScriptDirective); - } + return nodeStr(n, options); case "Directive": return path.call(print, "value"); // Babel 6 case "DirectiveLiteral": diff --git a/tests/js/directives/__snapshots__/jsfmt.spec.js.snap b/tests/js/directives/__snapshots__/jsfmt.spec.js.snap index 6621a5f46..375c49d23 100644 --- a/tests/js/directives/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/directives/__snapshots__/jsfmt.spec.js.snap @@ -82,6 +82,26 @@ printWidth: 80 ================================================================================ `; +exports[`issue-7346.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +('bar'); // parens should not be removed to avoid becoming a directive +\`foo\`; +'bar'; // parens should be added, see https://github.com/prettier/prettier/issues/7346#issuecomment-574823604 +'"'; + +=====================================output===================================== +("bar"); // parens should not be removed to avoid becoming a directive +\`foo\`; +("bar"); // parens should be added, see https://github.com/prettier/prettier/issues/7346#issuecomment-574823604 +('"'); + +================================================================================ +`; + exports[`last-line-0.js format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/js/directives/issue-7346.js b/tests/js/directives/issue-7346.js new file mode 100644 index 000000000..93c48da77 --- /dev/null +++ b/tests/js/directives/issue-7346.js @@ -0,0 +1,4 @@ +('bar'); // parens should not be removed to avoid becoming a directive +`foo`; +'bar'; // parens should be added, see https://github.com/prettier/prettier/issues/7346#issuecomment-574823604 +'"';