diff --git a/changelog_unreleased/javascript/pr-8554.md b/changelog_unreleased/javascript/pr-8554.md new file mode 100644 index 000000000000..6919d2be5d90 --- /dev/null +++ b/changelog_unreleased/javascript/pr-8554.md @@ -0,0 +1,22 @@ +#### Prefer a trailing comment when comment on between test and consequent on ternary ([#8554](https://github.com/prettier/prettier/pull/8554) by [@sosukesuzuki](https://github.com/sosukesuzuki)) + + +```js +// Input +test + // comment + ? foo + : bar; + +// Prettier (stable) +test + ? // comment + foo + : bar; + +// Prettier (master) +test + // comment + ? foo + : bar; +``` diff --git a/src/language-js/comments.js b/src/language-js/comments.js index 99224c58c67a..cce25a621a19 100644 --- a/src/language-js/comments.js +++ b/src/language-js/comments.js @@ -67,7 +67,14 @@ function handleOwnLineComment(comment, text, options, ast, isLastComment) { comment, options ) || - handleLabeledStatementComments(enclosingNode, comment) + handleLabeledStatementComments(enclosingNode, comment) || + handleTernaryTrailingComments( + enclosingNode, + precedingNode, + comment, + text, + options + ) ); } @@ -406,9 +413,7 @@ function handleConditionalExpressionComments( if ( (!precedingNode || !isSameLineAsPrecedingNode) && - enclosingNode && - (enclosingNode.type === "ConditionalExpression" || - enclosingNode.type === "TSConditionalType") && + isTernaryExpression(enclosingNode) && followingNode ) { addLeadingComment(followingNode, comment); @@ -942,6 +947,46 @@ function handleTSMappedTypeComments( return false; } +function hasQuestionBetweenTestAndComment(testNode, comment, text, options) { + const testNodeLocEnd = options.locEnd(testNode); + const commentLocStart = options.locStart(comment); + for (let i = testNodeLocEnd; i < commentLocStart; i++) { + i = privateUtil.getNextNonSpaceNonCommentCharacterIndexWithStartIndex( + text, + i + ); + if (text[i] === "?") { + return i < commentLocStart; + } + } + return false; +} + +function handleTernaryTrailingComments( + enclosingNode, + precedingNode, + comment, + text, + options +) { + // test + // // comment + // ? first + // : second + if ( + isTernaryTest(precedingNode, enclosingNode) && + // test ? + // // comment + // first + // : second + !hasQuestionBetweenTestAndComment(precedingNode, comment, text, options) + ) { + addTrailingComment(precedingNode, comment); + return true; + } + return false; +} + function isBlockComment(comment) { return comment.type === "Block" || comment.type === "CommentBlock"; } @@ -1008,6 +1053,20 @@ function getCommentChildNodes(node, options) { } } +function shouldIndentComment(path) { + return ( + isCommentParentTernaryTest(path) && + !isTernaryExpression(path.getParentNode(2)) + ); +} + +function shouldDedentComment(path) { + return ( + isCommentParentTernaryTest(path) && + isTernaryExpression(path.getParentNode(2)) + ); +} + function isTypeCastComment(comment) { return ( isBlockComment(comment) && @@ -1019,6 +1078,31 @@ function isTypeCastComment(comment) { ); } +function isTernaryExpression(node) { + return ( + node && + (node.type === "ConditionalExpression" || node.type === "TSConditionalType") + ); +} + +/** + * Check if node matches the ternary test node + */ +function isTernaryTest(node, ternary) { + if (!node || !isTernaryExpression(ternary)) { + return false; + } + const testField = ternary.test ? "test" : "extendsType"; + return node === ternary[testField]; +} + +function isCommentParentTernaryTest(commentPath) { + return isTernaryTest( + commentPath.getParentNode(), + commentPath.getParentNode(1) + ); +} + module.exports = { handleOwnLineComment, handleEndOfLineComment, @@ -1028,4 +1112,6 @@ module.exports = { isTypeCastComment, getGapRegex, getCommentChildNodes, + shouldIndentComment, + shouldDedentComment, }; diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index fd794a220981..07c9ee753e05 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -5447,4 +5447,6 @@ module.exports = { }, getGapRegex: handleComments.getGapRegex, getCommentChildNodes: handleComments.getCommentChildNodes, + shouldIndentComment: handleComments.shouldIndentComment, + shouldDedentComment: handleComments.shouldDedentComment, }; diff --git a/src/main/comments.js b/src/main/comments.js index 127f511985f3..8dcf4684146d 100644 --- a/src/main/comments.js +++ b/src/main/comments.js @@ -9,6 +9,7 @@ const { indent, lineSuffix, join, + dedent, cursor, } = require("../document").builders; const { @@ -431,9 +432,25 @@ function printTrailingComment(commentPath, options) { locStart ); - return lineSuffix( + const printed = lineSuffix( concat([hardline, isLineBeforeEmpty ? hardline : "", contents]) ); + + if ( + options.printer.shouldIndentComment && + options.printer.shouldIndentComment(commentPath) + ) { + return indent(printed); + } + + if ( + options.printer.shouldDedentComment && + options.printer.shouldDedentComment(commentPath) + ) { + return dedent(printed); + } + + return printed; } let printed = concat([" ", contents]); diff --git a/tests/js/conditional/__snapshots__/jsfmt.spec.js.snap b/tests/js/conditional/__snapshots__/jsfmt.spec.js.snap index 52296ec2f6fb..86b5fa8c1629 100644 --- a/tests/js/conditional/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/conditional/__snapshots__/jsfmt.spec.js.snap @@ -44,6 +44,63 @@ const { configureStore } = process.env.NODE_ENV === "production" ? require("./configureProdStore") // a : require("./configureDevStore"); // b +test + // ? + // comment + ? foo + : bar; + +test ? + // comment + foo + : bar; + +test ? + // comment + first + : test + // comment + ? first + : second; + +test + /* comment + comment + comment + comment */ + ? foo + : bar; + +test ? + /* comment + comment + comment + comment */ + foo + : bar; + +test + // ? + // comment + ? foo + : test2 + // comment + // comment + ? foo + : bar; + +test + /* comment + comment + comment + comment */ + ? foo + : test + /* comment + comment + comment */ + ? foo + : bar; test /* comment comment comment @@ -111,6 +168,26 @@ test test ? test /* c c */? foo : bar : bar; +(async () => { + (await test) + // comment + ? + foo : bar; +}); + +(test) +// comment +? +foo : bar; + +(test) ? +// comment +foo : bar; + +test ? +// comment +foo : bar; + =====================================output===================================== var inspect = 4 === util.inspect.length @@ -135,8 +212,8 @@ var inspect = }; const extractTextPluginOptions = shouldUseRelativeAssetPaths - ? // Making sure that the publicPath goes back to to build folder. - { publicPath: Array(cssFilename.split("/").length).join("../") } + // Making sure that the publicPath goes back to to build folder. + ? { publicPath: Array(cssFilename.split("/").length).join("../") } : {}; const extractTextPluginOptions2 = shouldUseRelativeAssetPaths @@ -153,6 +230,63 @@ const { configureStore } = ? require("./configureProdStore") // a : require("./configureDevStore"); // b +test + // ? + // comment + ? foo + : bar; + +test + ? // comment + foo + : bar; + +test + ? // comment + first + : test + // comment + ? first + : second; + +test + /* comment + comment + comment + comment */ + ? foo + : bar; + +test + ? /* comment + comment + comment + comment */ + foo + : bar; + +test + // ? + // comment + ? foo + : test2 + // comment + // comment + ? foo + : bar; + +test + /* comment + comment + comment + comment */ + ? foo + : test + /* comment + comment + comment */ + ? foo + : bar; test /* comment comment comment @@ -218,6 +352,28 @@ c */ : bar : bar; +async () => { + (await test) + // comment + ? foo + : bar; +}; + +test + // comment + ? foo + : bar; + +test + ? // comment + foo + : bar; + +test + ? // comment + foo + : bar; + ================================================================================ `; diff --git a/tests/js/conditional/comments.js b/tests/js/conditional/comments.js index 84d735742d6b..2813f09d933e 100644 --- a/tests/js/conditional/comments.js +++ b/tests/js/conditional/comments.js @@ -36,6 +36,63 @@ const { configureStore } = process.env.NODE_ENV === "production" ? require("./configureProdStore") // a : require("./configureDevStore"); // b +test + // ? + // comment + ? foo + : bar; + +test ? + // comment + foo + : bar; + +test ? + // comment + first + : test + // comment + ? first + : second; + +test + /* comment + comment + comment + comment */ + ? foo + : bar; + +test ? + /* comment + comment + comment + comment */ + foo + : bar; + +test + // ? + // comment + ? foo + : test2 + // comment + // comment + ? foo + : bar; + +test + /* comment + comment + comment + comment */ + ? foo + : test + /* comment + comment + comment */ + ? foo + : bar; test /* comment comment comment @@ -102,3 +159,23 @@ test test ? test /* c c */? foo : bar : bar; + +(async () => { + (await test) + // comment + ? + foo : bar; +}); + +(test) +// comment +? +foo : bar; + +(test) ? +// comment +foo : bar; + +test ? +// comment +foo : bar; diff --git a/tests/typescript/conditional-types/__snapshots__/jsfmt.spec.js.snap b/tests/typescript/conditional-types/__snapshots__/jsfmt.spec.js.snap index d109f4511d53..58467c69add8 100644 --- a/tests/typescript/conditional-types/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript/conditional-types/__snapshots__/jsfmt.spec.js.snap @@ -6,6 +6,55 @@ parsers: ["typescript"] printWidth: 80 | printWidth =====================================input====================================== +type A = test extends B + // ? + // comment + ? foo + : bar; + +type A = test extends B + /* comment + comment + comment + comment */ + ? foo + : bar; + +type A = test extends B + // ? + // comment + ? foo + : test2 extends C + // comment + // comment + ? foo + : bar; + +type B = test extends B + /* comment + comment + comment + comment */ + ? foo + : test2 extends C + /* comment + comment + comment */ + ? foo + : bar; + +type A = B extends test ? + // comment + foo + : bar; + +type A = B extends test ? + // comment + first + : B extends test + // comment + ? first + : second; type A = B extends T ? // comment foo @@ -79,6 +128,55 @@ type T = test extends B ? test extends B /* c c */? foo : bar : bar; =====================================output===================================== +type A = test extends B + // ? + // comment + ? foo + : bar; + +type A = test extends B + /* comment + comment + comment + comment */ + ? foo + : bar; + +type A = test extends B + // ? + // comment + ? foo + : test2 extends C + // comment + // comment + ? foo + : bar; + +type B = test extends B + /* comment + comment + comment + comment */ + ? foo + : test2 extends C + /* comment + comment + comment */ + ? foo + : bar; + +type A = B extends test + ? // comment + foo + : bar; + +type A = B extends test + ? // comment + first + : B extends test + // comment + ? first + : second; type A = B extends T ? // comment foo diff --git a/tests/typescript/conditional-types/comments.ts b/tests/typescript/conditional-types/comments.ts index bcbe41939355..5706d6ea23ce 100644 --- a/tests/typescript/conditional-types/comments.ts +++ b/tests/typescript/conditional-types/comments.ts @@ -1,3 +1,52 @@ +type A = test extends B + // ? + // comment + ? foo + : bar; + +type A = test extends B + /* comment + comment + comment + comment */ + ? foo + : bar; + +type A = test extends B + // ? + // comment + ? foo + : test2 extends C + // comment + // comment + ? foo + : bar; + +type B = test extends B + /* comment + comment + comment + comment */ + ? foo + : test2 extends C + /* comment + comment + comment */ + ? foo + : bar; + +type A = B extends test ? + // comment + foo + : bar; + +type A = B extends test ? + // comment + first + : B extends test + // comment + ? first + : second; type A = B extends T ? // comment foo