From d5ab6ab921d89ecf32fdf41266c5c1b43bbfd079 Mon Sep 17 00:00:00 2001
From: James Long <longster@gmail.com>
Date: Thu, 5 Jan 2017 15:26:45 -0500
Subject: [PATCH 1/3] Detect chained call expressions and format them
 specifically

---
 src/comments.js                               |   2 +-
 src/printer.js                                |  89 +++++++++++---
 .../stream/__snapshots__/jsfmt.spec.js.snap   |   7 +-
 .../promises/__snapshots__/jsfmt.spec.js.snap | 116 +++++++++++-------
 tests/union/__snapshots__/jsfmt.spec.js.snap  |  12 +-
 5 files changed, 157 insertions(+), 69 deletions(-)

diff --git a/src/comments.js b/src/comments.js
index bede556b043e..7c2179f26f8d 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -317,7 +317,7 @@ function printTrailingComment(commentPath, print) {
   //     }
   // }
 
-  parts.push(print(commentPath));
+  parts.push(print(commentPath), hardline);
 
   return concat(parts);
 }
diff --git a/src/printer.js b/src/printer.js
index efbc02462595..441febd7648d 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -294,19 +294,9 @@ function genericPrintNoParens(path, options, print) {
       " = ",
       path.call(print, "right")
     ]);
