From 84f72be9dc7cba2eae384a4aafddea1b836e70cb Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Fri, 28 May 2021 10:40:33 +0900 Subject: [PATCH] Avoid to break after assignment for call expression that has complex type parameters (#10949) * Avoid break-after-operator when call expr has complex type params * Add tests * Add changelog * Fix issue number * Update snapshpts * Address review * Support Flow * Rename parameters => arguments * Remove isCallExpression --- changelog_unreleased/typescript/10949.md | 27 ++++++ src/language-js/print/assignment.js | 40 ++++++++- .../__snapshots__/jsfmt.spec.js.snap | 83 +++++++++++++++++++ tests/format/flow/assignments/issue-10846.js | 38 +++++++++ .../__snapshots__/jsfmt.spec.js.snap | 83 +++++++++++++++++++ .../typescript/assignment/issue-10846.ts | 38 +++++++++ 6 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 changelog_unreleased/typescript/10949.md create mode 100644 tests/format/flow/assignments/issue-10846.js create mode 100644 tests/format/typescript/assignment/issue-10846.ts diff --git a/changelog_unreleased/typescript/10949.md b/changelog_unreleased/typescript/10949.md new file mode 100644 index 000000000..973b8b7eb --- /dev/null +++ b/changelog_unreleased/typescript/10949.md @@ -0,0 +1,27 @@ +#### Avoid breaking call expressions after assignments with complex type arguments (#10949 by @sosukesuzuki) + + +```ts +// Input +const foo = call<{ + prop1: string; + prop2: string; + prop3: string; +}>(); + +// Prettier stable +const foo = + call<{ + prop1: string; + prop2: string; + prop3: string; + }>(); + +// Prettier main +const foo = call<{ + prop1: string; + prop2: string; + prop3: string; +}>(); + +``` diff --git a/src/language-js/print/assignment.js b/src/language-js/print/assignment.js index 31d73ade1..2a2adfe76 100644 --- a/src/language-js/print/assignment.js +++ b/src/language-js/print/assignment.js @@ -3,7 +3,7 @@ const { isNonEmptyArray, getStringWidth } = require("../../common/util"); const { builders: { line, group, indent, indentIfBreak }, - utils: { cleanDoc }, + utils: { cleanDoc, willBreak }, } = require("../../document"); const { hasLeadingOwnLineComment, @@ -345,6 +345,10 @@ function isPoorlyBreakableMemberOrCallChain( return false; } + if (isCallExpressionWithComplexTypeArguments(node, print)) { + return false; + } + return path.call(goDeeper, "callee"); } @@ -415,6 +419,40 @@ function isObjectPropertyWithShortKey(node, keyDoc, options) { ); } +function isCallExpressionWithComplexTypeArguments(node, print) { + const typeArgs = getTypeArgumentsFromCallExpression(node); + if (isNonEmptyArray(typeArgs)) { + if (typeArgs.length > 1) { + return true; + } + if (typeArgs.length === 1) { + const firstArg = typeArgs[0]; + if ( + firstArg.type === "TSUnionType" || + firstArg.type === "UnionTypeAnnotation" || + firstArg.type === "TSIntersectionType" || + firstArg.type === "IntersectionTypeAnnotation" + ) { + return true; + } + } + const typeArgsKeyName = node.typeParameters + ? "typeParameters" + : "typeArguments"; + if (willBreak(print(typeArgsKeyName))) { + return true; + } + } + return false; +} + +function getTypeArgumentsFromCallExpression(node) { + return ( + (node.typeParameters && node.typeParameters.params) || + (node.typeArguments && node.typeArguments.params) + ); +} + module.exports = { printVariableDeclarator, printAssignmentExpression, diff --git a/tests/format/flow/assignments/__snapshots__/jsfmt.spec.js.snap b/tests/format/flow/assignments/__snapshots__/jsfmt.spec.js.snap index 9d921c9cb..d34376ab4 100644 --- a/tests/format/flow/assignments/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/flow/assignments/__snapshots__/jsfmt.spec.js.snap @@ -1,5 +1,88 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`issue-10846.js format 1`] = ` +====================================options===================================== +parsers: ["babel-flow", "flow"] +printWidth: 80 + | printWidth +=====================================input====================================== +const foo1 = + call<{ + prop1: string; + prop2: string; + prop3: string; + }>(); + +export const CallRecorderContext1 = + createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; + } | null>(null); + +export const CallRecorderContext2 = createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; +} | null>(null, "useless"); + +const foo2 = + call(); + +const foo3 = + call< + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + >(); + +const foo4 = + call< + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo + >(); + +=====================================output===================================== +const foo1 = call<{ + prop1: string, + prop2: string, + prop3: string, +}>(); + +export const CallRecorderContext1 = createContext<{ + deleteRecording: (id: string) => void, + deleteAll: () => void, +} | null>(null); + +export const CallRecorderContext2 = createContext<{ + deleteRecording: (id: string) => void, + deleteAll: () => void, +} | null>(null, "useless"); + +const foo2 = call< + Foooooo, + Foooooo, + Foooooo, + Foooooo, + Foooooo, + Foooooo, + Foooooo +>(); + +const foo3 = call< + Foooooooooooo | Foooooooooooo | Foooooooooooo | Foooooooooooo | Foooooooooooo +>(); + +const foo4 = call< + Foooooooooooo & Foooooooooooo & Foooooooooooo & Foooooooooooo & Foooooooooooo +>(); + +================================================================================ +`; + exports[`issue-10850.js format 1`] = ` ====================================options===================================== parsers: ["babel-flow", "flow"] diff --git a/tests/format/flow/assignments/issue-10846.js b/tests/format/flow/assignments/issue-10846.js new file mode 100644 index 000000000..61141c11b --- /dev/null +++ b/tests/format/flow/assignments/issue-10846.js @@ -0,0 +1,38 @@ +const foo1 = + call<{ + prop1: string; + prop2: string; + prop3: string; + }>(); + +export const CallRecorderContext1 = + createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; + } | null>(null); + +export const CallRecorderContext2 = createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; +} | null>(null, "useless"); + +const foo2 = + call(); + +const foo3 = + call< + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + >(); + +const foo4 = + call< + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo + >(); diff --git a/tests/format/typescript/assignment/__snapshots__/jsfmt.spec.js.snap b/tests/format/typescript/assignment/__snapshots__/jsfmt.spec.js.snap index 5f5eb0b6d..6fc1604d4 100644 --- a/tests/format/typescript/assignment/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/typescript/assignment/__snapshots__/jsfmt.spec.js.snap @@ -180,6 +180,89 @@ const firestorePersonallyIdentifiablePaths: Array = ================================================================================ `; +exports[`issue-10846.ts format 1`] = ` +====================================options===================================== +parsers: ["typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const foo = call<{ + prop1: string; + prop2: string; + prop3: string; +}>(); + +export const CallRecorderContext = + createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; + } | null>(null); + +export const CallRecorderContext = + createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; + } | null>(null, "useless"); + +const foo = + call(); + +const foo = + call< + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + >(); + +const foo = + call< + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo + >(); + +=====================================output===================================== +const foo = call<{ + prop1: string; + prop2: string; + prop3: string; +}>(); + +export const CallRecorderContext = createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; +} | null>(null); + +export const CallRecorderContext = createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; +} | null>(null, "useless"); + +const foo = call< + Foooooo, + Foooooo, + Foooooo, + Foooooo, + Foooooo, + Foooooo, + Foooooo +>(); + +const foo = call< + Foooooooooooo | Foooooooooooo | Foooooooooooo | Foooooooooooo | Foooooooooooo +>(); + +const foo = call< + Foooooooooooo & Foooooooooooo & Foooooooooooo & Foooooooooooo & Foooooooooooo +>(); + +================================================================================ +`; + exports[`issue-10850.ts format 1`] = ` ====================================options===================================== parsers: ["typescript"] diff --git a/tests/format/typescript/assignment/issue-10846.ts b/tests/format/typescript/assignment/issue-10846.ts new file mode 100644 index 000000000..db4b21118 --- /dev/null +++ b/tests/format/typescript/assignment/issue-10846.ts @@ -0,0 +1,38 @@ +const foo = call<{ + prop1: string; + prop2: string; + prop3: string; +}>(); + +export const CallRecorderContext = + createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; + } | null>(null); + +export const CallRecorderContext = + createContext<{ + deleteRecording: (id: string) => void; + deleteAll: () => void; + } | null>(null, "useless"); + +const foo = + call(); + +const foo = + call< + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + | Foooooooooooo + >(); + +const foo = + call< + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo & + Foooooooooooo + >();