feat(37782): 'declare method' quick fix for adding a private method

src/compiler/diagnosticMessages.json

@@ -5205,6 +5205,10 @@
         "category": "Message",
         "code": 90037
     },
+    "Declare private method '{0}'": {
+        "category": "Message",
+        "code": 90038
+    },
     "Declare a private field named '{0}'.": {
         "category": "Message",
         "code": 90053

src/services/codefixes/fixAddMissingMember.ts

@@ -13,19 +13,15 @@ namespace ts.codefix {
         errorCodes,
         getCodeActions(context) {
             const info = getInfo(context.sourceFile, context.span.start, context.program.getTypeChecker(), context.program);
-            if (!info) return undefined;
-
+            if (!info) {
+                return undefined;
+            }
             if (info.kind === InfoKind.Enum) {
                 const { token, parentDeclaration } = info;
                 const changes = textChanges.ChangeTracker.with(context, t => addEnumMemberDeclaration(t, context.program.getTypeChecker(), token, parentDeclaration));
                 return [createCodeFixAction(fixName, changes, [Diagnostics.Add_missing_enum_member_0, token.text], fixId, Diagnostics.Add_all_missing_members)];
             }
-            const { parentDeclaration, declSourceFile, inJs, makeStatic, token, call } = info;
-            const methodCodeAction = call && getActionForMethodDeclaration(context, declSourceFile, parentDeclaration, token, call, makeStatic, inJs);
-            const addMember = inJs && !isInterfaceDeclaration(parentDeclaration) ?
-                singleElementArray(getActionsForAddMissingMemberInJavascriptFile(context, declSourceFile, parentDeclaration, token, makeStatic)) :
-                getActionsForAddMissingMemberInTypeScriptFile(context, declSourceFile, parentDeclaration, token, makeStatic);
-            return concatenate(singleElementArray(methodCodeAction), addMember);
+            return concatenate(getActionsForMissingMethodDeclaration(context, info), getActionsForMissingMemberDeclaration(context, info));
         },
         fixIds: [fixId],
         getAllCodeActions: context => {
@@ -62,19 +58,18 @@ namespace ts.codefix {
                             return !!superInfos && superInfos.some(({ token }) => token.text === info.token.text);
                         })) continue;
 
-                        const { parentDeclaration, declSourceFile, inJs, makeStatic, token, call } = info;
-
+                        const { parentDeclaration, declSourceFile, modifierFlags, token, call, isJSFile } = info;
                         // Always prefer to add a method declaration if possible.
                         if (call && !isPrivateIdentifier(token)) {
-                            addMethodDeclaration(context, changes, declSourceFile, parentDeclaration, token, call, makeStatic, inJs);
+                            addMethodDeclaration(context, changes, call, token.text, modifierFlags & ModifierFlags.Static, parentDeclaration, declSourceFile, isJSFile);
                         }
                         else {
-                            if (inJs && !isInterfaceDeclaration(parentDeclaration)) {
-                                addMissingMemberInJs(changes, declSourceFile, parentDeclaration, token, makeStatic);
+                            if (isJSFile && !isInterfaceDeclaration(parentDeclaration)) {
+                                addMissingMemberInJs(changes, declSourceFile, parentDeclaration, token, !!(modifierFlags & ModifierFlags.Static));
                             }
                             else {
                                 const typeNode = getTypeNode(program.getTypeChecker(), parentDeclaration, token);
-                                addPropertyDeclaration(changes, declSourceFile, parentDeclaration, token.text, typeNode, makeStatic ? ModifierFlags.Static : 0);
+                                addPropertyDeclaration(changes, declSourceFile, parentDeclaration, token.text, typeNode, modifierFlags & ModifierFlags.Static);
                             }
                         }
                     }
@@ -104,12 +99,12 @@ namespace ts.codefix {
     }
     interface ClassOrInterfaceInfo {
         readonly kind: InfoKind.ClassOrInterface;
+        readonly call: CallExpression | undefined;
         readonly token: Identifier | PrivateIdentifier;
+        readonly modifierFlags: ModifierFlags;
         readonly parentDeclaration: ClassOrInterface;
-        readonly makeStatic: boolean;
         readonly declSourceFile: SourceFile;
-        readonly inJs: boolean;
-        readonly call: CallExpression | undefined;
+        readonly isJSFile: boolean;
     }
     type Info = EnumInfo | ClassOrInterfaceInfo;
 
@@ -144,9 +139,10 @@ namespace ts.codefix {
             }
 
             const declSourceFile = classOrInterface.getSourceFile();
-            const inJs = isSourceFileJS(declSourceFile);
+            const modifierFlags = (makeStatic ? ModifierFlags.Static : 0) | (startsWithUnderscore(token.text) ? ModifierFlags.Private : 0);
+            const isJSFile = isSourceFileJS(declSourceFile);
             const call = tryCast(parent.parent, isCallExpression);
-            return { kind: InfoKind.ClassOrInterface, token, parentDeclaration: classOrInterface, makeStatic, declSourceFile, inJs, call };
+            return { kind: InfoKind.ClassOrInterface, token, call, modifierFlags, parentDeclaration: classOrInterface, declSourceFile, isJSFile };
         }
 
         const enumDeclaration = find(symbol.declarations, isEnumDeclaration);
