Skip to content

Commit 683b3ec

Browse files
a-tarasyuksandersn
a-tarasyuk
and
sandersn
authored
feat(37782): 'declare method' quick fix for adding a private method (microsoft#37806)
* feat(37782): add quick-fix action to declare a private method for names that start from underscore * better merge order in messages json Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
1 parent e66ce87 commit 683b3ec

8 files changed

+158
-88
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5205,6 +5205,10 @@
52055205
"category": "Message",
52065206
"code": 90037
52075207
},
5208+
"Declare private method '{0}'": {
5209+
"category": "Message",
5210+
"code": 90038
5211+
},
52085212
"Declare a private field named '{0}'.": {
52095213
"category": "Message",
52105214
"code": 90053

‎src/services/codefixes/fixAddMissingMember.ts

Lines changed: 68 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,15 @@ namespace ts.codefix {
1313
errorCodes,
1414
getCodeActions(context) {
1515
const info = getInfo(context.sourceFile, context.span.start, context.program.getTypeChecker(), context.program);
16-
if (!info) return undefined;
17-
16+
if (!info) {
17+
return undefined;
18+
}
1819
if (info.kind === InfoKind.Enum) {
1920
const { token, parentDeclaration } = info;
2021
const changes = textChanges.ChangeTracker.with(context, t => addEnumMemberDeclaration(t, context.program.getTypeChecker(), token, parentDeclaration));
2122
return [createCodeFixAction(fixName, changes, [Diagnostics.Add_missing_enum_member_0, token.text], fixId, Diagnostics.Add_all_missing_members)];
2223
}
23-
const { parentDeclaration, declSourceFile, inJs, makeStatic, token, call } = info;
24-
const methodCodeAction = call && getActionForMethodDeclaration(context, declSourceFile, parentDeclaration, token, call, makeStatic, inJs);
25-
const addMember = inJs && !isInterfaceDeclaration(parentDeclaration) ?
26-
singleElementArray(getActionsForAddMissingMemberInJavascriptFile(context, declSourceFile, parentDeclaration, token, makeStatic)) :
27-
getActionsForAddMissingMemberInTypeScriptFile(context, declSourceFile, parentDeclaration, token, makeStatic);
28-
return concatenate(singleElementArray(methodCodeAction), addMember);
24+
return concatenate(getActionsForMissingMethodDeclaration(context, info), getActionsForMissingMemberDeclaration(context, info));
2925
},
3026
fixIds: [fixId],
3127
getAllCodeActions: context => {
@@ -62,19 +58,18 @@ namespace ts.codefix {
6258
return !!superInfos && superInfos.some(({ token }) => token.text === info.token.text);
6359
})) continue;
6460

65-
const { parentDeclaration, declSourceFile, inJs, makeStatic, token, call } = info;
66-
61+
const { parentDeclaration, declSourceFile, modifierFlags, token, call, isJSFile } = info;
6762
// Always prefer to add a method declaration if possible.
6863
if (call && !isPrivateIdentifier(token)) {
69-
addMethodDeclaration(context, changes, declSourceFile, parentDeclaration, token, call, makeStatic, inJs);
64+
addMethodDeclaration(context, changes, call, token.text, modifierFlags & ModifierFlags.Static, parentDeclaration, declSourceFile, isJSFile);
7065
}
7166
else {
72-
if (inJs && !isInterfaceDeclaration(parentDeclaration)) {
73-
addMissingMemberInJs(changes, declSourceFile, parentDeclaration, token, makeStatic);
67+
if (isJSFile && !isInterfaceDeclaration(parentDeclaration)) {
68+
addMissingMemberInJs(changes, declSourceFile, parentDeclaration, token, !!(modifierFlags & ModifierFlags.Static));
7469
}
7570
else {
7671
const typeNode = getTypeNode(program.getTypeChecker(), parentDeclaration, token);
77-
addPropertyDeclaration(changes, declSourceFile, parentDeclaration, token.text, typeNode, makeStatic ? ModifierFlags.Static : 0);
72+
addPropertyDeclaration(changes, declSourceFile, parentDeclaration, token.text, typeNode, modifierFlags & ModifierFlags.Static);
7873
}
7974
}
8075
}
@@ -104,12 +99,12 @@ namespace ts.codefix {
10499
}
105100
interface ClassOrInterfaceInfo {
106101
readonly kind: InfoKind.ClassOrInterface;
102+
readonly call: CallExpression | undefined;
107103
readonly token: Identifier | PrivateIdentifier;
104+
readonly modifierFlags: ModifierFlags;
108105
readonly parentDeclaration: ClassOrInterface;
109-
readonly makeStatic: boolean;
110106
readonly declSourceFile: SourceFile;
111-
readonly inJs: boolean;
112-
readonly call: CallExpression | undefined;
107+
readonly isJSFile: boolean;
113108
}
114109
type Info = EnumInfo | ClassOrInterfaceInfo;
115110

@@ -144,9 +139,10 @@ namespace ts.codefix {
144139
}
145140

146141
const declSourceFile = classOrInterface.getSourceFile();
147-
const inJs = isSourceFileJS(declSourceFile);
142+
const modifierFlags = (makeStatic ? ModifierFlags.Static : 0) | (startsWithUnderscore(token.text) ? ModifierFlags.Private : 0);
143+
const isJSFile = isSourceFileJS(declSourceFile);
148144
const call = tryCast(parent.parent, isCallExpression);
149-
return { kind: InfoKind.ClassOrInterface, token, parentDeclaration: classOrInterface, makeStatic, declSourceFile, inJs, call };
145+
return { kind: InfoKind.ClassOrInterface, token, call, modifierFlags, parentDeclaration: classOrInterface, declSourceFile, isJSFile };
150146
}
151147

