Exclude semicolon from range of ExpressionStatement (#18736)

changelog_unreleased/javascript/18736.md

@@ -0,0 +1,18 @@
+#### Preserve empty line before `;` in `no-semi` mode (#18736 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+a
+
+;[].b()
+
+// Prettier stable (--no-semi)
+a
+;[].b()
+
+// Prettier main
+a
+
+;[].b()
+```

src/language-js/comments/handle-comments.js

@@ -195,6 +195,7 @@ function handleIfStatementComments({
   enclosingNode,
   followingNode,
   text,
+  options,
 }) {
   if (enclosingNode?.type !== "IfStatement" || !followingNode) {
     return false;
@@ -232,23 +233,35 @@ function handleIfStatementComments({
     precedingNode === enclosingNode.consequent &&
     followingNode === enclosingNode.alternate
   ) {
-    const maybeElseTokenIndex = getNextNonSpaceNonCommentCharacterIndex(
-      text,
-      locEnd(enclosingNode.consequent),
+    const start = locStart(enclosingNode);
+    const end = locEnd(enclosingNode);
+    const ifStatementTxt = getTextWithoutComments(options, start, end);
+    let elseTokenIndex = getNextNonSpaceNonCommentCharacterIndex(
+      ifStatementTxt,
+      locEnd(enclosingNode.consequent) - start,
     );
+
+    if (elseTokenIndex !== false && ifStatementTxt[elseTokenIndex] === ";") {
+      elseTokenIndex = getNextNonSpaceNonCommentCharacterIndex(
+        ifStatementTxt,
+        elseTokenIndex + 1,
+      );
+    }
     const isElseToken =
-      maybeElseTokenIndex !== false &&
-      text.slice(maybeElseTokenIndex, maybeElseTokenIndex + 4) === "else";
+      elseTokenIndex !== false &&
+      ifStatementTxt.slice(elseTokenIndex, elseTokenIndex + 4) === "else";
 
     if (!isElseToken) {
       addTrailingComment(precedingNode, comment);
       return true;
     }
 
+    elseTokenIndex += start;
+
     // if comment is positioned between the `else` token and its body
     if (
       followingNode.type === "BlockStatement" &&
-      locStart(comment) >= maybeElseTokenIndex &&
+      locStart(comment) >= elseTokenIndex &&
       locEnd(comment) <= locStart(followingNode)
     ) {
       addLeadingComment(followingNode, comment);
@@ -261,7 +274,7 @@ function handleIfStatementComments({
     //   if (cond) a;
     //   else /* foo */ b;
     if (
-      locStart(comment) < maybeElseTokenIndex ||
+      locStart(comment) < elseTokenIndex ||
       enclosingNode.alternate.type === "BlockStatement"
     ) {
       if (precedingNode.type === "BlockStatement") {

src/language-js/loc.js

@@ -43,6 +43,15 @@ function locEnd(node) {
     return locEnd(node.alternate ?? node.consequent);
   }
 
+  if (
+    (node.type === "ExpressionStatement" || node.type === "Directive") &&
+    // @ts-expect-error -- Added in postprocess.js
+    node.__end
+  ) {
+    // @ts-expect-error -- safe
+    return node.__end;
+  }
+
   return locEndWithFullText(node);
 }
 

src/language-js/massage-ast/index.js

@@ -28,6 +28,8 @@ const ignoredProperties = new Set([
   // Hermes
   "trailingComma",
   "docblock",
+  // Prettier
+  "__end",
 ]);
 
 const removeTemplateElementsValue = (node) => {

src/language-js/parse/postprocess/index.js

@@ -1,7 +1,8 @@
 import * as assert from "#universal/assert";
-import { locEnd, locStart } from "../../loc.js";
+import { locEnd, locEndWithFullText, locStart } from "../../loc.js";
 import createTypeCheckFunction from "../../utilities/create-type-check-function.js";
 import getRaw from "../../utilities/get-raw.js";
+import getTextWithoutComments from "../../utilities/get-text-without-comments.js";
 import isBlockComment from "../../utilities/is-block-comment.js";
 import isLineComment from "../../utilities/is-line-comment.js";
 import isTypeCastComment from "../../utilities/is-type-cast-comment.js";
@@ -167,6 +168,11 @@ function postprocess(ast, options) {
             node.elementTypes = node.types;
           }
           break;
+
+        case "Directive":
+        case "ExpressionStatement":
+          addExpressionStatementEnd(node, { comments, text });
+          break;
       }
     },
     onLeave(node) {
@@ -254,4 +260,16 @@ function assertRaw(node, text) {
   assert.equal(raw, text.slice(locStart(node), locEnd(node)));
 }
 
+function addExpressionStatementEnd(node, { comments, text: originalText }) {
+  const start = locStart(node);
+  const end = locEndWithFullText(node);
+  const text = getTextWithoutComments(
+    { [Symbol.for("comments")]: comments, originalText },
+    start,
+    end,
+  );
+  const cleaned = (text.at(-1) === ";" ? text.slice(0, -1) : text).trimEnd();
+  node.__end = end - (text.length - cleaned.length);
+}
+
 export default postprocess;

src/language-js/print/ignored.js

@@ -8,7 +8,9 @@ function printIgnored(path, options /* , print*/) {
     options.semi &&
     (node.type === "BreakStatement" ||
       node.type === "ContinueStatement" ||
-      node.type === "VariableDeclaration")
+      node.type === "VariableDeclaration" ||
+      ((node.type === "ExpressionStatement" || node.type === "Directive") &&
+        node.__end))
   ) {
     text += ";";
   }

src/main/range.js

@@ -65,8 +65,9 @@ function findNodeAtOffset(
   predicate,
   ancestors = [],
   type,
+  locFunctions,
 ) {
-  const { locStart, locEnd } = options;
+  const { locStart, locEnd } = locFunctions;
   const start = locStart(node);
   const end = locEnd(node);
 
@@ -97,6 +98,7 @@ function findNodeAtOffset(
       predicate,
       nodeAndAncestors,
       type,
+      locFunctions,
     );
     if (childAndAncestors) {
       return childAndAncestors;
@@ -206,6 +208,8 @@ function calculateRange(text, opts, ast) {
     }
   }
 
+  const locFunctions =
+    opts.printer.features?.experimental_locForRangeFormat ?? opts;
   const startNodeAndAncestors = findNodeAtOffset(
     ast,
     start,
@@ -213,6 +217,7 @@ function calculateRange(text, opts, ast) {
     (node, parentNode) => isSourceElement(opts, node, parentNode),
     [],
     "rangeStart",
+    locFunctions,
   );
   if (!startNodeAndAncestors) {
     return;
@@ -229,6 +234,7 @@ function calculateRange(text, opts, ast) {
           (node) => isSourceElement(opts, node),
           [],
           "rangeEnd",
+          locFunctions,
         );
   if (!endNodeAndAncestors) {
     return;
@@ -251,9 +257,7 @@ function calculateRange(text, opts, ast) {
     );
   }
 
-  const { locStart, locEnd } =
-    opts.printer.features?.experimental_locForRangeFormat ?? opts;
-
+  const { locStart, locEnd } = locFunctions;
   return [
     Math.min(locStart(startNode), locStart(endNode)),
     Math.max(locEnd(startNode), locEnd(endNode)),

tests/config/run-format-test.js

@@ -22,11 +22,6 @@ const RANGE_END_PLACEHOLDER = "<<<PRETTIER_RANGE_END>>>";
 // TODO: these test files need fix
 const unstableTests = new Map(
   [
-    ["js/comments/dangling_array.js", (options) => options.semi === false],
-    ["js/comments/jsx.js", (options) => options.semi === false],
-    ["js/no-semi/comments.js", (options) => options.semi === false],
-    ["flow/no-semi/comments.js", (options) => options.semi === false],
-    ["js/ignore/semi/asi.js", (options) => options.semi === false],
     ["js/identifier/parentheses/let.js", (options) => options.semi === false],
     "js/comments/return-statement.js",
     "js/comments/tagged-template-literal.js",
@@ -53,6 +48,8 @@ const unstableTests = new Map(
       (options) => options.objectWrap !== "collapse",
     ],
     "typescript/call/callee-comments.ts",
+    "js/arrows/arrow-chain-with-trailing-comments.js",
+    "typescript/as/comments/18160.ts",
   ].map((fixture) => {
     const [file, isUnstable = () => true] = Array.isArray(fixture)
       ? fixture

tests/format/js/directives/comments/__snapshots__/format.test.js.snap

@@ -84,7 +84,7 @@ printWidth: 80
 =====================================input======================================
 'use strict' /* comment */;
 =====================================output=====================================
-"use strict" /* comment */;
+"use strict"; /* comment */
 
 ================================================================================
 `;
@@ -118,7 +118,7 @@ function foo() {
 }
 =====================================output=====================================
 function foo() {
-  "use strict" /* comment */;
+  "use strict"; /* comment */
 }
 
 ================================================================================

tests/format/js/ignore/__snapshots__/format.test.js.snap

@@ -405,7 +405,7 @@ function HelloWorld(x) {
     // eslint-disable-next-line
     x.a |
     x.b
-  ).call(null)
+  ).call(null);
 }
 
 ================================================================================

