Repository: jlongster/prettier (js)

Issue:
Long-length import statements could wrap
In the example:

```js
import someCoolUtilWithARatherLongName from '../../../../utils/someCoolUtilWithARatherLongName';
```

Which is 94 characters. Given that I want to wrap at 80 characters, my subjective ideal would be

```js
import someCoolUtilWithARatherLongName
  from '../../../../utils/someCoolUtilWithARatherLongName';
```

This isn't an issue with the `import { name } from 'source'` syntax.

Pull request description:
Break long imports
Fixes #576

Patch:
diff --git a/src/printer.js b/src/printer.js
index 941c9bb0a..d2a44514a 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -17,6 +17,7 @@ var group = docBuilders.group;
 var indent = docBuilders.indent;
 var conditionalGroup = docBuilders.conditionalGroup;
 var ifBreak = docBuilders.ifBreak;
+var breakParent = docBuilders.breakParent;
 
 var docUtils = require("./doc-utils");
 var willBreak = docUtils.willBreak;
@@ -261,6 +262,8 @@ function genericPrintNoParens(path, options, print) {
           !n.rest &&
           n.params[0].type === "Identifier" &&
           !n.params[0].typeAnnotation &&
+          !n.params[0].leadingComments &&
+          !n.params[0].trailingComments &&
           !n.predicate &&
           !n.returnType
       ) {
@@ -414,14 +417,15 @@ function genericPrintNoParens(path, options, print) {
     case "ImportDeclaration":
       parts.push("import ");
 
+      const fromParts = [];
+
       if (n.importKind && n.importKind !== "value") {
         parts.push(n.importKind + " ");
       }
 
+      var standalones = [];
+      var grouped = [];
       if (n.specifiers && n.specifiers.length > 0) {
-        var standalones = [];
-        var grouped = [];
-
         path.each(
           function(specifierPath) {
             var value = specifierPath.getValue();
@@ -465,12 +469,28 @@ function genericPrintNoParens(path, options, print) {
           );
         }
 
-        parts.push(" from ");
+        fromParts.push(grouped.length === 0 ? line : " ", "from ");
       }
 
-      parts.push(path.call(print, "source"), ";");
+      fromParts.push(path.call(print, "source"), ";");
 
-      return concat(parts);
+      // If there's a very long import, break the following way:
+      //
+      //   import veryLong
+      //     from 'verylong'
+      //
+      // In case there are grouped elements, they will already break the way
+      // we want and this break would take precedence instead.
+      if (grouped.length === 0) {
+        return group(
+          concat([
+            concat(parts),
+            indent(options.tabWidth, concat(fromParts))
+          ])
+        );
+      }
+
+      return concat([concat(parts), concat(fromParts)]);
 
     case "Import": {
       return "import";
@@ -1187,7 +1207,14 @@ function genericPrintNoParens(path, options, print) {
       ]);
     case "ClassBody":
       if (n.body.length === 0) {
-        return "{}";
+        return group(
+          concat([
+            "{",
+            comments.printDanglingComments(path, options),
+            softline,
+            "}"
+          ])
+        );
       }
 
       return concat([
@@ -1743,32 +1770,35 @@ function printArgumentsList(path, options, print) {
 
   if (groupLastArg) {
     const shouldBreak = printed.slice(0, -1).some(willBreak);
-    return conditionalGroup(
-      [
-        concat(["(", join(concat([", "]), printed), ")"]),
-        concat([
-          "(",
-          join(concat([",", line]), printed.slice(0, -1)),
-          printed.length > 1 ? ", " : "",
-          group(util.getLast(printed), { shouldBreak: true }),
-          ")"
-        ]),
-        group(
+    return concat([
+      printed.some(willBreak) ? breakParent : "",
+      conditionalGroup(
+        [
+          concat(["(", join(concat([", "]), printed), ")"]),
           concat([
             "(",
-            indent(
-              options.tabWidth,
-              concat([line, join(concat([",", line]), printed)])
-            ),
-            options.trailingComma ? "," : "",
-            line,
+            join(concat([",", line]), printed.slice(0, -1)),
+            printed.length > 1 ? ", " : "",
+            group(util.getLast(printed), { shouldBreak: true }),
             ")"
           ]),
-          { shouldBreak: true }
-        )
-      ],
-      { shouldBreak }
-    );
+          group(
+            concat([
+              "(",
+              indent(
+                options.tabWidth,
+                concat([line, join(concat([",", line]), printed)])
+              ),
+              options.trailingComma ? "," : "",
+              line,
+              ")"
+            ]),
+            { shouldBreak: true }
+          )
+        ],
+        { shouldBreak }
+      )
+    ]);
   }
 
   return group(
@@ -2093,7 +2123,11 @@ function printMemberChain(path, options, print) {
   //     .d()
   //     .e
 
-  // The first group is the first node followed by as many CallExpression as possible
+  // The first group is the first node followed by
+  //   - as many CallExpression as possible
+  //       < fn()()() >.something()
+  //   - then, as many MemberExpression as possible but the last one
+  //       < this.items >.something()
   var groups = [];
   var currentGroup = [printedNodes[0]];
   var i = 1;
@@ -2104,6 +2138,14 @@ function printMemberChain(path, options, print) {
       break;
     }
   }
+  for (; i + 1 < printedNodes.length; ++i) {
+    if (printedNodes[i].node.type === "MemberExpression" &&
+        printedNodes[i + 1].node.type === "MemberExpression") {
+      currentGroup.push(printedNodes[i]);
+    } else {
+      break;
+    }
+  }
   groups.push(currentGroup);
   currentGroup = [];
 


Test patch:
diff --git a/tests/classes/__snapshots__/jsfmt.spec.js.snap b/tests/classes/__snapshots__/jsfmt.spec.js.snap
index d5a3c2580..676701dae 100644
--- a/tests/classes/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/classes/__snapshots__/jsfmt.spec.js.snap
@@ -31,6 +31,27 @@ exports[`test call.js 1`] = `
 "
 `;
 
+exports[`test empty.js 1`] = `
+"class A {
+  // comment
+}
+
+class A { // comment
+}
+
+class A {
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+class A {
+  // comment
+}
+
+class A {} // comment
+
+class A {}
+"
+`;
+
 exports[`test member.js 1`] = `
 "(class {})[1];
 (class {}).a;
diff --git a/tests/classes/empty.js b/tests/classes/empty.js
new file mode 100644
index 000000000..c4141e05c
--- /dev/null
+++ b/tests/classes/empty.js
@@ -0,0 +1,9 @@
+class A {
+  // comment
+}
+
+class A { // comment
+}
+
+class A {
+}
diff --git a/tests/flow_comments/__snapshots__/jsfmt.spec.js.snap b/tests/flow_comments/__snapshots__/jsfmt.spec.js.snap
new file mode 100644
index 000000000..bdc17200d
--- /dev/null
+++ b/tests/flow_comments/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,29 @@
+exports[`test arrow.js 1`] = `
+"// Error
+const beep = (data/*: Object*/) => {}
+
+// OK
+const beep = (data/*: Object*/, secondData/*: Object*/) => {}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// Error
+const beep = (data: Object) => {};
+
+// OK
+const beep = (data: Object, secondData: Object) => {};
+"
+`;
+
+exports[`test arrow.js 2`] = `
+"// Error
+const beep = (data/*: Object*/) => {}
+
+// OK
+const beep = (data/*: Object*/, secondData/*: Object*/) => {}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// Error
+const beep = (data /*: Object*/) => {};
+
+// OK
+const beep = (data /*: Object*/, secondData /*: Object*/) => {};
+"
+`;
diff --git a/tests/flow_comments/arrow.js b/tests/flow_comments/arrow.js
new file mode 100644
index 000000000..7f5d39d75
--- /dev/null
+++ b/tests/flow_comments/arrow.js
@@ -0,0 +1,5 @@
+// Error
+const beep = (data/*: Object*/) => {}
+
+// OK
+const beep = (data/*: Object*/, secondData/*: Object*/) => {}
diff --git a/tests/flow_comments/jsfmt.spec.js b/tests/flow_comments/jsfmt.spec.js
new file mode 100644
index 000000000..d5ec770a9
--- /dev/null
+++ b/tests/flow_comments/jsfmt.spec.js
@@ -0,0 +1,2 @@
+run_spec(__dirname);
+run_spec(__dirname, {parser: 'babylon'});
diff --git a/tests/import/__snapshots__/jsfmt.spec.js.snap b/tests/import/__snapshots__/jsfmt.spec.js.snap
index 6ccad1ddc..921f31470 100644
--- a/tests/import/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/import/__snapshots__/jsfmt.spec.js.snap
@@ -52,6 +52,22 @@ import {fitsIn, oneLine} from \".\";
 "
 `;
 
+exports[`test long-line.js 1`] = `
+"import someCoolUtilWithARatherLongName from \'../../../../utils/someCoolUtilWithARatherLongName\';
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+import someCoolUtilWithARatherLongName
+  from \"../../../../utils/someCoolUtilWithARatherLongName\";
+"
+`;
+
+exports[`test long-line.js 2`] = `
+"import someCoolUtilWithARatherLongName from \'../../../../utils/someCoolUtilWithARatherLongName\';
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+import someCoolUtilWithARatherLongName
+  from \"../../../../utils/someCoolUtilWithARatherLongName\";
+"
+`;
+
 exports[`test multiple_standalones.js 1`] = `
 "import a, * as b from \'a\';
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/tests/import/long-line.js b/tests/import/long-line.js
new file mode 100644
index 000000000..10fb3bd16
--- /dev/null
+++ b/tests/import/long-line.js
@@ -0,0 +1,1 @@
+import someCoolUtilWithARatherLongName from '../../../../utils/someCoolUtilWithARatherLongName';
diff --git a/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap b/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap
index 93bce8cbf..d298b8359 100644
--- a/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap
@@ -35,6 +35,61 @@ export default function searchUsers(action$) {
 "
 `;
 
+exports[`test break-parent.js 1`] = `
+"({
+  processors: [
+    require(\"autoprefixer\", {
+      browsers: [\"> 1%\", \"last 2 versions\", \"ie >= 11\", \"Firefox ESR\"]
+    }),
+    require(\"postcss-url\")({
+      url: url =>
+        url.startsWith(\"/\") || /^[a-z]+:/.test(url) ? url : \`/static/\${url}\`
+    })
+  ]
+});
+
+true
+  ? test({
+      a: 1
+    })
+  : <div
+      a={123412342314}
+      b={123412341234}
+      c={123412341234}
+      d={123412341234}
+      e={123412341234}
+      f={123412341234}
+      g={123412341234}
+    />;
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+({
+  processors: [
+    require(\"autoprefixer\", {
+      browsers: [\"> 1%\", \"last 2 versions\", \"ie >= 11\", \"Firefox ESR\"]
+    }),
+    require(\"postcss-url\")({
+      url: url =>
+        url.startsWith(\"/\") || /^[a-z]+:/.test(url) ? url : \`/static/\${url}\`
+    })
+  ]
+});
+
+true
+  ? test({
+      a: 1
+    })
+  : <div
+      a={123412342314}
+      b={123412341234}
+      c={123412341234}
+      d={123412341234}
+      e={123412341234}
+      f={123412341234}
+      g={123412341234}
+    />;
+"
+`;
+
 exports[`test jsx.js 1`] = `
 "const els = items.map(item => (
   <div className=\"whatever\">
diff --git a/tests/last_argument_expansion/break-parent.js b/tests/last_argument_expansion/break-parent.js
new file mode 100644
index 000000000..698f0a022
--- /dev/null
+++ b/tests/last_argument_expansion/break-parent.js
@@ -0,0 +1,25 @@
+({
+  processors: [
+    require("autoprefixer", {
+      browsers: ["> 1%", "last 2 versions", "ie >= 11", "Firefox ESR"]
+    }),
+    require("postcss-url")({
+      url: url =>
+        url.startsWith("/") || /^[a-z]+:/.test(url) ? url : `/static/${url}`
+    })
+  ]
+});
+
+true
+  ? test({
+      a: 1
+    })
+  : <div
+      a={123412342314}
+      b={123412341234}
+      c={123412341234}
+      d={123412341234}
+      e={123412341234}
+      f={123412341234}
+      g={123412341234}
+    />;
diff --git a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap b/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
index 647e4a013..86427b531 100644
--- a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
@@ -166,3 +166,14 @@ method().then(x => x)[\"abc\"](x => x)[abc](x => x);
 ({}).a().b();
 "
 `;
+
+exports[`test this.js 1`] = `
+"const sel = this.connections
+  .concat(this.activities.concat(this.operators))
+  .filter(x => x.selected);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const sel = this.connections
+  .concat(this.activities.concat(this.operators))
+  .filter(x => x.selected);
+"
+`;
diff --git a/tests/method-chain/this.js b/tests/method-chain/this.js
new file mode 100644
index 000000000..da7fb1cef
--- /dev/null
+++ b/tests/method-chain/this.js
@@ -0,0 +1,3 @@
+const sel = this.connections
+  .concat(this.activities.concat(this.operators))
+  .filter(x => x.selected);