Merge pull request #4 from jlongster/call-chain

Initial attempt at chaining call expressions

src/comments.js

@@ -317,7 +317,7 @@ function printTrailingComment(commentPath, print) {
   //     }
   // }
 
-  parts.push(print(commentPath));
+  parts.push(print(commentPath), hardline);
 
   return concat(parts);
 }

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,23 @@ function genericPrintNoParens(path, options, print) {
     parts.push(";");
 
     return concat(parts);
-  case "CallExpression":
+  case "CallExpression": {
+    const parent = path.getParentNode();
+    // 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 printed = printMemberChain(n, options, print);
+      if(printed) {
+        return printed;
+      }
+    }
+
     return concat([
       path.call(print, "callee"),
       printArgumentsList(path, options, print)
     ]);
+  }
+
   case "ObjectExpression":
   case "ObjectPattern":
   case "ObjectTypeAnnotation":
@@ -1914,6 +1916,119 @@ 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]
+  );
+}
+
+// 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 ]);

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());
 "
 `;

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;
+  });
 "
 `;
 

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();
+  });
 "
 `;