tests/format/js/ignore/semi/__snapshots__/format.test.js.snap

@@ -14,7 +14,7 @@ foo;
 =====================================output=====================================
 foo
 // prettier-ignore
-;[1].map();
+;[1].map()
 
 ================================================================================
 `;
@@ -56,12 +56,12 @@ function foo() {
 
 =====================================output=====================================
 // prettier-ignore
-'use strict';
+'use strict'
 ;[].forEach()
 
 function foo() {
   // prettier-ignore
-  'use strict';
+  'use strict'
   ;[].forEach()
 }
 

tests/format/js/no-semi/__snapshots__/format.test.js.snap

@@ -562,6 +562,83 @@ x;
 ================================================================================
 `;
 
+exports[`issue-1510.js - {"semi":false} format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow"]
+printWidth: 80
+semi: false
+                                                                                | printWidth
+=====================================input======================================
+a;
+
+[].b();
+
+a; // comment
+
+[].b();
+
+a;
+
+// comment
+
+[].b();
+
+=====================================output=====================================
+a
+
+;[].b()
+
+a // comment
+
+;[].b()
+
+a
+
+// comment
+
+;[].b()
+
+================================================================================
+`;
+
+exports[`issue-1510.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+a;
+
+[].b();
+
+a; // comment
+
+[].b();
+
+a;
+
+// comment
+
+[].b();
+
+=====================================output=====================================
+a;
+
+[].b();
+
+a; // comment
+
+[].b();
+
+a;
+
+// comment
+
+[].b();
+
+================================================================================
+`;
+
 exports[`issue2006.js - {"semi":false} format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow"]

tests/format/js/no-semi/issue-1510.js

@@ -0,0 +1,13 @@
+a;
+
+[].b();
+
+a; // comment
+
+[].b();
+
+a;
+
+// comment
+
+[].b();

tests/format/markdown/markdown/__snapshots__/format.test.js.snap

@@ -1531,7 +1531,7 @@ matrix(
   1, 0, 0,
   0, 1, 0,
   0, 0, 1
-)
+);
 \`\`\`
 
 will be transformed to:
@@ -1544,7 +1544,7 @@ matrix(
   1, 0, 0,
   0, 1, 0,
   0, 0, 1
-)
+);
 \`\`\`
 
 ## Options
@@ -3503,7 +3503,7 @@ matrix(
   1, 0, 0,
   0, 1, 0,
   0, 0, 1
-)
+);
 \`\`\`
 
 will be transformed to:
@@ -3516,7 +3516,7 @@ matrix(
   1, 0, 0,
   0, 1, 0,
   0, 0, 1
-)
+);
 \`\`\`
 
 ## Options

tests/format/typescript/as/as-const/__snapshots__/format.test.js.snap

@@ -24,7 +24,7 @@ const;
 =====================================output=====================================
 1 /* comment */ as const;
 1 /* comment */ as const;
-1 as const /* comment */;
+1 as const; /* comment */
 1 as const; /* comment */
 
 1 as const; // comment

tests/format/typescript/webhost/__snapshots__/format.test.js.snap

@@ -121,7 +121,7 @@ namespace TypeScript.WebTsc {
     const fso = new RealActiveXObject("Scripting.FileSystemObject");
 
     const fileStream = new ActiveXObject("ADODB.Stream");
-    fileStream.Type = 2 /*text*/;
+    fileStream.Type = 2; /*text*/
 
     const args: string[] = [];
     for (let i = 0; i < WScript.Arguments.length; i++) {