Revert how if-else comments are handled (#1036)

Author: fvictorio

src/nodes/ExpressionStatement.js

@@ -1,6 +1,27 @@
+import { doc } from 'prettier';
+import { printComments } from '../common/printer-helpers.js';
+
+const { hardline } = doc.builders;
+
 export const ExpressionStatement = {
-  print: ({ node, path, print }) => [
-    path.call(print, 'expression'),
-    node.omitSemicolon ? '' : ';'
-  ]
+  print: ({ node, options, path, print }) => {
+    const parts = [];
+
+    const parent = path.getParentNode();
+
+    if (parent.type === 'IfStatement') {
+      if (node.comments?.length) {
+        const comments = printComments(node, path, options);
+        if (comments?.length) {
+          parts.push(comments);
+          parts.push(hardline);
+        }
+      }
+    }
+
+    parts.push(path.call(print, 'expression'));
+    parts.push(node.omitSemicolon ? '' : ';');
+
+    return parts;
+  }
 };

src/nodes/IfStatement.js

@@ -1,5 +1,8 @@
 import { doc } from 'prettier';
-import { printSeparatedItem } from '../common/printer-helpers.js';
+import {
+  printComments,
+  printSeparatedItem
+} from '../common/printer-helpers.js';
 
 const { group, hardline, indent, line } = doc.builders;
 
@@ -19,9 +22,10 @@ const printFalseBody = (node, path, print) =>
     ? [' ', path.call(print, 'falseBody')]
     : group(indent([line, path.call(print, 'falseBody')]));
 
-const printElse = (node, path, print) => {
+const printElse = (node, path, print, commentsBetweenIfAndElse) => {
   if (node.falseBody) {
-    const elseOnSameLine = node.trueBody.type === 'Block';
+    const elseOnSameLine =
+      node.trueBody.type === 'Block' && commentsBetweenIfAndElse.length === 0;
     return [
       elseOnSameLine ? ' ' : hardline,
       'else',
@@ -32,11 +36,22 @@ const printElse = (node, path, print) => {
 };
 
 export const IfStatement = {
-  print: ({ node, path, print }) => [
-    'if (',
-    printSeparatedItem(path.call(print, 'condition')),
-    ')',
-    printTrueBody(node, path, print),
-    printElse(node, path, print)
-  ]
+  print: ({ node, options, path, print }) => {
+    const comments = node.comments || [];
+    const commentsBetweenIfAndElse = comments.filter(
+      (comment) => !comment.leading && !comment.trailing
+    );
+
+    const parts = [];
+
+    parts.push('if (', printSeparatedItem(path.call(print, 'condition')), ')');
+    parts.push(printTrueBody(node, path, print));
+    if (commentsBetweenIfAndElse.length && node.falseBody) {
+      parts.push(hardline);
+      parts.push(printComments(node, path, options));
+    }
+    parts.push(printElse(node, path, print, commentsBetweenIfAndElse));
+
+    return parts;
+  }
 };

src/prettier-comments/language-js/comments.js

@@ -257,51 +257,21 @@ function handleIfStatementComments(
     precedingNode === enclosingNode.trueBody &&
     followingNode === enclosingNode.falseBody
   ) {
-    if (followingNode.type === "Block") {
-      if(followingNode.statements.length === 0) {
-        addDanglingComment(followingNode, comment);
-      } else {
-        addLeadingComment(followingNode.statements[followingNode.statements.length - 1], comment);
-      }
-    } else if (followingNode.type === "IfStatement") {
-      if (followingNode.trueBody.type === "Block") {
-        if(followingNode.trueBody.statements.length === 0) {
-          addDanglingComment(followingNode.trueBody, comment);
-        } else {
-          addLeadingComment(followingNode.trueBody.statements[0], comment);
-        }
-      } else {
-        addLeadingComment(followingNode.trueBody, comment);
-      }
+    if (precedingNode.type === "ExpressionStatement") {
+      addTrailingComment(precedingNode, comment);
     } else {
-      addLeadingComment(precedingNode, comment);
+      addDanglingComment(enclosingNode, comment);
     }
     return true;
   }
 
-  if (enclosingNode.trueBody === followingNode) {
-    if (followingNode.type === "Block") {
-      if(followingNode.statements.length === 0) {
-        addDanglingComment(followingNode, comment);
-      } else {
-        addLeadingComment(followingNode.statements[0], comment);
-      }
-    } else {
-      addLeadingComment(followingNode, comment);
-    }
+  if (followingNode.type === "ExpressionStatement") {
+    addBlockStatementFirstComment(followingNode, comment);
     return true;
   }
 
   if (followingNode.type === "IfStatement") {
-    if (followingNode.trueBody.type === "Block") {
-      if(followingNode.trueBody.statements.length === 0) {
-        addDanglingComment(followingNode.trueBody, comment);
-      } else {
-        addLeadingComment(followingNode.trueBody.statements[0], comment);
-      }
-    } else {
-      addLeadingComment(followingNode.trueBody, comment);
-    }
+    addBlockOrNotComment(followingNode.trueBody, comment);
     return true;
   }
 

tests/format/Comments/__snapshots__/jsfmt.spec.js.snap

@@ -260,18 +260,18 @@ contract Comments7 {
 
 contract Comments8 {
     function someFunction() {
-        if (something) {} else {
-            // comment
-        }
+        if (something) {}
+        // comment
+        else {}
     }
 }
 
 contract Comments8 {
     function someFunction() {
-        if (something) {} else {
-            /* comment
-             * comment */
-        }
+        if (something) {}
+        /* comment
+         * comment */
+        else {}
     }
 }
 

tests/format/Etc/__snapshots__/jsfmt.spec.js.snap

@@ -129,10 +129,10 @@ contract Contract {
     }
 
     function fun(uint256 a) returns (uint) {
-        if (something) foo();
-        else if (somethingElse)
+        if (something)
+            foo();
             // comment
-            bar();
+        else if (somethingElse) bar();
         else whatever();
         return;
     }

tests/format/IfStatements/__snapshots__/jsfmt.spec.js.snap

@@ -558,14 +558,17 @@ contract IfStatements {
             // Case 0-9
             if (47 < c && c < 58) {
                 return (true, c - 48);
-            } else if (64 < c && c < 71) {
-                // Case A-F
+            }
+            // Case A-F
+            else if (64 < c && c < 71) {
                 return (true, c - 55);
-            } else if (96 < c && c < 103) {
-                // Case a-f
+            }
+            // Case a-f
+            else if (96 < c && c < 103) {
                 return (true, c - 87);
-            } else {
-                // Else: not a hex char
+            }
+            // Else: not a hex char
+            else {
                 return (false, 0);
             }
         }

tests/format/IndexOf/__snapshots__/jsfmt.spec.js.snap

@@ -99,15 +99,17 @@ contract IndexOf {
         if (a.length < 1 || b.length < 1 || (b.length > a.length)) {
             whatwastheval = -1;
             return -1;
-        } else if (a.length > (2 ** 128 - 1)) {
-            // since we have to be able to return -1 (if the char isn't found or input error), this function must return an "int" type with a max length of (2^128 - 1)
+        } else if (
+            a.length > (2 ** 128 - 1)
+        ) // since we have to be able to return -1 (if the char isn't found or input error), this function must return an "int" type with a max length of (2^128 - 1)
+        {
             whatwastheval = -1;
             return -1;
         } else {
             uint subindex = 0;
             for (uint i = 0; i < a.length; i++) {
-                if (a[i] == b[0]) {
-                    // found the first char of b
+                if (a[i] == b[0]) // found the first char of b
+                {
                     subindex = 1;
                     while (
                         subindex < b.length &&