From c608a6eee960fc8d9d9205f80ef6baca6ce360d5 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Mon, 20 Sep 2021 17:44:47 -0700 Subject: [PATCH] add more tests and support more cases --- src/services/codefixes/helpers.ts | 23 ++-- src/services/completions.ts | 94 ++++++-------- .../fourslash/completionsOverridingMethod.ts | 121 +++++++++++++++++- 3 files changed, 165 insertions(+), 73 deletions(-) diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index 6ed8aced971..6fe8b12b5b3 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -18,7 +18,7 @@ namespace ts.codefix { const classMembers = classDeclaration.symbol.members!; for (const symbol of possiblyMissingSymbols) { if (!classMembers.has(symbol.escapedName)) { - addNewNodeForMemberSymbol(symbol, classDeclaration, sourceFile, context, preferences, importAdder, addClassElement); + addNewNodeForMemberSymbol(symbol, classDeclaration, sourceFile, context, preferences, importAdder, addClassElement, /* body */ undefined); } } } @@ -39,6 +39,7 @@ namespace ts.codefix { /** * `addClassElement` will not be called if we can't figure out a representation for `symbol` in `enclosingDeclaration`. + * @param body If defined, this will be the body of the member node passed to `addClassElement`. Otherwise, the body will default to a stub. */ export function addNewNodeForMemberSymbol( symbol: Symbol, @@ -48,6 +49,7 @@ namespace ts.codefix { preferences: UserPreferences, importAdder: ImportAdder | undefined, addClassElement: (node: AddNode) => void, + body: Block | undefined, ): void { const declarations = symbol.getDeclarations(); if (!(declarations && declarations.length)) { @@ -106,7 +108,7 @@ namespace ts.codefix { name, emptyArray, typeNode, - ambient ? undefined : createStubbedMethodBody(quotePreference))); + ambient ? undefined : body || createStubbedMethodBody(quotePreference))); } else { Debug.assertNode(accessor, isSetAccessorDeclaration, "The counterpart to a getter should be a setter"); @@ -117,7 +119,7 @@ namespace ts.codefix { modifiers, name, createDummyParameters(1, [parameterName], [typeNode], 1, /*inJs*/ false), - ambient ? undefined : createStubbedMethodBody(quotePreference))); + ambient ? undefined : body || createStubbedMethodBody(quotePreference))); } } break; @@ -139,7 +141,7 @@ namespace ts.codefix { if (declarations.length === 1) { Debug.assert(signatures.length === 1, "One declaration implies one signature"); const signature = signatures[0]; - outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : createStubbedMethodBody(quotePreference)); + outputMethod(quotePreference, signature, modifiers, name, ambient ? undefined : body || createStubbedMethodBody(quotePreference)); break; } @@ -151,11 +153,11 @@ namespace ts.codefix { if (!ambient) { if (declarations.length > signatures.length) { const signature = checker.getSignatureFromDeclaration(declarations[declarations.length - 1] as SignatureDeclaration)!; - outputMethod(quotePreference, signature, modifiers, name, createStubbedMethodBody(quotePreference)); + outputMethod(quotePreference, signature, modifiers, name, body || createStubbedMethodBody(quotePreference)); } else { Debug.assert(declarations.length === signatures.length, "Declarations and signatures should match count"); - addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, name, optional, modifiers, quotePreference)); + addClassElement(createMethodImplementingSignatures(checker, context, enclosingDeclaration, signatures, name, optional, modifiers, quotePreference, body)); } } break; @@ -365,6 +367,7 @@ namespace ts.codefix { optional: boolean, modifiers: readonly Modifier[] | undefined, quotePreference: QuotePreference, + body: Block | undefined, ): MethodDeclaration { /** This is *a* signature with the maximal number of arguments, * such that if there is a "maximal" signature without rest arguments, @@ -406,7 +409,8 @@ namespace ts.codefix { /*typeParameters*/ undefined, parameters, getReturnTypeFromSignatures(signatures, checker, context, enclosingDeclaration), - quotePreference); + quotePreference, + body); } function getReturnTypeFromSignatures(signatures: readonly Signature[], checker: TypeChecker, context: TypeConstructionContext, enclosingDeclaration: ClassLikeDeclaration): TypeNode | undefined { @@ -423,7 +427,8 @@ namespace ts.codefix { typeParameters: readonly TypeParameterDeclaration[] | undefined, parameters: readonly ParameterDeclaration[], returnType: TypeNode | undefined, - quotePreference: QuotePreference + quotePreference: QuotePreference, + body: Block | undefined ): MethodDeclaration { return factory.createMethodDeclaration( /*decorators*/ undefined, @@ -434,7 +439,7 @@ namespace ts.codefix { typeParameters, parameters, returnType, - createStubbedMethodBody(quotePreference)); + body || createStubbedMethodBody(quotePreference)); } function createStubbedMethodBody(quotePreference: QuotePreference) { diff --git a/src/services/completions.ts b/src/services/completions.ts index 795533cb7ba..d88e59d26b8 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -391,7 +391,8 @@ namespace ts.Completions { host: LanguageServiceHost, program: Program, compilerOptions: CompilerOptions, - log: Log, completionData: CompletionData, + log: Log, + completionData: CompletionData, preferences: UserPreferences, ): CompletionInfo | undefined { const { @@ -719,6 +720,7 @@ namespace ts.Completions { } // >> TODO: Find better location for code + // >> TODO: update this to `isMemberCompletion`... ? function isMethodOverrideCompletion(symbol: Symbol, location: Node): boolean { return !!(symbol.flags & SymbolFlags.Method) && isPropertyDeclaration(location.parent); // >> TODO: add more checks. e.g. the method suggestion could come from an interface the class implements, @@ -748,6 +750,14 @@ namespace ts.Completions { omitTrailingSemicolon: true, }); const importAdder = codefix.createImportAdder(sourceFile, program, preferences, host); + let body; + if (preferences.includeCompletionsWithSnippetText) { + isSnippet = true; + body = factory.createBlock([], /* multiline */ true); // TODO: add tabstop + } + else { + body = factory.createBlock([], /* multiline */ true); + } codefix.addNewNodeForMemberSymbol( symbol, classLikeDeclaration, @@ -755,66 +765,34 @@ namespace ts.Completions { { program, host }, preferences, importAdder, - /* addClassElement */ nodeToEntry); - - - function nodeToEntry(node: PropertyDeclaration | GetAccessorDeclaration | SetAccessorDeclaration | MethodDeclaration | FunctionExpression | ArrowFunction): void { - if (!isPropertyDeclaration(node) && node.body && isBlock(node.body)) { // Declaration has body, so we might need to transform this completion into a snippet. - factory.updateBlock(node.body, []); // TODO: add tabstop if editor supports snippets - if (preferences.includeCompletionsWithSnippetText) { - // TODO: add tabstop if editor supports snippets - isSnippet = true; + node => { + if (isClassDeclaration(classLikeDeclaration) && hasAbstractModifier(classLikeDeclaration)) { + // Add `abstract` modifier + node = factory.updateModifiers( + node, + concatenate([factory.createModifier(SyntaxKind.AbstractKeyword)], node.modifiers), + ); + if (isMethodDeclaration(node)) { + // Remove method body + // >> TODO: maybe move this up, when creating the body above? + node = factory.updateMethodDeclaration( + node, + node.decorators, + node.modifiers, + node.asteriskToken, + node.name, + node.questionToken, + node.typeParameters, + node.parameters, + node.type, + /* body */ undefined, + ); + } } insertText = printer.printNode(EmitHint.Unspecified, node, sourceFile); - } - else { // Declaration has no body or body is not a block. - insertText = printer.printNode(EmitHint.Unspecified, node, sourceFile); - } - } - // ** ----- ** // + }, + body); - // const methodDeclarations = symbol.declarations?.filter(isMethodDeclaration); - // // >> TODO: what should we do if we have more than 1 signature? when could that happen? - // if (methodDeclarations?.length === 1) { - // const originalDeclaration = methodDeclarations[0]; - // const modifiers = originalDeclaration.modifiers?.filter(modifier => { - // switch (modifier.kind) { // >> Simplify this if we only need to filter out "abstract" modifier. - // case SyntaxKind.AbstractKeyword: - // return false; - // default: - // return true; - // } - // }); - // if (options.noImplicitOverride) { - // modifiers?.push(factory.createModifier(SyntaxKind.OverrideKeyword)); - // // Assuming it's ok if this modifier is duplicated. - // } - - // const tabStop = preferences.includeCompletionsWithSnippetText ? "$1" : ""; - // const tabStopStatement = factory.createExpressionStatement(factory.createIdentifier(tabStop)); - // const completionDeclaration = factory.createMethodDeclaration( - // /*decorators*/ undefined, // I'm guessing we don't want to deal with decorators? - // /*modifiers*/ modifiers, - // /*asteriskToken*/ originalDeclaration.asteriskToken, - // /*name*/ name, - // /*questionToken*/ originalDeclaration.questionToken, - // /*typeParameters*/ originalDeclaration.typeParameters, - // /*parameters*/ originalDeclaration.parameters, - // /*type*/ originalDeclaration.type, - // /*body*/ factory.createBlock([tabStopStatement], /*multiLine*/ true)); - - // const printer = createPrinter({ - // removeComments: true, - // module: options.module, - // target: options.target, - // omitTrailingSemicolon: true, - // }); - // const insertText = printer.printNode(EmitHint.Unspecified, completionDeclaration, location.getSourceFile()); - // return { - // insertText, - // isSnippet: preferences.includeCompletionsWithSnippetText ? true as const : undefined, - // }; - // } return { insertText, isSnippet }; } diff --git a/tests/cases/fourslash/completionsOverridingMethod.ts b/tests/cases/fourslash/completionsOverridingMethod.ts index 28c101735a0..ca4717ea70e 100644 --- a/tests/cases/fourslash/completionsOverridingMethod.ts +++ b/tests/cases/fourslash/completionsOverridingMethod.ts @@ -1,11 +1,53 @@ /// -////abstract class Base { +// @Filename: a.ts +// Case: Concrete class implements abstract method +////abstract class ABase { //// abstract foo(param1: string, param2: boolean): Promise; ////} //// -////class Sub extends Base { -//// f/*a*/ +////class ASub extends ABase { +//// f/*a*/ +////} + +// @Filename: b.ts +// Case: Concrete class overrides concrete method +////class BBase { +//// foo(a: string, b: string): string { +//// return a + b; +//// } +////} +//// +////class BSub extends BBase { +//// f/*b*/ +////} + +// @Filename: c.ts +// Case: Multiple overrides, concrete class overrides concrete method +////class CBase { +//// foo(a: string | number): string { +//// return a + ""; +//// } +////} +//// +////class CSub extends CBase { +//// foo(a: string): string { +//// return add; +//// } +////} +//// +////class CSub2 extends CSub { +//// f/*c*/ +////} + +// @Filename: d.ts +// Case: Abstract class extends abstract class +////abstract class DBase { +//// abstract foo(a: string): string; +////} +//// +////abstract class DSub extends DBase { +//// f/*d*/ ////} // format.setFormatOptions({ @@ -20,20 +62,87 @@ verify.completions({ includeCompletionsWithInsertText: true, includeCompletionsWithSnippetText: true, }, - // exact: [ - // ], includes: [ { name: "foo", - isRecommended: true, sortText: completion.SortText.LocationPriority, replacementSpan: { fileName: "", pos: 0, end: 0, }, + isSnippet: true, insertText: "foo(param1: string, param2: boolean): Promise {\r\n}", } ], }); + +verify.completions({ + marker: "b", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: true, + }, + includes: [ + { + name: "foo", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + isSnippet: true, + insertText: +"foo(a: string, b: string): string {\r\n}", + } + ], +}); + +verify.completions({ + marker: "c", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: true, + }, + includes: [ + { + name: "foo", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + isSnippet: true, + insertText: +"foo(a: string): string {\r\n}", + } + ], +}); + +verify.completions({ + marker: "d", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: true, + }, + includes: [ + { + name: "foo", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + isSnippet: true, + insertText: +"abstract foo(a: string): string;", // Currently fails because no trailing semicolon + } + ], +}); \ No newline at end of file