152148
const enumDeclaration = find(symbol.declarations, isEnumDeclaration);
@@ -156,13 +152,22 @@ namespace ts.codefix {
156152
return undefined;
157153
}
158154

159-
function getActionsForAddMissingMemberInJavascriptFile(context: CodeFixContext, declSourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, token: Identifier | PrivateIdentifier, makeStatic: boolean): CodeFixAction | undefined {
160-
const changes = textChanges.ChangeTracker.with(context, t => addMissingMemberInJs(t, declSourceFile, classDeclaration, token, makeStatic));
155+
function getActionsForMissingMemberDeclaration(context: CodeFixContext, info: ClassOrInterfaceInfo): CodeFixAction[] | undefined {
156+
return info.isJSFile ? singleElementArray(createActionForAddMissingMemberInJavascriptFile(context, info)) :
157+
createActionsForAddMissingMemberInTypeScriptFile(context, info);
158+
}
159+
160+
function createActionForAddMissingMemberInJavascriptFile(context: CodeFixContext, { parentDeclaration, declSourceFile, modifierFlags, token }: ClassOrInterfaceInfo): CodeFixAction | undefined {
161+
if (isInterfaceDeclaration(parentDeclaration)) {
162+
return undefined;
163+
}
164+
165+
const changes = textChanges.ChangeTracker.with(context, t => addMissingMemberInJs(t, declSourceFile, parentDeclaration, token, !!(modifierFlags & ModifierFlags.Static)));
161166
if (changes.length === 0) {
162167
return undefined;
163168
}
164169

165-
const diagnostic = makeStatic ? Diagnostics.Initialize_static_property_0 :
170+
const diagnostic = modifierFlags & ModifierFlags.Static ? Diagnostics.Initialize_static_property_0 :
166171
isPrivateIdentifier(token) ? Diagnostics.Declare_a_private_field_named_0 : Diagnostics.Initialize_property_0_in_the_constructor;
167172

168173
return createCodeFixAction(fixName, changes, [diagnostic, token.text], fixId, Diagnostics.Add_all_missing_members);
@@ -209,18 +214,22 @@ namespace ts.codefix {
209214
return createStatement(createAssignment(createPropertyAccess(obj, propertyName), createIdentifier("undefined")));
210215
}
211216

212-
function getActionsForAddMissingMemberInTypeScriptFile(context: CodeFixContext, declSourceFile: SourceFile, classDeclaration: ClassOrInterface, token: Identifier | PrivateIdentifier, makeStatic: boolean): CodeFixAction[] | undefined {
213-
const typeNode = getTypeNode(context.program.getTypeChecker(), classDeclaration, token);
214-
const actions: CodeFixAction[] = [createAddPropertyDeclarationAction(context, declSourceFile, classDeclaration, token.text, typeNode, makeStatic ? ModifierFlags.Static : 0)];
215-
if (makeStatic || isPrivateIdentifier(token)) {
217+
function createActionsForAddMissingMemberInTypeScriptFile(context: CodeFixContext, { parentDeclaration, declSourceFile, modifierFlags, token }: ClassOrInterfaceInfo): CodeFixAction[] | undefined {
218+
const memberName = token.text;
219+
const isStatic = modifierFlags & ModifierFlags.Static;
220+
const typeNode = getTypeNode(context.program.getTypeChecker(), parentDeclaration, token);
221+
const addPropertyDeclarationChanges = (modifierFlags: ModifierFlags) => textChanges.ChangeTracker.with(context, t => addPropertyDeclaration(t, declSourceFile, parentDeclaration, memberName, typeNode, modifierFlags));
222+
223+
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)];
224+
if (isStatic || isPrivateIdentifier(token)) {
216225
return actions;
217226
}
218227

219-
if (startsWithUnderscore(token.text)) {
220-
actions.unshift(createAddPropertyDeclarationAction(context, declSourceFile, classDeclaration, token.text, typeNode, ModifierFlags.Private));
228+
if (modifierFlags & ModifierFlags.Private) {
229+
actions.unshift(createCodeFixActionWithoutFixAll(fixName, addPropertyDeclarationChanges(ModifierFlags.Private), [Diagnostics.Declare_private_property_0, memberName]));
221230
}
222231

223-
actions.push(createAddIndexSignatureAction(context, declSourceFile, classDeclaration, token.text, typeNode));
232+
actions.push(createAddIndexSignatureAction(context, declSourceFile, parentDeclaration, token.text, typeNode));
224233
return actions;
225234
}
226235

@@ -239,14 +248,6 @@ namespace ts.codefix {
239248
return typeNode || createKeywordTypeNode(SyntaxKind.AnyKeyword);
240249
}
241250

242-
function createAddPropertyDeclarationAction(context: CodeFixContext, declSourceFile: SourceFile, classDeclaration: ClassOrInterface, tokenName: string, typeNode: TypeNode, modifierFlags: ModifierFlags): CodeFixAction {
243-
const changes = textChanges.ChangeTracker.with(context, t => addPropertyDeclaration(t, declSourceFile, classDeclaration, tokenName, typeNode, modifierFlags));
244-
if (modifierFlags & ModifierFlags.Private) {
245-
return createCodeFixActionWithoutFixAll(fixName, changes, [Diagnostics.Declare_private_property_0, tokenName]);
246-
}
247-
return createCodeFixAction(fixName, changes, [modifierFlags & ModifierFlags.Static ? Diagnostics.Declare_static_property_0 : Diagnostics.Declare_property_0, tokenName], fixId, Diagnostics.Add_all_missing_members);
248-
}
249-
250251
function addPropertyDeclaration(changeTracker: textChanges.ChangeTracker, declSourceFile: SourceFile, classDeclaration: ClassOrInterface, tokenName: string, typeNode: TypeNode, modifierFlags: ModifierFlags): void {
251252
const property = createProperty(
252253
/*decorators*/ undefined,
@@ -297,41 +298,46 @@ namespace ts.codefix {
297298
return createCodeFixActionWithoutFixAll(fixName, changes, [Diagnostics.Add_index_signature_for_property_0, tokenName]);
298299
}
299300

300-
function getActionForMethodDeclaration(
301-
context: CodeFixContext,
302-
declSourceFile: SourceFile,
303-
classDeclaration: ClassOrInterface,
304-
token: Identifier | PrivateIdentifier,
305-
callExpression: CallExpression,
306-
makeStatic: boolean,
307-
inJs: boolean
308-
): CodeFixAction | undefined {
301+
function getActionsForMissingMethodDeclaration(context: CodeFixContext, info: ClassOrInterfaceInfo): CodeFixAction[] | undefined {
302+
const { parentDeclaration, declSourceFile, modifierFlags, token, call, isJSFile } = info;
303+
if (call === undefined) {
304+
return undefined;
305+
}
306+
309307
// Private methods are not implemented yet.
310-
if (isPrivateIdentifier(token)) { return undefined; }
311-
const changes = textChanges.ChangeTracker.with(context, t => addMethodDeclaration(context, t, declSourceFile, classDeclaration, token, callExpression, makeStatic, inJs));
312-
return createCodeFixAction(fixName, changes, [makeStatic ? Diagnostics.Declare_static_method_0 : Diagnostics.Declare_method_0, token.text], fixId, Diagnostics.Add_all_missing_members);
308+
if (isPrivateIdentifier(token)) {
309+
return undefined;
310+
}
311+
312+
const methodName = token.text;
313+
const addMethodDeclarationChanges = (modifierFlags: ModifierFlags) => textChanges.ChangeTracker.with(context, t => addMethodDeclaration(context, t, call, methodName, modifierFlags, parentDeclaration, declSourceFile, isJSFile));
314+
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)];
315+
if (modifierFlags & ModifierFlags.Private) {
316+
actions.unshift(createCodeFixActionWithoutFixAll(fixName, addMethodDeclarationChanges(ModifierFlags.Private), [Diagnostics.Declare_private_method_0, methodName]));
317+
}
318+
return actions;
313319
}
314320

315321
function addMethodDeclaration(
316322
context: CodeFixContextBase,
317-
changeTracker: textChanges.ChangeTracker,
318-
declSourceFile: SourceFile,
319-
typeDecl: ClassOrInterface,
320-
token: Identifier,
323+
changes: textChanges.ChangeTracker,
321324
callExpression: CallExpression,
322-
makeStatic: boolean,
323-
inJs: boolean
325+
methodName: string,
326+
modifierFlags: ModifierFlags,
327+
parentDeclaration: ClassOrInterface,
328+
sourceFile: SourceFile,
329+
isJSFile: boolean
324330
): void {
325-
const importAdder = createImportAdder(declSourceFile, context.program, context.preferences, context.host);
326-
const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic, typeDecl, importAdder);
327-
const containingMethodDeclaration = getAncestor(callExpression, SyntaxKind.MethodDeclaration);
328-
if (containingMethodDeclaration && containingMethodDeclaration.parent === typeDecl) {
329-
changeTracker.insertNodeAfter(declSourceFile, containingMethodDeclaration, methodDeclaration);
331+
const importAdder = createImportAdder(sourceFile, context.program, context.preferences, context.host);
332+
const methodDeclaration = createMethodFromCallExpression(context, importAdder, callExpression, methodName, modifierFlags, parentDeclaration, isJSFile);
333+
const containingMethodDeclaration = findAncestor(callExpression, n => isMethodDeclaration(n) || isConstructorDeclaration(n));
334+
if (containingMethodDeclaration && containingMethodDeclaration.parent === parentDeclaration) {
335+
changes.insertNodeAfter(sourceFile, containingMethodDeclaration, methodDeclaration);
330336
}
331337
else {
332-
changeTracker.insertNodeAtClassStart(declSourceFile, typeDecl, methodDeclaration);
338+
changes.insertNodeAtClassStart(sourceFile, parentDeclaration, methodDeclaration);
333339
}
334-
importAdder.writeFixes(changeTracker);
340+
importAdder.writeFixes(changes);
335341
}
336342

337343
function addEnumMemberDeclaration(changes: textChanges.ChangeTracker, checker: TypeChecker, token: Identifier, enumDeclaration: EnumDeclaration) {

‎src/services/codefixes/helpers.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,12 @@ namespace ts.codefix {
214214

215215
export function createMethodFromCallExpression(
216216
context: CodeFixContextBase,
217+
importAdder: ImportAdder,
217218
call: CallExpression,
218219
methodName: string,
219-
inJs: boolean,
220-
makeStatic: boolean,
220+
modifierFlags: ModifierFlags,
221221
contextNode: Node,
222-
importAdder: ImportAdder
222+
inJs: boolean
223223
): MethodDeclaration {
224224
const body = !isInterfaceDeclaration(contextNode);
225225
const { typeArguments, arguments: args, parent } = call;
@@ -234,7 +234,7 @@ namespace ts.codefix {
234234
const returnType = (inJs || !contextualType) ? undefined : checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);
235235
return createMethod(
236236
/*decorators*/ undefined,
237-
/*modifiers*/ makeStatic ? [createToken(SyntaxKind.StaticKeyword)] : undefined,
237+
/*modifiers*/ modifierFlags ? createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined,
238238
/*asteriskToken*/ isYieldExpression(parent) ? createToken(SyntaxKind.AsteriskToken) : undefined,
239239
methodName,
240240
/*questionToken*/ undefined,

‎tests/cases/fourslash/codeFixAddMissingMember18_declarePrivateMethod.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////class A {
4+
//// constructor() {
5+
//// this._foo();
6+
//// }
7+
////}
8+
9+
verify.codeFixAvailable([
10+
{ description: "Declare private method '_foo'" },
11+
{ description: "Declare method '_foo'" },
12+
{ description: "Declare private property '_foo'" },
13+
{ description: "Declare property '_foo'" },
14+
{ description: "Add index signature for property '_foo'" }
15+
])
16+
17+
verify.codeFix({
18+
description: [ts.Diagnostics.Declare_private_method_0.message, "_foo"],
19+
index: 0,
20+
newFileContent:
21+
`class A {
22+
constructor() {
23+
this._foo();
24+
}
25+
private _foo() {
26+
throw new Error("Method not implemented.");
27+
}
28+
}`
29+
});

‎tests/cases/fourslash/codeFixAddMissingMember19_declarePrivateMethod.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////class A {
4+
//// foo() {
5+
//// this._bar();
6+
//// }
7+
////}
8+
9+
verify.codeFixAvailable([
10+
{ description: "Declare private method '_bar'" },
11+
{ description: "Declare method '_bar'" },
12+
{ description: "Declare private property '_bar'" },
13+
{ description: "Declare property '_bar'" },
14+
{ description: "Add index signature for property '_bar'" }
15+
])
16+
17+
verify.codeFix({
18+
description: [ts.Diagnostics.Declare_private_method_0.message, "_bar"],
19+
index: 0,
20+
newFileContent:
21+
`class A {
22+
foo() {
23+
this._bar();
24+
}
25+
private _bar() {
26+
throw new Error("Method not implemented.");
27+
}
28+
}`
29+
});

‎tests/cases/fourslash/codeFixUndeclaredMethod.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ verify.codeFix({
1515
index: 0,
1616
newFileContent:
1717
`class A {
18-
foo1(arg0: number, arg1: number, arg2: number) {
19-
throw new Error("Method not implemented.");
20-
}
2118
constructor() {
2219
this.foo1(1,2,3);
2320
// 7 type args
2421
this.foo2<1,2,3,4,5,6,7>();
2522
// 8 type args
2623
this.foo3<1,2,3,4,5,6,7,8>();
2724
}
25+
foo1(arg0: number, arg1: number, arg2: number) {
26+
throw new Error("Method not implemented.");
27+
}
2828
}`,
2929
applyChanges: true,
3030
});
@@ -34,19 +34,19 @@ verify.codeFix({
3434
index: 0,
3535
newFileContent:
3636
`class A {
37-
foo2<T, U, V, W, X, Y, Z>() {
38-
throw new Error("Method not implemented.");
39-
}
40-
foo1(arg0: number, arg1: number, arg2: number) {
41-
throw new Error("Method not implemented.");
42-
}
4337
constructor() {
4438
this.foo1(1,2,3);
4539
// 7 type args
4640
this.foo2<1,2,3,4,5,6,7>();
4741
// 8 type args
4842
this.foo3<1,2,3,4,5,6,7,8>();
4943
}
44+
foo2<T, U, V, W, X, Y, Z>() {
45+
throw new Error("Method not implemented.");
46+
}
47+
foo1(arg0: number, arg1: number, arg2: number) {
48+
throw new Error("Method not implemented.");
49+
}
5050
}`,
5151
applyChanges: true,
5252
});
@@ -56,6 +56,13 @@ verify.codeFix({
5656
index: 0,
5757
newFileContent:
5858
`class A {
59+
constructor() {
60+
this.foo1(1,2,3);
61+
// 7 type args
62+
this.foo2<1,2,3,4,5,6,7>();
63+
// 8 type args
64+
this.foo3<1,2,3,4,5,6,7,8>();
65+
}
5966
foo3<T0, T1, T2, T3, T4, T5, T6, T7>() {
6067
throw new Error("Method not implemented.");
6168
}
@@ -65,12 +72,5 @@ verify.codeFix({
6572
foo1(arg0: number, arg1: number, arg2: number) {
6673
throw new Error("Method not implemented.");
6774
}
68-
constructor() {
69-
this.foo1(1,2,3);
70-
// 7 type args
71-
this.foo2<1,2,3,4,5,6,7>();
72-
// 8 type args
73-
this.foo3<1,2,3,4,5,6,7,8>();
74-
}
7575
}`
7676
});

0 commit comments

Comments
 (0)