From 5cdc3ab2dd44532e3d9ca700fcb7cf97680e0130 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Mon, 13 Feb 2023 10:11:40 +0800 Subject: [PATCH] Support print `ChainExpression` (#13735) --- src/language-js/clean.js | 12 ++ src/language-js/comments/printer-methods.js | 4 +- src/language-js/needs-parens.js | 103 +++++++++++++++--- src/language-js/parse/postprocess/index.js | 27 ----- src/language-js/print/assignment.js | 2 +- src/language-js/print/jsx.js | 2 + src/language-js/print/misc.js | 5 +- src/language-js/print/ternary.js | 1 + src/language-js/printer-estree.js | 4 + src/language-js/utils/index.js | 8 +- tests/config/utils/check-parsers.js | 2 +- .../__snapshots__/jsfmt.spec.js.snap | 29 +++++ .../format/js/chain-expression/jsfmt.spec.js | 1 + tests/format/js/chain-expression/test.js | 8 ++ .../__snapshots__/jsfmt.spec.js.snap | 35 ++++++ .../jsx/optional-chaining/jsfmt.spec.js | 1 + .../optional-chaining/optional-chaining.jsx | 11 ++ .../__snapshots__/jsfmt.spec.js.snap | 21 ++++ .../typescript/chain-expression/jsfmt.spec.js | 1 + .../typescript/chain-expression/test.ts | 4 + 20 files changed, 232 insertions(+), 49 deletions(-) create mode 100644 tests/format/js/chain-expression/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/format/js/chain-expression/jsfmt.spec.js create mode 100644 tests/format/js/chain-expression/test.js create mode 100644 tests/format/jsx/optional-chaining/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/format/jsx/optional-chaining/jsfmt.spec.js create mode 100644 tests/format/jsx/optional-chaining/optional-chaining.jsx create mode 100644 tests/format/typescript/chain-expression/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/format/typescript/chain-expression/jsfmt.spec.js create mode 100644 tests/format/typescript/chain-expression/test.ts diff --git a/src/language-js/clean.js b/src/language-js/clean.js index 8aa1c463b..006021fc6 100644 --- a/src/language-js/clean.js +++ b/src/language-js/clean.js @@ -203,6 +203,18 @@ function clean(ast, newObj, parent) { ) { return newObj.types[0]; } + + // We print `(a?.b!).c` as `(a?.b)!.c`, but `typescript` parse them differently + if ( + ast.type === "ChainExpression" && + ast.expression.type === "TSNonNullExpression" + ) { + // Ideally, we should swap these two nodes, but `type` is the only difference + [newObj.type, newObj.expression.type] = [ + newObj.expression.type, + newObj.type, + ]; + } } clean.ignoredProperties = ignoredProperties; diff --git a/src/language-js/comments/printer-methods.js b/src/language-js/comments/printer-methods.js index 66af10ee0..de7886e09 100644 --- a/src/language-js/comments/printer-methods.js +++ b/src/language-js/comments/printer-methods.js @@ -22,7 +22,9 @@ function canAttachComment(node) { node.type !== "TemplateElement" && node.type !== "Import" && // `babel-ts` doesn't have similar node for `class Foo { bar() /* bat */; }` - node.type !== "TSEmptyBodyFunctionExpression" + node.type !== "TSEmptyBodyFunctionExpression" && + // `babel` doesn't have similar node to attach comments + node.type !== "ChainExpression" ); } diff --git a/src/language-js/needs-parens.js b/src/language-js/needs-parens.js index b4bcc142d..b20a8f744 100644 --- a/src/language-js/needs-parens.js +++ b/src/language-js/needs-parens.js @@ -772,25 +772,15 @@ function needsParens(path, options) { default: return false; } - case "OptionalMemberExpression": - case "OptionalCallExpression": { - const parentParent = path.grandparent; - if ( - (key === "object" && parent.type === "MemberExpression") || - (key === "callee" && - (parent.type === "CallExpression" || - parent.type === "NewExpression")) || - (parent.type === "TSNonNullExpression" && - parentParent.type === "MemberExpression" && - parentParent.object === parent) - ) { - return true; - } - } - // fallthrough + case "OptionalCallExpression": case "CallExpression": case "MemberExpression": + if (shouldAddParenthesesToChainElement(path)) { + return true; + } + + // fallthrough case "TaggedTemplateExpression": case "TSNonNullExpression": if ( @@ -1021,4 +1011,85 @@ function shouldWrapFunctionForExportDefault(path, options) { ); } +/* +Matches following cases: + +```js +(a?.b).c; +(a?.()).b; +(a?.b!).c; +(a?.()!).b; +(a?.b)!.c; +(a?.())!.b; + +(a?.b)(); +(a?.())(); + +new (a?.b)(); +new (a?.())(); +``` +*/ +function shouldAddParenthesesToChainElement(path) { + // Babel, this was implemented before #13735, can use `path.match` as estree does + const { node, parent, grandparent, key } = path; + if ( + (node.type === "OptionalMemberExpression" || + node.type === "OptionalCallExpression") && + ((key === "object" && parent.type === "MemberExpression") || + (key === "callee" && + (parent.type === "CallExpression" || + parent.type === "NewExpression")) || + (parent.type === "TSNonNullExpression" && + grandparent.type === "MemberExpression" && + grandparent.object === parent)) + ) { + return true; + } + + // ESTree, same logic as babel + if ( + path.match( + () => node.type === "CallExpression" || node.type === "MemberExpression", + (node, name) => name === "expression" && node.type === "ChainExpression" + ) && + (path.match( + undefined, + undefined, + (node, name) => + (name === "callee" && + ((node.type === "CallExpression" && !node.optional) || + node.type === "NewExpression")) || + (name === "object" && + node.type === "MemberExpression" && + !node.optional) + ) || + path.match( + undefined, + undefined, + (node, name) => + name === "expression" && node.type === "TSNonNullExpression", + (node, name) => name === "object" && node.type === "MemberExpression" + )) + ) { + return true; + } + + // Babel treat `(a?.b!).c` and `(a?.b)!.c` the same, https://github.com/babel/babel/discussions/15077 + // Use this to align with babel + if ( + path.match( + () => node.type === "CallExpression" || node.type === "MemberExpression", + (node, name) => + name === "expression" && node.type === "TSNonNullExpression", + (node, name) => name === "expression" && node.type === "ChainExpression", + (node, name) => name === "object" && node.type === "MemberExpression" + ) + ) { + return true; + } + + // This function only handle cases above + return false; +} + export default needsParens; diff --git a/src/language-js/parse/postprocess/index.js b/src/language-js/parse/postprocess/index.js index e1c598985..cc07c76bc 100644 --- a/src/language-js/parse/postprocess/index.js +++ b/src/language-js/parse/postprocess/index.js @@ -61,10 +61,6 @@ function postprocess(ast, options) { ast = visitNode(ast, (node) => { switch (node.type) { - // Espree - case "ChainExpression": - return transformChainExpression(node.expression); - case "LogicalExpression": // We remove unneeded parens around same-operator LogicalExpressions if (isUnbalancedLogicalTree(node)) { @@ -207,29 +203,6 @@ function postprocess(ast, options) { } } -// This is a workaround to transform `ChainExpression` from `acorn`, `espree`, -// `meriyah`, and `typescript` into `babel` shape AST, we should do the opposite, -// since `ChainExpression` is the standard `estree` AST for `optional chaining` -// https://github.com/estree/estree/blob/master/es2020.md -function transformChainExpression(node) { - switch (node.type) { - case "CallExpression": - node.type = "OptionalCallExpression"; - node.callee = transformChainExpression(node.callee); - break; - case "MemberExpression": - node.type = "OptionalMemberExpression"; - node.object = transformChainExpression(node.object); - break; - // typescript - case "TSNonNullExpression": - node.expression = transformChainExpression(node.expression); - break; - // No default - } - return node; -} - function isUnbalancedLogicalTree(node) { return ( node.type === "LogicalExpression" && diff --git a/src/language-js/print/assignment.js b/src/language-js/print/assignment.js index 627897753..91e4c0414 100644 --- a/src/language-js/print/assignment.js +++ b/src/language-js/print/assignment.js @@ -331,7 +331,7 @@ function isPoorlyBreakableMemberOrCallChain( const goDeeper = () => isPoorlyBreakableMemberOrCallChain(path, options, print, true); - if (node.type === "TSNonNullExpression") { + if (node.type === "ChainExpression" || node.type === "TSNonNullExpression") { return path.call(goDeeper, "expression"); } diff --git a/src/language-js/print/jsx.js b/src/language-js/print/jsx.js index 3abf7d641..5d5ceef6b 100644 --- a/src/language-js/print/jsx.js +++ b/src/language-js/print/jsx.js @@ -499,6 +499,8 @@ function printJsxExpressionContainer(path, options, print) { (shouldInline(node.argument, node) || node.argument.type === "JSXElement")) || isCallExpression(node) || + (node.type === "ChainExpression" && + isCallExpression(node.expression)) || node.type === "FunctionExpression" || node.type === "TemplateLiteral" || node.type === "TaggedTemplateExpression" || diff --git a/src/language-js/print/misc.js b/src/language-js/print/misc.js index 367b0176f..6cfe49417 100644 --- a/src/language-js/print/misc.js +++ b/src/language-js/print/misc.js @@ -1,4 +1,5 @@ import { indent, line } from "../../document/builders.js"; +import { isCallExpression, isMemberExpression } from "../utils/index.js"; import { printTypeAnnotationProperty } from "./type-annotation.js"; /** @@ -21,8 +22,8 @@ function printOptionalToken(path) { return ""; } if ( - node.type === "OptionalCallExpression" || - (node.type === "OptionalMemberExpression" && node.computed) || + isCallExpression(node) || + (isMemberExpression(node) && node.computed) || node.type === "OptionalIndexedAccessType" ) { return "?."; diff --git a/src/language-js/print/ternary.js b/src/language-js/print/ternary.js index 349bc7a51..4de1be574 100644 --- a/src/language-js/print/ternary.js +++ b/src/language-js/print/ternary.js @@ -150,6 +150,7 @@ function shouldExtraIndentForConditionalExpression(path) { const node = path.getParentNode(ancestorCount); if ( + (node.type === "ChainExpression" && node.expression === child) || (isCallExpression(node) && node.callee === child) || (isMemberExpression(node) && node.object === child) || (node.type === "TSNonNullExpression" && node.expression === child) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 0b5f2bb25..da6b1c44f 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -247,6 +247,10 @@ function printPathNoParens(path, options, print, args) { danglingComment ? [" ", danglingComment] : "", ]; } + + case "ChainExpression": + return print("expression"); + // Babel non-standard node. Used for Closure-style type casts. See postprocess.js. case "ParenthesizedExpression": { const shouldHug = diff --git a/src/language-js/utils/index.js b/src/language-js/utils/index.js index 598cd25ec..2b021cbb4 100644 --- a/src/language-js/utils/index.js +++ b/src/language-js/utils/index.js @@ -63,7 +63,8 @@ function hasNakedLeftSide(node) { node.type === "BindExpression" || (node.type === "UpdateExpression" && !node.prefix) || isTSTypeExpression(node) || - node.type === "TSNonNullExpression" + node.type === "TSNonNullExpression" || + node.type === "ChainExpression" ); } @@ -546,6 +547,10 @@ function isSimpleTemplateLiteral(node) { return true; } + if (expr.type === "ChainExpression") { + expr = expr.expression; + } + // Allow `a.b.c`, `a.b[c]`, and `this.x.y` if (isMemberExpression(expr)) { let head = expr; @@ -897,6 +902,7 @@ function startsWithNoLookaheadToken(node, predicate) { return node.object && startsWithNoLookaheadToken(node.object, predicate); case "SequenceExpression": return startsWithNoLookaheadToken(node.expressions[0], predicate); + case "ChainExpression": case "TSSatisfiesExpression": case "TSAsExpression": case "TSNonNullExpression": diff --git a/tests/config/utils/check-parsers.js b/tests/config/utils/check-parsers.js index 368e5f637..166b37619 100644 --- a/tests/config/utils/check-parsers.js +++ b/tests/config/utils/check-parsers.js @@ -94,7 +94,7 @@ const categoryParsers = new Map([ "typescript", "babel-ts", ], - extensions: [".js"], + extensions: [".js", ".jsx"], }, ], [ diff --git a/tests/format/js/chain-expression/__snapshots__/jsfmt.spec.js.snap b/tests/format/js/chain-expression/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..0bda344cb --- /dev/null +++ b/tests/format/js/chain-expression/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,29 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`test.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "typescript", "flow"] +printWidth: 80 + | printWidth +=====================================input====================================== +(a?.b).c; +(a?.()).b; + +(a?.b)(); +(a?.())(); + +new (a?.b)(); +new (a?.())(); + +=====================================output===================================== +(a?.b).c; +(a?.()).b; + +(a?.b)(); +(a?.())(); + +new (a?.b)(); +new (a?.())(); + +================================================================================ +`; diff --git a/tests/format/js/chain-expression/jsfmt.spec.js b/tests/format/js/chain-expression/jsfmt.spec.js new file mode 100644 index 000000000..f7fd785f6 --- /dev/null +++ b/tests/format/js/chain-expression/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(import.meta, ["babel", "typescript", "flow"]); diff --git a/tests/format/js/chain-expression/test.js b/tests/format/js/chain-expression/test.js new file mode 100644 index 000000000..86b2e6bd9 --- /dev/null +++ b/tests/format/js/chain-expression/test.js @@ -0,0 +1,8 @@ +(a?.b).c; +(a?.()).b; + +(a?.b)(); +(a?.())(); + +new (a?.b)(); +new (a?.())(); diff --git a/tests/format/jsx/optional-chaining/__snapshots__/jsfmt.spec.js.snap b/tests/format/jsx/optional-chaining/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..959f68a90 --- /dev/null +++ b/tests/format/jsx/optional-chaining/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,35 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`optional-chaining.jsx format 1`] = ` +====================================options===================================== +parsers: ["flow", "babel", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +function foo() { + // https://github.com/marmelab/react-admin/blob/5ae855aa958ba54438b144bf0907b1437c5a5d77/examples/demo/src/orders/Totals.tsx#L38-L43 + return ( + + {record?.delivery_fees.toLocaleString(undefined, { + style: 'currency', + currency: 'USD', + })} + + ) +} + +=====================================output===================================== +function foo() { + // https://github.com/marmelab/react-admin/blob/5ae855aa958ba54438b144bf0907b1437c5a5d77/examples/demo/src/orders/Totals.tsx#L38-L43 + return ( + + {record?.delivery_fees.toLocaleString(undefined, { + style: "currency", + currency: "USD", + })} + + ); +} + +================================================================================ +`; diff --git a/tests/format/jsx/optional-chaining/jsfmt.spec.js b/tests/format/jsx/optional-chaining/jsfmt.spec.js new file mode 100644 index 000000000..3bfcf87d2 --- /dev/null +++ b/tests/format/jsx/optional-chaining/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(import.meta, ["flow", "babel", "typescript"]); diff --git a/tests/format/jsx/optional-chaining/optional-chaining.jsx b/tests/format/jsx/optional-chaining/optional-chaining.jsx new file mode 100644 index 000000000..3d5ffd50a --- /dev/null +++ b/tests/format/jsx/optional-chaining/optional-chaining.jsx @@ -0,0 +1,11 @@ +function foo() { + // https://github.com/marmelab/react-admin/blob/5ae855aa958ba54438b144bf0907b1437c5a5d77/examples/demo/src/orders/Totals.tsx#L38-L43 + return ( + + {record?.delivery_fees.toLocaleString(undefined, { + style: 'currency', + currency: 'USD', + })} + + ) +} diff --git a/tests/format/typescript/chain-expression/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/chain-expression/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 000000000..ca466a68c --- /dev/null +++ b/tests/format/typescript/chain-expression/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,21 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`test.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +(a?.b!).c; +(a?.()!).b; +(a?.b)!.c; +(a?.())!.b; + +=====================================output===================================== +(a?.b)!.c; +(a?.())!.b; +(a?.b)!.c; +(a?.())!.b; + +================================================================================ +`; diff --git a/tests/format/typescript/chain-expression/jsfmt.spec.js b/tests/format/typescript/chain-expression/jsfmt.spec.js new file mode 100644 index 000000000..29084a9c8 --- /dev/null +++ b/tests/format/typescript/chain-expression/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(import.meta, ["typescript"]); diff --git a/tests/format/typescript/chain-expression/test.ts b/tests/format/typescript/chain-expression/test.ts new file mode 100644 index 000000000..e12dd65ee --- /dev/null +++ b/tests/format/typescript/chain-expression/test.ts @@ -0,0 +1,4 @@ +(a?.b!).c; +(a?.()!).b; +(a?.b)!.c; +(a?.())!.b;