Improve comment printing around empty statement (#18108)

changelog_unreleased/javascript/18108.md

@@ -0,0 +1,25 @@
+#### Improve comment printing around empty statement (#18108 by @fisker)
+
+<!-- prettier-ignore -->
+```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 */ ;
+```

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

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

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

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

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

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;

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 */ ;
 
 ================================================================================
 `;

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

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

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

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
+;
+
 ================================================================================
 `;
 

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

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 */:;
+
 ================================================================================
 `;

tests/format/js/label/empty_label.js

@@ -1,2 +1,6 @@
 a:;
 b
+
+a: /* comment */;
+a: ;/* comment */
+a /* comment */:;