Repository: prettier/plugin-php
Pull request #781 (2018-10-11 12:47:12)

refactor: `getCommentChildNodes` function
1. Refactor
2. Simplify
3. Fix incorrect comment printing
4. Fix unstable comments

evilebottnawi: Feel free to feedback

Patch:
diff --git a/src/comments.js b/src/comments.js
index 636c1cc..ba90102 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -63,7 +63,8 @@ function handleOwnLineComment(comment, text, options, ast, isLastComment) {
       precedingNode,
       followingNode,
       comment
-    )
+    ) ||
+    handleDeclareComments(enclosingNode, precedingNode, followingNode, comment)
   );
 }
 
@@ -130,7 +131,8 @@ function handleEndOfLineComment(comment, text, options, ast, isLastComment) {
       precedingNode,
       followingNode,
       comment
-    )
+    ) ||
+    handleDeclareComments(enclosingNode, precedingNode, followingNode, comment)
   );
 }
 
@@ -670,6 +672,38 @@ function handleNamespaceComments(
   return false;
 }
 
+function handleDeclareComments(
+  enclosingNode,
+  precedingNode,
+  followingNode,
+  comment
+) {
+  if (!enclosingNode || enclosingNode.kind !== "declare") {
+    return false;
+  }
+
+  if (
+    !followingNode ||
+    enclosingNode.what[Object.keys(enclosingNode.what)[0]] === followingNode
+  ) {
+    if (enclosingNode.mode === "none") {
+      addTrailingComment(enclosingNode, comment);
+    } else {
+      addDanglingComment(enclosingNode, comment);
+    }
+
+    return true;
+  }
+
+  if (followingNode && precedingNode) {
+    addLeadingComment(followingNode, comment);
+
+    return true;
+  }
+
+  return false;
+}
+
 // https://github.com/prettier/prettier/blob/master/src/main/comments.js#L335
 function printComment(commentPath, options) {
   const comment = commentPath.getValue();
@@ -745,10 +779,42 @@ function isBlockComment(comment) {
   return comment.kind === "commentblock";
 }
 
+function getCommentChildNodes(node) {
+  if (typeof node !== "object") {
+    return [];
+  }
+
+  const getChildNodes = node =>
+    Object.keys(node)
+      .filter(
+        n =>
+          n !== "kind" &&
+          n !== "loc" &&
+          n !== "errors" &&
+          n !== "extra" &&
+          n !== "comments" &&
+          n !== "leadingComments" &&
+          n !== "enclosingNode" &&
+          n !== "precedingNode" &&
+          n !== "followingNode"
+      )
+      .map(n => node[n]);
+
+  return getChildNodes(node);
+}
+
+function canAttachComment(node) {
+  return (
+    node.kind && node.kind !== "commentblock" && node.kind !== "commentline"
+  );
+}
+
 module.exports = {
   handleOwnLineComment,
   handleEndOfLineComment,
   handleRemainingComment,
+  getCommentChildNodes,
+  canAttachComment,
   isBlockComment,
   printDanglingComments,
   hasLeadingComment,
diff --git a/src/index.js b/src/index.js
index 7be7624..15e0836 100644
--- a/src/index.js
+++ b/src/index.js
@@ -86,187 +86,8 @@ const printers = {
     print,
     insertPragma,
     massageAstNode: clean,
-    // @TODO: determine if it makes sense to abstract this into a "getChildNodes" util function
-    getCommentChildNodes(node) {
-      const nodeMap = [
-        {
-          kinds: ["constant", "property", "classconstant"],
-          children: { listNodes: [], nodes: ["value"] }
-        },
-        {
-          kinds: ["return"],
-          children: { listNodes: [], nodes: ["expr"] }
-        },
-        {
-          kinds: ["static", "array", "usegroup"],
-          children: { listNodes: ["items"], nodes: [] }
-        },
-        {
-          kinds: ["entry"],
-          children: { listNodes: [], nodes: ["key", "value"] }
-        },
-        {
-          kinds: ["assign"],
-          children: { listNodes: [], nodes: ["left", "right"] }
-        },
-        {
-          kinds: ["if"],
-          children: { listNodes: [], nodes: ["test", "body", "alternate"] }
-        },
-        {
-          kinds: ["block", "program", "namespace", "declare"],
-          children: { listNodes: ["children"], nodes: [] }
-        },
-        {
-          kinds: ["foreach"],
-          children: {
-            listNodes: [],
-            nodes: ["key", "source", "value", "body"]
-          }
-        },
-        {
-          kinds: ["try"],
-          children: {
-            listNodes: ["catches"],
-            nodes: ["always", "body"]
-          }
-        },
-        {
-          kinds: ["call", "new"],
-          children: { listNodes: ["arguments"], nodes: ["what"] }
-        },
-        {
-          kinds: ["offsetlookup", "staticlookup"],
-          children: { listNodes: [], nodes: ["what", "offset"] }
-        },
-        {
-          kinds: ["catch"],
-          children: {
-            listNodes: ["what"],
-            nodes: ["variable", "body"]
-          }
-        },
-        {
-          kinds: ["while", "do"],
-          children: { listNodes: [], nodes: ["test", "body"] }
-        },
-        {
-          kinds: ["for"],
-          children: {
-            listNodes: [],
-            nodes: ["init", "test", "increment", "body"]
-          }
-        },
-        {
-          kinds: ["trait", "class"],
-          children: { listNodes: ["implements", "body"], nodes: ["extends"] }
-        },
-        {
-          kinds: ["interface"],
-          children: { listNodes: ["extends", "body"], nodes: [] }
-        },
-        {
-          kinds: ["function", "method", "closure"],
-          children: {
-            listNodes: ["arguments", "uses"],
-            nodes: ["body", "type"]
-          }
-        },
-        {
-          kinds: ["switch", "case"],
-          children: { listNodes: [], nodes: ["test", "body"] }
-        },
-        {
-          kinds: ["bin"],
-          children: { listNodes: [], nodes: ["left", "right"] }
-        },
-        {
-          kinds: ["parameter"],
-          children: { listNodes: [], nodes: ["type", "value"] }
-        },
-        {
-          kinds: ["yieldfrom"],
-          children: { listNodes: [], nodes: ["value"] }
-        },
-        {
-          kinds: ["retif"],
-          children: { listNodes: [], nodes: ["trueExpr", "falseExpr", "test"] }
-        },
-        {
-          kinds: ["echo"],
-          children: { listNodes: ["arguments"], nodes: [] }
-        },
-        {
-          kinds: ["include"],
-          children: { listNodes: [], nodes: ["target"] }
-        },
-        {
-          kinds: ["throw", "clone", "pre", "post", "unary", "cast", "variadic"],
-          children: { listNodes: [], nodes: ["what"] }
-        },
-        {
-          kinds: ["list", "isset", "unset", "empty"],
-          children: { listNodes: ["arguments"], nodes: [] }
-        },
-        {
-          kinds: ["global"],
-          children: { listNodes: ["items"], nodes: [] }
-        },
-        {
-          kinds: ["print"],
-          children: { listNodes: [], nodes: ["arguments"] }
-        },
-        {
-          kinds: ["eval"],
-          children: { listNodes: [], nodes: ["source"] }
-        },
-        {
-          kinds: ["yield"],
-          children: { listNodes: [], nodes: ["key", "value"] }
-        },
-        {
-          kinds: ["traituse"],
-          children: { listNodes: ["traits", "adaptations"], nodes: [] }
-        },
-        {
-          kinds: ["traitprecedence"],
-          children: { listNodes: [], nodes: ["trait", "instead"] }
-        },
-        {
-          kinds: ["traitalias"],
-          children: { listNodes: [], nodes: ["trait"] }
-        },
-        {
-          kinds: ["silent"],
-          children: { listNodes: [], nodes: ["expr"] }
-        },
-        {
-          kinds: ["exit"],
-          children: { listNodes: [], nodes: ["status"] }
-        }
-      ];
-      const children = nodeMap.reduce((currentChildren, map) => {
-        if (map.kinds.includes(node.kind)) {
-          return [
-            ...map.children.listNodes.reduce((accumulator, listNode) => {
-              const childValue = node[listNode];
-              return childValue ? [...accumulator, ...childValue] : accumulator;
-            }, []),
-            ...map.children.nodes.reduce((accumulator, childNode) => {
-              const childValue = node[childNode];
-              return childValue ? [...accumulator, childValue] : accumulator;
-            }, [])
-          ];
-        }
-        return currentChildren;
-      }, false);
-      return children !== false ? children : [];
-    },
-    canAttachComment(node) {
-      return (
-        node.kind && node.kind !== "commentblock" && node.kind !== "commentline"
-      );
-    },
+    getCommentChildNodes: comments.getCommentChildNodes,
+    canAttachComment: comments.canAttachComment,
     isBlockComment: comments.isBlockComment,
     handleComments: {
       ownLine: comments.handleOwnLineComment,


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 78c92c3..a674745 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -1290,6 +1290,32 @@ declare(ticks=1):
     // Comment
 enddeclare;
 // Comment
+
+declare(ticks=1) // Comment
+{
+}
+
+declare(ticks=1) { // Comment
+}
+
+declare(ticks=1) // Comment
+{
+    $a = 1;
+}
+
+declare(ticks=1) { // Comment
+    $a = 1;
+}
+
+declare // Comment
+(ticks=1)
+{
+}
+
+declare /* Comment */ (/* Comment */ticks/* Comment */=/* Comment */1/* Comment */) {}
+
+declare // Comment
+(strict_types=1);
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 <?php
 
@@ -1316,6 +1342,33 @@ declare(ticks=1):
 enddeclare;
 // Comment
 
+declare(ticks=1) {
+    // Comment
+}
+
+declare(ticks=1) {
+    // Comment
+}
+
+declare(ticks=1) {
+    // Comment
+    $a = 1;
+}
+
+declare(ticks=1) {
+    // Comment
+    $a = 1;
+}
+
+declare(ticks=1) {
+    // Comment
+}
+
+declare(ticks=/* Comment */ /* Comment */ /* Comment */ /* Comment */ 1 /* Comment */) {
+}
+
+declare(strict_types=1); // Comment
+
 `;
 
 exports[`do.php - php-verify: do.php 1`] = `
@@ -1591,8 +1644,12 @@ for ($i = 1; $i <= 10; $i++) {
     // Comment
 } // Comment
 
-for ($i = 1; $i <= 10; $i++) {
-    /* Comments */ /* Comments */ /* Comments */ /* Comments */ /* Comments */ /* Comments */ /* Comments */ /* Comments */ /* Comments */ /* Comments */ echo $i;
+for (
+    /* Comments */ $i /* Comments */ = /* Comments */ 1 /* Comments */;
+    /* Comments */ $i /* Comments */ <= /* Comments */ 10 /* Comments */;
+    /* Comments */ $i++ /* Comments */
+) {
+    echo $i;
 }
 
 foreach ([1, 2, 3] as $i) {
@@ -3347,6 +3404,34 @@ $extractTextPluginOptions = $shouldUseRelativeAssetPaths // Making sure that the
 $var = $process->env->NODE_ENV === "production"
     ? call("./configureProdStore") // a
     : call("./configureDevStore"); // b
+
+$var = $a
+    // Comment
+    ? $b
+    // Comment
+    : $c;
+
+$var = $a
+    ? // Comment
+    $b
+    : // Comment
+    $c;
+
+$var = $a
+    ? $b
+    // Comment
+    : $c
+    // Comment
+    ;
+
+$var = $a
+    ? $b  // Comment
+    : $c  // Comment
+;
+
+$var = $a
+    ? $b // Comment
+    : $c; // Comment
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 <?php
 
@@ -3375,10 +3460,37 @@ $extractTextPluginOptions = $shouldUseRelativeAssetPaths // Making sure that the
     : [];
 
 $var =
-    $process->env->NODE_ENV === "production"
-        ? call("./configureProdStore") // a
+    $process->env->NODE_ENV ===
+    "production" // a
+        ? call("./configureProdStore")
         : call("./configureDevStore"); // b
 
+$var = $a
+    ? // Comment
+    $b
+    : // Comment
+    $c;
+
+$var = $a
+    ? // Comment
+    $b
+    : // Comment
+    $c;
+
+$var = $a
+    ? $b
+    : // Comment
+    $c;
+// Comment
+
+$var = $a
+    ? $b // Comment
+    : $c; // Comment
+
+$var = $a
+    ? $b // Comment
+    : $c; // Comment
+
 `;
 
 exports[`return.php - php-verify: return.php 1`] = `
@@ -3635,7 +3747,7 @@ class ImplementingClass
         /* Comment */ otherTrait /* Comment */;
     /* Comment */ /* Comment */ use testTrait /* Comment */,
         /* Comment */ implementingTrait /* Comment */ /* Comment */ {
-        /* Comment */ A /* Comment */ /* Comment */ /* Comment */ /* Comment */::testFunction insteadof C; /* Comment */ /* Comment */
+        /* Comment */ A /* Comment */ /* Comment */ /* Comment */::testFunction insteadof /* Comment */ C; /* Comment */ /* Comment */
         /* Comment */ B /* Comment */ /* Comment */ /* Comment */ /* Comment */::someOtherTestFunction as aliasedFunctionName; /* Comment */ /* Comment */
         /* Comment */ C /* Comment */ /* Comment */ /* Comment */ /* Comment */ /* Comment */::testFunction as private testVisibility; /* Comment */
     }
diff --git a/tests/comments/declare.php b/tests/comments/declare.php
index 883ae1f..c71dd33 100644
--- a/tests/comments/declare.php
+++ b/tests/comments/declare.php
@@ -22,3 +22,29 @@ declare(ticks=1):
     // Comment
 enddeclare;
 // Comment
+
+declare(ticks=1) // Comment
+{
+}
+
+declare(ticks=1) { // Comment
+}
+
+declare(ticks=1) // Comment
+{
+    $a = 1;
+}
+
+declare(ticks=1) { // Comment
+    $a = 1;
+}
+
+declare // Comment
+(ticks=1)
+{
+}
+
+declare /* Comment */ (/* Comment */ticks/* Comment */=/* Comment */1/* Comment */) {}
+
+declare // Comment
+(strict_types=1);
diff --git a/tests/comments/retif.php b/tests/comments/retif.php
index 7bb8465..1f2b0f7 100644
--- a/tests/comments/retif.php
+++ b/tests/comments/retif.php
@@ -27,3 +27,31 @@ $extractTextPluginOptions = $shouldUseRelativeAssetPaths // Making sure that the
 $var = $process->env->NODE_ENV === "production"
     ? call("./configureProdStore") // a
     : call("./configureDevStore"); // b
+
+$var = $a
+    // Comment
+    ? $b
+    // Comment
+    : $c;
+
+$var = $a
+    ? // Comment
+    $b
+    : // Comment
+    $c;
+
+$var = $a
+    ? $b
+    // Comment
+    : $c
+    // Comment
+    ;
+
+$var = $a
+    ? $b  // Comment
+    : $c  // Comment
+;
+
+$var = $a
+    ? $b // Comment
+    : $c; // Comment