Stabilize comment around break and continue in no-semi mode (#7161)

Co-authored-by: fisker <lionkay@gmail.com>

changelog_unreleased/javascript/7161.md

@@ -0,0 +1,39 @@
+#### Stabilize comment around `break` and `continue` in no-semi mode (#7161 by @thorn0, @fisker)
+
+<!-- prettier-ignore -->
+```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();
+  }
+}
+```

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);

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) {

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;
     }
   }
 }

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"]

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()
+  }
+}

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;

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 */

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"