From a749063a7d920344148abdfa2085d6433ea704fb Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 24 Mar 2020 01:16:28 +0900 Subject: [PATCH 01/16] Treat TSAsExpression as like BinaryishNode --- src/language-js/printer-estree.js | 38 +++++++++++-------- src/language-js/utils.js | 20 ++++++++++ .../__snapshots__/jsfmt.spec.js.snap | 8 ++-- .../__snapshots__/jsfmt.spec.js.snap | 18 ++++----- .../__snapshots__/jsfmt.spec.js.snap | 3 +- .../__snapshots__/jsfmt.spec.js.snap | 6 +-- 6 files changed, 59 insertions(+), 34 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 7c7440d16230..b7ef21361862 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -39,6 +39,7 @@ const { classChildNeedsASIProtection, classPropMayCauseASIProblems, conditionalExpressionChainContainsJSX, + convertToBinaryishNode, getFlowVariance, getLeftSidePathName, getParentExportDeclaration, @@ -553,11 +554,13 @@ function printPathNoParens(path, options, print, args) { ); case "BinaryExpression": case "LogicalExpression": - case "NGPipeExpression": { + case "NGPipeExpression": + case "TSAsExpression": { + const node = convertToBinaryishNode(n); const parent = path.getParentNode(); const parentParent = path.getParentNode(1); const isInsideParenthesis = - n !== parent.body && + node !== parent.body && (parent.type === "IfStatement" || parent.type === "WhileStatement" || parent.type === "SwitchStatement" || @@ -596,7 +599,7 @@ function printPathNoParens(path, options, print, args) { if ( ((parent.type === "CallExpression" || parent.type === "OptionalCallExpression") && - parent.callee === n) || + parent.callee === node) || parent.type === "UnaryExpression" || ((parent.type === "MemberExpression" || parent.type === "OptionalMemberExpression") && @@ -614,14 +617,14 @@ function printPathNoParens(path, options, print, args) { parent.type === "ThrowStatement" || (parent.type === "JSXExpressionContainer" && parentParent.type === "JSXAttribute") || - (n.operator !== "|" && parent.type === "JsExpressionRoot") || - (n.type !== "NGPipeExpression" && + (node.operator !== "|" && parent.type === "JsExpressionRoot") || + (node.type !== "NGPipeExpression" && ((parent.type === "NGRoot" && options.parser === "__ng_binding") || (parent.type === "NGMicrosyntaxExpression" && parentParent.type === "NGMicrosyntax" && parentParent.body.length === 1))) || - (n === parent.body && parent.type === "ArrowFunctionExpression") || - (n !== parent.body && parent.type === "ForStatement") || + (node === parent.body && parent.type === "ArrowFunctionExpression") || + (node !== parent.body && parent.type === "ForStatement") || (parent.type === "ConditionalExpression" && parentParent.type !== "ReturnStatement" && parentParent.type !== "ThrowStatement" && @@ -639,12 +642,12 @@ function printPathNoParens(path, options, print, args) { parent.type === "Property"; const samePrecedenceSubExpression = - isBinaryish(n.left) && shouldFlatten(n.operator, n.left.operator); + isBinaryish(node.left) && shouldFlatten(node.operator, node.left.operator); if ( shouldNotIndent || - (shouldInlineLogicalExpression(n) && !samePrecedenceSubExpression) || - (!shouldInlineLogicalExpression(n) && shouldIndentIfInlining) + (shouldInlineLogicalExpression(node) && !samePrecedenceSubExpression) || + (!shouldInlineLogicalExpression(node) && shouldIndentIfInlining) ) { return group(concat(parts)); } @@ -662,7 +665,7 @@ function printPathNoParens(path, options, print, args) { // // ) - const hasJSX = isJSXNode(n.right); + const hasJSX = isJSXNode(node.right); const rest = concat(hasJSX ? parts.slice(1, -1) : parts.slice(1)); const groupId = Symbol("logicalChain-" + ++uid); @@ -5713,10 +5716,13 @@ function printBinaryishExpressions( isInsideParenthesis ) { let parts = []; - const node = path.getValue(); + const node = convertToBinaryishNode(path.getValue()); // We treat BinaryExpression and LogicalExpression nodes the same. if (isBinaryish(node)) { + const leftNodeName = node.type === "TSAsExpression" ? "expression" : "left"; + const rightNodeName = + node.type === "TSAsExpression" ? "typeAnnotation" : "right"; // Put all operators with the same precedence level in the same // group. The reason we only need to do this with the `left` // expression is because given an expression like `1 + 2 - 3`, it @@ -5738,11 +5744,11 @@ function printBinaryishExpressions( /* isNested */ true, isInsideParenthesis ), - "left" + leftNodeName ) ); } else { - parts.push(path.call(print, "left")); + parts.push(path.call(print, leftNodeName)); } const shouldInline = shouldInlineLogicalExpression(node); @@ -5772,12 +5778,12 @@ function printBinaryishExpressions( : ""; const right = shouldInline - ? concat([operator, " ", path.call(print, "right"), rightSuffix]) + ? concat([operator, " ", path.call(print, rightNodeName), rightSuffix]) : concat([ lineBeforeOperator ? softline : "", operator, lineBeforeOperator ? " " : line, - path.call(print, "right"), + path.call(print, rightNodeName), rightSuffix, ]); diff --git a/src/language-js/utils.js b/src/language-js/utils.js index 905fe59e871a..20e4baa4ceb1 100644 --- a/src/language-js/utils.js +++ b/src/language-js/utils.js @@ -289,6 +289,7 @@ const binaryishNodeTypes = new Set([ "BinaryExpression", "LogicalExpression", "NGPipeExpression", + "TSAsExpression", ]); function isBinaryish(node) { return binaryishNodeTypes.has(node.type); @@ -1003,10 +1004,29 @@ function isTSXFile(options) { return options.filepath && /\.tsx$/i.test(options.filepath); } +function convertToBinaryishNode(node) { + if (node.type === "TSAsExpression") { + const binaryishNode = { ...node }; + + binaryishNode.operator = "as"; + + binaryishNode.left = node.expression; + delete binaryishNode.expression; + + binaryishNode.right = node.typeAnnotation; + delete binaryishNode.typeAnnotation; + + return binaryishNode; + } + + return node; +} + module.exports = { classChildNeedsASIProtection, classPropMayCauseASIProblems, conditionalExpressionChainContainsJSX, + convertToBinaryishNode, getFlowVariance, getLeftSidePathName, getParentExportDeclaration, diff --git a/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap index bd7c16c29ce7..a07ddfbe3dd7 100644 --- a/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap @@ -41,7 +41,7 @@ const bar8 = [1,2,3].reduce((carry, value) => { =====================================output===================================== const bar1 = [1, 2, 3].reduce((carry, value) => { return [...carry, value]; -}, ([] as unknown) as number[]); +}, [] as unknown as number[]); const bar2 = [1, 2, 3].reduce((carry, value) => { return [...carry, value]; @@ -51,7 +51,7 @@ const bar3 = [1, 2, 3].reduce( (carry, value) => { return [...carry, value]; }, - ([1, 2, 3] as unknown) as number[] + [1, 2, 3] as unknown as number[] ); const bar4 = [1, 2, 3].reduce( @@ -63,7 +63,7 @@ const bar4 = [1, 2, 3].reduce( const bar5 = [1, 2, 3].reduce((carry, value) => { return { ...carry, [value]: true }; -}, ({} as unknown) as { [key: number]: boolean }); +}, {} as unknown as { [key: number]: boolean }); const bar6 = [1, 2, 3].reduce((carry, value) => { return { ...carry, [value]: true }; @@ -73,7 +73,7 @@ const bar7 = [1, 2, 3].reduce( (carry, value) => { return { ...carry, [value]: true }; }, - ({ 1: true } as unknown) as { [key: number]: boolean } + { 1: true } as unknown as { [key: number]: boolean } ); const bar8 = [1, 2, 3].reduce( diff --git a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap index a90a79d45674..edab6a280a11 100644 --- a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap @@ -39,18 +39,18 @@ const state = JSON.stringify({ =====================================output===================================== const name = (description as DescriptionObject).name || (description as string); this.isTabActionBar((e.target || e.srcElement) as HTMLElement); -(originalError - ? wrappedError(errMsg, originalError) - : Error(errMsg)) as InjectionError; +(originalError ? wrappedError(errMsg, originalError) : Error(errMsg)) as + InjectionError; "current" in (props.pagination as Object); start + (yearSelectTotal as number); scrollTop > (visibilityHeight as number); -export default class Column extends (RcTable.Column as React.ComponentClass< - ColumnProps, - ColumnProps, - ColumnProps, - ColumnProps ->) {} +export default class Column extends (RcTable.Column as + React.ComponentClass< + ColumnProps, + ColumnProps, + ColumnProps, + ColumnProps + >) {} export const MobxTypedForm = class extends (Form as { new (): any }) {}; export abstract class MobxTypedForm1 extends (Form as { new (): any }) {} ({} as {}); diff --git a/tests/typescript_template_literals/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_template_literals/__snapshots__/jsfmt.spec.js.snap index abb2f54740fc..69c933d06a6b 100644 --- a/tests/typescript_template_literals/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_template_literals/__snapshots__/jsfmt.spec.js.snap @@ -25,7 +25,8 @@ const b = \`\${ }\`; const b = \`\${ (veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongFoo + - veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongBar) as veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongBaz + veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongBar) as + veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongBaz }\`; ================================================================================ diff --git a/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap index 33ccf96dbb43..17581a420a43 100644 --- a/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap @@ -269,10 +269,8 @@ type State = { | { discriminant: "BAZ"; baz: any } ); -const foo1 = [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as ( - | string - | undefined -)[]; +const foo1 = + [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as (string | undefined)[]; const foo2: ( | AAAAAAAAAAAAAAAAAAAAAA From f745e427c5e25c81e16c487033bbb94c8d3359b6 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 24 Mar 2020 01:20:53 +0900 Subject: [PATCH 02/16] Change test file extension, as.js => as.ts --- tests/typescript_as/__snapshots__/jsfmt.spec.js.snap | 2 +- tests/typescript_as/{as.js => as.ts} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/typescript_as/{as.js => as.ts} (100%) diff --git a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap index edab6a280a11..07374a375b7b 100644 --- a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`as.js 1`] = ` +exports[`as.ts 1`] = ` ====================================options===================================== parsers: ["typescript"] printWidth: 80 diff --git a/tests/typescript_as/as.js b/tests/typescript_as/as.ts similarity index 100% rename from tests/typescript_as/as.js rename to tests/typescript_as/as.ts From 4f6bc62dded4ee2733a4770f7ea9617138c8c298 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 24 Mar 2020 01:26:28 +0900 Subject: [PATCH 03/16] Add tests --- .../__snapshots__/jsfmt.spec.js.snap | 45 +++++++++++++++++-- tests/typescript_as/as.ts | 17 +++++-- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap index 07374a375b7b..7a9682cea1cd 100644 --- a/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_as/__snapshots__/jsfmt.spec.js.snap @@ -9,9 +9,12 @@ printWidth: 80 const name = (description as DescriptionObject).name || (description as string); this.isTabActionBar((e.target || e.srcElement) as HTMLElement); (originalError ? wrappedError(errMsg, originalError) : Error(errMsg)) as InjectionError; -'current' in (props.pagination as Object) -start + (yearSelectTotal as number) -scrollTop > (visibilityHeight as number) +'current' in (props.pagination as Object); +('current' in props.pagination) as Object; +start + (yearSelectTotal as number); +(start + yearSelectTotal) as number; +scrollTop > (visibilityHeight as number); +(scrollTop > visibilityHeight) as number; export default class Column extends (RcTable.Column as React.ComponentClass,ColumnProps,ColumnProps,ColumnProps>) {} export const MobxTypedForm = class extends (Form as { new (): any }) {} export abstract class MobxTypedForm1 extends (Form as { new (): any }) {} @@ -35,6 +38,14 @@ const state = JSON.stringify({ (bValue as boolean) ? 0 : -1; bValue ? 0 : -1; +const value1 = thisIsAReallyReallyReallyReallyReallyLongIdentifier as SomeInterface; +const value2 = thisIsAnIdentifier as thisIsAReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongInterface; +const value3 = thisIsAReallyLongIdentifier as (SomeInterface | SomeOtherInterface); +const value4 = thisIsAReallyLongIdentifier as { prop1: string, prop2: number, prop3: number }[]; +const value5 = thisIsAReallyReallyReallyReallyReallyReallyReallyReallyReallyLongIdentifier as [string, number]; + +const iter1 = createIterator(this.controller, child, this.tag as SyncFunctionComponent); +const iter2 = createIterator(self.controller, child, self.tag as SyncFunctionComponent); =====================================output===================================== const name = (description as DescriptionObject).name || (description as string); @@ -42,8 +53,11 @@ this.isTabActionBar((e.target || e.srcElement) as HTMLElement); (originalError ? wrappedError(errMsg, originalError) : Error(errMsg)) as InjectionError; "current" in (props.pagination as Object); +("current" in props.pagination) as Object; start + (yearSelectTotal as number); +(start + yearSelectTotal) as number; scrollTop > (visibilityHeight as number); +(scrollTop > visibilityHeight) as number; export default class Column extends (RcTable.Column as React.ComponentClass< ColumnProps, @@ -73,6 +87,31 @@ const state = JSON.stringify({ (bValue as boolean) ? 0 : -1; bValue ? 0 : -1; +const value1 = + thisIsAReallyReallyReallyReallyReallyLongIdentifier as SomeInterface; +const value2 = + thisIsAnIdentifier as + thisIsAReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongInterface; +const value3 = + thisIsAReallyLongIdentifier as SomeInterface | SomeOtherInterface; +const value4 = + thisIsAReallyLongIdentifier as + { prop1: string; prop2: number; prop3: number }[]; +const value5 = + thisIsAReallyReallyReallyReallyReallyReallyReallyReallyReallyLongIdentifier as + [string, number]; + +const iter1 = createIterator( + this.controller, + child, + this.tag as SyncFunctionComponent +); +const iter2 = createIterator( + self.controller, + child, + self.tag as SyncFunctionComponent +); + ================================================================================ `; diff --git a/tests/typescript_as/as.ts b/tests/typescript_as/as.ts index 596bdd8d2c92..cfdaced54745 100644 --- a/tests/typescript_as/as.ts +++ b/tests/typescript_as/as.ts @@ -1,9 +1,12 @@ const name = (description as DescriptionObject).name || (description as string); this.isTabActionBar((e.target || e.srcElement) as HTMLElement); (originalError ? wrappedError(errMsg, originalError) : Error(errMsg)) as InjectionError; -'current' in (props.pagination as Object) -start + (yearSelectTotal as number) -scrollTop > (visibilityHeight as number) +'current' in (props.pagination as Object); +('current' in props.pagination) as Object; +start + (yearSelectTotal as number); +(start + yearSelectTotal) as number; +scrollTop > (visibilityHeight as number); +(scrollTop > visibilityHeight) as number; export default class Column extends (RcTable.Column as React.ComponentClass,ColumnProps,ColumnProps,ColumnProps>) {} export const MobxTypedForm = class extends (Form as { new (): any }) {} export abstract class MobxTypedForm1 extends (Form as { new (): any }) {} @@ -27,3 +30,11 @@ const state = JSON.stringify({ (bValue as boolean) ? 0 : -1; bValue ? 0 : -1; +const value1 = thisIsAReallyReallyReallyReallyReallyLongIdentifier as SomeInterface; +const value2 = thisIsAnIdentifier as thisIsAReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongInterface; +const value3 = thisIsAReallyLongIdentifier as (SomeInterface | SomeOtherInterface); +const value4 = thisIsAReallyLongIdentifier as { prop1: string, prop2: number, prop3: number }[]; +const value5 = thisIsAReallyReallyReallyReallyReallyReallyReallyReallyReallyLongIdentifier as [string, number]; + +const iter1 = createIterator(this.controller, child, this.tag as SyncFunctionComponent); +const iter2 = createIterator(self.controller, child, self.tag as SyncFunctionComponent); From 143b43c5747625214c081d7eb15b908acd7dec38 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 24 Mar 2020 01:58:37 +0900 Subject: [PATCH 04/16] Remove old case for TSAsExpression --- src/language-js/printer-estree.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index b7ef21361862..cf6869fe47a7 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -3195,12 +3195,6 @@ function printPathNoParens(path, options, print, args) { return "unknown"; case "TSVoidKeyword": return "void"; - case "TSAsExpression": - return concat([ - path.call(print, "expression"), - " as ", - path.call(print, "typeAnnotation"), - ]); case "TSArrayType": return concat([path.call(print, "elementType"), "[]"]); case "TSPropertySignature": { From e460a0a86a67e4e1ab93e026c134c600fd8884a9 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Tue, 24 Mar 2020 01:59:14 +0900 Subject: [PATCH 05/16] Remove unnecesary condition --- src/language-js/printer-estree.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index cf6869fe47a7..fd489e845748 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -2494,7 +2494,6 @@ function printPathNoParens(path, options, print, args) { n.expressions[i].type === "OptionalMemberExpression" || n.expressions[i].type === "ConditionalExpression" || n.expressions[i].type === "SequenceExpression" || - n.expressions[i].type === "TSAsExpression" || isBinaryish(n.expressions[i]) ) { printed = concat([indent(concat([softline, printed])), softline]); From 805a48bbd7944e625639d081f5c82ac6435357ae Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Wed, 25 Mar 2020 15:04:24 +0900 Subject: [PATCH 06/16] Fix by lint --- src/language-js/printer-estree.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index fd489e845748..ad185f2b9477 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -642,7 +642,8 @@ function printPathNoParens(path, options, print, args) { parent.type === "Property"; const samePrecedenceSubExpression = - isBinaryish(node.left) && shouldFlatten(node.operator, node.left.operator); + isBinaryish(node.left) && + shouldFlatten(node.operator, node.left.operator); if ( shouldNotIndent || From 8f20f78938ed5ab446a1a79b14401615052a7e2b Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Wed, 25 Mar 2020 15:50:55 +0900 Subject: [PATCH 07/16] Update snapshots --- .../__snapshots__/jsfmt.spec.js.snap | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap index 17581a420a43..ca216209fe06 100644 --- a/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_union/__snapshots__/jsfmt.spec.js.snap @@ -269,8 +269,10 @@ type State = { | { discriminant: "BAZ"; baz: any } ); -const foo1 = - [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as (string | undefined)[]; +const foo1 = [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as ( + | string + | undefined +)[]; const foo2: ( | AAAAAAAAAAAAAAAAAAAAAA @@ -377,10 +379,8 @@ type State = { | { discriminant: "BAZ"; baz: any } ); -const foo1 = [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as ( - | string - | undefined -)[]; +const foo1 = + [abc, def, ghi, jkl, mno, pqr, stu, vwx, yz] as (string | undefined)[]; const foo2: ( | AAAAAAAAAAAAAAAAAAAAAA From fb0a5b7346adf35cc5a6d697f3970716fb4c5884 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 26 Mar 2020 03:30:50 +0900 Subject: [PATCH 08/16] Add changelog --- changelog_unreleased/typescript/pr-7869.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 changelog_unreleased/typescript/pr-7869.md diff --git a/changelog_unreleased/typescript/pr-7869.md b/changelog_unreleased/typescript/pr-7869.md new file mode 100644 index 000000000000..a242c4d324a8 --- /dev/null +++ b/changelog_unreleased/typescript/pr-7869.md @@ -0,0 +1,14 @@ +#### Wrap TSAsExpression ([#7869](https://github.com/prettier/prettier/pull/7869) by [@sosukesuzuki](https://github.com/sosukesuzuki)) + + +```ts +// Input +const varibale = foooooooooooooooooooooooooooooooooooooooooooooooooooo as SomeType; + +// Prettier stable +const varibale = foooooooooooooooooooooooooooooooooooooooooooooooooooo as SomeType; + +// Prettier master +const varibale = + foooooooooooooooooooooooooooooooooooooooooooooooooooo as SomeType; +``` From c3ad54586e752f3eced872974364606f7b0a7bfb Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 26 Mar 2020 11:53:54 +0900 Subject: [PATCH 09/16] Remove convetToBinaryishNode --- src/language-js/printer-estree.js | 54 +++++++++++++++++++------------ src/language-js/utils.js | 19 ----------- 2 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index ad185f2b9477..cbd3ce10a38b 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -39,7 +39,6 @@ const { classChildNeedsASIProtection, classPropMayCauseASIProblems, conditionalExpressionChainContainsJSX, - convertToBinaryishNode, getFlowVariance, getLeftSidePathName, getParentExportDeclaration, @@ -556,11 +555,13 @@ function printPathNoParens(path, options, print, args) { case "LogicalExpression": case "NGPipeExpression": case "TSAsExpression": { - const node = convertToBinaryishNode(n); + const leftNodeName = n.type === "TSAsExpression" ? "expression" : "left"; + const rightNodeName = + n.type === "TSAsExpression" ? "typeAnnotation" : "right"; const parent = path.getParentNode(); const parentParent = path.getParentNode(1); const isInsideParenthesis = - node !== parent.body && + n !== parent.body && (parent.type === "IfStatement" || parent.type === "WhileStatement" || parent.type === "SwitchStatement" || @@ -599,7 +600,7 @@ function printPathNoParens(path, options, print, args) { if ( ((parent.type === "CallExpression" || parent.type === "OptionalCallExpression") && - parent.callee === node) || + parent.callee === n) || parent.type === "UnaryExpression" || ((parent.type === "MemberExpression" || parent.type === "OptionalMemberExpression") && @@ -617,14 +618,14 @@ function printPathNoParens(path, options, print, args) { parent.type === "ThrowStatement" || (parent.type === "JSXExpressionContainer" && parentParent.type === "JSXAttribute") || - (node.operator !== "|" && parent.type === "JsExpressionRoot") || - (node.type !== "NGPipeExpression" && + (n.operator !== "|" && parent.type === "JsExpressionRoot") || + (n.type !== "NGPipeExpression" && ((parent.type === "NGRoot" && options.parser === "__ng_binding") || (parent.type === "NGMicrosyntaxExpression" && parentParent.type === "NGMicrosyntax" && parentParent.body.length === 1))) || - (node === parent.body && parent.type === "ArrowFunctionExpression") || - (node !== parent.body && parent.type === "ForStatement") || + (n === parent.body && parent.type === "ArrowFunctionExpression") || + (n !== parent.body && parent.type === "ForStatement") || (parent.type === "ConditionalExpression" && parentParent.type !== "ReturnStatement" && parentParent.type !== "ThrowStatement" && @@ -642,13 +643,13 @@ function printPathNoParens(path, options, print, args) { parent.type === "Property"; const samePrecedenceSubExpression = - isBinaryish(node.left) && - shouldFlatten(node.operator, node.left.operator); + isBinaryish(n[leftNodeName]) && + shouldFlatten(n.operator, n[leftNodeName].operator); if ( shouldNotIndent || - (shouldInlineLogicalExpression(node) && !samePrecedenceSubExpression) || - (!shouldInlineLogicalExpression(node) && shouldIndentIfInlining) + (shouldInlineLogicalExpression(n) && !samePrecedenceSubExpression) || + (!shouldInlineLogicalExpression(n) && shouldIndentIfInlining) ) { return group(concat(parts)); } @@ -666,7 +667,7 @@ function printPathNoParens(path, options, print, args) { // // ) - const hasJSX = isJSXNode(node.right); + const hasJSX = isJSXNode(n[rightNodeName]); const rest = concat(hasJSX ? parts.slice(1, -1) : parts.slice(1)); const groupId = Symbol("logicalChain-" + ++uid); @@ -5710,13 +5711,14 @@ function printBinaryishExpressions( isInsideParenthesis ) { let parts = []; - const node = convertToBinaryishNode(path.getValue()); + const node = path.getValue(); // We treat BinaryExpression and LogicalExpression nodes the same. if (isBinaryish(node)) { const leftNodeName = node.type === "TSAsExpression" ? "expression" : "left"; const rightNodeName = node.type === "TSAsExpression" ? "typeAnnotation" : "right"; + // Put all operators with the same precedence level in the same // group. The reason we only need to do this with the `left` // expression is because given an expression like `1 + 2 - 3`, it @@ -5726,7 +5728,7 @@ function printBinaryishExpressions( // precedence level and should be treated as a separate group, so // print them normally. (This doesn't hold for the `**` operator, // which is unique in that it is right-associative.) - if (shouldFlatten(node.operator, node.left.operator)) { + if (shouldFlatten(node.operator, node[leftNodeName].operator)) { // Flatten them out by recursively calling this function. parts = parts.concat( path.call( @@ -5745,14 +5747,24 @@ function printBinaryishExpressions( parts.push(path.call(print, leftNodeName)); } + const operator = + node.type === "NGPipeExpression" + ? "|" + : node.type === "TSAsExpression" + ? "as" + : node.operator; + const shouldInline = shouldInlineLogicalExpression(node); const lineBeforeOperator = - (node.operator === "|>" || + (operator === "|>" || node.type === "NGPipeExpression" || - (node.operator === "|" && options.parser === "__vue_expression")) && - !hasLeadingOwnLineComment(options.originalText, node.right, options); + (operator === "|" && options.parser === "__vue_expression")) && + !hasLeadingOwnLineComment( + options.originalText, + node[rightNodeName], + options + ); - const operator = node.type === "NGPipeExpression" ? "|" : node.operator; const rightSuffix = node.type === "NGPipeExpression" && node.arguments.length !== 0 ? group( @@ -5787,8 +5799,8 @@ function printBinaryishExpressions( const shouldGroup = !(isInsideParenthesis && node.type === "LogicalExpression") && parent.type !== node.type && - node.left.type !== node.type && - node.right.type !== node.type; + node[leftNodeName].type !== node.type && + node[rightNodeName].type !== node.type; parts.push(" ", shouldGroup ? group(right) : right); diff --git a/src/language-js/utils.js b/src/language-js/utils.js index 20e4baa4ceb1..48375f7003e8 100644 --- a/src/language-js/utils.js +++ b/src/language-js/utils.js @@ -1004,29 +1004,10 @@ function isTSXFile(options) { return options.filepath && /\.tsx$/i.test(options.filepath); } -function convertToBinaryishNode(node) { - if (node.type === "TSAsExpression") { - const binaryishNode = { ...node }; - - binaryishNode.operator = "as"; - - binaryishNode.left = node.expression; - delete binaryishNode.expression; - - binaryishNode.right = node.typeAnnotation; - delete binaryishNode.typeAnnotation; - - return binaryishNode; - } - - return node; -} - module.exports = { classChildNeedsASIProtection, classPropMayCauseASIProblems, conditionalExpressionChainContainsJSX, - convertToBinaryishNode, getFlowVariance, getLeftSidePathName, getParentExportDeclaration, From 5b5f4c1564e57a31f889448d103c0eed57a63aff Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 26 Mar 2020 11:56:05 +0900 Subject: [PATCH 10/16] Fix comment --- src/language-js/printer-estree.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index cbd3ce10a38b..7de1736fac15 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -5713,7 +5713,7 @@ function printBinaryishExpressions( let parts = []; const node = path.getValue(); - // We treat BinaryExpression and LogicalExpression nodes the same. + // We treat BinaryExpression, LogicalExpression, NGPipeExpression and TSAsExpression the same. if (isBinaryish(node)) { const leftNodeName = node.type === "TSAsExpression" ? "expression" : "left"; const rightNodeName = From 289b99340e9ae817c34c664166fd587edd5dea80 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Thu, 26 Mar 2020 12:22:21 +0900 Subject: [PATCH 11/16] Use getBinaryishNodeNames --- src/language-js/printer-estree.js | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 7de1736fac15..5a36d67d7ea2 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -555,9 +555,7 @@ function printPathNoParens(path, options, print, args) { case "LogicalExpression": case "NGPipeExpression": case "TSAsExpression": { - const leftNodeName = n.type === "TSAsExpression" ? "expression" : "left"; - const rightNodeName = - n.type === "TSAsExpression" ? "typeAnnotation" : "right"; + const { leftNodeName, rightNodeName } = getBinaryishNodeNames(n); const parent = path.getParentNode(); const parentParent = path.getParentNode(1); const isInsideParenthesis = @@ -5695,6 +5693,20 @@ function shouldInlineLogicalExpression(node) { return false; } +function getBinaryishNodeNames(node) { + const leftNodeName = node.type === "TSAsExpression" ? "expression" : "left"; + const rightNodeName = + node.type === "TSAsExpression" ? "typeAnnotation" : "right"; + const operator = + node.type === "NGPipeExpression" + ? "|" + : node.type === "TSAsExpression" + ? "as" + : node.operator; + + return { leftNodeName, rightNodeName, operator }; +} + // For binary expressions to be consistent, we need to group // subsequent operators with the same precedence level under a single // group. Otherwise they will be nested such that some of them break @@ -5715,9 +5727,9 @@ function printBinaryishExpressions( // We treat BinaryExpression, LogicalExpression, NGPipeExpression and TSAsExpression the same. if (isBinaryish(node)) { - const leftNodeName = node.type === "TSAsExpression" ? "expression" : "left"; - const rightNodeName = - node.type === "TSAsExpression" ? "typeAnnotation" : "right"; + const { leftNodeName, rightNodeName, operator } = getBinaryishNodeNames( + node + ); // Put all operators with the same precedence level in the same // group. The reason we only need to do this with the `left` @@ -5747,13 +5759,6 @@ function printBinaryishExpressions( parts.push(path.call(print, leftNodeName)); } - const operator = - node.type === "NGPipeExpression" - ? "|" - : node.type === "TSAsExpression" - ? "as" - : node.operator; - const shouldInline = shouldInlineLogicalExpression(node); const lineBeforeOperator = (operator === "|>" || From 27e67ef55806030195a9d4cf9c2db2c89fe70887 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Fri, 27 Mar 2020 01:25:45 +0900 Subject: [PATCH 12/16] Modify to wrap TSAsExpression with parens --- src/common/util.js | 5 +++++ src/language-js/printer-estree.js | 17 +++++++++++++---- .../__snapshots__/jsfmt.spec.js.snap | 8 ++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/common/util.js b/src/common/util.js index 51ab3a1dce40..b288568ee060 100644 --- a/src/common/util.js +++ b/src/common/util.js @@ -399,6 +399,11 @@ function shouldFlatten(parentOp, nodeOp) { return false; } + // foo as SomeType01 as SomeType02 --> (foo as SomeType01) as SomeType02 + if (parentOp === "as" && nodeOp === "as") { + return false; + } + return true; } diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 5a36d67d7ea2..2ecea55fe421 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -555,7 +555,7 @@ function printPathNoParens(path, options, print, args) { case "LogicalExpression": case "NGPipeExpression": case "TSAsExpression": { - const { leftNodeName, rightNodeName } = getBinaryishNodeNames(n); + const { leftNodeName, rightNodeName, operator } = getBinaryishNodeNames(n); const parent = path.getParentNode(); const parentParent = path.getParentNode(1); const isInsideParenthesis = @@ -616,7 +616,7 @@ function printPathNoParens(path, options, print, args) { parent.type === "ThrowStatement" || (parent.type === "JSXExpressionContainer" && parentParent.type === "JSXAttribute") || - (n.operator !== "|" && parent.type === "JsExpressionRoot") || + (operator !== "|" && parent.type === "JsExpressionRoot") || (n.type !== "NGPipeExpression" && ((parent.type === "NGRoot" && options.parser === "__ng_binding") || (parent.type === "NGMicrosyntaxExpression" && @@ -642,7 +642,10 @@ function printPathNoParens(path, options, print, args) { const samePrecedenceSubExpression = isBinaryish(n[leftNodeName]) && - shouldFlatten(n.operator, n[leftNodeName].operator); + shouldFlatten( + operator, + getBinaryishNodeNames(n[leftNodeName]).operator + ); if ( shouldNotIndent || @@ -5740,7 +5743,13 @@ function printBinaryishExpressions( // precedence level and should be treated as a separate group, so // print them normally. (This doesn't hold for the `**` operator, // which is unique in that it is right-associative.) - if (shouldFlatten(node.operator, node[leftNodeName].operator)) { + if ( + node.type === "NGPipeExpression" || + shouldFlatten( + operator, + getBinaryishNodeNames(node[leftNodeName]).operator + ) + ) { // Flatten them out by recursively calling this function. parts = parts.concat( path.call( diff --git a/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap index a07ddfbe3dd7..bd7c16c29ce7 100644 --- a/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_argument_expansion/__snapshots__/jsfmt.spec.js.snap @@ -41,7 +41,7 @@ const bar8 = [1,2,3].reduce((carry, value) => { =====================================output===================================== const bar1 = [1, 2, 3].reduce((carry, value) => { return [...carry, value]; -}, [] as unknown as number[]); +}, ([] as unknown) as number[]); const bar2 = [1, 2, 3].reduce((carry, value) => { return [...carry, value]; @@ -51,7 +51,7 @@ const bar3 = [1, 2, 3].reduce( (carry, value) => { return [...carry, value]; }, - [1, 2, 3] as unknown as number[] + ([1, 2, 3] as unknown) as number[] ); const bar4 = [1, 2, 3].reduce( @@ -63,7 +63,7 @@ const bar4 = [1, 2, 3].reduce( const bar5 = [1, 2, 3].reduce((carry, value) => { return { ...carry, [value]: true }; -}, {} as unknown as { [key: number]: boolean }); +}, ({} as unknown) as { [key: number]: boolean }); const bar6 = [1, 2, 3].reduce((carry, value) => { return { ...carry, [value]: true }; @@ -73,7 +73,7 @@ const bar7 = [1, 2, 3].reduce( (carry, value) => { return { ...carry, [value]: true }; }, - { 1: true } as unknown as { [key: number]: boolean } + ({ 1: true } as unknown) as { [key: number]: boolean } ); const bar8 = [1, 2, 3].reduce( From 515f688f8fe63bfcbeb24b9e2277fda1f285a1e1 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Fri, 27 Mar 2020 01:46:22 +0900 Subject: [PATCH 13/16] Fix by lint --- src/language-js/printer-estree.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 2ecea55fe421..f9c5579a9a7b 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -555,7 +555,9 @@ function printPathNoParens(path, options, print, args) { case "LogicalExpression": case "NGPipeExpression": case "TSAsExpression": { - const { leftNodeName, rightNodeName, operator } = getBinaryishNodeNames(n); + const { leftNodeName, rightNodeName, operator } = getBinaryishNodeNames( + n + ); const parent = path.getParentNode(); const parentParent = path.getParentNode(1); const isInsideParenthesis = From 8e0c384f173b534a20d328e2c6714f40657b8335 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Fri, 27 Mar 2020 20:01:59 +0900 Subject: [PATCH 14/16] Do not pass operator when node is TSAsExpression --- src/language-js/printer-estree.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index f9c5579a9a7b..423d9fb685b4 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -5747,10 +5747,8 @@ function printBinaryishExpressions( // which is unique in that it is right-associative.) if ( node.type === "NGPipeExpression" || - shouldFlatten( - operator, - getBinaryishNodeNames(node[leftNodeName]).operator - ) + (node.type !== "TSAsExpression" && + shouldFlatten(operator, node[leftNodeName].operator)) ) { // Flatten them out by recursively calling this function. parts = parts.concat( From eb9f330e62299488d5ecea76f41879f60930100d Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Fri, 27 Mar 2020 20:29:26 +0900 Subject: [PATCH 15/16] Refactor shouldNotIndent --- src/language-js/printer-estree.js | 44 ++++++++++++++----------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 423d9fb685b4..81bb89323aac 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -555,9 +555,7 @@ function printPathNoParens(path, options, print, args) { case "LogicalExpression": case "NGPipeExpression": case "TSAsExpression": { - const { leftNodeName, rightNodeName, operator } = getBinaryishNodeNames( - n - ); + const { rightNodeName, operator } = getBinaryishNodeNames(n); const parent = path.getParentNode(); const parentParent = path.getParentNode(1); const isInsideParenthesis = @@ -613,7 +611,7 @@ function printPathNoParens(path, options, print, args) { // Avoid indenting sub-expressions in some cases where the first sub-expression is already // indented accordingly. We should indent sub-expressions where the first case isn't indented. - const shouldNotIndent = + let shouldNotIndent = parent.type === "ReturnStatement" || parent.type === "ThrowStatement" || (parent.type === "JSXExpressionContainer" && @@ -633,27 +631,25 @@ function printPathNoParens(path, options, print, args) { parentParent.type !== "OptionalCallExpression") || parent.type === "TemplateLiteral"; - const shouldIndentIfInlining = - parent.type === "AssignmentExpression" || - parent.type === "VariableDeclarator" || - parent.type === "ClassProperty" || - parent.type === "TSAbstractClassProperty" || - parent.type === "ClassPrivateProperty" || - parent.type === "ObjectProperty" || - parent.type === "Property"; - - const samePrecedenceSubExpression = - isBinaryish(n[leftNodeName]) && - shouldFlatten( - operator, - getBinaryishNodeNames(n[leftNodeName]).operator - ); + if (!shouldNotIndent) { + if (shouldInlineLogicalExpression(n)) { + const samePrecedenceSubExpression = + isBinaryish(n.left) && shouldFlatten(operator, n.left.operator); + shouldNotIndent = !samePrecedenceSubExpression; + } else { + const shouldIndentIfInlining = + parent.type === "AssignmentExpression" || + parent.type === "VariableDeclarator" || + parent.type === "ClassProperty" || + parent.type === "TSAbstractClassProperty" || + parent.type === "ClassPrivateProperty" || + parent.type === "ObjectProperty" || + parent.type === "Property"; + shouldNotIndent = shouldIndentIfInlining; + } + } - if ( - shouldNotIndent || - (shouldInlineLogicalExpression(n) && !samePrecedenceSubExpression) || - (!shouldInlineLogicalExpression(n) && shouldIndentIfInlining) - ) { + if (shouldNotIndent) { return group(concat(parts)); } From 0b7a7ac3dd0f9323ffcec768ab8ce8209a3b311b Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Fri, 27 Mar 2020 17:40:40 +0200 Subject: [PATCH 16/16] Update util.js --- src/common/util.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/common/util.js b/src/common/util.js index b288568ee060..51ab3a1dce40 100644 --- a/src/common/util.js +++ b/src/common/util.js @@ -399,11 +399,6 @@ function shouldFlatten(parentOp, nodeOp) { return false; } - // foo as SomeType01 as SomeType02 --> (foo as SomeType01) as SomeType02 - if (parentOp === "as" && nodeOp === "as") { - return false; - } - return true; }