diff --git a/src/comments.js b/src/comments.js index bede556b043e..7c2179f26f8d 100644 --- a/src/comments.js +++ b/src/comments.js @@ -317,7 +317,7 @@ function printTrailingComment(commentPath, print) { // } // } - parts.push(print(commentPath)); + parts.push(print(commentPath), hardline); return concat(parts); } diff --git a/src/printer.js b/src/printer.js index 372c68a9d5a0..a4f7e960ae18 100644 --- a/src/printer.js +++ b/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 ]); diff --git a/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap b/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap index 331dc90020ee..c8c6576d768d 100644 --- a/tests/node_tests/stream/__snapshots__/jsfmt.spec.js.snap +++ b/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()); " `; diff --git a/tests/promises/__snapshots__/jsfmt.spec.js.snap b/tests/promises/__snapshots__/jsfmt.spec.js.snap index 724968c31c4b..530fc7333ebb 100644 --- a/tests/promises/__snapshots__/jsfmt.spec.js.snap +++ b/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; + }); " `; diff --git a/tests/union/__snapshots__/jsfmt.spec.js.snap b/tests/union/__snapshots__/jsfmt.spec.js.snap index d3340c8952d4..33a6c05e81ee 100644 --- a/tests/union/__snapshots__/jsfmt.spec.js.snap +++ b/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(); + }); " `;