Repository: prettier/plugin-php
Pull request #569 (2018-09-03 13:49:15)

feat: better print for/foreach comments
Logic from prettier


Patch:
diff --git a/src/comments.js b/src/comments.js
index dd63d86..9e31a13 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -27,12 +27,12 @@ args:
 */
 
 const handleOwnLineComment = (comment, text, options, ast, isLastComment) => {
-  const { enclosingNode } = comment;
+  const { precedingNode, enclosingNode, followingNode } = comment;
   return (
     handleClass(comment) ||
     handleFunctionParameter(comment, text, options) ||
     handleFunction(comment, text, options) ||
-    handleForLoop(comment) ||
+    handleForComments(enclosingNode, precedingNode, followingNode, comment) ||
     handleTryCatch(comment) ||
     handleAlternate(comment) ||
     handleOnlyComments(enclosingNode, ast, comment, isLastComment) ||
@@ -46,7 +46,6 @@ const handleEndOfLineComment = (comment, text, options, ast, isLastComment) => {
     handleClass(comment) ||
     handleFunctionParameter(comment, text, options) ||
     handleFunction(comment, text, options) ||
-    handleForLoop(comment) ||
     handleTryCatch(comment) ||
     handleOnlyComments(enclosingNode, ast, comment, isLastComment)
   );
@@ -58,7 +57,6 @@ const handleRemainingComment = (comment, text, options, ast, isLastComment) => {
     handleClass(comment) ||
     handleFunctionParameter(comment, text, options) ||
     handleFunction(comment, text, options) ||
-    handleForLoop(comment) ||
     handleTryCatch(comment) ||
     handleBreakAndContinueStatementComments(comment) ||
     handleGoto(comment) ||
@@ -68,26 +66,26 @@ const handleRemainingComment = (comment, text, options, ast, isLastComment) => {
   );
 };
 
-const handleForLoop = comment => {
-  const { enclosingNode, followingNode } = comment;
+const handleForComments = (
+  enclosingNode,
+  precedingNode,
+  followingNode,
+  comment
+) => {
   if (
     enclosingNode &&
     (enclosingNode.kind === "for" || enclosingNode.kind === "foreach")
   ) {
+    // For a shortform for loop (where the body is just one node), add
+    // this as a leading comment to the body
     if (enclosingNode.body && enclosingNode.body.kind !== "block") {
-      // for a shortform for loop (where the body is just one node), add
-      // this as a leading comment to the body
       addLeadingComment(followingNode, comment);
-      return true;
-    } else if (
-      // for an empty for loop where the body is only made up of comments, we
-      // need to attach this as a dangling comment on the for loop itself
-      !(enclosingNode.body && enclosingNode.body.children.length > 0)
-    ) {
-      addDanglingComment(enclosingNode, comment);
-      return true;
+    } else {
+      addLeadingComment(enclosingNode, comment);
     }
+    return true;
   }
+
   return false;
 };
 
diff --git a/src/printer.js b/src/printer.js
index ca231e4..eb32daf 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -1563,66 +1563,105 @@ function printNode(path, options, print) {
     case "for": {
       const body = printBodyControlStructure(path, print, "body", options);
 
+      // We want to keep dangling comments above the loop to stay consistent.
+      // Any comment positioned between the for statement and the parentheses
+      // is going to be printed before the statement.
+      const dangling = comments.printDanglingComments(
+        path,
+        options,
+        /* sameLine */ true
+      );
+      const printedComments = dangling ? concat([dangling, softline]) : "";
+
       if (!node.init.length && !node.test.length && !node.increment.length) {
-        return concat([group(concat(["for (;;)", body]))]);
+        return concat([printedComments, group(concat(["for (;;)", body]))]);
       }
 
       return concat([
-        "for (",
+        printedComments,
         group(
           concat([
-            indent(
+            "for (",
+            group(
               concat([
-                softline,
-                group(
-                  concat([join(concat([",", line]), path.map(print, "init"))])
-                ),
-                ";",
-                line,
-                group(
-                  concat([join(concat([",", line]), path.map(print, "test"))])
+                indent(
+                  concat([
+                    softline,
+                    group(
+                      concat([
+                        join(concat([",", line]), path.map(print, "init"))
+                      ])
+                    ),
+                    ";",
+                    line,
+                    group(
+                      concat([
+                        join(concat([",", line]), path.map(print, "test"))
+                      ])
+                    ),
+                    ";",
+                    line,
+                    group(
+                      join(concat([",", line]), path.map(print, "increment"))
+                    )
+                  ])
                 ),
-                ";",
-                line,
-                group(join(concat([",", line]), path.map(print, "increment")))
+                softline
               ])
             ),
-            softline
+            ")",
+            body
           ])
-        ),
-        ")",
-        body
+        )
       ]);
     }
-    case "foreach":
+    case "foreach": {
+      const body = printBodyControlStructure(path, print, "body", options);
+
+      // We want to keep dangling comments above the loop to stay consistent.
+      // Any comment positioned between the for statement and the parentheses
+      // is going to be printed before the statement.
+      const dangling = comments.printDanglingComments(
+        path,
+        options,
+        /* sameLine */ true
+      );
+      const printedComments = dangling ? concat([dangling, softline]) : "";
+
       return concat([
-        "foreach (",
+        printedComments,
         group(
           concat([
-            indent(
+            "foreach (",
+            group(
               concat([
-                softline,
-                path.call(print, "source"),
-                line,
-                "as ",
-                group(
-                  node.key
-                    ? indent(
-                        join(concat([" =>", line]), [
-                          path.call(print, "key"),
-                          path.call(print, "value")
-                        ])
-                      )
-                    : path.call(print, "value")
-                )
+                indent(
+                  concat([
+                    softline,
+                    path.call(print, "source"),
+                    line,
+                    "as ",
+                    group(
+                      node.key
+                        ? indent(
+                            join(concat([" =>", line]), [
+                              path.call(print, "key"),
+                              path.call(print, "value")
+                            ])
+                          )
+                        : path.call(print, "value")
+                    )
+                  ])
+                ),
+                softline
               ])
             ),
-            softline
+            ")",
+            body
           ])
-        ),
-        ")",
-        printBodyControlStructure(path, print, "body", options)
+        )
       ]);
+    }
     case "switch":
       return concat([
         group(


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 1b2be7b..fa73157 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -939,6 +939,9 @@ foreach([1,2,3] as $i)
 for($i = 1; $i < 10; $i++)
     // this comment will screw things up
     echo $i;
+
+for (;;) { // Comment
+}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 <?php
 // Comment
@@ -968,6 +971,10 @@ for ($i = 1; $i < 10; $i++) {
     echo $i;
 }
 
+for (;;) {
+    // Comment
+}
+
 `;
 
 exports[`foreach.php 1`] = `
@@ -983,6 +990,25 @@ foreach ($iterable as $value) { // Comment
 foreach (/* Comments */ $iterable /* Comments */ as /* Comments */ $value /* Comments */) {}
 
 foreach (/* Comments */ $iterable /* Comments */ as /* Comments */ $key /* Comments */ => /* Comments */ $value /* Comments */) {}
+
+foreach ($x
+/*a*/
+as //b
+$y) //c
+{};
+
+foreach ($x as /*a*/ //b
+$y) {}; //c
+
+foreach ($x /*a*/ as $y) {}; //b //c
+
+foreach ($x
+//a
+as $y) {};
+
+foreach($x as
+//a
+$y) {};
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 <?php
 // Comment
@@ -993,18 +1019,41 @@ foreach ($iterable as $value) {
     // Comment
 } // Comment
 
-foreach ($iterable as $value) {/* Comments */
-    /* Comments */
-    /* Comments */
-    /* Comments */
+foreach (
+    /* Comments */ $iterable /* Comments */
+    as /* Comments */ $value /* Comments */
+) {
+}
+
+foreach (
+    /* Comments */ $iterable /* Comments */
+    as /* Comments */ $key /* Comments */ =>
+        /* Comments */ $value /* Comments */
+) {
+}
+
+/*a*/
+foreach (
+    $x //b
+    as $y //c
+) {
+}
+
+foreach (
+    $x /*a*/ //b
+    as $y
+) {
+} //c
+
+foreach ($x /*a*/ as $y) {
+} //b //c
+
+//a
+foreach ($x as $y) {
 }
 
-foreach ($iterable as $key => $value) {/* Comments */
-    /* Comments */
-    /* Comments */
-    /* Comments */
-    /* Comments */
-    /* Comments */
+//a
+foreach ($x as $y) {
 }
 
 `;
diff --git a/tests/comments/for.php b/tests/comments/for.php
index 83ca2f3..f8b5831 100644
--- a/tests/comments/for.php
+++ b/tests/comments/for.php
@@ -23,3 +23,6 @@ foreach([1,2,3] as $i)
 for($i = 1; $i < 10; $i++)
     // this comment will screw things up
     echo $i;
+
+for (;;) { // Comment
+}
diff --git a/tests/comments/foreach.php b/tests/comments/foreach.php
index f8e55e4..8f03480 100644
--- a/tests/comments/foreach.php
+++ b/tests/comments/foreach.php
@@ -10,3 +10,22 @@ foreach ($iterable as $value) { // Comment
 foreach (/* Comments */ $iterable /* Comments */ as /* Comments */ $value /* Comments */) {}
 
 foreach (/* Comments */ $iterable /* Comments */ as /* Comments */ $key /* Comments */ => /* Comments */ $value /* Comments */) {}
+
+foreach ($x
+/*a*/
+as //b
+$y) //c
+{};
+
+foreach ($x as /*a*/ //b
+$y) {}; //c
+
+foreach ($x /*a*/ as $y) {}; //b //c
+
+foreach ($x
+//a
+as $y) {};
+
+foreach($x as
+//a
+$y) {};