Group first arg for inline functions (#947)

* Group first arg for inline functions

* Group first arg. Unless there are comments.

* Group first arg. Allow second arg to be empty object/array.

This implementation has a side effect of disallowing empty objects/arrays in the should group last arg heuristic.

src/printer.js

@@ -1925,25 +1925,43 @@ function printMethod(path, options, print) {
   return concat(parts);
 }
 
+function couldGroupArg(arg) {
+  return ((arg.type === "ObjectExpression" && arg.properties.length > 0) ||
+      (arg.type === "ArrayExpression" && arg.elements.length > 0) ||
+      arg.type === "FunctionExpression" ||
+      (arg.type === "ArrowFunctionExpression" &&
+        (arg.body.type === "BlockStatement" ||
+          arg.body.type === "ArrowFunctionExpression" ||
+          arg.body.type === "ObjectExpression" ||
+          arg.body.type === "ArrayExpression" ||
+          arg.body.type === "CallExpression" ||
+          arg.body.type === "JSXElement")));
+}
+
 function shouldGroupLastArg(args) {
   const lastArg = util.getLast(args);
   const penultimateArg = util.getPenultimate(args);
   return (!lastArg.comments || !lastArg.comments.length) &&
-    (lastArg.type === "ObjectExpression" ||
-      lastArg.type === "ArrayExpression" ||
-      lastArg.type === "FunctionExpression" ||
-      (lastArg.type === "ArrowFunctionExpression" &&
-        (lastArg.body.type === "BlockStatement" ||
-          lastArg.body.type === "ArrowFunctionExpression" ||
-          lastArg.body.type === "ObjectExpression" ||
-          lastArg.body.type === "ArrayExpression" ||
-          lastArg.body.type === "CallExpression" ||
-          lastArg.body.type === "JSXElement"))) &&
+    couldGroupArg(lastArg) &&
     // If the last two arguments are of the same type,
     // disable last element expansion.
     (!penultimateArg || penultimateArg.type !== lastArg.type);
 }
 
+function shouldGroupFirstArg(args) {
+  if (args.length !== 2) {
+    return false;
+  }
+
+  const firstArg = args[0];
+  const secondArg = args[1];
+  return (!firstArg.comments || !firstArg.comments.length) &&
+    (firstArg.type === 'FunctionExpression' ||
+      (firstArg.type === 'ArrowFunctionExpression' &&
+        firstArg.body.type === 'BlockStatement')) &&
+        !couldGroupArg(secondArg);
+}
+
 function printArgumentsList(path, options, print) {
   var printed = path.map(print, "arguments");
 
@@ -1959,20 +1977,31 @@ function printArgumentsList(path, options, print) {
   // This is just an optimization; I think we could return the
   // conditional group for all function calls, but it's more expensive
   // so only do it for specific forms.
-  if (shouldGroupLastArg(args)) {
-    const shouldBreak = printed.slice(0, -1).some(willBreak);
+  const shouldGroupFirst = shouldGroupFirstArg(args);
+  if (shouldGroupFirst || shouldGroupLastArg(args)) {
+    const shouldBreak = shouldGroupFirst
+      ? printed.slice(1).some(willBreak)
+      : printed.slice(0, -1).some(willBreak);
     return concat([
       printed.some(willBreak) ? breakParent : "",
       conditionalGroup(
         [
           concat(["(", join(concat([", "]), printed), ")"]),
-          concat([
-            "(",
-            join(concat([",", line]), printed.slice(0, -1)),
-            printed.length > 1 ? ", " : "",
-            group(util.getLast(printed), { shouldBreak: true }),
-            ")"
-          ]),
+          shouldGroupFirst
+            ? concat([
+                "(",
+                group(printed[0], { shouldBreak: true }),
+                printed.length > 1 ? ", " : "",
+                join(concat([",", line]), printed.slice(1)),
+                ")"
+              ])
+            : concat([
+                "(",
+                join(concat([",", line]), printed.slice(0, -1)),
+                printed.length > 1 ? ", " : "",
+                group(util.getLast(printed), { shouldBreak: true }),
+                ")"
+              ]),
           group(
             concat([
               "(",
@@ -2042,8 +2071,8 @@ function printFunctionParams(path, print, options) {
   const canHaveTrailingComma = !(lastParam &&
     lastParam.type === "RestElement") && !fun.rest;
 
-  // If the parent is a call with the last argument expansion and this is the
-  // params of the last argument, we dont want the arguments to break and instead
+  // If the parent is a call with the first/last argument expansion and this is the
+  // params of the first/last argument, we dont want the arguments to break and instead
   // want the whole expression to be on a new line.
   //
   // Good:                 Bad:
@@ -2055,8 +2084,10 @@ function printFunctionParams(path, print, options) {
   const parent = path.getParentNode();
   if (
     (parent.type === "CallExpression" || parent.type === "NewExpression") &&
-    util.getLast(parent.arguments) === path.getValue() &&
-    shouldGroupLastArg(parent.arguments)
+    ((util.getLast(parent.arguments) === path.getValue() &&
+      shouldGroupLastArg(parent.arguments)) ||
+      (parent.arguments[0] === path.getValue() &&
+        shouldGroupFirstArg(parent.arguments)))
   ) {
     return concat(["(", join(", ", printed), ")"]);
   }

tests/first_argument_expansion/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,200 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`test.js 1`] = `
+"setTimeout(function() {
+  thing();
+}, 500);
+
+[\\"a\\",\\"b\\",\\"c\\"].reduce(function(item, thing) {
+  return thing + \\" \\" + item;
+}, \\"letters:\\")
+
+func(() => {
+  thing();
+}, identifier);
+
+func(function() {
+  thing();
+}, this.props.timeout * 1000);
+
+func((that) => {
+  thing();
+}, this.props.getTimeout());
+
+func(() => {
+  thing();
+}, true);
+
+func(() => {
+  thing();
+}, null);
+
+func(() => {
+  thing();
+}, undefined);
+
+func(() => {
+  thing();
+}, /regex.*?/);
+
+compose((a) => {
+  return a.thing;
+}, b => b * b);
+
+somthing.reduce(function(item, thing) {
+  return thing.blah =  item;
+}, {})
+
+somthing.reduce(function(item, thing) {
+  return thing.push(item);
+}, [])
+
+reallyLongLongLongLongLongLongLongLongLongLongLongLongLongLongMethod((f, g, h) => {
+  return f.pop();
+}, true);
+
+// Don't do the rest of these
+
+func(function() {
+  thing();
+}, true, false);
+
+func(() => {
+  thing();
+}, {yes: true, cats: 5});
+
+compose((a) => {
+  return a.thing;
+}, b => {
+  return b + \\"\\";
+});
+
+compose((a) => {
+  return a.thing;
+}, b => [1, 2, 3, 4, 5]);
+
+renderThing(a =>
+  <div>Content. So much to say. Oh my. Are we done yet?</div>
+,args);
+
+setTimeout(
+  // Something
+  function() {
+    thing();
+  },
+  500
+);
+
+setTimeout(/* blip */ function() {
+  thing();
+}, 500);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+setTimeout(function() {
+  thing();
+}, 500);
+
+[\\"a\\", \\"b\\", \\"c\\"].reduce(function(item, thing) {
+  return thing + \\" \\" + item;
+}, \\"letters:\\");
+
+func(() => {
+  thing();
+}, identifier);
+
+func(function() {
+  thing();
+}, this.props.timeout * 1000);
+
+func(that => {
+  thing();
+}, this.props.getTimeout());
+
+func(() => {
+  thing();
+}, true);
+
+func(() => {
+  thing();
+}, null);
+
+func(() => {
+  thing();
+}, undefined);
+
+func(() => {
+  thing();
+}, /regex.*?/);
+
+compose(a => {
+  return a.thing;
+}, b => b * b);
+
+somthing.reduce(function(item, thing) {
+  return (thing.blah = item);
+}, {});
+
+somthing.reduce(function(item, thing) {
+  return thing.push(item);
+}, []);
+
+reallyLongLongLongLongLongLongLongLongLongLongLongLongLongLongMethod(
+  (f, g, h) => {
+    return f.pop();
+  },
+  true
+);
+
+// Don't do the rest of these
+
+func(
+  function() {
+    thing();
+  },
+  true,
+  false
+);
+
+func(
+  () => {
+    thing();
+  },
+  { yes: true, cats: 5 }
+);
+
+compose(
+  a => {
+    return a.thing;
+  },
+  b => {
+    return b + \\"\\";
+  }
+);
+
+compose(
+  a => {
+    return a.thing;
+  },
+  b => [1, 2, 3, 4, 5]
+);
+
+renderThing(
+  a => <div>Content. So much to say. Oh my. Are we done yet?</div>,
+  args
+);
+
+setTimeout(
+  // Something
+  function() {
+    thing();
+  },
+  500
+);
+
+setTimeout(
+  /* blip */ function() {
+    thing();
+  },
+  500
+);
+"
+`;

tests/first_argument_expansion/jsfmt.spec.js

@@ -0,0 +1 @@
+run_spec(__dirname);

tests/first_argument_expansion/test.js

@@ -0,0 +1,87 @@
+setTimeout(function() {
+  thing();
+}, 500);
+
+["a","b","c"].reduce(function(item, thing) {
+  return thing + " " + item;
+}, "letters:")
+
+func(() => {
+  thing();
+}, identifier);
+
+func(function() {
+  thing();
+}, this.props.timeout * 1000);
+
+func((that) => {
+  thing();
+}, this.props.getTimeout());
+
+func(() => {
+  thing();
+}, true);
+
+func(() => {
+  thing();
+}, null);
+
+func(() => {
+  thing();
+}, undefined);
+
+func(() => {
+  thing();
+}, /regex.*?/);
+
+compose((a) => {
+  return a.thing;
+}, b => b * b);
+
+somthing.reduce(function(item, thing) {
+  return thing.blah =  item;
+}, {})
+
+somthing.reduce(function(item, thing) {
+  return thing.push(item);
+}, [])
+
+reallyLongLongLongLongLongLongLongLongLongLongLongLongLongLongMethod((f, g, h) => {
+  return f.pop();
+}, true);
+
+// Don't do the rest of these
+
+func(function() {
+  thing();
+}, true, false);
+
+func(() => {
+  thing();
+}, {yes: true, cats: 5});
+
+compose((a) => {
+  return a.thing;
+}, b => {
+  return b + "";
+});
+
+compose((a) => {
+  return a.thing;
+}, b => [1, 2, 3, 4, 5]);
+
+renderThing(a =>
+  <div>Content. So much to say. Oh my. Are we done yet?</div>
+,args);
+
+setTimeout(
+  // Something
+  function() {
+    thing();
+  },
+  500
+);
+
+setTimeout(/* blip */ function() {
+  thing();
+}, 500);

tests/flow/arraylib/__snapshots__/jsfmt.spec.js.snap

@@ -94,12 +94,9 @@ function reduce_test() {
     return previousValue + currentValue + array[index];
   });
 
-  [0, 1, 2, 3, 4].reduce(
-    function(previousValue, currentValue, index, array) {
-      return previousValue + currentValue + array[index];
-    },
-    10
-  );
+  [0, 1, 2, 3, 4].reduce(function(previousValue, currentValue, index, array) {
+    return previousValue + currentValue + array[index];
+  }, 10);
 
   var total = [0, 1, 2, 3].reduce(function(a, b) {
     return a + b;

tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap

@@ -1597,12 +1597,9 @@ exports[`test20.js 1`] = `
 
 [0, 1].reduce((x, y, i) => y);
 
-[\\"a\\", \\"b\\"].reduce(
-  (regex, representation, index) => {
-    return regex + (index ? \\"|\\" : \\"\\") + \\"(\\" + representation + \\")\\";
-  },
-  \\"\\"
-);
+[\\"a\\", \\"b\\"].reduce((regex, representation, index) => {
+  return regex + (index ? \\"|\\" : \\"\\") + \\"(\\" + representation + \\")\\";
+}, \\"\\");
 
 [\\"\\"].reduce((acc, str) => acc * str.length);
 "

tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap

@@ -154,11 +154,113 @@ exports[`overflow.js 1`] = `
 "SuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLongCall((err, result) => {
   // comment
 });
+
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, no, []);
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, []);
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, [
+  // Comments
+]);
+func(five, six, seven, eig, is, this, too, long, yes, [
+  // Comments
+]);
+
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, no, {});
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, {});
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, {
+  // Comments
+});
+
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 SuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLongCall(
   (err, result) => {
     // comment
   }
 );
+
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, no, []);
+func(
+  one,
+  two,
+  three,
+  four,
+  five,
+  six,
+  seven,
+  eig,
+  is,
+  this,
+  too,
+  long,
+  yes,
+  []
+);
+func(
+  one,
+  two,
+  three,
+  four,
+  five,
+  six,
+  seven,
+  eig,
+  is,
+  this,
+  too,
+  long,
+  yes,
+  [
+    // Comments
+  ]
+);
+func(
+  five,
+  six,
+  seven,
+  eig,
+  is,
+  this,
+  too,
+  long,
+  yes,
+  [
+    // Comments
+  ]
+);
+
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, no, {});
+func(
+  one,
+  two,
+  three,
+  four,
+  five,
+  six,
+  seven,
+  eig,
+  is,
+  this,
+  too,
+  long,
+  yes,
+  {}
+);
+func(
+  one,
+  two,
+  three,
+  four,
+  five,
+  six,
+  seven,
+  eig,
+  is,
+  this,
+  too,
+  long,
+  yes,
+  {
+    // Comments
+  }
+);
 "
 `;

tests/last_argument_expansion/overflow.js

@@ -1,3 +1,19 @@
 SuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLongCall((err, result) => {
   // comment
 });
+
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, no, []);
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, []);
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, [
+  // Comments
+]);
+func(five, six, seven, eig, is, this, too, long, yes, [
+  // Comments
+]);
+
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, no, {});
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, {});
+func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, {
+  // Comments
+});
+