Repository: prettier/plugin-php
Pull request #571 (2018-09-03 18:22:45)

feat: better print `assign` comments

evilebottnawi: /cc @czosel 

Patch:
diff --git a/src/comments.js b/src/comments.js
index 9e31a13..5172bee 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -8,7 +8,8 @@ const {
   isNextLineEmpty
 } = require("prettier").util;
 const { concat, join, indent, hardline } = require("prettier").doc.builders;
-
+// TODO: remove after resolve https://github.com/prettier/prettier/pull/5049
+const { hasNewline } = require("./util");
 /*
 Comment functions are meant to inspect various edge cases using given comment nodes,
 with information about where those comment nodes exist in the tree (ie enclosingNode,
@@ -353,6 +354,15 @@ function hasTrailingComment(node) {
   return node.comments && node.comments.some(comment => comment.trailing);
 }
 
+function hasLeadingOwnLineComment(text, node, options) {
+  return (
+    node.comments &&
+    node.comments.some(
+      comment => comment.leading && hasNewline(text, options.locEnd(comment))
+    )
+  );
+}
+
 function printComments(comments, options) {
   const parts = [];
   comments.forEach((comment, index, comments) => {
@@ -376,5 +386,6 @@ module.exports = {
   printDanglingComments,
   hasLeadingComment,
   hasTrailingComment,
+  hasLeadingOwnLineComment,
   printComments
 };
diff --git a/src/printer.js b/src/printer.js
index eb32daf..77ab3e8 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -1168,7 +1168,13 @@ function isLookupNodeChain(node) {
   return isLookupNodeChain(node.what);
 }
 
-function printAssignmentRight(leftNode, rightNode, printedRight) {
+function printAssignmentRight(leftNode, rightNode, printedRight, options) {
+  if (
+    comments.hasLeadingOwnLineComment(options.originalText, rightNode, options)
+  ) {
+    return indent(concat([hardline, printedRight]));
+  }
+
   const canBreak =
     (rightNode.kind === "bin" && !shouldInlineLogicalExpression(rightNode)) ||
     (rightNode.kind === "retif" &&
@@ -1491,7 +1497,8 @@ function printNode(path, options, print) {
         path.call(print, "left"),
         concat([" ", node.operator]),
         node.right,
-        path.call(print, "right")
+        path.call(print, "right"),
+        options
       );
     }
     case "if": {
diff --git a/src/util.js b/src/util.js
index 9af1de1..7c29521 100644
--- a/src/util.js
+++ b/src/util.js
@@ -446,6 +446,91 @@ function getNodeKindIncludingLogical(node) {
   return node.kind;
 }
 
+function skip(chars) {
+  return (text, index, opts) => {
+    const backwards = opts && opts.backwards;
+
+    // Allow `skip` functions to be threaded together without having
+    // to check for failures (did someone say monads?).
+    if (index === false) {
+      return false;
+    }
+
+    const { length } = text;
+    let cursor = index;
+    while (cursor >= 0 && cursor < length) {
+      const c = text.charAt(cursor);
+      if (chars instanceof RegExp) {
+        if (!chars.test(c)) {
+          return cursor;
+        }
+      } else if (chars.indexOf(c) === -1) {
+        return cursor;
+      }
+
+      backwards ? cursor-- : cursor++;
+    }
+
+    if (cursor === -1 || cursor === length) {
+      // If we reached the beginning or end of the file, return the
+      // out-of-bounds cursor. It's up to the caller to handle this
+      // correctly. We don't want to indicate `false` though if it
+      // actually skipped valid characters.
+      return cursor;
+    }
+    return false;
+  };
+}
+
+const skipSpaces = skip(" \t");
+
+// This one doesn't use the above helper function because it wants to
+// test \r\n in order and `skip` doesn't support ordering and we only
+// want to skip one newline. It's simple to implement.
+function skipNewline(text, index, opts) {
+  const backwards = opts && opts.backwards;
+  if (index === false) {
+    return false;
+  }
+
+  const atIndex = text.charAt(index);
+  if (backwards) {
+    if (text.charAt(index - 1) === "\r" && atIndex === "\n") {
+      return index - 2;
+    }
+    if (
+      atIndex === "\n" ||
+      atIndex === "\r" ||
+      atIndex === "\u2028" ||
+      atIndex === "\u2029"
+    ) {
+      return index - 1;
+    }
+  } else {
+    if (atIndex === "\r" && text.charAt(index + 1) === "\n") {
+      return index + 2;
+    }
+    if (
+      atIndex === "\n" ||
+      atIndex === "\r" ||
+      atIndex === "\u2028" ||
+      atIndex === "\u2029"
+    ) {
+      return index + 1;
+    }
+  }
+
+  return index;
+}
+
+// TODO: remove after resolve https://github.com/prettier/prettier/pull/5049
+function hasNewline(text, index, opts) {
+  opts = opts || {};
+  const idx = skipSpaces(text, opts.backwards ? index - 1 : index, opts);
+  const idx2 = skipNewline(text, idx, opts);
+  return idx !== idx2;
+}
+
 module.exports = {
   printNumber,
   getPrecedence,
@@ -471,5 +556,6 @@ module.exports = {
   getFirstNestedChildNode,
   getLastNestedChildNode,
   isProgramLikeNode,
-  getNodeKindIncludingLogical
+  getNodeKindIncludingLogical,
+  hasNewline
 };


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index fa73157..6f4c1e3 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -133,6 +133,113 @@ $array /* Comment */ = /* Comment */ []; /* Comment */
 
 `;
 
+exports[`assign.php 1`] = `
+<?php
+$var =
+    // Comment
+    'some' . 'long' . 'string';
+
+$var =
+    // Comment
+
+    'some' . 'long' . 'string';
+
+$var =
+
+    // Comment
+
+    'some' . 'long' . 'string';
+
+$var =
+    /* comment */
+    'some' . 'long' . 'string';
+
+$var =
+    /**
+     * multi-line
+     */
+    'some' . 'long' . 'string';
+
+$var =
+    /* inline */ 'some' . 'long' . 'string' . 'some' . 'long' . 'string' . 'some' . 'long' . 'string' . 'some' . 'long' . 'string';
+
+$var = // Comment
+    // Comment
+    'some' . 'long' . 'string';
+
+$var = // Comment
+    'some' . 'long' . 'string';
+
+$var = /* Comment */
+    // Comment
+    'some' . 'long' . 'string';
+
+$var = // Comment/
+    /* Comment */
+    'some' . 'long' . 'string';
+
+$var = /* Comment */
+    /* Comment */'some' .
+    'long' .
+    'string';
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<?php
+$var =
+    // Comment
+    'some' . 'long' . 'string';
+
+$var =
+    // Comment
+
+    'some' . 'long' . 'string';
+
+$var =
+    // Comment
+
+    'some' . 'long' . 'string';
+
+$var =
+    /* comment */
+    'some' . 'long' . 'string';
+
+$var =
+    /**
+     * multi-line
+     */
+    'some' . 'long' . 'string';
+
+$var =
+    /* inline */ 'some' .
+    'long' .
+    'string' .
+    'some' .
+    'long' .
+    'string' .
+    'some' .
+    'long' .
+    'string' .
+    'some' .
+    'long' .
+    'string';
+
+$var = // Comment
+    // Comment
+    'some' . 'long' . 'string';
+
+$var = 'some' . 'long' . 'string'; // Comment
+
+$var /* Comment */ =
+    // Comment
+    'some' . 'long' . 'string';
+
+$var = // Comment/
+    /* Comment */
+    'some' . 'long' . 'string';
+
+$var /* Comment */ = /* Comment */ 'some' . 'long' . 'string';
+
+`;
+
 exports[`bin.php 1`] = `
 <?php
 
diff --git a/tests/comments/assign.php b/tests/comments/assign.php
new file mode 100644
index 0000000..e534791
--- /dev/null
+++ b/tests/comments/assign.php
@@ -0,0 +1,48 @@
+<?php
+$var =
+    // Comment
+    'some' . 'long' . 'string';
+
+$var =
+    // Comment
+
+    'some' . 'long' . 'string';
+
+$var =
+
+    // Comment
+
+    'some' . 'long' . 'string';
+
+$var =
+    /* comment */
+    'some' . 'long' . 'string';
+
+$var =
+    /**
+     * multi-line
+     */
+    'some' . 'long' . 'string';
+
+$var =
+    /* inline */ 'some' . 'long' . 'string' . 'some' . 'long' . 'string' . 'some' . 'long' . 'string' . 'some' . 'long' . 'string';
+
+$var = // Comment
+    // Comment
+    'some' . 'long' . 'string';
+
+$var = // Comment
+    'some' . 'long' . 'string';
+
+$var = /* Comment */
+    // Comment
+    'some' . 'long' . 'string';
+
+$var = // Comment/
+    /* Comment */
+    'some' . 'long' . 'string';
+
+$var = /* Comment */
+    /* Comment */'some' .
+    'long' .
+    'string';