From 627c95e3f5fca9499c174655abc1292e8a305a11 Mon Sep 17 00:00:00 2001 From: Markus Johnsson Date: Wed, 28 Feb 2018 08:55:07 +0100 Subject: [PATCH 1/5] Infer parameter names and types when applying Delcare Method codefix (#22180) --- src/services/codefixes/fixAddMissingMember.ts | 10 +++---- src/services/codefixes/helpers.ts | 21 +++++++++++---- .../fourslash/codeFixAddMissingMember9.ts | 24 +++++++++++++++++ .../codeFixUndeclaredAcrossFiles1.ts | 7 ++++- .../codeFixUndeclaredAcrossFiles3.ts | 26 +++++++++++++++++++ .../codeFixUndeclaredInStaticMethod.ts | 14 +++++----- .../fourslash/codeFixUndeclaredMethod.ts | 6 ++--- 7 files changed, 87 insertions(+), 21 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingMember9.ts create mode 100644 tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index 7dbbc39843f..2adbfd1dd85 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -31,7 +31,7 @@ namespace ts.codefix { // Always prefer to add a method declaration if possible. if (call) { - addMethodDeclaration(changes, classDeclarationSourceFile, classDeclaration, token, call, makeStatic, inJs); + addMethodDeclaration(context, changes, classDeclarationSourceFile, classDeclaration, token, call, makeStatic, inJs); } else { if (inJs) { @@ -181,14 +181,14 @@ namespace ts.codefix { return { description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Add_index_signature_for_property_0), [tokenName]), changes, fixId: undefined }; } - function getActionForMethodDeclaration(context: CodeFixContext, classDeclarationSourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, token: Identifier, callExpression: CallExpression, makeStatic: boolean, inJs: boolean): CodeFixAction | undefined { + function getActionForMethodDeclaration(context: CodeFixContextBase, classDeclarationSourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, token: Identifier, callExpression: CallExpression, makeStatic: boolean, inJs: boolean): CodeFixAction | undefined { const description = formatStringFromArgs(getLocaleSpecificMessage(makeStatic ? Diagnostics.Declare_static_method_0 : Diagnostics.Declare_method_0), [token.text]); - const changes = textChanges.ChangeTracker.with(context, t => addMethodDeclaration(t, classDeclarationSourceFile, classDeclaration, token, callExpression, makeStatic, inJs)); + const changes = textChanges.ChangeTracker.with(context, t => addMethodDeclaration(context, t, classDeclarationSourceFile, classDeclaration, token, callExpression, makeStatic, inJs)); return { description, changes, fixId }; } - function addMethodDeclaration(changeTracker: textChanges.ChangeTracker, classDeclarationSourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, token: Identifier, callExpression: CallExpression, makeStatic: boolean, inJs: boolean) { - const methodDeclaration = createMethodFromCallExpression(callExpression, token.text, inJs, makeStatic); + function addMethodDeclaration(context: CodeFixContextBase, changeTracker: textChanges.ChangeTracker, classDeclarationSourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, token: Identifier, callExpression: CallExpression, makeStatic: boolean, inJs: boolean) { + const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic); changeTracker.insertNodeAtClassStart(classDeclarationSourceFile, classDeclaration, methodDeclaration); } } diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index 6a45c66ca81..f746a08561c 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -107,7 +107,18 @@ namespace ts.codefix { return nodes && createNodeArray(nodes.map(getSynthesizedDeepClone)); } - export function createMethodFromCallExpression({ typeArguments, arguments: args }: CallExpression, methodName: string, inJs: boolean, makeStatic: boolean): MethodDeclaration { + export function createMethodFromCallExpression(context: CodeFixContextBase, { typeArguments, arguments: args }: CallExpression, methodName: string, inJs: boolean, makeStatic: boolean): MethodDeclaration { + const checker = context.program.getTypeChecker(); + const types = map(args, + (arg) => { + let type = checker.getTypeAtLocation(arg); + // Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {" + type = checker.getBaseTypeOfLiteralType(type); + return checker.typeToTypeNode(type); + }); + const names = map(args, (arg) => + isIdentifier(arg) ? arg.text : + isPropertyAccessExpression(arg) ? arg.name.text : undefined); return createMethod( /*decorators*/ undefined, /*modifiers*/ makeStatic ? [createToken(SyntaxKind.StaticKeyword)] : undefined, @@ -116,12 +127,12 @@ namespace ts.codefix { /*questionToken*/ undefined, /*typeParameters*/ inJs ? undefined : map(typeArguments, (_, i) => createTypeParameterDeclaration(CharacterCodes.T + typeArguments.length - 1 <= CharacterCodes.Z ? String.fromCharCode(CharacterCodes.T + i) : `T${i}`)), - /*parameters*/ createDummyParameters(args.length, /*names*/ undefined, /*minArgumentCount*/ undefined, inJs), + /*parameters*/ createDummyParameters(args.length, names, types, /*minArgumentCount*/ undefined, inJs), /*type*/ inJs ? undefined : createKeywordTypeNode(SyntaxKind.AnyKeyword), createStubbedMethodBody()); } - function createDummyParameters(argCount: number, names: string[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] { + function createDummyParameters(argCount: number, names: string[] | undefined, types: TypeNode[], minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] { const parameters: ParameterDeclaration[] = []; for (let i = 0; i < argCount; i++) { const newParameter = createParameter( @@ -130,7 +141,7 @@ namespace ts.codefix { /*dotDotDotToken*/ undefined, /*name*/ names && names[i] || `arg${i}`, /*questionToken*/ minArgumentCount !== undefined && i >= minArgumentCount ? createToken(SyntaxKind.QuestionToken) : undefined, - /*type*/ inJs ? undefined : createKeywordTypeNode(SyntaxKind.AnyKeyword), + /*type*/ inJs ? undefined : types && types[i] || createKeywordTypeNode(SyntaxKind.AnyKeyword), /*initializer*/ undefined); parameters.push(newParameter); } @@ -157,7 +168,7 @@ namespace ts.codefix { const maxNonRestArgs = maxArgsSignature.parameters.length - (maxArgsSignature.hasRestParameter ? 1 : 0); const maxArgsParameterSymbolNames = maxArgsSignature.parameters.map(symbol => symbol.name); - const parameters = createDummyParameters(maxNonRestArgs, maxArgsParameterSymbolNames, minArgumentCount, /*inJs*/ false); + const parameters = createDummyParameters(maxNonRestArgs, maxArgsParameterSymbolNames, /* types */ undefined, minArgumentCount, /*inJs*/ false); if (someSigHasRestParameter) { const anyArrayType = createArrayTypeNode(createKeywordTypeNode(SyntaxKind.AnyKeyword)); diff --git a/tests/cases/fourslash/codeFixAddMissingMember9.ts b/tests/cases/fourslash/codeFixAddMissingMember9.ts new file mode 100644 index 00000000000..afe401f350b --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingMember9.ts @@ -0,0 +1,24 @@ +/// + +////class C { +//// z: boolean = true; +//// method() { +//// const x = 0; +//// this.y(x, "a", this.z); +//// } +////} + +verify.codeFixAll({ + fixId: "addMissingMember", + newFileContent: +`class C { + y(x: number, arg1: string, z: boolean): any { + throw new Error("Method not implemented."); + } + z: boolean = true; + method() { + const x = 0; + this.y(x, "a", this.z); + } +}`, +}); diff --git a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts index a946b60f173..5d494e877e3 100644 --- a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts +++ b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles1.ts @@ -10,6 +10,7 @@ //// let c = new X.C; //// c.m1(); //// c.y = {}; +//// c.m2(c); // @Filename: f1.ts //// export class C {[| @@ -21,14 +22,18 @@ verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); +verify.getAndApplyCodeFix(/*errorCode*/undefined, 0); verify.rangeIs(` + m2(c: C): any { + throw new Error("Method not implemented."); + } y: {}; m1(): any { throw new Error("Method not implemented."); } static x: any; - static m0(arg0: any, arg1: any, arg2: any): any { + static m0(arg0: number, arg1: string, arg2: undefined[]): any { throw new Error("Method not implemented."); } `); \ No newline at end of file diff --git a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts new file mode 100644 index 00000000000..2b07493f923 --- /dev/null +++ b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts @@ -0,0 +1,26 @@ +/// + +// @allowJs: true +// @checkJs: true + +// @Filename: f3.ts +//// import { C } from "./f1"; +//// import { D } from "./f2"; +//// const c = new C(); +//// c.m0(new D()); + +// @Filename: f2.ts +//// export class D { } + +// @Filename: f1.ts +//// export class C {[| +//// |]x: number; +//// } + +verify.getAndApplyCodeFix(/*errorCode*/ undefined, 0); + +verify.rangeIs(` + m0(arg0: D): any { + throw new Error("Method not implemented."); + } +`); \ No newline at end of file diff --git a/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts b/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts index 0bd4ac3a879..4552f204df1 100644 --- a/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts +++ b/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts @@ -13,7 +13,7 @@ verify.codeFix({ description: "Declare static method 'm1'", index: 0, newRangeContent: ` - static m1(arg0: any, arg1: any, arg2: any): any { + static m1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } `, @@ -23,10 +23,10 @@ verify.codeFix({ description: "Declare static method 'm2'", index: 0, newRangeContent: ` - static m2(arg0: any, arg1: any): any { + static m2(arg0: number, arg1: number): any { throw new Error("Method not implemented."); } - static m1(arg0: any, arg1: any, arg2: any): any { + static m1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } `, @@ -37,10 +37,10 @@ verify.codeFix({ index: 0, newRangeContent: ` static prop1: number; - static m2(arg0: any, arg1: any): any { + static m2(arg0: number, arg1: number): any { throw new Error("Method not implemented."); } - static m1(arg0: any, arg1: any, arg2: any): any { + static m1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } `, @@ -52,10 +52,10 @@ verify.codeFix({ newRangeContent: ` static prop2: string; static prop1: number; - static m2(arg0: any, arg1: any): any { + static m2(arg0: number, arg1: number): any { throw new Error("Method not implemented."); } - static m1(arg0: any, arg1: any, arg2: any): any { + static m1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } `, diff --git a/tests/cases/fourslash/codeFixUndeclaredMethod.ts b/tests/cases/fourslash/codeFixUndeclaredMethod.ts index 70ed58b995b..795b7052a68 100644 --- a/tests/cases/fourslash/codeFixUndeclaredMethod.ts +++ b/tests/cases/fourslash/codeFixUndeclaredMethod.ts @@ -14,7 +14,7 @@ verify.codeFix({ description: "Declare method 'foo1'", index: 0, newRangeContent: ` - foo1(arg0: any, arg1: any, arg2: any): any { + foo1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } `, @@ -27,7 +27,7 @@ verify.codeFix({ foo2(): any { throw new Error("Method not implemented."); } - foo1(arg0: any, arg1: any, arg2: any): any { + foo1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } ` @@ -43,7 +43,7 @@ verify.codeFix({ foo2(): any { throw new Error("Method not implemented."); } - foo1(arg0: any, arg1: any, arg2: any): any { + foo1(arg0: number, arg1: number, arg2: number): any { throw new Error("Method not implemented."); } ` From 67eb6fc85920b8443bc8138eccd39224462930fc Mon Sep 17 00:00:00 2001 From: Markus Johnsson Date: Wed, 28 Feb 2018 12:07:41 +0100 Subject: [PATCH 2/5] Added more tests for #22180 --- .../codeFixUndeclaredMethodFunctionArgs.ts | 31 +++++++++++++++++++ ...odeFixUndeclaredMethodObjectLiteralArgs.ts | 17 ++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts create mode 100644 tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts diff --git a/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts b/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts new file mode 100644 index 00000000000..1c06cfc3afd --- /dev/null +++ b/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts @@ -0,0 +1,31 @@ +/// + +//// class A {[| +//// |]constructor() { +//// this.foo1(() => 1, () => "2", () => false); +//// this.foo2((a: number) => a, (b: string) => b, (c: boolean) => c); +//// } +//// } + +verify.codeFix({ + description: "Declare method 'foo1'", + index: 0, + newRangeContent: ` + foo1(arg0: () => number, arg1: () => string, arg2: () => boolean): any { + throw new Error("Method not implemented."); + } + `, +}); + +verify.codeFix({ + description: "Declare method 'foo2'", + index: 0, + newRangeContent: ` + foo2(arg0: (a: number) => number, arg1: (b: string) => string, arg2: (c: boolean) => boolean): any { + throw new Error("Method not implemented."); + } + foo1(arg0: () => number, arg1: () => string, arg2: () => boolean): any { + throw new Error("Method not implemented."); + } + ` +}); diff --git a/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts b/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts new file mode 100644 index 00000000000..bd434cf9d9d --- /dev/null +++ b/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts @@ -0,0 +1,17 @@ +/// + +//// class A {[| +//// |]constructor() { +//// this.foo1(null, {}, { a: 1, b: "2"}); +//// } +//// } + +verify.codeFix({ + description: "Declare method 'foo1'", + index: 0, + newRangeContent: ` + foo1(arg0: null, arg1: {}, arg2: { a: number; b: string; }): any { + throw new Error("Method not implemented."); + } + ` +}); From 1e058cd1d7cce51283b4e60bd21475daf2a0140b Mon Sep 17 00:00:00 2001 From: Markus Johnsson Date: Sat, 3 Mar 2018 06:58:47 +0100 Subject: [PATCH 3/5] Remove parens from single parameter arrow functions --- src/services/codefixes/helpers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index f746a08561c..e09df4b379a 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -110,13 +110,13 @@ namespace ts.codefix { export function createMethodFromCallExpression(context: CodeFixContextBase, { typeArguments, arguments: args }: CallExpression, methodName: string, inJs: boolean, makeStatic: boolean): MethodDeclaration { const checker = context.program.getTypeChecker(); const types = map(args, - (arg) => { + arg => { let type = checker.getTypeAtLocation(arg); // Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {" type = checker.getBaseTypeOfLiteralType(type); return checker.typeToTypeNode(type); }); - const names = map(args, (arg) => + const names = map(args, arg => isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) ? arg.name.text : undefined); return createMethod( From 8bc92e4a6363804436dba0dc37b33b1d862019bd Mon Sep 17 00:00:00 2001 From: Markus Johnsson Date: Sun, 10 Jun 2018 19:58:32 +0200 Subject: [PATCH 4/5] Update test to reflect new behavior --- tests/cases/fourslash/codeFixAddMissingMember9.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/cases/fourslash/codeFixAddMissingMember9.ts b/tests/cases/fourslash/codeFixAddMissingMember9.ts index afe401f350b..b583cd28e9a 100644 --- a/tests/cases/fourslash/codeFixAddMissingMember9.ts +++ b/tests/cases/fourslash/codeFixAddMissingMember9.ts @@ -10,15 +10,16 @@ verify.codeFixAll({ fixId: "addMissingMember", + fixAllDescription: "Add all missing members", newFileContent: `class C { - y(x: number, arg1: string, z: boolean): any { - throw new Error("Method not implemented."); - } z: boolean = true; method() { const x = 0; this.y(x, "a", this.z); } + y(x: number, arg1: string, z: boolean): any { + throw new Error("Method not implemented."); + } }`, }); From f0c52a454819528a1e019f96131a94b1079f5c0a Mon Sep 17 00:00:00 2001 From: Markus Johnsson Date: Tue, 19 Jun 2018 13:21:07 +0200 Subject: [PATCH 5/5] Accept baseline --- tests/baselines/reference/api/tsserverlibrary.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index a9fe9a32556..eaac2305bc7 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -11625,7 +11625,7 @@ declare namespace ts.codefix { * @returns Empty string iff there are no member insertions. */ function createMissingMemberNodes(classDeclaration: ClassLikeDeclaration, possiblyMissingSymbols: ReadonlyArray, checker: TypeChecker, preferences: UserPreferences, out: (node: ClassElement) => void): void; - function createMethodFromCallExpression({ typeArguments, arguments: args, parent: parent }: CallExpression, methodName: string, inJs: boolean, makeStatic: boolean, preferences: UserPreferences): MethodDeclaration; + function createMethodFromCallExpression(context: CodeFixContextBase, { typeArguments, arguments: args, parent: parent }: CallExpression, methodName: string, inJs: boolean, makeStatic: boolean, preferences: UserPreferences): MethodDeclaration; } declare namespace ts.codefix { }