From 17f0dfbf9c621aa765268ef666b533a28f3b8618 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Mon, 27 Apr 2020 18:59:39 +0300 Subject: [PATCH] fix(38177): add auto-import for missing argument type in new functions/methods --- src/services/codefixes/helpers.ts | 28 +++++------ src/services/refactors/extractSymbol.ts | 5 +- tests/cases/fourslash/extract-method32.ts | 35 ++++++++++++++ tests/cases/fourslash/extract-method33.ts | 44 +++++++++++++++++ tests/cases/fourslash/extract-method34.ts | 54 +++++++++++++++++++++ tests/cases/fourslash/extract-method35.ts | 38 +++++++++++++++ tests/cases/fourslash/extract-method36.ts | 47 +++++++++++++++++++ tests/cases/fourslash/extract-method37.ts | 57 +++++++++++++++++++++++ 8 files changed, 294 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/extract-method32.ts create mode 100644 tests/cases/fourslash/extract-method33.ts create mode 100644 tests/cases/fourslash/extract-method34.ts create mode 100644 tests/cases/fourslash/extract-method35.ts create mode 100644 tests/cases/fourslash/extract-method36.ts create mode 100644 tests/cases/fourslash/extract-method37.ts diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index 7f14de02c61..51fe5b8fad2 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -226,18 +226,8 @@ namespace ts.codefix { const scriptTarget = getEmitScriptTarget(context.program.getCompilerOptions()); const checker = context.program.getTypeChecker(); const tracker = getNoopSymbolTrackerWithResolver(context); - const types = map(args, arg => { - const type = checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)); - const typeNode = checker.typeToTypeNode(type, contextNode, /*flags*/ undefined, tracker); - if (typeNode?.kind === SyntaxKind.ImportType) { - const importableReference = tryGetAutoImportableReferenceFromImportTypeNode(typeNode, type, scriptTarget); - if (importableReference) { - importSymbols(importAdder, importableReference.symbols); - return importableReference.typeReference; - } - } - return typeNode; - }); + const types = map(args, arg => + typeToAutoImportableTypeNode(checker, importAdder, checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)), contextNode, scriptTarget, /*flags*/ undefined, tracker)); const names = map(args, arg => isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) && isIdentifier(arg.name) ? arg.name.text : undefined); const contextualType = checker.getContextualType(call); @@ -255,6 +245,18 @@ namespace ts.codefix { body ? createStubbedMethodBody(context.preferences) : undefined); } + export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined { + const typeNode = checker.typeToTypeNode(type, contextNode, flags, tracker); + if (typeNode && isImportTypeNode(typeNode)) { + const importableReference = tryGetAutoImportableReferenceFromImportTypeNode(typeNode, type, scriptTarget); + if (importableReference) { + importSymbols(importAdder, importableReference.symbols); + return importableReference.typeReference; + } + } + return typeNode; + } + function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] { const parameters: ParameterDeclaration[] = []; for (let i = 0; i < argCount; i++) { @@ -456,7 +458,7 @@ namespace ts.codefix { return createQualifiedName(replaceFirstIdentifierOfEntityName(name.left, newIdentifier), name.right); } - function importSymbols(importAdder: ImportAdder, symbols: readonly Symbol[]) { + export function importSymbols(importAdder: ImportAdder, symbols: readonly Symbol[]) { symbols.forEach(s => importAdder.addImportFromExportedSymbol(s, /*usageIsTypeOnly*/ true)); } } diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 30dcaeaebfa..64e9bb6f3a2 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -719,6 +719,8 @@ namespace ts.refactor.extractSymbol { context: RefactorContext): RefactorEditInfo { const checker = context.program.getTypeChecker(); + const scriptTarget = getEmitScriptTarget(context.program.getCompilerOptions()); + const importAdder = codefix.createImportAdder(context.file, context.program, context.preferences, context.host); // Make a unique name for the extracted function const file = scope.getSourceFile(); @@ -737,7 +739,7 @@ namespace ts.refactor.extractSymbol { let type = checker.getTypeOfSymbolAtLocation(usage.symbol, usage.node); // Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {" type = checker.getBaseTypeOfLiteralType(type); - typeNode = checker.typeToTypeNode(type, scope, NodeBuilderFlags.NoTruncation); + typeNode = codefix.typeToAutoImportableTypeNode(checker, importAdder, type, scope, scriptTarget, NodeBuilderFlags.NoTruncation); } const paramDecl = createParameter( @@ -823,6 +825,7 @@ namespace ts.refactor.extractSymbol { else { changeTracker.insertNodeAtEndOfScope(context.file, scope, newFunction); } + importAdder.writeFixes(changeTracker); const newNodes: Node[] = []; // replace range with function call diff --git a/tests/cases/fourslash/extract-method32.ts b/tests/cases/fourslash/extract-method32.ts new file mode 100644 index 00000000000..229561e9795 --- /dev/null +++ b/tests/cases/fourslash/extract-method32.ts @@ -0,0 +1,35 @@ +/// + +// @Filename: /a.ts +////export interface A { +//// x: number; +////} +////export const a: A = { x: 1 }; + +// @Filename: /b.ts +////import { a } from "./a"; +//// +////function foo() { +//// const arg = a; +//// /*a*/console.log(arg);/*b*/ +////} + +goTo.file("/b.ts"); +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to function in module scope", + newContent: +`import { a, A } from "./a"; + +function foo() { + const arg = a; + /*RENAME*/newFunction(arg); +} + +function newFunction(arg: A) { + console.log(arg); +} +` +}); diff --git a/tests/cases/fourslash/extract-method33.ts b/tests/cases/fourslash/extract-method33.ts new file mode 100644 index 00000000000..96d4216c844 --- /dev/null +++ b/tests/cases/fourslash/extract-method33.ts @@ -0,0 +1,44 @@ +/// + +// @Filename: /a.ts +////export interface A { +//// x: number; +////} + +// @Filename: /b.ts +////import { A } from "./a"; +////export interface B { +//// payload: T; +////} +////export const b: B = { +//// payload: { x: 1 } +////} + +// @Filename: /c.ts +////import { b } from "./b"; +//// +////function foo() { +//// const prop = b; +//// /*a*/console.log(prop);/*b*/ +////} + +goTo.file("/c.ts"); +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to function in module scope", + newContent: +`import { b, B } from "./b"; +import { A } from "./a"; + +function foo() { + const prop = b; + /*RENAME*/newFunction(prop); +} + +function newFunction(prop: B) { + console.log(prop); +} +` +}); diff --git a/tests/cases/fourslash/extract-method34.ts b/tests/cases/fourslash/extract-method34.ts new file mode 100644 index 00000000000..7ffb72e7af5 --- /dev/null +++ b/tests/cases/fourslash/extract-method34.ts @@ -0,0 +1,54 @@ +/// + +// @Filename: /a.ts +////export interface A { +//// x: number; +////} + +// @Filename: /b.ts +////export interface B { +//// payload: T; +////} + +// @Filename: /c.ts +////import { A } from "./a"; +////import { B } from "./b"; +////export interface C { +//// payload: T; +////} +//// +////export const c: C> = { +//// payload: { +//// payload: { x: 1 } +//// } +////} + +// @Filename: /d.ts +////import { c } from "./c"; +//// +////function foo() { +//// const prop = c; +//// /*a*/console.log(prop);/*b*/ +////} + +goTo.file("/c.ts"); +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to function in module scope", + newContent: +`import { c, C } from "./c"; +import { B } from "./b"; +import { A } from "./a"; + +function foo() { + const prop = c; + /*RENAME*/newFunction(prop); +} + +function newFunction(prop: C>) { + console.log(prop); +} +` +}); diff --git a/tests/cases/fourslash/extract-method35.ts b/tests/cases/fourslash/extract-method35.ts new file mode 100644 index 00000000000..67a3edd0b4a --- /dev/null +++ b/tests/cases/fourslash/extract-method35.ts @@ -0,0 +1,38 @@ +/// + +// @Filename: /a.ts +////export interface A { +//// x: number; +////} +////export const a: A = { x: 1 }; + +// @Filename: /b.ts +////import { a } from "./a"; +//// +////class Foo { +//// foo() { +//// const arg = a; +//// /*a*/console.log(arg);/*b*/ +//// } +////} + +goTo.file("/b.ts"); +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to method in class 'Foo'", + newContent: +`import { a, A } from "./a"; + +class Foo { + foo() { + const arg = a; + this./*RENAME*/newMethod(arg); + } + + private newMethod(arg: A) { + console.log(arg); + } +}` +}); diff --git a/tests/cases/fourslash/extract-method36.ts b/tests/cases/fourslash/extract-method36.ts new file mode 100644 index 00000000000..0b3e93e2b95 --- /dev/null +++ b/tests/cases/fourslash/extract-method36.ts @@ -0,0 +1,47 @@ +/// + +// @Filename: /a.ts +////export interface A { +//// x: number; +////} + +// @Filename: /b.ts +////import { A } from "./a"; +////export interface B { +//// payload: T; +////} +////export const b: B = { +//// payload: { x: 1 } +////} + +// @Filename: /c.ts +////import { b } from "./b"; +//// +////class Foo { +//// foo() { +//// const arg = b; +//// /*a*/console.log(arg);/*b*/ +//// } +////} + +goTo.file("/c.ts"); +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to method in class 'Foo'", + newContent: +`import { b, B } from "./b"; +import { A } from "./a"; + +class Foo { + foo() { + const arg = b; + this./*RENAME*/newMethod(arg); + } + + private newMethod(arg: B) { + console.log(arg); + } +}` +}); diff --git a/tests/cases/fourslash/extract-method37.ts b/tests/cases/fourslash/extract-method37.ts new file mode 100644 index 00000000000..1f71cd09818 --- /dev/null +++ b/tests/cases/fourslash/extract-method37.ts @@ -0,0 +1,57 @@ +/// + +// @Filename: /a.ts +////export interface A { +//// x: number; +////} + +// @Filename: /b.ts +////export interface B { +//// payload: T; +////} + +// @Filename: /c.ts +////import { A } from "./a"; +////import { B } from "./b"; +////export interface C { +//// payload: T; +////} +//// +////export const c: C> = { +//// payload: { +//// payload: { x: 1 } +//// } +////} + +// @Filename: /d.ts +////import { c } from "./c"; +//// +////class Foo { +//// foo() { +//// const arg = c; +//// /*a*/console.log(arg);/*b*/ +//// } +////} + +goTo.file("/d.ts"); +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to method in class 'Foo'", + newContent: +`import { c, C } from "./c"; +import { B } from "./b"; +import { A } from "./a"; + +class Foo { + foo() { + const arg = c; + this./*RENAME*/newMethod(arg); + } + + private newMethod(arg: C>) { + console.log(arg); + } +}` +});