From a92f7a13b27dc1c59dc091c822f5222d1c9e8c67 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Fri, 10 Apr 2020 11:07:52 +0300 Subject: [PATCH 1/2] feat(37782): add quick-fix action to declare a private method for names that start from underscore --- src/compiler/diagnosticMessages.json | 4 + src/services/codefixes/fixAddMissingMember.ts | 131 +++++++++--------- src/services/codefixes/helpers.ts | 14 +- ...AddMissingMember18_declarePrivateMethod.ts | 29 ++++ ...AddMissingMember19_declarePrivateMethod.ts | 29 ++++ .../fourslash/codeFixUndeclaredMethod.ts | 32 ++--- .../codeFixUndeclaredMethodFunctionArgs.ts | 7 +- ...odeFixUndeclaredMethodObjectLiteralArgs.ts | 7 +- 8 files changed, 156 insertions(+), 97 deletions(-) create mode 100644 tests/cases/fourslash/codeFixAddMissingMember18_declarePrivateMethod.ts create mode 100644 tests/cases/fourslash/codeFixAddMissingMember19_declarePrivateMethod.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 29bf61b2f752b..dfda7f40a80df 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5181,6 +5181,10 @@ "category": "Message", "code": 90035 }, + "Declare private method '{0}'": { + "category": "Message", + "code": 90036 + }, "Declare a private field named '{0}'.": { "category": "Message", "code": 90053 diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index 7a5098d60655a..1e78c351db258 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -13,23 +13,19 @@ 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, context.preferences); - 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 => { - const { program, preferences } = context; + const { program } = context; const checker = program.getTypeChecker(); const seen = createMap(); @@ -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, preferences); + 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,43 @@ 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, - preferences: UserPreferences, - ): 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, preferences)); - 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, - preferences: UserPreferences, + methodName: string, + modifierFlags: ModifierFlags, + parentDeclaration: ClassOrInterface, + sourceFile: SourceFile, + isJSFile: boolean ): void { - const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic, preferences, typeDecl); - const containingMethodDeclaration = getAncestor(callExpression, SyntaxKind.MethodDeclaration); - - if (containingMethodDeclaration && containingMethodDeclaration.parent === typeDecl) { - changeTracker.insertNodeAfter(declSourceFile, containingMethodDeclaration, methodDeclaration); + const methodDeclaration = createMethodFromCallExpression(context, 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); } } diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index 48f2ee9858496..0a0510ee4b725 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -212,15 +212,7 @@ namespace ts.codefix { return signatureDeclaration; } - export function createMethodFromCallExpression( - context: CodeFixContextBase, - call: CallExpression, - methodName: string, - inJs: boolean, - makeStatic: boolean, - preferences: UserPreferences, - contextNode: Node, - ): MethodDeclaration { + export function createMethodFromCallExpression(context: CodeFixContextBase, call: CallExpression, methodName: string, modifierFlags: ModifierFlags, contextNode: Node, inJs: boolean): MethodDeclaration { const body = !isInterfaceDeclaration(contextNode); const { typeArguments, arguments: args, parent } = call; const checker = context.program.getTypeChecker(); @@ -234,7 +226,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, @@ -242,7 +234,7 @@ namespace ts.codefix { createTypeParameterDeclaration(CharacterCodes.T + typeArguments!.length - 1 <= CharacterCodes.Z ? String.fromCharCode(CharacterCodes.T + i) : `T${i}`)), /*parameters*/ createDummyParameters(args.length, names, types, /*minArgumentCount*/ undefined, inJs), /*type*/ returnType, - body ? createStubbedMethodBody(preferences) : undefined); + body ? createStubbedMethodBody(context.preferences) : undefined); } function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] { diff --git a/tests/cases/fourslash/codeFixAddMissingMember18_declarePrivateMethod.ts b/tests/cases/fourslash/codeFixAddMissingMember18_declarePrivateMethod.ts new file mode 100644 index 0000000000000..d0fa87d6a0d95 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingMember18_declarePrivateMethod.ts @@ -0,0 +1,29 @@ +/// + +////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."); + } +}` +}); diff --git a/tests/cases/fourslash/codeFixAddMissingMember19_declarePrivateMethod.ts b/tests/cases/fourslash/codeFixAddMissingMember19_declarePrivateMethod.ts new file mode 100644 index 0000000000000..72f4e27b5fe36 --- /dev/null +++ b/tests/cases/fourslash/codeFixAddMissingMember19_declarePrivateMethod.ts @@ -0,0 +1,29 @@ +/// + +////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."); + } +}` +}); diff --git a/tests/cases/fourslash/codeFixUndeclaredMethod.ts b/tests/cases/fourslash/codeFixUndeclaredMethod.ts index 32713e5dd19b8..ac7a8b32b2f51 100644 --- a/tests/cases/fourslash/codeFixUndeclaredMethod.ts +++ b/tests/cases/fourslash/codeFixUndeclaredMethod.ts @@ -15,9 +15,6 @@ 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 @@ -25,6 +22,9 @@ verify.codeFix({ // 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,12 +34,6 @@ verify.codeFix({ index: 0, newFileContent: `class A { - foo2() { - 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 @@ -47,6 +41,12 @@ verify.codeFix({ // 8 type args this.foo3<1,2,3,4,5,6,7,8>(); } + foo2() { + 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() { 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>(); - } }` }); diff --git a/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts b/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts index c4056625b70b9..faa5a165ace73 100644 --- a/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts +++ b/tests/cases/fourslash/codeFixUndeclaredMethodFunctionArgs.ts @@ -1,10 +1,11 @@ /// -//// class A {[| -//// |]constructor() { +//// class A { +//// constructor() { //// this.foo1(() => 1, () => "2", () => false); //// this.foo2((a: number) => a, (b: string) => b, (c: boolean) => c); -//// } +//// }[| +//// |] //// } verify.codeFix({ diff --git a/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts b/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts index 5c0698ad653e9..9d339c50fae04 100644 --- a/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts +++ b/tests/cases/fourslash/codeFixUndeclaredMethodObjectLiteralArgs.ts @@ -1,11 +1,12 @@ /// -//// 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({ From 232896b0d498754217e515de68818c9797083ec4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Wed, 6 May 2020 11:47:52 -0700 Subject: [PATCH 2/2] better merge order in messages json --- src/compiler/diagnosticMessages.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 058d06618d076..78c047a5e4a95 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5197,15 +5197,15 @@ "category": "Message", "code": 90035 }, - "Declare private method '{0}'": { + "Replace '{0}' with 'Promise<{1}>'": { "category": "Message", "code": 90036 }, - "Replace '{0}' with 'Promise<{1}>'": { + "Fix all incorrect return type of an async functions": { "category": "Message", "code": 90037 }, - "Fix all incorrect return type of an async functions": { + "Declare private method '{0}'": { "category": "Message", "code": 90038 },