Improve comment handling between if statement condition and the "body" (#17998)

changelog_unreleased/javascript/17998.md

@@ -0,0 +1,31 @@
+#### Improve comment handing inside if statement (#17998 by @fisker)
+
+<!-- prettier-ignore -->
+```jsx
+// Input
+if (foo) // comment
+{
+  doThing();
+} else // comment for else
+{
+  doSomethingElse();
+}
+
+// Prettier stable
+if (foo) {
+  // comment
+  doThing();
+} // comment for else
+else {
+  doSomethingElse();
+}
+
+// Prettier main
+if (foo) // comment
+{
+  doThing();
+} else // comment for else
+{
+  doSomethingElse();
+}
+```

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

@@ -198,6 +198,17 @@ function handleIfStatementComments({
     return true;
   }
 
+  // if comment is positioned between the condition and its body
+  if (
+    followingNode.type === "BlockStatement" &&
+    followingNode === enclosingNode.consequent &&
+    locStart(comment) >= locEnd(precedingNode) &&
+    locEnd(comment) <= locStart(followingNode)
+  ) {
+    addLeadingComment(followingNode, comment);
+    return true;
+  }
+
   // Comments before `else`:
   // - treat as trailing comments of the consequent, if it's a BlockStatement
   // - treat as a dangling comment otherwise
@@ -209,6 +220,17 @@ function handleIfStatementComments({
       text,
       locEnd(enclosingNode.consequent),
     );
+
+    // if comment is positioned between the `else` token and its body
+    if (
+      followingNode.type === "BlockStatement" &&
+      locStart(comment) >= maybeElseTokenIndex &&
+      locEnd(comment) <= locStart(followingNode)
+    ) {
+      addLeadingComment(followingNode, comment);
+      return true;
+    }
+
     // With the above conditions alone, this code would also match. This is a false positive.
     // So, ignore cases where the token "else" appears immediately after the consequent:
     //

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

@@ -2632,9 +2632,9 @@ true
 else /* comment */
 {true}
 
-if (14) // comment
-/* comment */
-// comment
+if (14) // comment 1
+/* comment 2 */
+// comment 3
 {true}
 else if (15) // comment
 /* comment */
@@ -2642,8 +2642,8 @@ else if (15) // comment
 true
 
 =====================================output=====================================
-if (1) {
-  // comment
+if (1) // comment
+{
   false
 }
 // comment
@@ -2666,33 +2666,33 @@ if (5)
   // comment
   true
 
-if (6) {
-  // comment
+if (6) // comment
+{
   true
 } else if (7)
   // comment
   true
-// comment
-else {
+else // comment
+{
   true
 }
 
-if (8) {
-  // comment
-  // comment
+if (8) // comment
+// comment
+{
   true
 } else if (9)
   // comment
   // comment
   true
+else // comment
 // comment
-// comment
-else {
+{
   true
 }
 
-if (10) {
-  /* comment */ // comment
+if (10) /* comment */ // comment
+{
   true
 } else if (11) /* comment */ true
 else if (12)
@@ -2701,14 +2701,15 @@ else if (12)
 else if (13)
   /* comment */ /* comment */ // comment
   true
-/* comment */ else {
+else /* comment */
+{
   true
 }
 
-if (14) {
-  // comment
-  /* comment */
-  // comment
+if (14) // comment 1
+/* comment 2 */
+// comment 3
+{
   true
 } else if (15)
   // comment
@@ -2778,9 +2779,9 @@ true
 else /* comment */
 {true}
 
-if (14) // comment
-/* comment */
-// comment
+if (14) // comment 1
+/* comment 2 */
+// comment 3
 {true}
 else if (15) // comment
 /* comment */
@@ -2788,8 +2789,8 @@ else if (15) // comment
 true
 
 =====================================output=====================================
-if (1) {
-  // comment
+if (1) // comment
+{
   false;
 }
 // comment
@@ -2812,33 +2813,33 @@ if (5)
   // comment
   true;
 
-if (6) {
-  // comment
+if (6) // comment
+{
   true;
 } else if (7)
   // comment
   true;
-// comment
-else {
+else // comment
+{
   true;
 }
 
-if (8) {
-  // comment
-  // comment
+if (8) // comment
+// comment
+{
   true;
 } else if (9)
   // comment
   // comment
   true;
+else // comment
 // comment
-// comment
-else {
+{
   true;
 }
 
-if (10) {
-  /* comment */ // comment
+if (10) /* comment */ // comment
+{
   true;
 } else if (11) /* comment */ true;
 else if (12)
@@ -2847,14 +2848,15 @@ else if (12)
 else if (13)
   /* comment */ /* comment */ // comment
   true;
-/* comment */ else {
+else /* comment */
+{
   true;
 }
 
-if (14) {
-  // comment
-  /* comment */
-  // comment
+if (14) // comment 1
+/* comment 2 */
+// comment 3
+{
   true;
 } else if (15)
   // comment

tests/format/js/comments/if.js

@@ -51,9 +51,9 @@ true
 else /* comment */
 {true}
 
-if (14) // comment
-/* comment */
-// comment
+if (14) // comment 1
+/* comment 2 */
+// comment 3
 {true}
 else if (15) // comment
 /* comment */

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

@@ -38,6 +38,88 @@ else stuff;
 ================================================================================
 `;
 
+exports[`comment-between-condition-and-body.js format 1`] = `
+====================================options=====================================
+parsers: ["babel", "flow", "typescript"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+if (1) { // foo may not exist
+  doThing(foo);
+}
+if (1){}else { // foo may not exist
+  doThing(foo);
+}
+
+if (2)
+// foo may not exist
+{
+  doThing(foo);
+}
+if (2){}else
+// foo may not exist
+{
+  doThing(foo);
+}
+
+if (3)// foo may not exist
+{
+  doThing(foo);
+}
+if (3){}else// foo may not exist
+{
+  doThing(foo);
+}
+
+if (4)/* foo may not exist */{
+  doThing(foo);
+}
+if (4){}else/* foo may not exist */{
+  doThing(foo);
+}
+
+=====================================output=====================================
+if (1) {
+  // foo may not exist
+  doThing(foo);
+}
+if (1) {
+} else {
+  // foo may not exist
+  doThing(foo);
+}
+
+if (2) // foo may not exist
+{
+  doThing(foo);
+}
+if (2) {
+} else // foo may not exist
+{
+  doThing(foo);
+}
+
+if (3) // foo may not exist
+{
+  doThing(foo);
+}
+if (3) {
+} else // foo may not exist
+{
+  doThing(foo);
+}
+
+if (4) /* foo may not exist */ {
+  doThing(foo);
+}
+if (4) {
+} else /* foo may not exist */ {
+  doThing(foo);
+}
+
+================================================================================
+`;
+
 exports[`else.js format 1`] = `
 ====================================options=====================================
 parsers: ["babel", "flow", "typescript"]
@@ -243,13 +325,13 @@ async function f5() {
 
 =====================================output=====================================
 async function f1() {
-  if (untrackedChoice === 0) {
-    /* Cancel */ return null;
-  } else if (untrackedChoice === 1) {
-    /* Add */ await repository.addAll(Array.from(untrackedChanges.keys()));
+  if (untrackedChoice === 0) /* Cancel */ {
+    return null;
+  } else if (untrackedChoice === 1) /* Add */ {
+    await repository.addAll(Array.from(untrackedChanges.keys()));
     shouldAmend = true;
-  } else if (untrackedChoice === 2) {
-    /* Allow Untracked */ allowUntracked = true;
+  } else if (untrackedChoice === 2) /* Allow Untracked */ {
+    allowUntracked = true;
   }
 }
 
@@ -272,13 +354,13 @@ async function f3() {
 }
 
 async function f4() {
-  if (untrackedChoice === 0) {
-    /* Cancel */ return null;
-  } else if (untrackedChoice === 1) {
-    /* Add */ await repository.addAll(Array.from(untrackedChanges.keys()));
+  if (untrackedChoice === 0) /* Cancel */ {
+    return null;
+  } else if (untrackedChoice === 1) /* Add */ {
+    await repository.addAll(Array.from(untrackedChanges.keys()));
     shouldAmend = true;
-  } else if (untrackedChoice === 2) {
-    /* Allow Untracked */ allowUntracked = true;
+  } else if (untrackedChoice === 2) /* Allow Untracked */ {
+    allowUntracked = true;
   }
 }
 
@@ -386,8 +468,7 @@ if (code === 92 /* '\\' */) {
 if (code === 92 /* '\\' */ /* '\\' */) {
 }
 
-if (code === 92) {
-  /* '\\' */
+if (code === 92) /* '\\' */ {
 }
 if (code === 92) {
   /* '\\' */

tests/format/js/if/comment-between-condition-and-body.js

@@ -0,0 +1,33 @@
+if (1) { // foo may not exist
+  doThing(foo);
+}
+if (1){}else { // foo may not exist
+  doThing(foo);
+}
+
+if (2)
+// foo may not exist
+{
+  doThing(foo);
+}
+if (2){}else
+// foo may not exist
+{
+  doThing(foo);
+}
+
+if (3)// foo may not exist
+{
+  doThing(foo);
+}
+if (3){}else// foo may not exist
+{
+  doThing(foo);
+}
+
+if (4)/* foo may not exist */{
+  doThing(foo);
+}
+if (4){}else/* foo may not exist */{
+  doThing(foo);
+}