From e079f71e7da3164b429a3dffa0fd52ed846cbd51 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 12 Oct 2021 17:51:33 -0700 Subject: [PATCH] more tests and fixes --- src/services/completions.ts | 88 ++++++++----- .../fourslash/completionsOverridingMethod.ts | 50 ++++++++ .../fourslash/completionsOverridingMethod3.ts | 38 ++++++ .../fourslash/completionsOverridingMethod4.ts | 67 ++++++++++ .../fourslash/completionsOverridingMethod5.ts | 116 ++++++++++++++++++ 5 files changed, 325 insertions(+), 34 deletions(-) create mode 100644 tests/cases/fourslash/completionsOverridingMethod3.ts create mode 100644 tests/cases/fourslash/completionsOverridingMethod4.ts create mode 100644 tests/cases/fourslash/completionsOverridingMethod5.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 9f80cfb83b6..ac0c61818b9 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -702,10 +702,9 @@ namespace ts.Completions { } if (isClassLikeMemberCompletion(symbol, location)) { - ({ insertText, isSnippet } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location)); + ({ insertText, isSnippet } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken)); } - // >> Should this actually be here and not at the very end of the function? if (insertText !== undefined && !preferences.includeCompletionsWithInsertText) { return undefined; } @@ -772,16 +771,29 @@ namespace ts.Completions { }; } - // >> TODO: Find better location for code function isClassLikeMemberCompletion(symbol: Symbol, location: Node): boolean { + // TODO: support JS files. + if (isInJSFile(location)) { + return false; + } + const memberFlags = - // SymbolFlags.Method - // | SymbolFlags.Accessor - // | SymbolFlags.Property SymbolFlags.ClassMember & SymbolFlags.EnumMemberExcludes; - // >> TODO: Flags: Constructor? Signature? - return !!findAncestor(location, isClassLike) && !!(symbol.flags & memberFlags); + /* In + `class C { + | + }` + `location` is a class-like declaration. + In + `class C { + m| + }` + `location` is an identifier declaration, `location.parent` is a class element declaration, + and `location.parent.parent` is a class-like declaration. + */ + return !!(symbol.flags & memberFlags) && + (isClassLike(location) || (isClassElement(location.parent) && isClassLike(location.parent.parent))); } function getEntryForMemberCompletion( @@ -791,7 +803,9 @@ namespace ts.Completions { preferences: UserPreferences, name: string, symbol: Symbol, - location: Node): { insertText: string, isSnippet?: true } { + location: Node, + contextToken: Node | undefined, + ): { insertText: string, isSnippet?: true } { const classLikeDeclaration = findAncestor(location, isClassLike); if (!classLikeDeclaration) { return { insertText: name }; @@ -815,7 +829,7 @@ namespace ts.Completions { if (preferences.includeCompletionsWithSnippetText) { isSnippet = true; // We are adding a final tabstop (i.e. $0) in the body of the suggested member, if it has one. - // NOTE: this assumes we won't have more than one body in the completion nodes. + // Note: this assumes we won't have more than one body in the completion nodes, which should be the case. const emptyStatement = factory.createExpressionStatement(factory.createIdentifier("")); setSnippetElement(emptyStatement, { kind: SnippetKind.TabStop, order: 0 }); body = factory.createBlock([emptyStatement], /* multiline */ true); @@ -835,34 +849,36 @@ namespace ts.Completions { // `addNewNodeForMemberSymbol` calls this callback function for each new member node // it adds for the given member symbol. // We store these member nodes in the `completionNodes` array. - // Note that there might be: + // Note: there might be: // - No nodes if `addNewNodeForMemberSymbol` cannot figure out a node for the member; // - One node; // - More than one node if the member is overloaded (e.g. a method with overload signatures). node => { - // >> TODO: making it abstract. might not need it after all. - // if (hasAbstractModifier(classLikeDeclaration)) { - // Add `abstract` modifier - // node = factory.updateModifiers( - // node, - // concatenate([factory.createModifier(SyntaxKind.AbstractKeyword)], node.modifiers), - // ); - // if (isMethodDeclaration(node)) { - // // Remove method body - // node = factory.updateMethodDeclaration( - // node, - // node.decorators, - // node.modifiers, - // node.asteriskToken, - // node.name, - // node.questionToken, - // node.typeParameters, - // node.parameters, - // node.type, - // /* body */ undefined, - // ); - // } - // } + // Check if the suggested method should be abstract. + // e.g. in `abstract class C { abstract | }`, we should offer abstract method signatures at position `|`. + // Note: We are relying on checking if the context token is `abstract`, + // since other visibility modifiers (e.g. `protected`) should come *before* `abstract`. + // However, that is not true for the e.g. `override` modifier, so this check has its limitations. + const isAbstract = contextToken && + (isAbstractModifier(contextToken) || + (isIdentifier(contextToken) && contextToken.escapedText === "abstract")); + if (isAbstract) { + if (isMethodDeclaration(node)) { + // Remove method body + node = factory.updateMethodDeclaration( + node, + node.decorators, + node.modifiers, + node.asteriskToken, + node.name, + node.questionToken, + node.typeParameters, + node.parameters, + node.type, + /* body */ undefined, + ); + } + } if (isClassElement(node) && checker.getMemberOverrideModifierDiagnostic(classLikeDeclaration, node) === MemberOverrideDiagnostic.NeedsOverride) { node = factory.updateModifiers( @@ -904,6 +920,10 @@ namespace ts.Completions { setSnippetElement(node, { kind: SnippetKind.Placeholder, order }); order += 1; } + else if (isTypeParameterDeclaration(node) && parent && isFunctionLikeDeclaration(parent)) { + setSnippetElement(node, { kind: SnippetKind.Placeholder, order }); + order += 1; + } forEachChild(node, child => addSnippetsWorker(child, node)); } diff --git a/tests/cases/fourslash/completionsOverridingMethod.ts b/tests/cases/fourslash/completionsOverridingMethod.ts index ec166f52a14..62f9ed1d768 100644 --- a/tests/cases/fourslash/completionsOverridingMethod.ts +++ b/tests/cases/fourslash/completionsOverridingMethod.ts @@ -95,6 +95,23 @@ //// static /*h2*/ ////} +// @Filename: i.ts +// Case: Generic method +////class IBase { +//// met(t: T): T { +//// return t; +//// } +//// metcons(t: T): T { +//// return t; +//// } +////} +//// +////class ISub extends IBase { +//// /*i*/ +////} + + + // format.setFormatOptions({ // newLineCharacter: "\n", // }); @@ -285,4 +302,37 @@ verify.completions({ "met(n: number): number {\r\n}\r\n", } ], +}); + +verify.completions({ + marker: "i", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + }, + includes: [ + { + name: "met", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: +"met(t: T): T {\r\n}\r\n", + }, + { + name: "metcons", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: +"metcons(t: T): T {\r\n}\r\n", + } + ], }); \ No newline at end of file diff --git a/tests/cases/fourslash/completionsOverridingMethod3.ts b/tests/cases/fourslash/completionsOverridingMethod3.ts new file mode 100644 index 00000000000..6ff8a51808c --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod3.ts @@ -0,0 +1,38 @@ +/// + +// @Filename: boo.d.ts +// Case: Declaration files +////interface Ghost { +//// boo(): string; +////} +//// +////declare class Poltergeist implements Ghost { +//// /*b*/ +////} + +// format.setFormatOptions({ +// newLineCharacter: "\n", +// }); +// format.setOption("newline", "\n"); + +verify.completions({ + marker: "b", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + }, + includes: [ + { + name: "boo", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: +"boo(): string;\r\n", + } + ], +}); \ No newline at end of file diff --git a/tests/cases/fourslash/completionsOverridingMethod4.ts b/tests/cases/fourslash/completionsOverridingMethod4.ts new file mode 100644 index 00000000000..8fb1acfcfcf --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod4.ts @@ -0,0 +1,67 @@ +/// + +// @Filename: secret.ts +// Case: accessibility modifier inheritance +////class Secret { +//// #secret(): string { +//// return "secret"; +//// } +//// +//// private tell(): string { +//// return this.#secret(); +//// } +//// +//// protected hint(): string { +//// return "hint"; +//// } +//// +//// public refuse(): string { +//// return "no comments"; +//// } +////} +//// +////class Gossip extends Secret { +//// /* no telling secrets */ +//// /*a*/ +////} + + +// format.setFormatOptions({ +// newLineCharacter: "\n", +// }); +// format.setOption("newline", "\n"); + +verify.completions({ + marker: "a", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + }, + excludes: [ + "tell", + "#secret", + ], + includes: [ + { + name: "hint", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "protected hint(): string {\r\n}\r\n", + }, + { + name: "refuse", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "public refuse(): string {\r\n}\r\n", + } + ], +}); \ No newline at end of file diff --git a/tests/cases/fourslash/completionsOverridingMethod5.ts b/tests/cases/fourslash/completionsOverridingMethod5.ts new file mode 100644 index 00000000000..81cbe6863d9 --- /dev/null +++ b/tests/cases/fourslash/completionsOverridingMethod5.ts @@ -0,0 +1,116 @@ +/// + +// @Filename: a.ts +// Case: abstract methods +////abstract class Ab { +//// abstract met(n: string): void; +//// met2(n: number): void { +//// +//// } +////} +//// +////abstract class Abc extends Ab { +//// /*a*/ +//// abstract /*b*/ +//// abstract m/*c*/ +////} + +// format.setFormatOptions({ +// newLineCharacter: "\n", +// }); +// format.setOption("newline", "\n"); + +verify.completions({ + marker: "a", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + }, + includes: [ + { + name: "met", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "met(n: string): void {\r\n}\r\n", + }, + { + name: "met2", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "met2(n: number): void {\r\n}\r\n", + } + ], +}); + +verify.completions({ + marker: "b", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + }, + includes: [ + { + name: "met", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "met(n: string): void;\r\n", + }, + { + name: "met2", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "met2(n: number): void;\r\n", + } + ], +}); + +verify.completions({ + marker: "c", + isNewIdentifierLocation: true, + preferences: { + includeCompletionsWithInsertText: true, + includeCompletionsWithSnippetText: false, + }, + includes: [ + { + name: "met", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "met(n: string): void;\r\n", + }, + { + name: "met2", + sortText: completion.SortText.LocationPriority, + replacementSpan: { + fileName: "", + pos: 0, + end: 0, + }, + insertText: "met2(n: number): void;\r\n", + } + ], +}); + +