Add newline for bracket-less arrow functions that return calls (#927)

I've tried a lot of places where to fix this and this is the only one that gives correct results. This is likely not exhaustive but works for that use case. It's been reported twice in issues and I've seen it happen a few other times so we probably want to get it fixed.

Fixes #922
Fixes #797

src/printer.js

@@ -50,13 +50,13 @@ function shouldPrintComma(options, level) {
   }
 }
 
-function genericPrint(path, options, printPath) {
+function genericPrint(path, options, printPath, args) {
   assert.ok(path instanceof FastPath);
 
   var node = path.getValue();
   var parts = [];
   var needsParens = false;
-  var linesWithoutParens = genericPrintNoParens(path, options, printPath);
+  var linesWithoutParens = genericPrintNoParens(path, options, printPath, args);
 
   if (!node || isEmpty(linesWithoutParens)) {
     return linesWithoutParens;
@@ -122,7 +122,7 @@ function genericPrint(path, options, printPath) {
   return concat(parts);
 }
 
-function genericPrintNoParens(path, options, print) {
+function genericPrintNoParens(path, options, print, args) {
   var n = path.getValue();
 
   if (!n) {
@@ -275,7 +275,7 @@ function genericPrintNoParens(path, options, print) {
     case "FunctionDeclaration":
     case "FunctionExpression":
       return printFunctionDeclaration(path, print, options)
-    case "ArrowFunctionExpression":
+    case "ArrowFunctionExpression": {
       if (n.async) parts.push("async ");
 
       if (n.typeParameters) {
@@ -325,12 +325,28 @@ function genericPrintNoParens(path, options, print) {
         return group(collapsed);
       }
 
+      // if the arrow function is expanded as last argument, we are adding a
+      // level of indentation and need to add a softline to align the closing )
+      // with the opening (.
+      const shouldAddSoftLine = args && args.expandLastArg;
+
       return group(
         concat([
           concat(parts),
-          group(indent(concat([line, body])))
+          group(
+            concat([
+              indent(concat([line, body])),
+              shouldAddSoftLine
+                ? concat([
+                    ifBreak(shouldPrintComma(options, "all") ? "," : ""),
+                    softline
+                  ])
+                : ''
+            ])
+          ),
         ])
       );
+    }
     case "MethodDefinition":
       if (n.static) {
         parts.push("static ");
@@ -2129,11 +2145,29 @@ function printArgumentsList(path, options, print) {
     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");
+    }
+
     return concat([
       printed.some(willBreak) ? breakParent : "",
       conditionalGroup(
         [
-          concat(["(", join(concat([", "]), printed), ")"]),
+          concat(["(", join(concat([", "]), printedLastArgExpanded), ")"]),
           shouldGroupFirst
             ? concat([
                 "(",
@@ -2146,7 +2180,10 @@ function printArgumentsList(path, options, print) {
                 "(",
                 join(concat([",", line]), printed.slice(0, -1)),
                 printed.length > 1 ? ", " : "",
-                group(util.getLast(printed), { shouldBreak: true }),
+                group(
+                  util.getLast(printedLastArgExpanded),
+                  { shouldBreak: true }
+                ),
                 ")"
               ]),
           group(
@@ -3371,11 +3408,12 @@ function printArrayItems(path, options, printPath, print) {
   return concat(printedElements);
 }
 
+
 function printAstToDoc(ast, options) {
-  function printGenerically(path) {
+  function printGenerically(path, args) {
     return comments.printComments(
       path,
-      p => genericPrint(p, options, printGenerically),
+      p => genericPrint(p, options, printGenerically, args),
       options
     );
   }

tests/arrow-call/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,171 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`arrow_call.js 1`] = `
+"const testResults = results.testResults.map(testResult =>
+  formatResult(testResult, formatter, reporter)
+);
+
+it('mocks regexp instances', () => {
+  expect(
+    () => moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/)),
+  ).not.toThrow();
+});
+
+expect(() => asyncRequest({ url: \\"/test-endpoint\\" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ url: \\"/test-endpoint-but-with-a-long-url\\" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ url: \\"/test-endpoint-but-with-a-suuuuuuuuper-long-url\\" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint\\" }))
+  .not.toThrowError();
+
+expect(() => asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint-but-with-a-long-url\\" }))
+  .not.toThrowError();
+
+const a = Observable
+  .fromPromise(axiosInstance.post('/carts/mine'))
+  .map((response) => response.data)
+
+const b = Observable.fromPromise(axiosInstance.get(url))
+  .map((response) => response.data)
+
+func(
+  veryLoooooooooooooooooooooooongName,
+  veryLooooooooooooooooooooooooongName =>
+    veryLoooooooooooooooongName.something()
+);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const testResults = results.testResults.map(testResult =>
+  formatResult(testResult, formatter, reporter)
+);
+
+it(\\"mocks regexp instances\\", () => {
+  expect(() =>
+    moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/))
+  ).not.toThrow();
+});
+
+expect(() => asyncRequest({ url: \\"/test-endpoint\\" })).toThrowError(
+  /Required parameter/
+);
+
+expect(() =>
+  asyncRequest({ url: \\"/test-endpoint-but-with-a-long-url\\" })
+).toThrowError(/Required parameter/);
+
+expect(() =>
+  asyncRequest({ url: \\"/test-endpoint-but-with-a-suuuuuuuuper-long-url\\" })
+).toThrowError(/Required parameter/);
+
+expect(() =>
+  asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint\\" })
+).not.toThrowError();
+
+expect(() =>
+  asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint-but-with-a-long-url\\" })
+).not.toThrowError();
+
+const a = Observable.fromPromise(axiosInstance.post(\\"/carts/mine\\")).map(
+  response => response.data
+);
+
+const b = Observable.fromPromise(axiosInstance.get(url)).map(
+  response => response.data
+);
+
+func(
+  veryLoooooooooooooooooooooooongName,
+  veryLooooooooooooooooooooooooongName =>
+    veryLoooooooooooooooongName.something()
+);
+"
+`;
+
+exports[`arrow_call.js 2`] = `
+"const testResults = results.testResults.map(testResult =>
+  formatResult(testResult, formatter, reporter)
+);
+
+it('mocks regexp instances', () => {
+  expect(
+    () => moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/)),
+  ).not.toThrow();
+});
+
+expect(() => asyncRequest({ url: \\"/test-endpoint\\" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ url: \\"/test-endpoint-but-with-a-long-url\\" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ url: \\"/test-endpoint-but-with-a-suuuuuuuuper-long-url\\" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint\\" }))
+  .not.toThrowError();
+
+expect(() => asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint-but-with-a-long-url\\" }))
+  .not.toThrowError();
+
+const a = Observable
+  .fromPromise(axiosInstance.post('/carts/mine'))
+  .map((response) => response.data)
+
+const b = Observable.fromPromise(axiosInstance.get(url))
+  .map((response) => response.data)
+
+func(
+  veryLoooooooooooooooooooooooongName,
+  veryLooooooooooooooooooooooooongName =>
+    veryLoooooooooooooooongName.something()
+);
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+const testResults = results.testResults.map(testResult =>
+  formatResult(testResult, formatter, reporter),
+);
+
+it(\\"mocks regexp instances\\", () => {
+  expect(() =>
+    moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/)),
+  ).not.toThrow();
+});
+
+expect(() => asyncRequest({ url: \\"/test-endpoint\\" })).toThrowError(
+  /Required parameter/,
+);
+
+expect(() =>
+  asyncRequest({ url: \\"/test-endpoint-but-with-a-long-url\\" }),
+).toThrowError(/Required parameter/);
+
+expect(() =>
+  asyncRequest({ url: \\"/test-endpoint-but-with-a-suuuuuuuuper-long-url\\" }),
+).toThrowError(/Required parameter/);
+
+expect(() =>
+  asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint\\" }),
+).not.toThrowError();
+
+expect(() =>
+  asyncRequest({ type: \\"foo\\", url: \\"/test-endpoint-but-with-a-long-url\\" }),
+).not.toThrowError();
+
+const a = Observable.fromPromise(axiosInstance.post(\\"/carts/mine\\")).map(
+  response => response.data,
+);
+
+const b = Observable.fromPromise(axiosInstance.get(url)).map(
+  response => response.data,
+);
+
+func(
+  veryLoooooooooooooooooooooooongName,
+  veryLooooooooooooooooooooooooongName =>
+    veryLoooooooooooooooongName.something(),
+);
+"
+`;

tests/arrow-call/arrow_call.js

@@ -0,0 +1,37 @@
+const testResults = results.testResults.map(testResult =>
+  formatResult(testResult, formatter, reporter)
+);
+
+it('mocks regexp instances', () => {
+  expect(
+    () => moduleMocker.generateFromMetadata(moduleMocker.getMetadata(/a/)),
+  ).not.toThrow();
+});
+
+expect(() => asyncRequest({ url: "/test-endpoint" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ url: "/test-endpoint-but-with-a-long-url" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ url: "/test-endpoint-but-with-a-suuuuuuuuper-long-url" }))
+  .toThrowError(/Required parameter/);
+
+expect(() => asyncRequest({ type: "foo", url: "/test-endpoint" }))
+  .not.toThrowError();
+
+expect(() => asyncRequest({ type: "foo", url: "/test-endpoint-but-with-a-long-url" }))
+  .not.toThrowError();
+
+const a = Observable
+  .fromPromise(axiosInstance.post('/carts/mine'))
+  .map((response) => response.data)
+
+const b = Observable.fromPromise(axiosInstance.get(url))
+  .map((response) => response.data)
+
+func(
+  veryLoooooooooooooooooooooooongName,
+  veryLooooooooooooooooooooooooongName =>
+    veryLoooooooooooooooongName.something()
+);

tests/arrow-call/jsfmt.spec.js

@@ -0,0 +1,2 @@
+run_spec(__dirname);
+run_spec(__dirname, { trailingComma: 'all' });

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

@@ -156,14 +156,17 @@ Seq(typeDef.interface.groups).forEach(group =>
     markdownDoc(member.doc, {
       typePath: typePath.concat(memberName.slice(1)),
       signatures: member.signatures
-    })));
+    })
+  )
+);
 
 const promiseFromCallback = fn =>
   new Promise((resolve, reject) =>
     fn((err, result) => {
       if (err) return reject(err);
       return resolve(result);
-    }));
+    })
+  );
 
 runtimeAgent.getProperties(
   objectId,

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

@@ -32,7 +32,9 @@ export default function searchUsers(action$) {
               .getJSON(\`https://api.github.com/search/users?q=\${q}\`)
               .map(res => res.items)
               .map(receiveUsers)
-          )));
+          )
+        )
+    );
 }
 "
 `;

tests/method-chain/__snapshots__/jsfmt.spec.js.snap

@@ -212,7 +212,8 @@ export default function theFunction(action$, store) {
     })
       .filter(data => theFilter(data))
       .map(({ theType, ...data }) => theMap(theType, data))
-      .retryWhen(errors => errors));
+      .retryWhen(errors => errors)
+  );
 }
 
 function f() {