From 01dc7aec658ec5041a5d1f0318b82a42614abeb8 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 28 Aug 2019 03:50:24 -0700 Subject: [PATCH] fix: more readable parentheses for new-call (#6412) --- CHANGELOG.unreleased.md | 22 +++++++ src/language-js/needs-parens.js | 63 +++++++------------ .../__snapshots__/jsfmt.spec.js.snap | 10 ++- tests/new_expression/new_expression.js | 3 + .../__snapshots__/jsfmt.spec.js.snap | 14 ++++- tests/typescript_non_null/parens.ts | 4 ++ 6 files changed, 70 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 47f1c08b5..71d5e7010 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -44,6 +44,26 @@ const link = http://example.com; --> +#### JavaScript: More readable parentheses for new-call ([#6412] by [@bakkot]) + + +```js +// Input +var a = new (x().y)(); +var a = new (x().y.z)(); +var a = new (x().y().z)(); + +// Output (Prettier stable) +var a = new (x()).y(); +var a = new (x()).y.z(); +var a = new (x().y()).z(); + +// Output (Prettier master) +var a = new (x().y)(); +var a = new (x().y.z)(); +var a = new (x().y().z)(); +``` + #### MDX: fix text with whitespace after JSX trim incorrectly ([#6340] by [@JounQin]) Previous versions format text with whitespace after JSX incorrectly in mdx, this has been fixed in this version. @@ -429,8 +449,10 @@ const foo = [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as ( [#6301]: https://github.com/prettier/prettier/pull/6301 [#6307]: https://github.com/prettier/prettier/pull/6307 [#6340]: https://github.com/prettier/prettier/pull/6340 +[#6412]: https://github.com/prettier/prettier/pull/6412 [@duailibe]: https://github.com/duailibe [@gavinjoyce]: https://github.com/gavinjoyce [@sosukesuzuki]: https://github.com/sosukesuzuki [@g-harel]: https://github.com/g-harel [@jounqin]: https://github.com/JounQin +[@bakkot]: https://gibhub.com/bakkot diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js index a9cc0e587..014c16da6 100644 --- a/src/language-js/needs-parens.js +++ b/src/language-js/needs-parens.js @@ -225,36 +225,6 @@ function needsParens(path, options) { } switch (node.type) { - case "CallExpression": { - let firstParentNotMemberExpression = parent; - let i = 0; - // tagged templates are basically member expressions from a grammar perspective - // see https://tc39.github.io/ecma262/#prod-MemberExpression - // so are typescript's non-null assertions, though there's no grammar to point to - while ( - firstParentNotMemberExpression && - ((firstParentNotMemberExpression.type === "MemberExpression" && - firstParentNotMemberExpression.object === - path.getParentNode(i - 1)) || - firstParentNotMemberExpression.type === "TaggedTemplateExpression" || - firstParentNotMemberExpression.type === "TSNonNullExpression") - ) { - firstParentNotMemberExpression = path.getParentNode(++i); - } - - if ( - firstParentNotMemberExpression.type === "NewExpression" && - firstParentNotMemberExpression.callee === path.getParentNode(i - 1) - ) { - return true; - } - - if (parent.type === "BindExpression" && parent.callee === node) { - return true; - } - return false; - } - case "SpreadElement": case "SpreadProperty": return ( @@ -696,24 +666,35 @@ function needsParens(path, options) { case "OptionalMemberExpression": return parent.type === "MemberExpression"; + case "CallExpression": case "MemberExpression": + case "TaggedTemplateExpression": + case "TSNonNullExpression": if ( - parent.type === "BindExpression" && + (parent.type === "BindExpression" || parent.type === "NewExpression") && name === "callee" && parent.callee === node ) { - let object = node.object; + let object = node; while (object) { - if (object.type === "CallExpression") { - return true; + switch (object.type) { + case "CallExpression": + return true; + case "MemberExpression": + case "BindExpression": + object = object.object; + break; + // tagged templates are basically member expressions from a grammar perspective + // see https://tc39.github.io/ecma262/#prod-MemberExpression + case "TaggedTemplateExpression": + object = object.tag; + break; + case "TSNonNullExpression": + object = object.expression; + break; + default: + return false; } - if ( - object.type !== "MemberExpression" && - object.type !== "BindExpression" - ) { - break; - } - object = object.object; } } return false; diff --git a/tests/new_expression/__snapshots__/jsfmt.spec.js.snap b/tests/new_expression/__snapshots__/jsfmt.spec.js.snap index 15e40b214..187ddb955 100644 --- a/tests/new_expression/__snapshots__/jsfmt.spec.js.snap +++ b/tests/new_expression/__snapshots__/jsfmt.spec.js.snap @@ -32,14 +32,20 @@ new (createObj()).prop(a()); new (x()\`\`.y)(); new e[f().x].y(); new e[f()].y(); +new (a().b)(); +new (a().b().c)(); +new (a\`\`()); =====================================output===================================== new (memoize.Cache || MapCache)(); new (typeof this == "function" ? this : Dict())(); -new (createObj()).prop(a()); -new (x())\`\`.y(); +new (createObj().prop)(a()); +new (x()\`\`.y)(); new e[f().x].y(); new e[f()].y(); +new (a().b)(); +new (a().b().c)(); +new (a\`\`())(); ================================================================================ `; diff --git a/tests/new_expression/new_expression.js b/tests/new_expression/new_expression.js index c85c355a8..2aa1d5e02 100644 --- a/tests/new_expression/new_expression.js +++ b/tests/new_expression/new_expression.js @@ -4,3 +4,6 @@ new (createObj()).prop(a()); new (x()``.y)(); new e[f().x].y(); new e[f()].y(); +new (a().b)(); +new (a().b().c)(); +new (a``()); diff --git a/tests/typescript_non_null/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_non_null/__snapshots__/jsfmt.spec.js.snap index 39235371d..ee61557e8 100644 --- a/tests/typescript_non_null/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_non_null/__snapshots__/jsfmt.spec.js.snap @@ -63,6 +63,10 @@ const b = c!(); const c = new (d()!)(); const c = new (d()!); const c = new (d()!.e)(); +new (x()\`\`.y!)(); +new (x()\`\`!.y)(); +new (x()!\`\`.y)(); +new (x!()\`\`.y)(); =====================================output===================================== (a ? b : c)![tokenKey]; @@ -81,9 +85,13 @@ const a = b()!(); // parens aren't necessary const b = c!(); // parens are necessary if the expression result is called as a constructor -const c = new (d())!(); -const c = new (d())!(); -const c = new (d())!.e(); +const c = new (d()!)(); +const c = new (d()!)(); +const c = new (d()!.e)(); +new (x()\`\`.y!)(); +new (x()\`\`!.y)(); +new (x()!\`\`.y)(); +new (x!()\`\`.y)(); ================================================================================ `; diff --git a/tests/typescript_non_null/parens.ts b/tests/typescript_non_null/parens.ts index b96c63dda..5f1315db7 100644 --- a/tests/typescript_non_null/parens.ts +++ b/tests/typescript_non_null/parens.ts @@ -17,3 +17,7 @@ const b = c!(); const c = new (d()!)(); const c = new (d()!); const c = new (d()!.e)(); +new (x()``.y!)(); +new (x()``!.y)(); +new (x()!``.y)(); +new (x!()``.y)();