From f5e3980c3085f6fb2b166c8e3f91e8db317d6eb2 Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Sat, 17 Jan 2026 23:38:52 +0200 Subject: [PATCH] Stabilize comment around `break` and `continue` in no-semi mode (#7161) Co-authored-by: fisker --- changelog_unreleased/javascript/7161.md | 39 ++++++++ src/language-js/comments/handle-comments.js | 33 +++---- src/language-js/parse/postprocess/index.js | 17 ++++ src/main/comments/attach.js | 35 +++++-- .../__snapshots__/format.test.js.snap | 97 ++++++++++++++++++- .../comments/break-continue-statements-3.js | 16 +++ .../js/for/__snapshots__/format.test.js.snap | 38 ++++---- ...ntinue-and-break-comment-without-blocks.js | 16 +-- .../debug-print-comments.js.snap | 3 - 9 files changed, 232 insertions(+), 62 deletions(-) create mode 100644 changelog_unreleased/javascript/7161.md create mode 100644 tests/format/js/comments/break-continue-statements-3.js diff --git a/changelog_unreleased/javascript/7161.md b/changelog_unreleased/javascript/7161.md new file mode 100644 index 000000000..c2ee5f908 --- /dev/null +++ b/changelog_unreleased/javascript/7161.md @@ -0,0 +1,39 @@ +#### Stabilize comment around `break` and `continue` in no-semi mode (#7161 by @thorn0, @fisker) + + +```jsx +// Input +for (;;) { + if (condition) { + break; // breaking comment + + (possibleArray || []).sort() + } +} + +// Prettier stable (--no-semi, first format) +for (;;) { + if (condition) { + break // breaking comment + + ;(possibleArray || []).sort() + } +} + +// Prettier stable (--no-semi, second format) +for (;;) { + if (condition) { + break // breaking comment + ;(possibleArray || []).sort() + } +} + +// Prettier main (--no-semi, both first and second format) +for (;;) { + if (condition) { + break; // breaking comment + + (possibleArray || []).sort(); + } +} +``` diff --git a/src/language-js/comments/handle-comments.js b/src/language-js/comments/handle-comments.js index abaaca59b..c52dab6e1 100644 --- a/src/language-js/comments/handle-comments.js +++ b/src/language-js/comments/handle-comments.js @@ -44,6 +44,7 @@ import isTypeCastComment from "../utilities/is-type-cast-comment.js"; options: any, ast: NodeMap["File"] | NodeMap["Program"], isLastComment: boolean, + placement: "ownLine" | "endOfLine" | "remaining" }} CommentContext */ @@ -79,7 +80,6 @@ function handleOwnLineComment(context) { handleAssignmentPatternComments, handleMethodNameComments, handleLabeledStatementComments, - handleBreakAndContinueStatementComments, handleNestedConditionalExpressionComments, handleCommentsInDestructuringPattern, handleTSMappedTypeComments, @@ -107,7 +107,6 @@ function handleEndOfLineComment(context) { handlePropertyComments, handleOnlyComments, handleVariableDeclaratorComments, - handleBreakAndContinueStatementComments, handleSwitchDefaultCaseComments, handleLastUnionElementInExpression, handleLastBinaryOperatorOperand, @@ -132,7 +131,6 @@ function handleRemainingComment(context) { handleOnlyComments, handleCommentAfterArrowParams, handleFunctionNameComments, - handleBreakAndContinueStatementComments, handleTSFunctionTrailingComments, handleBinaryCastExpressionComment, ].some((fn) => fn(context)); @@ -237,6 +235,14 @@ function handleIfStatementComments({ text, locEnd(enclosingNode.consequent), ); + const isElseToken = + maybeElseTokenIndex !== false && + text.slice(maybeElseTokenIndex, maybeElseTokenIndex + 4) === "else"; + + if (!isElseToken) { + addTrailingComment(precedingNode, comment); + return true; + } // if comment is positioned between the `else` token and its body if ( @@ -285,11 +291,6 @@ function handleIfStatementComments({ return true; } - if (followingNode.type === "IfStatement") { - addBlockOrNotComment(followingNode.consequent, comment); - return true; - } - // For comments positioned after the condition parenthesis in an if statement // before the consequent without brackets on, such as // if (a) /* comment */ true, @@ -540,6 +541,7 @@ const isPropertyLikeNode = createTypeCheckFunction([ "TSParameterProperty", ]); function handleMethodNameComments({ + placement, comment, precedingNode, enclosingNode, @@ -571,7 +573,7 @@ function handleMethodNameComments({ if ( precedingNode?.type === "Decorator" && isPropertyLikeNode(enclosingNode) && - (isLineComment(comment) || comment.placement === "ownLine") + (isLineComment(comment) || placement === "ownLine") ) { addTrailingComment(precedingNode, comment); return true; @@ -777,18 +779,6 @@ function handleLabeledStatementComments({ comment, enclosingNode }) { return false; } -function handleBreakAndContinueStatementComments({ comment, enclosingNode }) { - if ( - (enclosingNode?.type === "ContinueStatement" || - enclosingNode?.type === "BreakStatement") && - !enclosingNode.label - ) { - addTrailingComment(enclosingNode, comment); - return true; - } - return false; -} - function handleCallExpressionComments({ comment, precedingNode, @@ -903,6 +893,7 @@ function handleForComments({ comment, enclosingNode, followingNode }) { if ( (enclosingNode?.type === "ForInStatement" || enclosingNode?.type === "ForOfStatement") && + followingNode && followingNode !== enclosingNode.body ) { addLeadingComment(enclosingNode, comment); diff --git a/src/language-js/parse/postprocess/index.js b/src/language-js/parse/postprocess/index.js index 08fa714b9..2651081fd 100644 --- a/src/language-js/parse/postprocess/index.js +++ b/src/language-js/parse/postprocess/index.js @@ -173,6 +173,23 @@ function postprocess(ast, options) { node.elementTypes = node.types; } break; + + case "BreakStatement": + case "ContinueStatement": { + const start = locStart(node); + if (node.label) { + node.range = [start, locEnd(node.label)]; + } else { + node.range = [ + start, + start + + (node.type === "BreakStatement" + ? "break".length + : "continue".length), + ]; + } + break; + } } }, onLeave(node) { diff --git a/src/main/comments/attach.js b/src/main/comments/attach.js index 851cea323..8c576d4bd 100644 --- a/src/main/comments/attach.js +++ b/src/main/comments/attach.js @@ -139,8 +139,14 @@ function attachComments(ast, options) { options, ast, isLastComment: comments.length - 1 === index, + // TODO: Move placement here + placement: undefined, })); + // For easier debug, save these to comment even `avoidAstMutation` + const attachPropertiesToComment = + process.env.NODE_ENV !== "production" || !avoidAstMutation; + for (const [index, context] of decoratedComments.entries()) { const { comment, @@ -153,18 +159,28 @@ function attachComments(ast, options) { isLastComment, } = context; + const placement = isOwnLineComment(text, options, decoratedComments, index) + ? "ownLine" + : isEndOfLineComment(text, options, decoratedComments, index) + ? "endOfLine" + : "remaining"; + let args; if (avoidAstMutation) { + context.placement = placement; args = [context]; } else { - comment.enclosingNode = enclosingNode; - comment.precedingNode = precedingNode; - comment.followingNode = followingNode; args = [comment, text, options, ast, isLastComment]; } - if (isOwnLineComment(text, options, decoratedComments, index)) { - comment.placement = "ownLine"; + if (attachPropertiesToComment) { + comment.placement = placement; + comment.enclosingNode = enclosingNode; + comment.precedingNode = precedingNode; + comment.followingNode = followingNode; + } + + if (placement === "ownLine") { // If a comment exists on its own line, prefer a leading comment. // We also need to check if it's the first line of the file. if (handleOwnLineComment(...args)) { @@ -181,8 +197,7 @@ function attachComments(ast, options) { /* c8 ignore next */ addDanglingComment(ast, comment); } - } else if (isEndOfLineComment(text, options, decoratedComments, index)) { - comment.placement = "endOfLine"; + } else if (placement === "endOfLine") { if (handleEndOfLineComment(...args)) { // We're good } else if (precedingNode) { @@ -199,7 +214,8 @@ function attachComments(ast, options) { addDanglingComment(ast, comment); } } else { - comment.placement = "remaining"; + // Remaining + // eslint-disable-next-line no-lonely-if if (handleRemainingComment(...args)) { // We're good } else if (precedingNode && followingNode) { @@ -232,7 +248,7 @@ function attachComments(ast, options) { breakTies(tiesToBreak, options); - if (!avoidAstMutation) { + if (attachPropertiesToComment) { for (const comment of comments) { // These node references were useful for breaking ties, but we // don't need them anymore, and they create cycles in the AST that @@ -240,6 +256,7 @@ function attachComments(ast, options) { delete comment.precedingNode; delete comment.enclosingNode; delete comment.followingNode; + delete comment.placement; } } } diff --git a/tests/format/js/comments/__snapshots__/format.test.js.snap b/tests/format/js/comments/__snapshots__/format.test.js.snap index 4c88c9d61..94a046599 100644 --- a/tests/format/js/comments/__snapshots__/format.test.js.snap +++ b/tests/format/js/comments/__snapshots__/format.test.js.snap @@ -1400,9 +1400,9 @@ for (;;) { loop: for (;;) { break /* comment */ loop; - break loop /* comment */; + break loop; /* comment */ continue /* comment */ loop; - continue loop /* comment */; + continue loop; /* comment */ } ================================================================================ @@ -1605,6 +1605,99 @@ for (;;) { ================================================================================ `; +exports[`break-continue-statements-3.js - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +// https://github.com/prettier/prettier/issues/7126 +for (;;) { + if (condition){ continue + + // breaking comment + ;(possibleArray || []).sort() + } +} + +lbl: for (;;) { + if (condition){ continue lbl + + // breaking comment + ;(possibleArray || []).sort() + } +} + +=====================================output===================================== +// https://github.com/prettier/prettier/issues/7126 +for (;;) { + if (condition) { + continue + + // breaking comment + ;(possibleArray || []).sort() + } +} + +lbl: for (;;) { + if (condition) { + continue lbl + + // breaking comment + ;(possibleArray || []).sort() + } +} + +================================================================================ +`; + +exports[`break-continue-statements-3.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +// https://github.com/prettier/prettier/issues/7126 +for (;;) { + if (condition){ continue + + // breaking comment + ;(possibleArray || []).sort() + } +} + +lbl: for (;;) { + if (condition){ continue lbl + + // breaking comment + ;(possibleArray || []).sort() + } +} + +=====================================output===================================== +// https://github.com/prettier/prettier/issues/7126 +for (;;) { + if (condition) { + continue; + + // breaking comment + (possibleArray || []).sort(); + } +} + +lbl: for (;;) { + if (condition) { + continue lbl; + + // breaking comment + (possibleArray || []).sort(); + } +} + +================================================================================ +`; + exports[`call_comment.js - {"semi":false} format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/format/js/comments/break-continue-statements-3.js b/tests/format/js/comments/break-continue-statements-3.js new file mode 100644 index 000000000..b5c85630d --- /dev/null +++ b/tests/format/js/comments/break-continue-statements-3.js @@ -0,0 +1,16 @@ +// https://github.com/prettier/prettier/issues/7126 +for (;;) { + if (condition){ continue + + // breaking comment + ;(possibleArray || []).sort() + } +} + +lbl: for (;;) { + if (condition){ continue lbl + + // breaking comment + ;(possibleArray || []).sort() + } +} diff --git a/tests/format/js/for/__snapshots__/format.test.js.snap b/tests/format/js/for/__snapshots__/format.test.js.snap index e3a4fb72f..efbe39dc0 100644 --- a/tests/format/js/for/__snapshots__/format.test.js.snap +++ b/tests/format/js/for/__snapshots__/format.test.js.snap @@ -440,11 +440,11 @@ do { } while (true); label1: for (;;) { - continue label1 /* comment */; + continue label1; /* comment */ } label2: { - break label2 /* comment */; + break label2; /* comment */ } ================================================================================ @@ -741,19 +741,19 @@ for (;;) break ; for (const f of []) continue -// comment +// comment3 ; for (const f of []) break -// comment +// comment4 ; for (const f in {}) continue -// comment +// comment5 ; for (const f in {}) break -// comment +// comment6 ; for(;;) continue // comment @@ -801,19 +801,19 @@ for (;;) break ; for (const f of []) continue -/* comment */ +/* comment 33 */ ; for (const f of []) break -/* comment */ +/* comment 34 */ ; for (const f in {}) continue -/* comment */ +/* comment 35 */ ; for (const f in {}) break -/* comment */ +/* comment 36 */ ; label1: for (;;) continue label1 /* comment */ @@ -841,19 +841,19 @@ for (;;) for (const f of []) continue; - // comment + // comment3 for (const f of []) break; - // comment + // comment4 for (const f in {}) continue; - // comment + // comment5 for (const f in {}) break; - // comment + // comment6 for (;;) continue; // comment @@ -895,21 +895,21 @@ for (;;) for (const f of []) continue; - /* comment */ + /* comment 33 */ for (const f of []) break; - /* comment */ + /* comment 34 */ for (const f in {}) continue; - /* comment */ + /* comment 35 */ for (const f in {}) break; - /* comment */ + /* comment 36 */ -label1: for (;;) continue label1 /* comment */; +label1: for (;;) continue label1; /* comment */ label1: for (;;) continue label1; diff --git a/tests/format/js/for/continue-and-break-comment-without-blocks.js b/tests/format/js/for/continue-and-break-comment-without-blocks.js index d68bba16e..298663ccb 100644 --- a/tests/format/js/for/continue-and-break-comment-without-blocks.js +++ b/tests/format/js/for/continue-and-break-comment-without-blocks.js @@ -7,19 +7,19 @@ for (;;) break ; for (const f of []) continue -// comment +// comment3 ; for (const f of []) break -// comment +// comment4 ; for (const f in {}) continue -// comment +// comment5 ; for (const f in {}) break -// comment +// comment6 ; for(;;) continue // comment @@ -67,19 +67,19 @@ for (;;) break ; for (const f of []) continue -/* comment */ +/* comment 33 */ ; for (const f of []) break -/* comment */ +/* comment 34 */ ; for (const f in {}) continue -/* comment */ +/* comment 35 */ ; for (const f in {}) break -/* comment */ +/* comment 36 */ ; label1: for (;;) continue label1 /* comment */ diff --git a/tests/integration/__tests__/__snapshots__/debug-print-comments.js.snap b/tests/integration/__tests__/__snapshots__/debug-print-comments.js.snap index 647afcc39..bec3f81ed 100644 --- a/tests/integration/__tests__/__snapshots__/debug-print-comments.js.snap +++ b/tests/integration/__tests__/__snapshots__/debug-print-comments.js.snap @@ -12,7 +12,6 @@ exports[`prints information for debugging comment attachment with --debug-print- "start": { "line": 1, "column": 0, "index": 0 }, "end": { "line": 1, "column": 7, "index": 7 } }, - "placement": "endOfLine", "leading": true, "trailing": false, "nodeDescription": "ExpressionStatement" @@ -26,7 +25,6 @@ exports[`prints information for debugging comment attachment with --debug-print- "start": { "line": 2, "column": 16, "index": 24 }, "end": { "line": 2, "column": 23, "index": 31 } }, - "placement": "remaining", "leading": false, "trailing": true, "nodeDescription": "Identifier foo" @@ -40,7 +38,6 @@ exports[`prints information for debugging comment attachment with --debug-print- "start": { "line": 2, "column": 26, "index": 34 }, "end": { "line": 2, "column": 30, "index": 38 } }, - "placement": "remaining", "leading": false, "trailing": true, "nodeDescription": "ExpressionStatement"