From 14179ef98b40e5d784af7bfc94450d9af776c989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 29 May 2019 14:53:19 +0800 Subject: [PATCH 1/3] add support for extract as interface --- src/compiler/diagnosticMessages.json | 4 + src/services/refactors/extractType.ts | 83 ++++++++++++++++--- .../cases/fourslash/refactorExtractType60.ts | 16 ++++ .../cases/fourslash/refactorExtractType61.ts | 18 ++++ .../cases/fourslash/refactorExtractType62.ts | 7 ++ .../cases/fourslash/refactorExtractType63.ts | 7 ++ .../cases/fourslash/refactorExtractType64.ts | 7 ++ .../cases/fourslash/refactorExtractType65.ts | 7 ++ .../cases/fourslash/refactorExtractType66.ts | 16 ++++ 9 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/refactorExtractType60.ts create mode 100644 tests/cases/fourslash/refactorExtractType61.ts create mode 100644 tests/cases/fourslash/refactorExtractType62.ts create mode 100644 tests/cases/fourslash/refactorExtractType63.ts create mode 100644 tests/cases/fourslash/refactorExtractType64.ts create mode 100644 tests/cases/fourslash/refactorExtractType65.ts create mode 100644 tests/cases/fourslash/refactorExtractType66.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 5d6dc7d6df411..50743ef944e6f 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4990,6 +4990,10 @@ "category": "Message", "code": 95079 }, + "Extract to interface": { + "category": "Message", + "code": 95080 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 02f6cd39a1885..5a32af4a5dfb8 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -2,6 +2,7 @@ namespace ts.refactor { const refactorName = "Extract type"; const extractToTypeAlias = "Extract to type alias"; + const extractToInterface = "Extract to interface"; const extractToTypeDef = "Extract to typedef"; registerRefactor(refactorName, { getAvailableActions(context): ReadonlyArray { @@ -11,23 +12,32 @@ namespace ts.refactor { return [{ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Extract_type), - actions: [info.isJS ? { + actions: info.isJS ? [{ name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef) - } : { - name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias) - }] + }] : append([{ + name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias) + }], info.typeElements && { + name: extractToInterface, description: getLocaleSpecificMessage(Diagnostics.Extract_to_interface) + }) }]; }, getEditsForAction(context, actionName): RefactorEditInfo { - Debug.assert(actionName === extractToTypeAlias || actionName === extractToTypeDef); + Debug.assert(actionName === extractToTypeAlias || actionName === extractToTypeDef || actionName === extractToInterface); const { file } = context; const info = Debug.assertDefined(getRangeToExtract(context)); - Debug.assert(actionName === extractToTypeAlias && !info.isJS || actionName === extractToTypeDef && info.isJS); + Debug.assert( + info.isJS ? actionName === extractToTypeDef : ( + (actionName === extractToInterface && !!info.typeElements) || actionName === extractToTypeAlias + ) + ); const name = getUniqueName("NewType", file); const edits = textChanges.ChangeTracker.with(context, changes => info.isJS ? - doTypedefChange(changes, file, name, info.firstStatement, info.selection, info.typeParameters) : - doTypeAliasChange(changes, file, name, info.firstStatement, info.selection, info.typeParameters)); + doTypedefChange(changes, file, name, info) : + info.typeElements ? + doInterfaceChange(changes, file, name, info) : + doTypeAliasChange(changes, file, name, info) + ); const renameFilename = file.fileName; const renameLocation = getRenameLocation(edits, renameFilename, name, /*preferLastLocation*/ false); @@ -35,7 +45,15 @@ namespace ts.refactor { } }); - interface Info { isJS: boolean; selection: TypeNode; firstStatement: Statement; typeParameters: ReadonlyArray; } + interface TypeAliasInfo { + isJS: boolean; selection: TypeNode; firstStatement: Statement; typeParameters: ReadonlyArray; typeElements: undefined; + } + + interface InterfaceInfo { + isJS: boolean; selection: TypeNode; firstStatement: Statement; typeParameters: ReadonlyArray; typeElements: ReadonlyArray; + } + + type Info = TypeAliasInfo | InterfaceInfo; function getRangeToExtract(context: RefactorContext): Info | undefined { const { file, startPosition } = context; @@ -51,7 +69,29 @@ namespace ts.refactor { const typeParameters = collectTypeParameters(checker, selection, firstStatement, file); if (!typeParameters) return undefined; - return { isJS, selection, firstStatement, typeParameters }; + const types = flattenTypeLiteralNodeReference(checker, selection); + const typeElements = types && flatMap(types, type => type.members); + return { isJS, selection, firstStatement, typeParameters, typeElements } as Info; + } + + function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode | undefined): ReadonlyArray | undefined { + if (!node) return undefined; + if (isIntersectionTypeNode(node)) { + const result: TypeLiteralNode[] = []; + for (const type of node.types) { + const flattenedTypes = flattenTypeLiteralNodeReference(checker, type); + if (!flattenedTypes) return undefined; + addRange(result, flattenedTypes); + } + return result; + } + else if (isParenthesizedTypeNode(node)) { + return flattenTypeLiteralNodeReference(checker, node.type); + } + else if (isTypeLiteralNode(node)) { + return [node]; + } + return undefined; } function isStatementAndHasJSDoc(n: Node): n is (Statement & HasJSDoc) { @@ -107,7 +147,9 @@ namespace ts.refactor { } } - function doTypeAliasChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, firstStatement: Statement, selection: TypeNode, typeParameters: ReadonlyArray) { + function doTypeAliasChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: TypeAliasInfo) { + const { firstStatement, selection, typeParameters } = info; + const newTypeNode = createTypeAliasDeclaration( /* decorators */ undefined, /* modifiers */ undefined, @@ -119,7 +161,24 @@ namespace ts.refactor { changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); } - function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, firstStatement: Statement, selection: TypeNode, typeParameters: ReadonlyArray) { + function doInterfaceChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: InterfaceInfo) { + const { firstStatement, selection, typeParameters, typeElements } = info; + + const newTypeNode = createInterfaceDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + name, + typeParameters, + /* heritageClauses */ undefined, + typeElements + ); + changes.insertNodeBefore(file, firstStatement, newTypeNode, /* blankLineBetween */ true); + changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); + } + + function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: Info) { + const { firstStatement, selection, typeParameters } = info; + const node = createNode(SyntaxKind.JSDocTypedefTag); node.tagName = createIdentifier("typedef"); // TODO: jsdoc factory https://github.com/Microsoft/TypeScript/pull/29539 node.fullName = createIdentifier(name); diff --git a/tests/cases/fourslash/refactorExtractType60.ts b/tests/cases/fourslash/refactorExtractType60.ts new file mode 100644 index 0000000000000..0efa996114c7f --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType60.ts @@ -0,0 +1,16 @@ +/// + +//// function foo(a: /*a*/{ a: number | string, b: string }/*b*/) { } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to interface", + actionDescription: "Extract to interface", + newContent: `interface /*RENAME*/NewType { + a: number | string; + b: string; +} + +function foo(a: NewType) { }`, +}); diff --git a/tests/cases/fourslash/refactorExtractType61.ts b/tests/cases/fourslash/refactorExtractType61.ts new file mode 100644 index 0000000000000..e87b01bc320f7 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType61.ts @@ -0,0 +1,18 @@ +/// + +//// function foo(a: /*a*/{ a: number | string, b: string } & { c: string } & { d: boolean }/*b*/) { } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to interface", + actionDescription: "Extract to interface", + newContent: `interface /*RENAME*/NewType { + a: number | string; + b: string; + c: string; + d: boolean; +} + +function foo(a: NewType) { }`, +}); diff --git a/tests/cases/fourslash/refactorExtractType62.ts b/tests/cases/fourslash/refactorExtractType62.ts new file mode 100644 index 0000000000000..f4d4e0f53e52a --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType62.ts @@ -0,0 +1,7 @@ +/// + +//// type T = { c: string } +//// function foo(a: /*a*/{ a: number | string, b: string } & T/*b*/) { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract type", "Extract to interface") diff --git a/tests/cases/fourslash/refactorExtractType63.ts b/tests/cases/fourslash/refactorExtractType63.ts new file mode 100644 index 0000000000000..0aef1c28249ba --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType63.ts @@ -0,0 +1,7 @@ +/// + +//// type T = { c: string } & { d: boolean } +//// function foo(a: /*a*/{ a: number | string, b: string } & T/*b*/) { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract type", "Extract to interface") diff --git a/tests/cases/fourslash/refactorExtractType64.ts b/tests/cases/fourslash/refactorExtractType64.ts new file mode 100644 index 0000000000000..d54c1e16eef8e --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType64.ts @@ -0,0 +1,7 @@ +/// + +//// type T = { c: string } & Record +//// function foo(a: /*a*/{ a: number | string, b: string } & T/*b*/) { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract type", "Extract to interface") diff --git a/tests/cases/fourslash/refactorExtractType65.ts b/tests/cases/fourslash/refactorExtractType65.ts new file mode 100644 index 0000000000000..00a999dae3469 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType65.ts @@ -0,0 +1,7 @@ +/// + +//// type T = { c: string } +//// function foo(a: /*a*/T/*b*/) { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract type", "Extract to interface") diff --git a/tests/cases/fourslash/refactorExtractType66.ts b/tests/cases/fourslash/refactorExtractType66.ts new file mode 100644 index 0000000000000..2d28fe030c484 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType66.ts @@ -0,0 +1,16 @@ +/// + +//// function foo(a: /*a*/{ a: string } & { b: U }/*b*/) { } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract type", + actionName: "Extract to interface", + actionDescription: "Extract to interface", + newContent: `interface /*RENAME*/NewType { + a: string; + b: U; +} + +function foo(a: NewType) { }`, +}); From 30d6eeb26d67619b90c2f755d1642f883c1be14e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 29 May 2019 15:57:41 +0800 Subject: [PATCH 2/3] fix action assert --- src/services/refactors/extractType.ts | 31 +++++++++++++++------------ 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 5a32af4a5dfb8..b83192d3221b2 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -22,22 +22,25 @@ namespace ts.refactor { }]; }, getEditsForAction(context, actionName): RefactorEditInfo { - Debug.assert(actionName === extractToTypeAlias || actionName === extractToTypeDef || actionName === extractToInterface); const { file } = context; const info = Debug.assertDefined(getRangeToExtract(context)); - Debug.assert( - info.isJS ? actionName === extractToTypeDef : ( - (actionName === extractToInterface && !!info.typeElements) || actionName === extractToTypeAlias - ) - ); const name = getUniqueName("NewType", file); - const edits = textChanges.ChangeTracker.with(context, changes => info.isJS ? - doTypedefChange(changes, file, name, info) : - info.typeElements ? - doInterfaceChange(changes, file, name, info) : - doTypeAliasChange(changes, file, name, info) - ); + const edits = textChanges.ChangeTracker.with(context, changes => { + switch (actionName) { + case extractToTypeAlias: + Debug.assert(!info.isJS); + return doTypeAliasChange(changes, file, name, info); + case extractToTypeDef: + Debug.assert(info.isJS); + return doTypedefChange(changes, file, name, info); + case extractToInterface: + Debug.assert(!info.isJS && !!info.typeElements); + return doInterfaceChange(changes, file, name, info as InterfaceInfo); + default: + Debug.fail("Unexpected action"); + } + }); const renameFilename = file.fileName; const renameLocation = getRenameLocation(edits, renameFilename, name, /*preferLastLocation*/ false); @@ -46,7 +49,7 @@ namespace ts.refactor { }); interface TypeAliasInfo { - isJS: boolean; selection: TypeNode; firstStatement: Statement; typeParameters: ReadonlyArray; typeElements: undefined; + isJS: boolean; selection: TypeNode; firstStatement: Statement; typeParameters: ReadonlyArray; typeElements?: ReadonlyArray; } interface InterfaceInfo { @@ -71,7 +74,7 @@ namespace ts.refactor { const types = flattenTypeLiteralNodeReference(checker, selection); const typeElements = types && flatMap(types, type => type.members); - return { isJS, selection, firstStatement, typeParameters, typeElements } as Info; + return { isJS, selection, firstStatement, typeParameters, typeElements }; } function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode | undefined): ReadonlyArray | undefined { From 9fdc263c048f9aa6a3630c0cd9249e6ad9673627 Mon Sep 17 00:00:00 2001 From: kingwl Date: Tue, 27 Aug 2019 16:24:35 +0800 Subject: [PATCH 3/3] Donot provide convert to interface if duplicate member --- src/services/refactors/extractType.ts | 29 ++++++++++--------- .../cases/fourslash/refactorExtractType67.ts | 6 ++++ .../cases/fourslash/refactorExtractType68.ts | 6 ++++ 3 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/refactorExtractType67.ts create mode 100644 tests/cases/fourslash/refactorExtractType68.ts diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 1ab6800e11020..8944e5fbb50ae 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -22,25 +22,23 @@ namespace ts.refactor { }]; }, getEditsForAction(context, actionName): RefactorEditInfo { - Debug.assert(actionName === extractToTypeAlias || actionName === extractToTypeDef, "Unexpected action name"); const { file } = context; const info = Debug.assertDefined(getRangeToExtract(context), "Expected to find a range to extract"); - Debug.assert(actionName === extractToTypeAlias && !info.isJS || actionName === extractToTypeDef && info.isJS, "Invalid actionName/JS combo"); const name = getUniqueName("NewType", file); const edits = textChanges.ChangeTracker.with(context, changes => { switch (actionName) { case extractToTypeAlias: - Debug.assert(!info.isJS); + Debug.assert(!info.isJS, "Invalid actionName/JS combo"); return doTypeAliasChange(changes, file, name, info); case extractToTypeDef: - Debug.assert(info.isJS); + Debug.assert(info.isJS, "Invalid actionName/JS combo"); return doTypedefChange(changes, file, name, info); case extractToInterface: - Debug.assert(!info.isJS && !!info.typeElements); + Debug.assert(!info.isJS && !!info.typeElements, "Invalid actionName/JS combo"); return doInterfaceChange(changes, file, name, info as InterfaceInfo); default: - Debug.fail("Unexpected action"); + Debug.fail("Unexpected action name"); } }); @@ -74,19 +72,22 @@ namespace ts.refactor { const typeParameters = collectTypeParameters(checker, selection, firstStatement, file); if (!typeParameters) return undefined; - const types = flattenTypeLiteralNodeReference(checker, selection); - const typeElements = types && flatMap(types, type => type.members); + const typeElements = flattenTypeLiteralNodeReference(checker, selection); return { isJS, selection, firstStatement, typeParameters, typeElements }; } - function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode | undefined): ReadonlyArray | undefined { + function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode | undefined): ReadonlyArray | undefined { if (!node) return undefined; if (isIntersectionTypeNode(node)) { - const result: TypeLiteralNode[] = []; + const result: TypeElement[] = []; + const seen = createMap(); for (const type of node.types) { - const flattenedTypes = flattenTypeLiteralNodeReference(checker, type); - if (!flattenedTypes) return undefined; - addRange(result, flattenedTypes); + const flattenedTypeMembers = flattenTypeLiteralNodeReference(checker, type); + if (!flattenedTypeMembers || !flattenedTypeMembers.every(type => type.name && addToSeen(seen, getNameFromPropertyName(type.name) as string))) { + return undefined; + } + + addRange(result, flattenedTypeMembers); } return result; } @@ -94,7 +95,7 @@ namespace ts.refactor { return flattenTypeLiteralNodeReference(checker, node.type); } else if (isTypeLiteralNode(node)) { - return [node]; + return node.members; } return undefined; } diff --git a/tests/cases/fourslash/refactorExtractType67.ts b/tests/cases/fourslash/refactorExtractType67.ts new file mode 100644 index 0000000000000..8fbcfe1a88143 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType67.ts @@ -0,0 +1,6 @@ +/// + +//// function foo(a: /*a*/{ a: number | string, b: string } & { b: string } & { d: boolean }/*b*/) { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract type", "Extract to interface") diff --git a/tests/cases/fourslash/refactorExtractType68.ts b/tests/cases/fourslash/refactorExtractType68.ts new file mode 100644 index 0000000000000..ce852d76efdb8 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractType68.ts @@ -0,0 +1,6 @@ +/// + +//// function foo(a: /*a*/{ a: number | string, b: string } & { b: number } & { d: boolean }/*b*/) { } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract type", "Extract to interface") \ No newline at end of file