-  case "MemberExpression":
-
-    parts.push(path.call(print, "object"));
-
-    var property = path.call(print, "property");
-
-    if (n.computed) {
-      parts.push("[", property, "]");
-    } else {
-      parts.push(".", property);
-    }
-
-    return concat(parts);
+  case "MemberExpression": {
+    return concat([path.call(print, "object"), printMemberLookup(path, print)]);
+  }
   case "MetaProperty":
     return concat([
       path.call(print, "meta"),
@@ -620,11 +610,72 @@ function genericPrintNoParens(path, options, print) {
     parts.push(";");
 
     return concat(parts);
-  case "CallExpression":
+  case "CallExpression": {
+    const parent = path.getParentNode();
+    // We detect calls on member expressions specially to format a
+    // comman pattern better. The pattern we are looking for is this:
+    //
+    // arr
+    //   .map(x => x + 1)
+    //   .filter(x => x > 10)
+    //   .some(x => x % 2)
+    //
+    // where there is a "chain" of function calls on the result of
+    // previous expressions. We want to format them like above so they
+    // are consistently aligned.
+    //
+    // The way we do is by eagerly traversing the AST tree and
+    // re-shape it into a list of calls on member expressions. This
+    // lets us implement a heuristic easily for when we want to format
+    // it: if there are more than 1 calls on a member expression that
+    // pass in a function, we treat it like the above.
+    if(n.callee.type === "MemberExpression") {
+      const nodes = [];
+      let curr = n;
+      // Traverse down and gather up all of the calls on member
+      // expressions. This flattens it out into a list that we can
+      // easily analyze.
+      while(curr.type === "CallExpression" && curr.callee.type === "MemberExpression") {
+        nodes.push({ property: curr.callee.property, call: curr });
+        curr = curr.callee.object;
+      }
+
+      // There are two kinds of formats we want to specialize: first,
+      // if there are multiple calls on lookups we want to group them
+      // together so they will all break at the same time. Second,
+      // it's a chain if there 2 or more calls pass in functions and
+      // we want to forcibly break all of the lookups onto new lines
+      // and indent them.
+      const hasMultipleLookups = nodes.length > 1;
+      const isChain = nodes.filter(n => {
+        return n.call.arguments.length > 0 &&
+          (n.call.arguments[0].type === "FunctionExpression" ||
+           n.call.arguments[0].type === "ArrowFunctionExpression");
+      }).length > 1;
+
+      if(hasMultipleLookups) {
+        return group(concat([
+          print(FastPath.from(curr)),
+          concat(nodes.reverse().map(node => {
+            const printed = concat([
+              isChain ? hardline : softline,
+                ".",
+                print(FastPath.from(node.property)),
+                printArgumentsList(FastPath.from(node.call), options, print)
+            ]);
+
+            return isChain ? indent(options.tabWidth, printed) : printed;
+          }))
+        ]));
+      }
+    }
+
     return concat([
       path.call(print, "callee"),
       printArgumentsList(path, options, print)
     ]);
+  }
+
   case "ObjectExpression":
   case "ObjectPattern":
   case "ObjectTypeAnnotation":
@@ -1914,6 +1965,16 @@ function printClass(path, print) {
   return parts;
 }
 
+function printMemberLookup(path, print) {
+  const property = path.call(print, "property");
+  const n = path.getValue();
+  return concat(
+    n.computed ?
+      ["[", property, "]"] :
+      [".", property]
+  );
+}
+
 function adjustClause(clause, options, forceSpace) {
   if (isCurlyBracket(clause) || forceSpace) {
     return concat([ " ", clause ]);
diff --git a/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap b/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap
index 331dc90020ee..076d52431a6d 100644
--- a/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap
@@ -67,8 +67,9 @@ class MyWriteStream extends stream.Writable {}
 class MyDuplex extends stream.Duplex {}
 class MyTransform extends stream.Duplex {}
 
-new MyReadStream().pipe(
-  new MyDuplex()
-).pipe(new MyTransform()).pipe(new MyWriteStream());
+new MyReadStream()
+.pipe(new MyDuplex())
+.pipe(new MyTransform())
+.pipe(new MyWriteStream());
 "
 `;
diff --git a/tests/promises/__snapshots__/jsfmt.spec.js.snap b/tests/promises/__snapshots__/jsfmt.spec.js.snap
index 724968c31c4b..530fc7333ebb 100644
--- a/tests/promises/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/promises/__snapshots__/jsfmt.spec.js.snap
@@ -529,59 +529,83 @@ Promise.reject(Promise.resolve(0)).then(function(num) {
   var b: number = num;
 });
 
-Promise.resolve(0).then(function(num) {
-  return \"asdf\";
-}).then(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .resolve(0)
+  .then(function(num) {
+    return \"asdf\";
+  })
+  .then(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.resolve(0).then(function(num) {
-  return Promise.resolve(\"asdf\");
-}).then(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .resolve(0)
+  .then(function(num) {
+    return Promise.resolve(\"asdf\");
+  })
+  .then(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.resolve(0).then(function(num) {
-  return Promise.resolve(Promise.resolve(\"asdf\"));
-}).then(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .resolve(0)
+  .then(function(num) {
+    return Promise.resolve(Promise.resolve(\"asdf\"));
+  })
+  .then(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.resolve(0).then(function(num) {
-  throw \"str\";
-}).catch(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .resolve(0)
+  .then(function(num) {
+    throw \"str\";
+  })
+  .catch(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.reject(0).catch(function(num) {
-  return \"asdf\";
-}).then(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .reject(0)
+  .catch(function(num) {
+    return \"asdf\";
+  })
+  .then(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.reject(0).catch(function(num) {
-  return Promise.resolve(\"asdf\");
-}).then(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .reject(0)
+  .catch(function(num) {
+    return Promise.resolve(\"asdf\");
+  })
+  .then(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.reject(0).catch(function(num) {
-  return Promise.resolve(Promise.resolve(\"asdf\"));
-}).then(function(str) {
-  var a: string = str;
-  var b: number = str;
-});
+Promise
+  .reject(0)
+  .catch(function(num) {
+    return Promise.resolve(Promise.resolve(\"asdf\"));
+  })
+  .then(function(str) {
+    var a: string = str;
+    var b: number = str;
+  });
 
-Promise.resolve(0).catch(function(err) {}).then(function(num) {
-  var a: number = num;
-  var b: string = num;
-});
+Promise
+  .resolve(0)
+  .catch(function(err) {})
+  .then(function(num) {
+    var a: number = num;
+    var b: string = num;
+  });
 "
 `;
 
diff --git a/tests/union/__snapshots__/jsfmt.spec.js.snap b/tests/union/__snapshots__/jsfmt.spec.js.snap
index d3340c8952d4..33a6c05e81ee 100644
--- a/tests/union/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/union/__snapshots__/jsfmt.spec.js.snap
@@ -86,11 +86,13 @@ exports[`test issue-198.js 1`] = `
 // This should fail because str is string, not number
 var p = new Promise(function(resolve, reject) {
   resolve(5);
-}).then(function(num) {
-  return num.toFixed();
-}).then(function(str) {
-  return str.toFixed();
-});
+})
+  .then(function(num) {
+    return num.toFixed();
+  })
+  .then(function(str) {
+    return str.toFixed();
+  });
 "
 `;
 

From fe4484d2e3c17ea5fa7897c01bfd97406fc2ac5a Mon Sep 17 00:00:00 2001
From: James Long <longster@gmail.com>
Date: Sun, 8 Jan 2017 14:00:41 -0500
Subject: [PATCH 2/3] Update call/member expression chaining heuristic to
 account for NewExpression's

---
 src/printer.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/printer.js b/src/printer.js
index 441febd7648d..b3c5f506d794 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -650,7 +650,8 @@ function genericPrintNoParens(path, options, print) {
       const isChain = nodes.filter(n => {
         return n.call.arguments.length > 0 &&
           (n.call.arguments[0].type === "FunctionExpression" ||
-           n.call.arguments[0].type === "ArrowFunctionExpression");
+           n.call.arguments[0].type === "ArrowFunctionExpression" ||
+           n.call.arguments[0].type === "NewExpression");
       }).length > 1;
 
       if(hasMultipleLookups) {

From c10681d148c7151dac3ffbdd8359cd2d6531fb40 Mon Sep 17 00:00:00 2001
From: James Long <longster@gmail.com>
Date: Sun, 8 Jan 2017 14:44:44 -0500
Subject: [PATCH 3/3] Pull out special chain printing into utility function

---
 src/printer.js                                | 163 ++++++++++++------
 .../stream/__snapshots__/jsfmt.spec.js.snap   |   6 +-
 2 files changed, 111 insertions(+), 58 deletions(-)

diff --git a/src/printer.js b/src/printer.js
index b3c5f506d794..da5a48805c74 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -612,62 +612,12 @@ function genericPrintNoParens(path, options, print) {
     return concat(parts);
   case "CallExpression": {
     const parent = path.getParentNode();
-    // We detect calls on member expressions specially to format a
-    // comman pattern better. The pattern we are looking for is this:
-    //
-    // arr
-    //   .map(x => x + 1)
-    //   .filter(x => x > 10)
-    //   .some(x => x % 2)
-    //
-    // where there is a "chain" of function calls on the result of
-    // previous expressions. We want to format them like above so they
-    // are consistently aligned.
-    //
-    // The way we do is by eagerly traversing the AST tree and
-    // re-shape it into a list of calls on member expressions. This
-    // lets us implement a heuristic easily for when we want to format
-    // it: if there are more than 1 calls on a member expression that
-    // pass in a function, we treat it like the above.
+    // We detect calls on member lookups and possibly print them in a
+    // special chain format. See `printMemberChain` for more info.
     if(n.callee.type === "MemberExpression") {
-      const nodes = [];
-      let curr = n;
-      // Traverse down and gather up all of the calls on member
-      // expressions. This flattens it out into a list that we can
-      // easily analyze.
-      while(curr.type === "CallExpression" && curr.callee.type === "MemberExpression") {
-        nodes.push({ property: curr.callee.property, call: curr });
-        curr = curr.callee.object;
-      }
-
-      // There are two kinds of formats we want to specialize: first,
-      // if there are multiple calls on lookups we want to group them
-      // together so they will all break at the same time. Second,
-      // it's a chain if there 2 or more calls pass in functions and
-      // we want to forcibly break all of the lookups onto new lines
-      // and indent them.
-      const hasMultipleLookups = nodes.length > 1;
-      const isChain = nodes.filter(n => {
-        return n.call.arguments.length > 0 &&
-          (n.call.arguments[0].type === "FunctionExpression" ||
-           n.call.arguments[0].type === "ArrowFunctionExpression" ||
-           n.call.arguments[0].type === "NewExpression");
-      }).length > 1;
-
-      if(hasMultipleLookups) {
-        return group(concat([
-          print(FastPath.from(curr)),
-          concat(nodes.reverse().map(node => {
-            const printed = concat([
-              isChain ? hardline : softline,
-                ".",
-                print(FastPath.from(node.property)),
-                printArgumentsList(FastPath.from(node.call), options, print)
-            ]);
-
-            return isChain ? indent(options.tabWidth, printed) : printed;
-          }))
-        ]));
+      const printed = printMemberChain(n, options, print);
+      if(printed) {
+        return printed;
       }
     }
 
@@ -1976,6 +1926,109 @@ function printMemberLookup(path, print) {
   );
 }
 
+// We detect calls on member expressions specially to format a
+// comman pattern better. The pattern we are looking for is this:
+//
+// arr
+//   .map(x => x + 1)
+//   .filter(x => x > 10)
+//   .some(x => x % 2)
+//
+// where there is a "chain" of function calls on the result of
+// previous expressions. We want to format them like above so they
+// are consistently aligned.
+//
+// The way we do is by eagerly traversing the AST tree and
+// re-shape it into a list of calls on member expressions. This
+// lets us implement a heuristic easily for when we want to format
+// it: if there are more than 1 calls on a member expression that
+// pass in a function, we treat it like the above.
+function printMemberChain(node, options, print) {
+  const nodes = [];
+  let curr = node;
+  // Traverse down and gather up all of the calls on member
+  // expressions. This flattens it out into a list that we can
+  // easily analyze.
+  while(curr.type === "CallExpression" && curr.callee.type === "MemberExpression") {
+    nodes.push({ property: curr.callee.property, call: curr });
+    curr = curr.callee.object;
+  }
+  nodes.reverse();
+
+  // There are two kinds of formats we want to specialize: first,
+  // if there are multiple calls on lookups we want to group them
+  // together so they will all break at the same time. Second,
+  // it's a chain if there 2 or more calls pass in functions and
+  // we want to forcibly break all of the lookups onto new lines
+  // and indent them.
+  function argIsFunction(call) {
+    if(call.arguments.length > 0) {
+      const type = call.arguments[0].type;
+      return type === "FunctionExpression" ||
+        type === "ArrowFunctionExpression" ||
+        type === "NewExpression";
+    }
+    return false;
+  }
+  const hasMultipleLookups = nodes.length > 1;
+  const isChain = (
+    hasMultipleLookups &&
+      nodes.filter(n => argIsFunction(n.call)).length > 1
+  );
+
+  if(hasMultipleLookups) {
+    const currPrinted = print(FastPath.from(curr));
+    const nodesPrinted = nodes.map(node => ({
+      property: print(FastPath.from(node.property)),
+      args: printArgumentsList(FastPath.from(node.call), options, print)
+    }));
+    const fullyExpanded = (
+      concat([
+        currPrinted,
+        concat(nodesPrinted.map(node => {
+          return indent(
+            options.tabWidth,
+            concat([
+              hardline,
+              ".",
+              node.property,
+              node.args
+            ])
+          );
+        }))
+      ])
+    );
+
+    // If it's a chain, force it to be fully expanded and print a
+    // newline before each lookup. If we're not sure if it's a
+    // chain (it *might* be printed on one line, but if gets too
+    // long it will be printed as a chain), we need to use
+    // `conditionalGroup` to describe both of these
+    // representations. We cannot describe both at the same time
+    // because the fully expanded form adds indentation, which
+    // messes up anything with hard lines.
+    if(isChain) {
+      return fullyExpanded;
+    }
+    else {
+      return conditionalGroup([
+        concat([
+          currPrinted,
+          concat(nodesPrinted.map(node => {
+            return concat([
+              ".",
+              node.property,
+              node.args
+            ]);
+          }))
+        ]),
+
+        fullyExpanded
+      ]);
+    }
+  }
+}
+
 function adjustClause(clause, options, forceSpace) {
   if (isCurlyBracket(clause) || forceSpace) {
     return concat([ " ", clause ]);
diff --git a/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap b/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap
index 076d52431a6d..c8c6576d768d 100644
--- a/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap
@@ -68,8 +68,8 @@ class MyDuplex extends stream.Duplex {}
 class MyTransform extends stream.Duplex {}
 
 new MyReadStream()
-.pipe(new MyDuplex())
-.pipe(new MyTransform())
-.pipe(new MyWriteStream());
+  .pipe(new MyDuplex())
+  .pipe(new MyTransform())
+  .pipe(new MyWriteStream());
 "
 `;