@@ -156,13 +152,22 @@ namespace ts.codefix {
         return undefined;
     }
 
-    function getActionsForAddMissingMemberInJavascriptFile(context: CodeFixContext, declSourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, token: Identifier | PrivateIdentifier, makeStatic: boolean): CodeFixAction | undefined {
-        const changes = textChanges.ChangeTracker.with(context, t => addMissingMemberInJs(t, declSourceFile, classDeclaration, token, makeStatic));
+    function getActionsForMissingMemberDeclaration(context: CodeFixContext, info: ClassOrInterfaceInfo): CodeFixAction[] | undefined {
+        return info.isJSFile ? singleElementArray(createActionForAddMissingMemberInJavascriptFile(context, info)) :
+            createActionsForAddMissingMemberInTypeScriptFile(context, info);
+    }
+
+    function createActionForAddMissingMemberInJavascriptFile(context: CodeFixContext, { parentDeclaration, declSourceFile, modifierFlags, token }: ClassOrInterfaceInfo): CodeFixAction | undefined {
+        if (isInterfaceDeclaration(parentDeclaration)) {
+            return undefined;
+        }
+
+        const changes = textChanges.ChangeTracker.with(context, t => addMissingMemberInJs(t, declSourceFile, parentDeclaration, token, !!(modifierFlags & ModifierFlags.Static)));
         if (changes.length === 0) {
             return undefined;
         }
 
-        const diagnostic = makeStatic ? Diagnostics.Initialize_static_property_0 :
+        const diagnostic = modifierFlags & ModifierFlags.Static ? Diagnostics.Initialize_static_property_0 :
             isPrivateIdentifier(token) ? Diagnostics.Declare_a_private_field_named_0 : Diagnostics.Initialize_property_0_in_the_constructor;
 
         return createCodeFixAction(fixName, changes, [diagnostic, token.text], fixId, Diagnostics.Add_all_missing_members);
@@ -209,18 +214,22 @@ namespace ts.codefix {
         return createStatement(createAssignment(createPropertyAccess(obj, propertyName), createIdentifier("undefined")));
     }
 
-    function getActionsForAddMissingMemberInTypeScriptFile(context: CodeFixContext, declSourceFile: SourceFile, classDeclaration: ClassOrInterface, token: Identifier | PrivateIdentifier, makeStatic: boolean): CodeFixAction[] | undefined {
-        const typeNode = getTypeNode(context.program.getTypeChecker(), classDeclaration, token);
-        const actions: CodeFixAction[] = [createAddPropertyDeclarationAction(context, declSourceFile, classDeclaration, token.text, typeNode, makeStatic ? ModifierFlags.Static : 0)];
-        if (makeStatic || isPrivateIdentifier(token)) {
+    function createActionsForAddMissingMemberInTypeScriptFile(context: CodeFixContext, { parentDeclaration, declSourceFile, modifierFlags, token }: ClassOrInterfaceInfo): CodeFixAction[] | undefined {
+        const memberName = token.text;
+        const isStatic = modifierFlags & ModifierFlags.Static;
+        const typeNode = getTypeNode(context.program.getTypeChecker(), parentDeclaration, token);
+        const addPropertyDeclarationChanges = (modifierFlags: ModifierFlags) => textChanges.ChangeTracker.with(context, t => addPropertyDeclaration(t, declSourceFile, parentDeclaration, memberName, typeNode, modifierFlags));
+
+        const actions = [createCodeFixAction(fixName, addPropertyDeclarationChanges(modifierFlags & ModifierFlags.Static), [isStatic ? Diagnostics.Declare_static_property_0 : Diagnostics.Declare_property_0, memberName], fixId, Diagnostics.Add_all_missing_members)];
+        if (isStatic || isPrivateIdentifier(token)) {
             return actions;
         }
 
-        if (startsWithUnderscore(token.text)) {
-            actions.unshift(createAddPropertyDeclarationAction(context, declSourceFile, classDeclaration, token.text, typeNode, ModifierFlags.Private));
+        if (modifierFlags & ModifierFlags.Private) {
+            actions.unshift(createCodeFixActionWithoutFixAll(fixName, addPropertyDeclarationChanges(ModifierFlags.Private), [Diagnostics.Declare_private_property_0, memberName]));
         }
 
-        actions.push(createAddIndexSignatureAction(context, declSourceFile, classDeclaration, token.text, typeNode));
+        actions.push(createAddIndexSignatureAction(context, declSourceFile, parentDeclaration, token.text, typeNode));
         return actions;
     }
 
@@ -239,14 +248,6 @@ namespace ts.codefix {
         return typeNode || createKeywordTypeNode(SyntaxKind.AnyKeyword);
     }
 
-    function createAddPropertyDeclarationAction(context: CodeFixContext, declSourceFile: SourceFile, classDeclaration: ClassOrInterface, tokenName: string, typeNode: TypeNode, modifierFlags: ModifierFlags): CodeFixAction {
-        const changes = textChanges.ChangeTracker.with(context, t => addPropertyDeclaration(t, declSourceFile, classDeclaration, tokenName, typeNode, modifierFlags));
-        if (modifierFlags & ModifierFlags.Private) {
-            return createCodeFixActionWithoutFixAll(fixName, changes, [Diagnostics.Declare_private_property_0, tokenName]);
-        }
-        return createCodeFixAction(fixName, changes, [modifierFlags & ModifierFlags.Static ? Diagnostics.Declare_static_property_0 : Diagnostics.Declare_property_0, tokenName], fixId, Diagnostics.Add_all_missing_members);
-    }
-
     function addPropertyDeclaration(changeTracker: textChanges.ChangeTracker, declSourceFile: SourceFile, classDeclaration: ClassOrInterface, tokenName: string, typeNode: TypeNode, modifierFlags: ModifierFlags): void {
         const property = createProperty(
             /*decorators*/ undefined,
@@ -297,41 +298,46 @@ namespace ts.codefix {
         return createCodeFixActionWithoutFixAll(fixName, changes, [Diagnostics.Add_index_signature_for_property_0, tokenName]);
     }
 
-    function getActionForMethodDeclaration(
-        context: CodeFixContext,
-        declSourceFile: SourceFile,
-        classDeclaration: ClassOrInterface,
-        token: Identifier | PrivateIdentifier,
-        callExpression: CallExpression,
-        makeStatic: boolean,
-        inJs: boolean
-    ): CodeFixAction | undefined {
+    function getActionsForMissingMethodDeclaration(context: CodeFixContext, info: ClassOrInterfaceInfo): CodeFixAction[] | undefined {
+        const { parentDeclaration, declSourceFile, modifierFlags, token, call, isJSFile } = info;
+        if (call === undefined) {
+            return undefined;
+        }
+
         // Private methods are not implemented yet.
-        if (isPrivateIdentifier(token)) { return undefined; }
-        const changes = textChanges.ChangeTracker.with(context, t => addMethodDeclaration(context, t, declSourceFile, classDeclaration, token, callExpression, makeStatic, inJs));
-        return createCodeFixAction(fixName, changes, [makeStatic ? Diagnostics.Declare_static_method_0 : Diagnostics.Declare_method_0, token.text], fixId, Diagnostics.Add_all_missing_members);
+        if (isPrivateIdentifier(token)) {
+            return undefined;
+        }
+
+        const methodName = token.text;
+        const addMethodDeclarationChanges = (modifierFlags: ModifierFlags) => textChanges.ChangeTracker.with(context, t => addMethodDeclaration(context, t, call, methodName, modifierFlags, parentDeclaration, declSourceFile, isJSFile));
+        const actions = [createCodeFixAction(fixName, addMethodDeclarationChanges(modifierFlags & ModifierFlags.Static), [modifierFlags & ModifierFlags.Static ? Diagnostics.Declare_static_method_0 : Diagnostics.Declare_method_0, methodName], fixId, Diagnostics.Add_all_missing_members)];
+        if (modifierFlags & ModifierFlags.Private) {
+            actions.unshift(createCodeFixActionWithoutFixAll(fixName, addMethodDeclarationChanges(ModifierFlags.Private), [Diagnostics.Declare_private_method_0, methodName]));
+        }
+        return actions;
     }
 
     function addMethodDeclaration(
         context: CodeFixContextBase,
-        changeTracker: textChanges.ChangeTracker,
-        declSourceFile: SourceFile,
-        typeDecl: ClassOrInterface,
-        token: Identifier,
+        changes: textChanges.ChangeTracker,
         callExpression: CallExpression,
-        makeStatic: boolean,
-        inJs: boolean
+        methodName: string,
+        modifierFlags: ModifierFlags,
+        parentDeclaration: ClassOrInterface,
+        sourceFile: SourceFile,
+        isJSFile: boolean
     ): void {
-        const importAdder = createImportAdder(declSourceFile, context.program, context.preferences, context.host);
-        const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic, typeDecl, importAdder);
-        const containingMethodDeclaration = getAncestor(callExpression, SyntaxKind.MethodDeclaration);
-        if (containingMethodDeclaration && containingMethodDeclaration.parent === typeDecl) {
-            changeTracker.insertNodeAfter(declSourceFile, containingMethodDeclaration, methodDeclaration);
+        const importAdder = createImportAdder(sourceFile, context.program, context.preferences, context.host);
+        const methodDeclaration = createMethodFromCallExpression(context, importAdder, callExpression, methodName, modifierFlags, parentDeclaration, isJSFile);
+        const containingMethodDeclaration = findAncestor(callExpression, n => isMethodDeclaration(n) || isConstructorDeclaration(n));
+        if (containingMethodDeclaration && containingMethodDeclaration.parent === parentDeclaration) {
+            changes.insertNodeAfter(sourceFile, containingMethodDeclaration, methodDeclaration);
         }
         else {
-            changeTracker.insertNodeAtClassStart(declSourceFile, typeDecl, methodDeclaration);
+            changes.insertNodeAtClassStart(sourceFile, parentDeclaration, methodDeclaration);
         }
-        importAdder.writeFixes(changeTracker);
+        importAdder.writeFixes(changes);
     }
 
     function addEnumMemberDeclaration(changes: textChanges.ChangeTracker, checker: TypeChecker, token: Identifier, enumDeclaration: EnumDeclaration) {

src/services/codefixes/helpers.ts

@@ -214,12 +214,12 @@ namespace ts.codefix {
 
     export function createMethodFromCallExpression(
         context: CodeFixContextBase,
+        importAdder: ImportAdder,
         call: CallExpression,
         methodName: string,
-        inJs: boolean,
-        makeStatic: boolean,
+        modifierFlags: ModifierFlags,
         contextNode: Node,
-        importAdder: ImportAdder
+        inJs: boolean
     ): MethodDeclaration {
         const body = !isInterfaceDeclaration(contextNode);
         const { typeArguments, arguments: args, parent } = call;
@@ -234,7 +234,7 @@ namespace ts.codefix {
         const returnType = (inJs || !contextualType) ? undefined : checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);
         return createMethod(
             /*decorators*/ undefined,
-            /*modifiers*/ makeStatic ? [createToken(SyntaxKind.StaticKeyword)] : undefined,
+            /*modifiers*/ modifierFlags ? createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined,
             /*asteriskToken*/ isYieldExpression(parent) ? createToken(SyntaxKind.AsteriskToken) : undefined,
             methodName,
             /*questionToken*/ undefined,

tests/cases/fourslash/codeFixAddMissingMember18_declarePrivateMethod.ts

@@ -0,0 +1,29 @@
+/// <reference path='fourslash.ts' />
+
+////class A {
+////    constructor() {
+////        this._foo();
+////    }
+////}
+
+verify.codeFixAvailable([
+    { description: "Declare private method '_foo'" },
+    { description: "Declare method '_foo'" },
+    { description: "Declare private property '_foo'" },
+    { description: "Declare property '_foo'" },
+    { description: "Add index signature for property '_foo'" }
+])
+
+verify.codeFix({
+    description: [ts.Diagnostics.Declare_private_method_0.message, "_foo"],
+    index: 0,
+    newFileContent:
+`class A {
+    constructor() {
+        this._foo();
+    }
+    private _foo() {
+        throw new Error("Method not implemented.");
+    }
+}`
+});

tests/cases/fourslash/codeFixAddMissingMember19_declarePrivateMethod.ts

@@ -0,0 +1,29 @@
+/// <reference path='fourslash.ts' />
+
+////class A {
+////    foo() {
+////        this._bar();
+////    }
+////}
+
+verify.codeFixAvailable([
+    { description: "Declare private method '_bar'" },
+    { description: "Declare method '_bar'" },
+    { description: "Declare private property '_bar'" },
+    { description: "Declare property '_bar'" },
+    { description: "Add index signature for property '_bar'" }
+])
+
+verify.codeFix({
+    description: [ts.Diagnostics.Declare_private_method_0.message, "_bar"],
+    index: 0,
+    newFileContent:
+`class A {
+    foo() {
+        this._bar();
+    }
+    private _bar() {
+        throw new Error("Method not implemented.");
+    }
+}`
+});

tests/cases/fourslash/codeFixUndeclaredMethod.ts

@@ -15,16 +15,16 @@ verify.codeFix({
     index: 0,
     newFileContent:
 `class A {
-    foo1(arg0: number, arg1: number, arg2: number) {
-        throw new Error("Method not implemented.");
-    }
     constructor() {
         this.foo1(1,2,3);
         // 7 type args
         this.foo2<1,2,3,4,5,6,7>();
         // 8 type args
         this.foo3<1,2,3,4,5,6,7,8>();
     }
+    foo1(arg0: number, arg1: number, arg2: number) {
+        throw new Error("Method not implemented.");
+    }
 }`,
     applyChanges: true,
 });
@@ -34,19 +34,19 @@ verify.codeFix({
     index: 0,
     newFileContent:
 `class A {
-    foo2<T, U, V, W, X, Y, Z>() {
-        throw new Error("Method not implemented.");
-    }
-    foo1(arg0: number, arg1: number, arg2: number) {
-        throw new Error("Method not implemented.");
-    }
     constructor() {
         this.foo1(1,2,3);
         // 7 type args
         this.foo2<1,2,3,4,5,6,7>();
         // 8 type args
         this.foo3<1,2,3,4,5,6,7,8>();
     }
+    foo2<T, U, V, W, X, Y, Z>() {
+        throw new Error("Method not implemented.");
+    }
+    foo1(arg0: number, arg1: number, arg2: number) {
+        throw new Error("Method not implemented.");
+    }
 }`,
     applyChanges: true,
 });
@@ -56,6 +56,13 @@ verify.codeFix({
     index: 0,
     newFileContent:
 `class A {
+    constructor() {
+        this.foo1(1,2,3);
+        // 7 type args
+        this.foo2<1,2,3,4,5,6,7>();
+        // 8 type args
+        this.foo3<1,2,3,4,5,6,7,8>();
+    }
     foo3<T0, T1, T2, T3, T4, T5, T6, T7>() {
         throw new Error("Method not implemented.");
     }
@@ -65,12 +72,5 @@ verify.codeFix({
     foo1(arg0: number, arg1: number, arg2: number) {
         throw new Error("Method not implemented.");
     }
-    constructor() {
-        this.foo1(1,2,3);
-        // 7 type args
-        this.foo2<1,2,3,4,5,6,7>();
-        // 8 type args
-        this.foo3<1,2,3,4,5,6,7,8>();
-    }
 }`
 });

tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts

@@ -1,10 +1,11 @@
 /// <reference path='fourslash.ts' />
 
-//// class A {[|
-////     |]constructor() {
+//// class A {
+////     constructor() {
 ////         this.foo1(() => 1, () => "2", () => false);
 ////         this.foo2((a: number) => a, (b: string) => b, (c: boolean) => c);
-////     }
+////     }[|
+////     |]
 //// }
 
 verify.codeFix({

tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts

@@ -1,11 +1,12 @@
 /// <reference path='fourslash.ts' />
 
-//// class A {[|
-////     |]constructor() {
+//// class A {
+////     constructor() {
 ////         this.foo1(null, {}, { a: 1, b: "2"});
 ////         const bar = this.foo2(null, {}, { a: 1, b: "2"});
 ////         const baz: number = this.foo3(null, {}, { a: 1, b: "2"});
-////     }
+////     }[|
+////     |]
 //// }
 
 verify.codeFix({