From 028d9e0ea7da33a4dd63d08e2ef9dc9451c2a374 Mon Sep 17 00:00:00 2001 From: Lucas Azzola Date: Sun, 16 Jul 2017 19:47:06 +1000 Subject: [PATCH] Improve bind expression formatting (#2493) * Improve bind expression formatting, fixes #1400 * Fix parenthesis with bind expressions, fixes #1446 --- src/fast-path.js | 1 + src/printer.js | 37 ++++--- .../__snapshots__/jsfmt.spec.js.snap | 101 ++++++++++++++++++ tests/bind_expressions/bind_parens.js | 3 + tests/bind_expressions/jsfmt.spec.js | 2 + tests/bind_expressions/method_chain.js | 17 +++ 6 files changed, 147 insertions(+), 14 deletions(-) create mode 100644 tests/bind_expressions/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/bind_expressions/bind_parens.js create mode 100644 tests/bind_expressions/jsfmt.spec.js create mode 100644 tests/bind_expressions/method_chain.js diff --git a/src/fast-path.js b/src/fast-path.js index 6491eca0b..fae6826d2 100644 --- a/src/fast-path.js +++ b/src/fast-path.js @@ -295,6 +295,7 @@ FastPath.prototype.needsParens = function(options) { case "UnaryExpression": case "SpreadElement": case "SpreadProperty": + case "BindExpression": case "AwaitExpression": case "TSAsExpression": case "TSNonNullExpression": diff --git a/src/printer.js b/src/printer.js index 5d7631ab2..11d284688 100644 --- a/src/printer.js +++ b/src/printer.js @@ -415,7 +415,7 @@ function genericPrintNoParens(path, options, print, args) { parts.push(path.call(print, "object")); } - parts.push("::", path.call(print, "callee")); + parts.push(printBindExpressionCallee(path, options, print)); return concat(parts); case "Path": @@ -889,7 +889,7 @@ function genericPrintNoParens(path, options, print, args) { // We detect calls on member lookups and possibly print them in a // special chain format. See `printMemberChain` for more info. - if (!isNew && n.callee.type === "MemberExpression") { + if (!isNew && isMemberish(n.callee)) { return printMemberChain(path, options, print); } @@ -3530,6 +3530,10 @@ function printMemberLookup(path, options, print) { ); } +function printBindExpressionCallee(path, options, print) { + return concat(["::", path.call(print, "callee")]); +} + // We detect calls on member expressions specially to format a // common pattern better. The pattern we are looking for is this: // @@ -3553,10 +3557,7 @@ function printMemberChain(path, options, print) { function rec(path) { const node = path.getValue(); - if ( - node.type === "CallExpression" && - node.callee.type === "MemberExpression" - ) { + if (node.type === "CallExpression" && isMemberish(node.callee)) { printedNodes.unshift({ node: node, printed: comments.printComments( @@ -3570,12 +3571,15 @@ function printMemberChain(path, options, print) { ) }); path.call(callee => rec(callee), "callee"); - } else if (node.type === "MemberExpression") { + } else if (isMemberish(node)) { printedNodes.unshift({ node: node, printed: comments.printComments( path, - () => printMemberLookup(path, options, print), + () => + node.type === "MemberExpression" + ? printMemberLookup(path, options, print) + : printBindExpressionCallee(path, options, print), options ) }); @@ -3633,8 +3637,8 @@ function printMemberChain(path, options, print) { } for (; i + 1 < printedNodes.length; ++i) { if ( - printedNodes[i].node.type === "MemberExpression" && - printedNodes[i + 1].node.type === "MemberExpression" + isMemberish(printedNodes[i].node) && + isMemberish(printedNodes[i + 1].node) ) { currentGroup.push(printedNodes[i]); } else { @@ -3650,10 +3654,7 @@ function printMemberChain(path, options, print) { // MemberExpression let hasSeenCallExpression = false; for (; i < printedNodes.length; ++i) { - if ( - hasSeenCallExpression && - printedNodes[i].node.type === "MemberExpression" - ) { + if (hasSeenCallExpression && isMemberish(printedNodes[i].node)) { // [0] should be appended at the end of the group instead of the // beginning of the next one if ( @@ -4239,6 +4240,13 @@ function isBinaryish(node) { return node.type === "BinaryExpression" || node.type === "LogicalExpression"; } +function isMemberish(node) { + return ( + node.type === "MemberExpression" || + (node.type === "BindExpression" && node.object) + ); +} + function shouldInlineLogicalExpression(node) { if (node.type !== "LogicalExpression") { return false; @@ -4559,6 +4567,7 @@ function hasNakedLeftSide(node) { node.type === "MemberExpression" || node.type === "SequenceExpression" || node.type === "TaggedTemplateExpression" || + (node.type === "BindExpression" && !node.object) || (node.type === "UpdateExpression" && !node.prefix) ); } diff --git a/tests/bind_expressions/__snapshots__/jsfmt.spec.js.snap b/tests/bind_expressions/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..1a6ad657b --- /dev/null +++ b/tests/bind_expressions/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,101 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`bind_parens.js 1`] = ` +(a || b)::c; +a || (b::c); +::obj.prop; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +(a || b)::c; +a || b::c; +::obj.prop; + +`; + +exports[`bind_parens.js 2`] = ` +(a || b)::c; +a || (b::c); +::obj.prop; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +;(a || b)::c +a || b::c +;::obj.prop + +`; + +exports[`method_chain.js 1`] = ` +import {interval} from 'rxjs/observable/interval'; +import {filter} from 'rxjs/operator/filter'; +import {take} from 'rxjs/operator/take'; +import {map} from 'rxjs/operator/map'; +import {throttle} from 'rxjs/operator/throttle'; +import {takeUntil} from 'rxjs/operator/takeUntil'; + +function test(observable) { + return observable + ::filter(data => data.someTest) + ::throttle(() => + interval(10) + ::take(1) + ::takeUntil(observable::filter(data => someOtherTest)) + ) + ::map(someFunction); +} +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +import { interval } from "rxjs/observable/interval"; +import { filter } from "rxjs/operator/filter"; +import { take } from "rxjs/operator/take"; +import { map } from "rxjs/operator/map"; +import { throttle } from "rxjs/operator/throttle"; +import { takeUntil } from "rxjs/operator/takeUntil"; + +function test(observable) { + return observable + ::filter(data => data.someTest) + ::throttle(() => + interval(10) + ::take(1) + ::takeUntil(observable::filter(data => someOtherTest)) + ) + ::map(someFunction); +} + +`; + +exports[`method_chain.js 2`] = ` +import {interval} from 'rxjs/observable/interval'; +import {filter} from 'rxjs/operator/filter'; +import {take} from 'rxjs/operator/take'; +import {map} from 'rxjs/operator/map'; +import {throttle} from 'rxjs/operator/throttle'; +import {takeUntil} from 'rxjs/operator/takeUntil'; + +function test(observable) { + return observable + ::filter(data => data.someTest) + ::throttle(() => + interval(10) + ::take(1) + ::takeUntil(observable::filter(data => someOtherTest)) + ) + ::map(someFunction); +} +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +import { interval } from "rxjs/observable/interval" +import { filter } from "rxjs/operator/filter" +import { take } from "rxjs/operator/take" +import { map } from "rxjs/operator/map" +import { throttle } from "rxjs/operator/throttle" +import { takeUntil } from "rxjs/operator/takeUntil" + +function test(observable) { + return observable + ::filter(data => data.someTest) + ::throttle(() => + interval(10) + ::take(1) + ::takeUntil(observable::filter(data => someOtherTest)) + ) + ::map(someFunction) +} + +`; diff --git a/tests/bind_expressions/bind_parens.js b/tests/bind_expressions/bind_parens.js new file mode 100644 index 000000000..77481c2f4 --- /dev/null +++ b/tests/bind_expressions/bind_parens.js @@ -0,0 +1,3 @@ +(a || b)::c; +a || (b::c); +::obj.prop; diff --git a/tests/bind_expressions/jsfmt.spec.js b/tests/bind_expressions/jsfmt.spec.js new file mode 100644 index 000000000..309a8f318 --- /dev/null +++ b/tests/bind_expressions/jsfmt.spec.js @@ -0,0 +1,2 @@ +run_spec(__dirname, { parser: "babylon" }); +run_spec(__dirname, { parser: "babylon", semi: false }); diff --git a/tests/bind_expressions/method_chain.js b/tests/bind_expressions/method_chain.js new file mode 100644 index 000000000..11a8166d0 --- /dev/null +++ b/tests/bind_expressions/method_chain.js @@ -0,0 +1,17 @@ +import {interval} from 'rxjs/observable/interval'; +import {filter} from 'rxjs/operator/filter'; +import {take} from 'rxjs/operator/take'; +import {map} from 'rxjs/operator/map'; +import {throttle} from 'rxjs/operator/throttle'; +import {takeUntil} from 'rxjs/operator/takeUntil'; + +function test(observable) { + return observable + ::filter(data => data.someTest) + ::throttle(() => + interval(10) + ::take(1) + ::takeUntil(observable::filter(data => someOtherTest)) + ) + ::map(someFunction); +}