From 40ab73cc0d1c7327316e68972866e7783465a509 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Sat, 7 Jan 2023 16:43:44 +0800 Subject: [PATCH] Fix comments after directive (#14081) --- changelog_unreleased/javascript/14081.md | 13 + src/language-js/print/literal.js | 14 +- src/language-js/print/misc.js | 19 + src/language-js/printer-estree.js | 28 +- .../comments/__snapshots__/jsfmt.spec.js.snap | 569 ++++++++++++++++++ .../js/directives/comments/jsfmt.spec.js | 55 ++ tests/format/markdown/auto-link/jsfmt.spec.js | 1 + 7 files changed, 672 insertions(+), 27 deletions(-) create mode 100644 changelog_unreleased/javascript/14081.md create mode 100644 tests/format/js/directives/comments/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/format/js/directives/comments/jsfmt.spec.js diff --git a/changelog_unreleased/javascript/14081.md b/changelog_unreleased/javascript/14081.md new file mode 100644 index 000000000000..92a794cbc73d --- /dev/null +++ b/changelog_unreleased/javascript/14081.md @@ -0,0 +1,13 @@ +#### Fix comments after directive (#14081 by @fisker) + + +```jsx +// Input +"use strict" /* comment */; + +// Prettier stable (with other js parsers except `babel`) +Error: Comment "comment" was not printed. Please report this error! + +// Prettier main + +``` diff --git a/src/language-js/print/literal.js b/src/language-js/print/literal.js index a614bd5f4ba1..252cf4925a50 100644 --- a/src/language-js/print/literal.js +++ b/src/language-js/print/literal.js @@ -1,6 +1,7 @@ "use strict"; const { printString, printNumber } = require("../../common/util.js"); const { replaceTextEndOfLine } = require("../../document/doc-utils.js"); +const { printDirective } = require("./misc.js"); function printLiteral(path, options /*, print*/) { const node = path.getNode(); @@ -41,7 +42,9 @@ function printLiteral(path, options /*, print*/) { } if (typeof value === "string") { - return replaceTextEndOfLine(printString(node.raw, options)); + return isDirective(path) + ? printDirective(node.raw, options) + : replaceTextEndOfLine(printString(node.raw, options)); } return String(value); @@ -49,6 +52,15 @@ function printLiteral(path, options /*, print*/) { } } +function isDirective(path) { + if (path.getName() !== "expression") { + return; + } + + const parent = path.getParentNode(); + return parent.type === "ExpressionStatement" && parent.directive; +} + function printBigInt(raw) { return raw.toLowerCase(); } diff --git a/src/language-js/print/misc.js b/src/language-js/print/misc.js index 3d5a7d0f8194..44ae0ce9c761 100644 --- a/src/language-js/print/misc.js +++ b/src/language-js/print/misc.js @@ -93,6 +93,24 @@ function printRestSpread(path, options, print) { return ["...", print("argument"), printTypeAnnotation(path, options, print)]; } +function printDirective(rawText, options) { + const rawContent = rawText.slice(1, -1); + + // Check for the alternate quote, to determine if we're allowed to swap + // the quotes on a DirectiveLiteral. + if (rawContent.includes('"') || rawContent.includes("'")) { + return rawText; + } + + const enclosingQuote = options.singleQuote ? "'" : '"'; + + // 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 + return enclosingQuote + rawContent + enclosingQuote; +} + module.exports = { printOptionalToken, printDefiniteToken, @@ -102,4 +120,5 @@ module.exports = { printTypeAnnotation, printRestSpread, adjustClause, + printDirective, }; diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index f068fa92729c..20ea246499d5 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -25,7 +25,6 @@ const { isLineComment, isNextLineEmpty, needsHardlineAfterDanglingComment, - rawText, hasIgnoreComment, isCallExpression, isMemberExpression, @@ -50,6 +49,7 @@ const { adjustClause, printRestSpread, printDefiniteToken, + printDirective, } = require("./print/misc.js"); const { printImportDeclaration, @@ -219,11 +219,6 @@ function printPathNoParens(path, options, print, args) { case "EmptyStatement": return ""; case "ExpressionStatement": { - // Detect Flow and TypeScript directives - if (node.directive) { - return [printDirective(node.expression, options), semi]; - } - if ( options.parser === "__vue_event_binding" || options.parser === "__vue_ts_event_binding" @@ -424,7 +419,7 @@ function printPathNoParens(path, options, print, args) { case "Directive": return [print("value"), semi]; // Babel 6 case "DirectiveLiteral": - return printDirective(node, options); + return printDirective(node.extra.raw, options); case "UnaryExpression": parts.push(node.operator); @@ -890,25 +885,6 @@ function printPathNoParens(path, options, print, args) { } } -function printDirective(node, options) { - const raw = rawText(node); - const rawContent = raw.slice(1, -1); - - // Check for the alternate quote, to determine if we're allowed to swap - // the quotes on a DirectiveLiteral. - if (rawContent.includes('"') || rawContent.includes("'")) { - return raw; - } - - const enclosingQuote = options.singleQuote ? "'" : '"'; - - // 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 - return enclosingQuote + rawContent + enclosingQuote; -} - function canAttachComment(node) { return ( node.type && diff --git a/tests/format/js/directives/comments/__snapshots__/jsfmt.spec.js.snap b/tests/format/js/directives/comments/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000000..62f891b2ff16 --- /dev/null +++ b/tests/format/js/directives/comments/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,569 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`snippet: #0 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +/* comment */ 'use strict'; +=====================================output===================================== +/* comment */ "use strict" + +================================================================================ +`; + +exports[`snippet: #0 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +/* comment */ 'use strict'; +=====================================output===================================== +/* comment */ "use strict"; + +================================================================================ +`; + +exports[`snippet: #1 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + /* comment */ 'use strict'; +} +=====================================output===================================== +function foo() { + /* comment */ "use strict" +} + +================================================================================ +`; + +exports[`snippet: #1 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + /* comment */ 'use strict'; +} +=====================================output===================================== +function foo() { + /* comment */ "use strict"; +} + +================================================================================ +`; + +exports[`snippet: #2 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +'use strict' /* comment */; +=====================================output===================================== +"use strict" /* comment */ + +================================================================================ +`; + +exports[`snippet: #2 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +'use strict' /* comment */; +=====================================output===================================== +"use strict" /* comment */; + +================================================================================ +`; + +exports[`snippet: #3 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + 'use strict' /* comment */; +} +=====================================output===================================== +function foo() { + "use strict" /* comment */ +} + +================================================================================ +`; + +exports[`snippet: #3 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + 'use strict' /* comment */; +} +=====================================output===================================== +function foo() { + "use strict" /* comment */; +} + +================================================================================ +`; + +exports[`snippet: #4 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +// comment +'use strict'; +=====================================output===================================== +// comment +"use strict" + +================================================================================ +`; + +exports[`snippet: #4 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +// comment +'use strict'; +=====================================output===================================== +// comment +"use strict"; + +================================================================================ +`; + +exports[`snippet: #5 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + // comment + 'use strict'; +} +=====================================output===================================== +function foo() { + // comment + "use strict" +} + +================================================================================ +`; + +exports[`snippet: #5 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + // comment + 'use strict'; +} +=====================================output===================================== +function foo() { + // comment + "use strict"; +} + +================================================================================ +`; + +exports[`snippet: #6 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +'use strict' // comment +=====================================output===================================== +"use strict" // comment + +================================================================================ +`; + +exports[`snippet: #6 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +'use strict' // comment +=====================================output===================================== +"use strict"; // comment + +================================================================================ +`; + +exports[`snippet: #7 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + 'use strict' // comment +} +=====================================output===================================== +function foo() { + "use strict" // comment +} + +================================================================================ +`; + +exports[`snippet: #7 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + 'use strict' // comment +} +=====================================output===================================== +function foo() { + "use strict"; // comment +} + +================================================================================ +`; + +exports[`snippet: #8 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +'use strict'; +/* comment */ +(function () {})(); +=====================================output===================================== +"use strict" +/* comment */ +;(function () {})() + +================================================================================ +`; + +exports[`snippet: #8 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +'use strict'; +/* comment */ +(function () {})(); +=====================================output===================================== +"use strict"; +/* comment */ +(function () {})(); + +================================================================================ +`; + +exports[`snippet: #9 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + 'use strict'; + /* comment */ + (function () {})(); +} +=====================================output===================================== +function foo() { + "use strict" + /* comment */ + ;(function () {})() +} + +================================================================================ +`; + +exports[`snippet: #9 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + 'use strict'; + /* comment */ + (function () {})(); +} +=====================================output===================================== +function foo() { + "use strict"; + /* comment */ + (function () {})(); +} + +================================================================================ +`; + +exports[`snippet: #10 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +/* comment */ +'use strict'; +(function () {})(); +=====================================output===================================== +/* comment */ +"use strict" +;(function () {})() + +================================================================================ +`; + +exports[`snippet: #10 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +/* comment */ +'use strict'; +(function () {})(); +=====================================output===================================== +/* comment */ +"use strict"; +(function () {})(); + +================================================================================ +`; + +exports[`snippet: #11 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + /* comment */ + 'use strict'; + (function () {})(); +} +=====================================output===================================== +function foo() { + /* comment */ + "use strict" + ;(function () {})() +} + +================================================================================ +`; + +exports[`snippet: #11 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + /* comment */ + 'use strict'; + (function () {})(); +} +=====================================output===================================== +function foo() { + /* comment */ + "use strict"; + (function () {})(); +} + +================================================================================ +`; + +exports[`snippet: #12 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +'use strict'; +// comment +(function () {})(); +=====================================output===================================== +"use strict" +// comment +;(function () {})() + +================================================================================ +`; + +exports[`snippet: #12 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +'use strict'; +// comment +(function () {})(); +=====================================output===================================== +"use strict"; +// comment +(function () {})(); + +================================================================================ +`; + +exports[`snippet: #13 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + 'use strict'; + // comment + (function () {})(); +} +=====================================output===================================== +function foo() { + "use strict" + // comment + ;(function () {})() +} + +================================================================================ +`; + +exports[`snippet: #13 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + 'use strict'; + // comment + (function () {})(); +} +=====================================output===================================== +function foo() { + "use strict"; + // comment + (function () {})(); +} + +================================================================================ +`; + +exports[`snippet: #14 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +// comment +'use strict'; +(function () {})(); +=====================================output===================================== +// comment +"use strict" +;(function () {})() + +================================================================================ +`; + +exports[`snippet: #14 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +// comment +'use strict'; +(function () {})(); +=====================================output===================================== +// comment +"use strict"; +(function () {})(); + +================================================================================ +`; + +exports[`snippet: #15 - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +function foo() { + // comment + 'use strict'; + (function () {})(); +} +=====================================output===================================== +function foo() { + // comment + "use strict" + ;(function () {})() +} + +================================================================================ +`; + +exports[`snippet: #15 format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + // comment + 'use strict'; + (function () {})(); +} +=====================================output===================================== +function foo() { + // comment + "use strict"; + (function () {})(); +} + +================================================================================ +`; diff --git a/tests/format/js/directives/comments/jsfmt.spec.js b/tests/format/js/directives/comments/jsfmt.spec.js new file mode 100644 index 000000000000..e85b3df3ed9b --- /dev/null +++ b/tests/format/js/directives/comments/jsfmt.spec.js @@ -0,0 +1,55 @@ +const { outdent } = require("outdent"); +const indent = (text) => + text + .split("\n") + .map((line) => (line ? ` ${line}` : line)) + .join("\n"); +// TODO: Remove this when we drop support for Node.js v10 +// eslint-disable-next-line unicorn/prefer-spread +const flat = (array) => [].concat(...array); + +const snippets = flat( + [ + "/* comment */ 'use strict';", + "'use strict' /* comment */;", + outdent` + // comment + 'use strict'; + `, + outdent` + 'use strict' // comment + `, + outdent` + 'use strict'; + /* comment */ + (function () {})(); + `, + outdent` + /* comment */ + 'use strict'; + (function () {})(); + `, + outdent` + 'use strict'; + // comment + (function () {})(); + `, + outdent` + // comment + 'use strict'; + (function () {})(); + `, + ].map((code) => [ + code, + outdent` + function foo() { + ${indent(code)} + } + `, + ]) +); + +run_spec({ dirname: __dirname, snippets }, ["babel", "flow", "typescript"]); +run_spec({ dirname: __dirname, snippets }, ["babel", "flow", "typescript"], { + semi: false, +}); diff --git a/tests/format/markdown/auto-link/jsfmt.spec.js b/tests/format/markdown/auto-link/jsfmt.spec.js index 176a31f1886b..8f65afa30464 100644 --- a/tests/format/markdown/auto-link/jsfmt.spec.js +++ b/tests/format/markdown/auto-link/jsfmt.spec.js @@ -3,6 +3,7 @@ * @param {Array>} array * @returns {Array} */ +// TODO: Remove this when we drop support for Node.js v10 // eslint-disable-next-line unicorn/prefer-spread const flat = (array) => [].concat(...array);