From e0eb438e7bf55abaf50d38accba876bd7a349ef5 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Tue, 11 Apr 2017 08:50:49 -0700 Subject: [PATCH] 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 | 56 +++++- .../__snapshots__/jsfmt.spec.js.snap | 171 ++++++++++++++++++ tests/arrow-call/arrow_call.js | 37 ++++ tests/arrow-call/jsfmt.spec.js | 2 + tests/arrows/__snapshots__/jsfmt.spec.js.snap | 7 +- .../__snapshots__/jsfmt.spec.js.snap | 4 +- .../__snapshots__/jsfmt.spec.js.snap | 3 +- 7 files changed, 267 insertions(+), 13 deletions(-) create mode 100644 tests/arrow-call/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/arrow-call/arrow_call.js create mode 100644 tests/arrow-call/jsfmt.spec.js diff --git a/src/printer.js b/src/printer.js index 7217b1da..ebc89aaf 100644 --- a/src/printer.js +++ b/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 ); } diff --git a/tests/arrow-call/__snapshots__/jsfmt.spec.js.snap b/tests/arrow-call/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 00000000..d5f35820 --- /dev/null +++ b/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(), +); +" +`; diff --git a/tests/arrow-call/arrow_call.js b/tests/arrow-call/arrow_call.js new file mode 100644 index 00000000..c0f58510 --- /dev/null +++ b/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() +); diff --git a/tests/arrow-call/jsfmt.spec.js b/tests/arrow-call/jsfmt.spec.js new file mode 100644 index 00000000..220f446b --- /dev/null +++ b/tests/arrow-call/jsfmt.spec.js @@ -0,0 +1,2 @@ +run_spec(__dirname); +run_spec(__dirname, { trailingComma: 'all' }); diff --git a/tests/arrows/__snapshots__/jsfmt.spec.js.snap b/tests/arrows/__snapshots__/jsfmt.spec.js.snap index 48998b7b..10d1c4f8 100644 --- a/tests/arrows/__snapshots__/jsfmt.spec.js.snap +++ b/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, diff --git a/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap b/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap index a814bab1..4400a42c 100644 --- a/tests/last_argument_expansion/__snapshots__/jsfmt.spec.js.snap +++ b/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) - ))); + ) + ) + ); } " `; diff --git a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap b/tests/method-chain/__snapshots__/jsfmt.spec.js.snap index c781d206..9e11c5e4 100644 --- a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap +++ b/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() {