Preserve blank line before comments above else (#18813)

changelog_unreleased/javascript/18813.md

@@ -0,0 +1,30 @@
+#### Fix comments above `else` (#18813 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+/* Case 1 */
+if (1) {
+}
+
+/* Case 2 */
+else if (2) {
+}
+
+// Prettier stable
+/* Case 1 */
+if (1) {
+} else if (2) {
+
+/* Case 2 */
+}
+
+// Prettier main
+/* Case 1 */
+if (1) {
+}
+
+/* Case 2 */
+else if (2) {
+}
+```

src/language-js/comments/attach/handle-if-statement-comments.js

@@ -93,12 +93,14 @@ function handleCommentsBetween({
     return true;
   }
 
+  const isConsequentBlockStatement = precedingNode.type === "BlockStatement";
+
   // Comments before `else`:
   // - treat as trailing comments of the consequent, if it's a BlockStatement
   // - treat as a dangling comment otherwise
-
   if (
-    isSingleLineComment(comment, text) &&
+    !isConsequentBlockStatement &&
+    isSingleLineComment &&
     // Comment and `precedingNode` are on same line
     !hasNewlineInRange(text, locEnd(precedingNode), locStart(comment))
   ) {
@@ -106,6 +108,7 @@ function handleCommentsBetween({
     //   if (cond1) expr1; // comment A
     //   else if (cond2) expr2; // comment A
     //   else expr3;
+
     addTrailingComment(precedingNode, comment);
     return true;
   }

src/language-js/print/if-statement.js

@@ -1,6 +1,9 @@
 import { group, hardline } from "../../document/index.js";
 import { printDanglingComments } from "../../main/comments/print.js";
-import { CommentCheckFlags, hasComment } from "../utilities/comments.js";
+import hasNewline from "../../utilities/has-newline.js";
+import { locEnd, locStart } from "../loc.js";
+import { CommentCheckFlags, getComments } from "../utilities/comments.js";
+import { isPreviousLineEmpty } from "../utilities/is-previous-line-empty.js";
 import { needsHardlineAfterDanglingComment } from "../utilities/needs-hardline-after-dangling-comment.js";
 import {
   printIfStatementAlternate,
@@ -15,35 +18,59 @@ import { printIfStatementCondition } from "./miscellaneous.js";
 
 function printIfStatement(path, options, print) {
   const { node } = path;
-  const consequent = printIfStatementConsequent(path, options, print);
   const opening = group([
     "if (",
     printIfStatementCondition(path, options, print),
     ")",
-    consequent,
+    printIfStatementConsequent(path, options, print),
   ]);
-  /** @type{Doc[]} */
+
+  if (!node.alternate) {
+    return opening;
+  }
+
+  const { consequent } = node;
+  const isConsequentBlockStatement = consequent.type === "BlockStatement";
+
+  /** @type {Doc[]} */
   const parts = [opening];
+  let needSpace = isConsequentBlockStatement;
+  if (!isConsequentBlockStatement) {
+    parts.push(hardline);
+    needSpace = false;
+  }
 
-  if (node.alternate) {
-    const commentOnOwnLine =
-      hasComment(
-        node.consequent,
-        CommentCheckFlags.Trailing | CommentCheckFlags.Line,
-      ) || needsHardlineAfterDanglingComment(node);
-    const elseOnSameLine =
-      node.consequent.type === "BlockStatement" && !commentOnOwnLine;
-    parts.push(elseOnSameLine ? " " : hardline);
+  const danglingComments = getComments(node, CommentCheckFlags.Dangling);
+  if (danglingComments.length > 0) {
+    const [firstComment] = danglingComments;
+
+    if (isPreviousLineEmpty(firstComment, options)) {
+      parts.push(isConsequentBlockStatement ? [hardline, hardline] : hardline);
+    } else if (
+      hasNewline(options.originalText, locStart(firstComment), {
+        backwards: true,
+      })
+    ) {
+      parts.push(isConsequentBlockStatement ? hardline : "");
+    } else {
+      parts.push(" ");
+    }
 
-    if (hasComment(node, CommentCheckFlags.Dangling)) {
     parts.push(
       printDanglingComments(path, options),
-        commentOnOwnLine ? hardline : " ",
+      needsHardlineAfterDanglingComment(node) ||
+        hasNewline(options.originalText, locEnd(danglingComments.at(-1)))
+        ? hardline
+        : " ",
     );
+    needSpace = false;
   }
 
-    parts.push("else", group(printIfStatementAlternate(path, options, print)));
-  }
+  parts.push(
+    needSpace ? " " : "",
+    "else",
+    group(printIfStatementAlternate(path, options, print)),
+  );
 
   return parts;
 }

src/language-js/utilities/comments.js

@@ -54,8 +54,8 @@ const getCommentTestFunction = (flags, fn) => {
 };
 /**
  * @param {Node} node
- * @param {number | function} [flags]
- * @param {function} [fn]
+ * @param {number | ((comment: Comment) => boolean)} [flags]
+ * @param {(comment: Comment) => boolean} [fn]
  * @returns {boolean}
  */
 function hasComment(node, flags, fn) {
@@ -68,8 +68,8 @@ function hasComment(node, flags, fn) {
 
 /**
  * @param {Node} node
- * @param {number | function} [flags]
- * @param {function} [fn]
+ * @param {number | ((comment: Comment) => boolean)} [flags]
+ * @param {(comment: Comment) => boolean} [fn]
  * @returns {Comment[]}
  */
 function getComments(node, flags, fn) {

src/language-js/utilities/is-previous-line-empty.js

@@ -0,0 +1,18 @@
+import isPreviousLineEmptyBeforeIndex from "../../utilities/is-previous-line-empty.js";
+import { locStart } from "../loc.js";
+
+/**
+@import {
+  Node,
+  Comment,
+} from "../types/estree.js";
+*/
+
+/**
+ * @param {Node | Comment} node
+ * @returns {boolean}
+ */
+const isPreviousLineEmpty = (node, { originalText }) =>
+  isPreviousLineEmptyBeforeIndex(originalText, locStart(node));
+
+export { isPreviousLineEmpty };

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

@@ -1,5 +1,141 @@
 // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
 
+exports[`blank-lines.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+// 11
+if (1) {
+  a();
+}
+
+// 12
+else if (2) {
+  a();
+}
+
+// 13
+else {
+  a();
+}
+
+/* 21 */
+if (1) {
+  a();
+}
+
+/* 22 */
+else if (2) {
+  a();
+}
+
+/* 23 */
+else {
+  a();
+}
+
+// 31
+if (1)
+  a();
+
+// 32
+else if (2)
+  a();
+
+// 33
+else
+  a();
+
+// 31
+if (1)
+  a();
+
+// 32
+else if (2)
+  a();
+
+// 33
+else
+  a();
+
+/* 41 */ if (1) {
+  a();
+}
+
+/* 42 */ else if (2) {
+  a();
+}
+
+/* 43 */ else {
+  a();
+}
+
+=====================================output=====================================
+// 11
+if (1) {
+  a();
+}
+
+// 12
+else if (2) {
+  a();
+}
+
+// 13
+else {
+  a();
+}
+
+/* 21 */
+if (1) {
+  a();
+}
+
+/* 22 */
+else if (2) {
+  a();
+}
+
+/* 23 */
+else {
+  a();
+}
+
+// 31
+if (1) a();
+
+// 32
+else if (2) a();
+
+// 33
+else a();
+
+// 31
+if (1) a();
+
+// 32
+else if (2) a();
+
+// 33
+else a();
+
+/* 41 */ if (1) {
+  a();
+}
+
+/* 42 */ else if (2) {
+  a();
+}
+
+/* 43 */ else {
+  a();
+}
+
+================================================================================
+`;
+
 exports[`comment_before_else.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]

tests/format/js/if/blank-lines.js

@@ -0,0 +1,65 @@
+// 11
+if (1) {
+  a();
+}
+
+// 12
+else if (2) {
+  a();
+}
+
+// 13
+else {
+  a();
+}
+
+/* 21 */
+if (1) {
+  a();
+}
+
+/* 22 */
+else if (2) {
+  a();
+}
+
+/* 23 */
+else {
+  a();
+}
+
+// 31
+if (1)
+  a();
+
+// 32
+else if (2)
+  a();
+
+// 33
+else
+  a();
+
+// 31
+if (1)
+  a();
+
+// 32
+else if (2)
+  a();
+
+// 33
+else
+  a();
+
+/* 41 */ if (1) {
+  a();
+}
+
+/* 42 */ else if (2) {
+  a();
+}
+
+/* 43 */ else {
+  a();
+}