From 1a7d4b34ba0959629ffa44a5198b424074fb37b6 Mon Sep 17 00:00:00 2001 From: Vyacheslav Pukhanov Date: Sun, 27 May 2018 23:36:25 +0300 Subject: [PATCH 1/3] addMethodDeclaration: add after quickfix location if possible (#22674) --- src/services/codefixes/fixAddMissingMember.ts | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index 2eafb5cdd6f..ccf5829f363 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -199,6 +199,34 @@ namespace ts.codefix { preferences: UserPreferences, ): void { const methodDeclaration = createMethodFromCallExpression(callExpression, token.text, inJs, makeStatic, preferences); - changeTracker.insertNodeAtClassStart(classDeclarationSourceFile, classDeclaration, methodDeclaration); + const currentMethod = getNodeToInsertMethodAfter(classDeclaration, callExpression); + + if (currentMethod) { + changeTracker.insertNodeAfter(classDeclarationSourceFile, currentMethod, methodDeclaration); + } + else { + changeTracker.insertNodeAtClassStart(classDeclarationSourceFile, classDeclaration, methodDeclaration); + } + } + + // Gets the MethodDeclaration of a method of the cls class that contains the node, or undefined if the node is not in a method or that method is not in the cls class. + function getNodeToInsertMethodAfter(cls: ClassLikeDeclaration, node: Node): MethodDeclaration | undefined { + const nodeMethod = getParentMethodDeclaration(node); + if (nodeMethod) { + const isClsMethod = contains(cls.members, nodeMethod); + if (isClsMethod) { + return nodeMethod; + } + } + return undefined; + } + + // Gets the MethodDeclaration of the method that contains the node, or undefined if the node is not in a method. + function getParentMethodDeclaration(node: Node): MethodDeclaration | undefined { + while (node) { + if (isMethodDeclaration(node)) break; + node = node.parent; + } + return node; } } From 1bc5977e3ad060c8fa8ec9a02dab6b1b0cbfec25 Mon Sep 17 00:00:00 2001 From: Vyacheslav Pukhanov Date: Mon, 28 May 2018 00:09:47 +0300 Subject: [PATCH 2/3] Update undeclared method quickfix tests --- .../fourslash/codeFixAddMissingMember_all.ts | 6 +-- .../codeFixAddMissingMember_all_js.ts | 6 +-- .../codeFixUndeclaredInStaticMethod.ts | 42 +++++++++---------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/cases/fourslash/codeFixAddMissingMember_all.ts b/tests/cases/fourslash/codeFixAddMissingMember_all.ts index 5a2f8606f2d..021968a8c6a 100644 --- a/tests/cases/fourslash/codeFixAddMissingMember_all.ts +++ b/tests/cases/fourslash/codeFixAddMissingMember_all.ts @@ -14,13 +14,13 @@ verify.codeFixAll({ newFileContent: `class C { x: number; - y(): any { - throw new Error("Method not implemented."); - } method() { this.x = 0; this.y(); this.x = ""; } + y(): any { + throw new Error("Method not implemented."); + } }`, }); diff --git a/tests/cases/fourslash/codeFixAddMissingMember_all_js.ts b/tests/cases/fourslash/codeFixAddMissingMember_all_js.ts index 5e5a2895fdc..7c900be76f3 100644 --- a/tests/cases/fourslash/codeFixAddMissingMember_all_js.ts +++ b/tests/cases/fourslash/codeFixAddMissingMember_all_js.ts @@ -18,9 +18,6 @@ verify.codeFixAll({ fixAllDescription: "Add all missing members", newFileContent: `class C { - y() { - throw new Error("Method not implemented."); - } constructor() { this.x = undefined; } @@ -29,5 +26,8 @@ verify.codeFixAll({ this.y(); this.x; } + y() { + throw new Error("Method not implemented."); + } }`, }); diff --git a/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts b/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts index 60693a22fca..9faa15745fc 100644 --- a/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts +++ b/tests/cases/fourslash/codeFixUndeclaredInStaticMethod.ts @@ -14,15 +14,15 @@ verify.codeFix({ index: 0, newFileContent: `class A { - static m1(arg0: any, arg1: any, arg2: any): any { - throw new Error("Method not implemented."); - } static foo0() { this.m1(1,2,3); A.m2(1,2); this.prop1 = 10; A.prop2 = "asdf"; } + static m1(arg0: any, arg1: any, arg2: any): any { + throw new Error("Method not implemented."); + } }`, }); @@ -31,18 +31,18 @@ verify.codeFix({ index: 0, newFileContent: `class A { - static m2(arg0: any, arg1: any): any { - throw new Error("Method not implemented."); - } - static m1(arg0: any, arg1: any, arg2: any): any { - throw new Error("Method not implemented."); - } static foo0() { this.m1(1,2,3); A.m2(1,2); this.prop1 = 10; A.prop2 = "asdf"; } + static m2(arg0: any, arg1: any): any { + throw new Error("Method not implemented."); + } + static m1(arg0: any, arg1: any, arg2: any): any { + throw new Error("Method not implemented."); + } }`, }); @@ -52,18 +52,18 @@ verify.codeFix({ newFileContent: `class A { static prop1: number; - static m2(arg0: any, arg1: any): any { - throw new Error("Method not implemented."); - } - static m1(arg0: any, arg1: any, arg2: any): any { - throw new Error("Method not implemented."); - } static foo0() { this.m1(1,2,3); A.m2(1,2); this.prop1 = 10; A.prop2 = "asdf"; } + static m2(arg0: any, arg1: any): any { + throw new Error("Method not implemented."); + } + static m1(arg0: any, arg1: any, arg2: any): any { + throw new Error("Method not implemented."); + } }`, }); @@ -74,17 +74,17 @@ verify.codeFix({ `class A { static prop1: number; static prop2: string; - static m2(arg0: any, arg1: any): any { - throw new Error("Method not implemented."); - } - static m1(arg0: any, arg1: any, arg2: any): any { - throw new Error("Method not implemented."); - } static foo0() { this.m1(1,2,3); A.m2(1,2); this.prop1 = 10; A.prop2 = "asdf"; } + static m2(arg0: any, arg1: any): any { + throw new Error("Method not implemented."); + } + static m1(arg0: any, arg1: any, arg2: any): any { + throw new Error("Method not implemented."); + } }`, }); From 2ea66a6dbcb3f90790a1de07f15c3ea124c484b8 Mon Sep 17 00:00:00 2001 From: Vyacheslav Pukhanov Date: Wed, 30 May 2018 16:58:23 +0300 Subject: [PATCH 3/3] Refactor and inline getNodeToInsertMethodAfter --- src/services/codefixes/fixAddMissingMember.ts | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index ccf5829f363..53492211042 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -199,34 +199,13 @@ namespace ts.codefix { preferences: UserPreferences, ): void { const methodDeclaration = createMethodFromCallExpression(callExpression, token.text, inJs, makeStatic, preferences); - const currentMethod = getNodeToInsertMethodAfter(classDeclaration, callExpression); + const containingMethodDeclaration = getAncestor(callExpression, SyntaxKind.MethodDeclaration); - if (currentMethod) { - changeTracker.insertNodeAfter(classDeclarationSourceFile, currentMethod, methodDeclaration); + if (containingMethodDeclaration && containingMethodDeclaration.parent === classDeclaration) { + changeTracker.insertNodeAfter(classDeclarationSourceFile, containingMethodDeclaration, methodDeclaration); } else { changeTracker.insertNodeAtClassStart(classDeclarationSourceFile, classDeclaration, methodDeclaration); } } - - // Gets the MethodDeclaration of a method of the cls class that contains the node, or undefined if the node is not in a method or that method is not in the cls class. - function getNodeToInsertMethodAfter(cls: ClassLikeDeclaration, node: Node): MethodDeclaration | undefined { - const nodeMethod = getParentMethodDeclaration(node); - if (nodeMethod) { - const isClsMethod = contains(cls.members, nodeMethod); - if (isClsMethod) { - return nodeMethod; - } - } - return undefined; - } - - // Gets the MethodDeclaration of the method that contains the node, or undefined if the node is not in a method. - function getParentMethodDeclaration(node: Node): MethodDeclaration | undefined { - while (node) { - if (isMethodDeclaration(node)) break; - node = node.parent; - } - return node; - } }