Skip to content

Commit a92f7a1

Browse files
a-tarasyuka-tarasyuk
committed
feat(37782): add quick-fix action to declare a private method for names that start from underscore
1 parent 52dc9f2 commit a92f7a1

8 files changed

+156
-97
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5181,6 +5181,10 @@
51815181
"category": "Message",
51825182
"code": 90035
51835183
},
5184+
"Declare private method '{0}'": {
5185+
"category": "Message",
5186+
"code": 90036
5187+
},
51845188
"Declare a private field named '{0}'.": {
51855189
"category": "Message",
51865190
"code": 90053

‎src/services/codefixes/fixAddMissingMember.ts

Lines changed: 67 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,19 @@ 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, context.preferences);
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 => {
32-
const { program, preferences } = context;
28+
const { program } = context;
3329
const checker = program.getTypeChecker();
3430
const seen = createMap<true>();
3531

@@ -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, preferences);
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,43 @@ 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-
preferences: UserPreferences,
309-
): 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+
310307
// Private methods are not implemented yet.
311-
if (isPrivateIdentifier(token)) { return undefined; }
312-
const changes = textChanges.ChangeTracker.with(context, t => addMethodDeclaration(context, t, declSourceFile, classDeclaration, token, callExpression, makeStatic, inJs, preferences));
313-
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;
314319
}
315320

316321
function addMethodDeclaration(
317322
context: CodeFixContextBase,
318-
changeTracker: textChanges.ChangeTracker,
319-
declSourceFile: SourceFile,
320-
typeDecl: ClassOrInterface,
321-
token: Identifier,
323+
changes: textChanges.ChangeTracker,
322324
callExpression: CallExpression,
323-
makeStatic: boolean,
324-
inJs: boolean,
325-
preferences: UserPreferences,
325+
methodName: string,
326+
modifierFlags: ModifierFlags,
327+
parentDeclaration: ClassOrInterface,
328+
sourceFile: SourceFile,
329+
isJSFile: boolean
326330
): void {
327-
const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic, preferences, typeDecl);
328-
const containingMethodDeclaration = getAncestor(callExpression, SyntaxKind.MethodDeclaration);
329-
330-
if (containingMethodDeclaration && containingMethodDeclaration.parent === typeDecl) {
331-
changeTracker.insertNodeAfter(declSourceFile, containingMethodDeclaration, methodDeclaration);
331+
const methodDeclaration = createMethodFromCallExpression(context, callExpression, methodName, modifierFlags, parentDeclaration, isJSFile);
332+
const containingMethodDeclaration = findAncestor(callExpression, n => isMethodDeclaration(n) || isConstructorDeclaration(n));
333+
if (containingMethodDeclaration && containingMethodDeclaration.parent === parentDeclaration) {
334+
changes.insertNodeAfter(sourceFile, containingMethodDeclaration, methodDeclaration);
332335
}
333336
else {
334-
changeTracker.insertNodeAtClassStart(declSourceFile, typeDecl, methodDeclaration);
337+
changes.insertNodeAtClassStart(sourceFile, parentDeclaration, methodDeclaration);
335338
}
336339
}
337340

‎src/services/codefixes/helpers.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,7 @@ namespace ts.codefix {
212212
return signatureDeclaration;
213213
}
214214

215-
export function createMethodFromCallExpression(
216-
context: CodeFixContextBase,
217-
call: CallExpression,
218-
methodName: string,
219-
inJs: boolean,
220-
makeStatic: boolean,
221-
preferences: UserPreferences,
222-
contextNode: Node,
223-
): MethodDeclaration {
215+
export function createMethodFromCallExpression(context: CodeFixContextBase, call: CallExpression, methodName: string, modifierFlags: ModifierFlags, contextNode: Node, inJs: boolean): MethodDeclaration {
224216
const body = !isInterfaceDeclaration(contextNode);
225217
const { typeArguments, arguments: args, parent } = call;
226218
const checker = context.program.getTypeChecker();
@@ -234,15 +226,15 @@ namespace ts.codefix {
234226
const returnType = (inJs || !contextualType) ? undefined : checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker);
235227
return createMethod(
236228
/*decorators*/ undefined,
237-
/*modifiers*/ makeStatic ? [createToken(SyntaxKind.StaticKeyword)] : undefined,
229+
/*modifiers*/ modifierFlags ? createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined,
238230
/*asteriskToken*/ isYieldExpression(parent) ? createToken(SyntaxKind.AsteriskToken) : undefined,
239231
methodName,
240232
/*questionToken*/ undefined,
241233
/*typeParameters*/ inJs ? undefined : map(typeArguments, (_, i) =>
242234
createTypeParameterDeclaration(CharacterCodes.T + typeArguments!.length - 1 <= CharacterCodes.Z ? String.fromCharCode(CharacterCodes.T + i) : `T${i}`)),
243235
/*parameters*/ createDummyParameters(args.length, names, types, /*minArgumentCount*/ undefined, inJs),
244236
/*type*/ returnType,
245-
body ? createStubbedMethodBody(preferences) : undefined);
237+
body ? createStubbedMethodBody(context.preferences) : undefined);
246238
}
247239

248240
function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] {

‎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+
});

0 commit comments

Comments
 (0)