diff --git a/changelog_unreleased/javascript/18108.md b/changelog_unreleased/javascript/18108.md new file mode 100644 index 000000000..05e03e671 --- /dev/null +++ b/changelog_unreleased/javascript/18108.md @@ -0,0 +1,25 @@ +#### Improve comment printing around empty statement (#18108 by @fisker) + + +```jsx +// Input +for ( + index = 0; + doSomething(foo[index]) !== bar && doSomething(foo[index]) !== baz; + index ++ +) /* No op */; + +// Prettier stable +for ( + index = 0; + doSomething(foo[index]) !== bar && doSomething(foo[index]) !== baz; + index++ /* No op */ +); + +// Prettier main +for ( + index = 0; + doSomething(foo[index]) !== bar && doSomething(foo[index]) !== baz; + index++ +) /* No op */ ; +``` diff --git a/src/language-js/clean.js b/src/language-js/clean.js index 6fc821c41..d49d36539 100644 --- a/src/language-js/clean.js +++ b/src/language-js/clean.js @@ -1,5 +1,6 @@ import { isArrayExpression, + isMeaningfulEmptyStatement, isNumericLiteral, isStringLiteral, } from "./utils/index.js"; @@ -26,7 +27,7 @@ const removeTemplateElementsValue = (node) => { } }; -function clean(original, cloned) { +function clean(original, cloned, parent) { if (original.type === "Program") { delete cloned.sourceType; } @@ -39,7 +40,10 @@ function clean(original, cloned) { } // We remove extra `;` and add them when needed - if (original.type === "EmptyStatement") { + if ( + original.type === "EmptyStatement" && + !isMeaningfulEmptyStatement({ node: original, parent }) + ) { return null; } diff --git a/src/language-js/comments/printer-methods.js b/src/language-js/comments/printer-methods.js index daf0ab056..a3bc7f412 100644 --- a/src/language-js/comments/printer-methods.js +++ b/src/language-js/comments/printer-methods.js @@ -4,6 +4,7 @@ import { getFunctionParameters, hasNodeIgnoreComment, isJsxElement, + isMeaningfulEmptyStatement, isMethod, isUnionType, } from "../utils/index.js"; @@ -14,7 +15,6 @@ import { */ const isNodeCantAttachComment = createTypeCheckFunction([ - "EmptyStatement", "TemplateElement", // There is no similar node in Babel AST // ```ts @@ -84,12 +84,20 @@ const isClassMethodCantAttachComment = (node, [parent]) => @returns {boolean} */ function canAttachComment(node, ancestors) { - return !( + if ( isNodeCantAttachComment(node) || isChildWontPrint(node, ancestors) || // @ts-expect-error -- safe isClassMethodCantAttachComment(node, ancestors) - ); + ) { + return false; + } + + if (node.type === "EmptyStatement") { + return isMeaningfulEmptyStatement({ node, parent: ancestors[0] }); + } + + return true; } /** diff --git a/src/language-js/print/estree.js b/src/language-js/print/estree.js index d8dd34d1b..2ccd0309c 100644 --- a/src/language-js/print/estree.js +++ b/src/language-js/print/estree.js @@ -18,6 +18,7 @@ import { isArrayExpression, isCallExpression, isLiteral, + isMeaningfulEmptyStatement, isMemberExpression, isMethod, isNextLineEmpty, @@ -101,8 +102,6 @@ function printEstree(path, options, print, args) { // Babel extension. case "File": return printHtmlBinding(path, options, print) ?? print("program"); - case "EmptyStatement": - return ""; case "ExpressionStatement": return printExpressionStatement(path, options, print); @@ -493,11 +492,11 @@ function printEstree(path, options, print, args) { options.semi ? ";" : "", ]; case "LabeledStatement": - if (node.body.type === "EmptyStatement") { - return [print("label"), ":;"]; - } - - return [print("label"), ": ", print("body")]; + return [ + print("label"), + `:${node.body.type === "EmptyStatement" && !hasComment(node.body, CommentCheckFlags.Leading) ? "" : " "}`, + print("body"), + ]; case "TryStatement": return [ "try ", @@ -627,6 +626,11 @@ function printEstree(path, options, print, args) { case "VoidPattern": return "void"; + case "EmptyStatement": + if (isMeaningfulEmptyStatement(path)) { + return ";"; + } + // Fall through case "InterpreterDirective": // Printed as comment default: /* c8 ignore next */ diff --git a/src/language-js/print/misc.js b/src/language-js/print/misc.js index bf08be750..e8ad0e422 100644 --- a/src/language-js/print/misc.js +++ b/src/language-js/print/misc.js @@ -1,5 +1,10 @@ import { indent, line } from "../../document/builders.js"; -import { isCallExpression, isMemberExpression } from "../utils/index.js"; +import { + CommentCheckFlags, + hasComment, + isCallExpression, + isMemberExpression, +} from "../utils/index.js"; import { printTypeAnnotationProperty } from "./type-annotation.js"; /** @@ -96,7 +101,7 @@ function printBindExpressionCallee(path, options, print) { function adjustClause(node, clause, forceSpace) { if (node.type === "EmptyStatement") { - return ";"; + return hasComment(node, CommentCheckFlags.Leading) ? [" ", clause] : clause; } if (node.type === "BlockStatement" || forceSpace) { diff --git a/src/language-js/utils/index.js b/src/language-js/utils/index.js index 61672144d..bbd1bc05b 100644 --- a/src/language-js/utils/index.js +++ b/src/language-js/utils/index.js @@ -1144,4 +1144,5 @@ export { shouldPrintComma, startsWithNoLookaheadToken, }; +export { default as isMeaningfulEmptyStatement } from "./is-meaningful-empty-statement.js"; export { default as isNodeMatches } from "./is-node-matches.js"; diff --git a/src/language-js/utils/is-meaningful-empty-statement.js b/src/language-js/utils/is-meaningful-empty-statement.js new file mode 100644 index 000000000..e5d1242a2 --- /dev/null +++ b/src/language-js/utils/is-meaningful-empty-statement.js @@ -0,0 +1,26 @@ +/* The argument is a duck type of AstPath, so we can use it in comment attach and ast massage */ +function isMeaningfulEmptyStatement({ node, parent }) { + if (node?.type !== "EmptyStatement") { + return false; + } + + if (parent.type === "IfStatement") { + return parent.consequent === node || parent.alternate === node; + } + + if ( + parent.type === "DoWhileStatement" || + parent.type === "ForInStatement" || + parent.type === "ForOfStatement" || + parent.type === "ForStatement" || + parent.type === "LabeledStatement" || + parent.type === "WithStatement" || + parent.type === "WhileStatement" + ) { + return parent.body === node; + } + + return false; +} + +export default isMeaningfulEmptyStatement; diff --git a/tests/format/js/comments/__snapshots__/format.test.js.snap b/tests/format/js/comments/__snapshots__/format.test.js.snap index a4835402b..9bc3d111f 100644 --- a/tests/format/js/comments/__snapshots__/format.test.js.snap +++ b/tests/format/js/comments/__snapshots__/format.test.js.snap @@ -1703,11 +1703,10 @@ for // comment for /* comment */(;;); =====================================output===================================== -// comment -for (;;); +for (;;) // comment +; -/* comment */ -for (;;); +for (;;) /* comment */ ; ================================================================================ `; @@ -1724,11 +1723,10 @@ for // comment for /* comment */(;;); =====================================output===================================== -// comment -for (;;); +for (;;) // comment +; -/* comment */ -for (;;); +for (;;) /* comment */ ; ================================================================================ `; diff --git a/tests/format/js/explicit-resource-management/__snapshots__/format.test.js.snap b/tests/format/js/explicit-resource-management/__snapshots__/format.test.js.snap index 9318ad720..efe996c67 100644 --- a/tests/format/js/explicit-resource-management/__snapshots__/format.test.js.snap +++ b/tests/format/js/explicit-resource-management/__snapshots__/format.test.js.snap @@ -34,7 +34,7 @@ printWidth: 80 /*1*/ for /* 2 */ await /*3*/ ( /*4*/ using /*5*/ fo /*6*/ of /*7*/ of /*8*/) /*9*/; =====================================output===================================== -/*1*/ for await (/* 2 */ /*3*/ /*4*/ using /*5*/ fo /*6*/ of /*7*/ of /*8*/ /*9*/); +/*1*/ for await (/* 2 */ /*3*/ /*4*/ using /*5*/ fo /*6*/ of /*7*/ of /*8*/) /*9*/ ; ================================================================================ `; @@ -649,17 +649,16 @@ async function f() { } { /*0*/ for ( - /*1*/ /*2*/ await using /*3*/ /*4*/ b /*5*/ = - /*6*/ x /*7*/ /*8*/ /*9*/ /*10*/; + /*1*/ /*2*/ await using /*3*/ /*4*/ b /*5*/ = /*6*/ x /*7*/ /*8*/ /*9*/; ; - ); + ) /*10*/ ; } { - /*0*/ for (/*1*/ /*2*/ await using /*3*/ /*4*/ b /*5*/ of /*6*/ x /*7*/ /*8*/); + /*0*/ for (/*1*/ /*2*/ await using /*3*/ /*4*/ b /*5*/ of /*6*/ x /*7*/) /*8*/ ; } { - /*0*/ for await (/*1*/ /*2*/ /*3*/ await using /*4*/ /*5*/ b /*6*/ of /*7*/ x /*8*/ /*9*/); + /*0*/ for await (/*1*/ /*2*/ /*3*/ await using /*4*/ /*5*/ b /*6*/ of /*7*/ x /*8*/) /*9*/ ; } } diff --git a/tests/format/js/for/9812-unstable.js b/tests/format/js/for/9812-unstable.js index fb334ddb5..c7ec7608e 100644 --- a/tests/format/js/for/9812-unstable.js +++ b/tests/format/js/for/9812-unstable.js @@ -5,3 +5,7 @@ for(x of y) for(x of y) // comment {} + +for(x of y) +// comment +; diff --git a/tests/format/js/for/9812.js b/tests/format/js/for/9812.js index d9c6d5316..fdbd503b2 100644 --- a/tests/format/js/for/9812.js +++ b/tests/format/js/for/9812.js @@ -2,10 +2,6 @@ for (const p of ['fullName', 'organ', 'position', 'rank']) // comment form.setValue(`${prefix}.data.${p}`, response[p]) -for(x of y) -// comment -; - for(x of y) // comment bar(); diff --git a/tests/format/js/for/__snapshots__/format.test.js.snap b/tests/format/js/for/__snapshots__/format.test.js.snap index 350175e25..30b864e01 100644 --- a/tests/format/js/for/__snapshots__/format.test.js.snap +++ b/tests/format/js/for/__snapshots__/format.test.js.snap @@ -10,10 +10,6 @@ for (const p of ['fullName', 'organ', 'position', 'rank']) // comment form.setValue(\`\${prefix}.data.\${p}\`, response[p]) -for(x of y) -// comment -; - for(x of y) // comment bar(); @@ -67,9 +63,6 @@ for (const p of ["fullName", "organ", "position", "rank"]) // comment form.setValue(\`\${prefix}.data.\${p}\`, response[p]); -// comment -for (x of y); - for (x of y) // comment bar(); @@ -135,6 +128,10 @@ for(x of y) // comment {} +for(x of y) +// comment +; + =====================================output===================================== for (x of y) // comment { @@ -145,6 +142,9 @@ for (x of y) // comment { } +for (x of y) // comment +; + ================================================================================ `; diff --git a/tests/format/js/if/__snapshots__/format.test.js.snap b/tests/format/js/if/__snapshots__/format.test.js.snap index 28495b8a0..39af73281 100644 --- a/tests/format/js/if/__snapshots__/format.test.js.snap +++ b/tests/format/js/if/__snapshots__/format.test.js.snap @@ -254,7 +254,7 @@ function a() { else return 2; if (a) - if (b /* comment 6f */); + if (b); /* comment 6f */ else return 2; } diff --git a/tests/format/js/label/__snapshots__/format.test.js.snap b/tests/format/js/label/__snapshots__/format.test.js.snap index de94d2e41..dcc55812a 100644 --- a/tests/format/js/label/__snapshots__/format.test.js.snap +++ b/tests/format/js/label/__snapshots__/format.test.js.snap @@ -55,9 +55,17 @@ printWidth: 80 a:; b +a: /* comment */; +a: ;/* comment */ +a /* comment */:; + =====================================output===================================== a:; b; +a: /* comment */ ; +a:; /* comment */ +a /* comment */:; + ================================================================================ `; diff --git a/tests/format/js/label/empty_label.js b/tests/format/js/label/empty_label.js index 294408ee3..24020c100 100644 --- a/tests/format/js/label/empty_label.js +++ b/tests/format/js/label/empty_label.js @@ -1,2 +1,6 @@ a:; b + +a: /* comment */; +a: ;/* comment */ +a /* comment */:;