Fix overflow for last argument expansion (#1224)

In #847, I used a heuristic to find if the element was going to be expanded. But, it wasn't 100% accurate because we couldn't know in which conditionalGroup we would land. We added a way for the parent to tell that function if we should be in `expandLastArg`. By replacing the condition by this variable, it now fixes the issues!

This is so good that adding the right abstraction fixes problems across the board :)

Fixes #997

src/printer.js

@@ -301,7 +301,12 @@ function genericPrintNoParens(path, options, print, args) {
         parts.push(
           group(
             concat([
-              printFunctionParams(path, print, options),
+              printFunctionParams(
+                path,
+                print,
+                options,
+                args && (args.expandLastArg || args.expandFirstArg)
+              ),
               printReturnType(path, print)
             ])
           )
@@ -2137,35 +2142,38 @@ function printArgumentsList(path, options, print) {
   // conditional group for all function calls, but it's more expensive
   // so only do it for specific forms.
   const shouldGroupFirst = shouldGroupFirstArg(args);
-  if (shouldGroupFirst || shouldGroupLastArg(args)) {
+  const shouldGroupLast = shouldGroupLastArg(args);
+  if (shouldGroupFirst || shouldGroupLast) {
     const shouldBreak = shouldGroupFirst
       ? printed.slice(1).some(willBreak)
       : printed.slice(0, -1).some(willBreak);
 
-    let printedLastArgExpanded;
-    if (shouldGroupFirst) {
-      printedLastArgExpanded = printed;
-    } else {
-      // We want to print the last argument with a special flag
-      let i = 0;
-      path.each(function(argPath) {
-        if (i++ === args.length - 1) {
-          printedLastArgExpanded = printed
-            .slice(0, -1)
-            .concat(argPath.call(p => print(p, { expandLastArg: true })));
-        }
-      }, "arguments");
-    }
+    // We want to print the last argument with a special flag
+    let printedExpanded;
+    let i = 0;
+    path.each(function(argPath) {
+      if (shouldGroupFirst && i === 0) {
+        printedExpanded =
+          [argPath.call(p => print(p, { expandFirstArg: true }))]
+            .concat(printed.slice(1));
+      }
+      if (shouldGroupLast && i === args.length - 1) {
+        printedExpanded = printed
+          .slice(0, -1)
+          .concat(argPath.call(p => print(p, { expandLastArg: true })));
+      }
+      i++;
+    }, "arguments");
 
     return concat([
       printed.some(willBreak) ? breakParent : "",
       conditionalGroup(
         [
-          concat(["(", join(concat([", "]), printedLastArgExpanded), ")"]),
+          concat(["(", join(concat([", "]), printedExpanded), ")"]),
           shouldGroupFirst
             ? concat([
                 "(",
-                group(printed[0], { shouldBreak: true }),
+                group(printedExpanded[0], { shouldBreak: true }),
                 printed.length > 1 ? ", " : "",
                 join(concat([",", line]), printed.slice(1)),
                 ")"
@@ -2174,7 +2182,7 @@ function printArgumentsList(path, options, print) {
                 "(",
                 join(concat([",", line]), printed.slice(0, -1)),
                 printed.length > 1 ? ", " : "",
-                group(util.getLast(printedLastArgExpanded), {
+                group(util.getLast(printedExpanded), {
                   shouldBreak: true
                 }),
                 ")"
@@ -2207,7 +2215,7 @@ function printArgumentsList(path, options, print) {
   );
 }
 
-function printFunctionParams(path, print, options) {
+function printFunctionParams(path, print, options, expandArg) {
   var fun = path.getValue();
   // namedTypes.Function.assert(fun);
   var paramsField = fun.type === "TSFunctionType" ? "parameters" : "params";
@@ -2250,15 +2258,8 @@ function printFunctionParams(path, print, options) {
   //     }                     b,
   //   })                    ) => {
   //                         })
-  const parent = path.getParentNode();
-  if (
-    (parent.type === "CallExpression" || parent.type === "NewExpression") &&
-    ((util.getLast(parent.arguments) === path.getValue() &&
-      shouldGroupLastArg(parent.arguments)) ||
-      (parent.arguments[0] === path.getValue() &&
-        shouldGroupFirstArg(parent.arguments)))
-  ) {
-    return concat(["(", join(", ", printed), ")"]);
+  if (expandArg) {
+    return group(concat(["(", join(", ", printed), ")"]));
   }
 
   // Single object destructuring should hug
@@ -2280,6 +2281,7 @@ function printFunctionParams(path, print, options) {
     return concat(["(", join(", ", printed), ")"]);
   }
 
+  const parent = path.getParentNode();
   const isFlowShorthandWithOneArg =
     (isObjectTypePropertyAFunction(parent) ||
       isTypeAnnotationAFunction(parent) ||

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

@@ -172,6 +172,38 @@ func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, {
   // Comments
 });
 
+foo(
+  (
+    one,
+    two,
+    three,
+    four,
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+    eleven,
+    twelve,
+    thirteen,
+    fourteen,
+  ) => {},
+);
+
+const contentTypes = function(tile, singleSelection) {
+  return compute(
+    function contentTypesContentTypes(
+      tile,
+      searchString = '',
+      filteredContentTypes = [],
+      contentTypesArray = [],
+      selectedGroup,
+      singleSelection) {
+      selectedGroup = (tile.state && tile.state.group) || selectedGroup;
+    }
+  );
+};
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 SuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperSuperLongCall(
   (err, result) => {
@@ -265,4 +297,36 @@ func(
   }
 );
 
+foo(
+  (
+    one,
+    two,
+    three,
+    four,
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+    eleven,
+    twelve,
+    thirteen,
+    fourteen
+  ) => {}
+);
+
+const contentTypes = function(tile, singleSelection) {
+  return compute(function contentTypesContentTypes(
+    tile,
+    searchString = "",
+    filteredContentTypes = [],
+    contentTypesArray = [],
+    selectedGroup,
+    singleSelection
+  ) {
+    selectedGroup = (tile.state && tile.state.group) || selectedGroup;
+  });
+};
+
 `;

tests/last_argument_expansion/overflow.js

@@ -17,3 +17,35 @@ func(one, two, three, four, five, six, seven, eig, is, this, too, long, yes, {
   // Comments
 });
 
+foo(
+  (
+    one,
+    two,
+    three,
+    four,
+    five,
+    six,
+    seven,
+    eight,
+    nine,
+    ten,
+    eleven,
+    twelve,
+    thirteen,
+    fourteen,
+  ) => {},
+);
+
+const contentTypes = function(tile, singleSelection) {
+  return compute(
+    function contentTypesContentTypes(
+      tile,
+      searchString = '',
+      filteredContentTypes = [],
+      contentTypesArray = [],
+      selectedGroup,
+      singleSelection) {
+      selectedGroup = (tile.state && tile.state.group) || selectedGroup;
+    }
+  );
+};