diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index a56d21ae345..74790400b98 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -3417,6 +3417,17 @@ namespace FourSlashInterface { export class VerifyNegatable { public not: VerifyNegatable; + public allowedClassElementKeywords = [ + "public", + "private", + "protected", + "static", + "abstract", + "readonly", + "get", + "set", + "constructor" + ]; constructor(protected state: FourSlash.TestState, private negative = false) { if (!negative) { @@ -3453,6 +3464,12 @@ namespace FourSlashInterface { this.state.verifyCompletionListIsEmpty(this.negative); } + public completionListContainsClassElementKeywords() { + for (const keyword of this.allowedClassElementKeywords) { + this.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword"); + } + } + public completionListIsGlobal(expected: boolean) { this.state.verifyCompletionListIsGlobal(expected); } diff --git a/src/services/completions.ts b/src/services/completions.ts index 44915a958c2..da59dab8828 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -988,6 +988,10 @@ namespace ts.Completions { return undefined; } + function isFromClassElementDeclaration(node: Node) { + return isClassElement(node.parent) && isClassLike(node.parent.parent); + } + /** * Returns the immediate owning class declaration of a context token, * on the condition that one exists and that the context implies completion should be given. @@ -1010,6 +1014,13 @@ namespace ts.Completions { return location; } break; + + default: + if (isFromClassElementDeclaration(contextToken) && + (isClassMemberCompletionKeyword(contextToken.kind) || + isClassMemberCompletionKeywordText(contextToken.getText()))) { + return contextToken.parent.parent as ClassLikeDeclaration; + } } } @@ -1148,7 +1159,7 @@ namespace ts.Completions { isFunction(containingNodeKind); case SyntaxKind.StaticKeyword: - return containingNodeKind === SyntaxKind.PropertyDeclaration; + return containingNodeKind === SyntaxKind.PropertyDeclaration && !isClassLike(contextToken.parent.parent); case SyntaxKind.DotDotDotToken: return containingNodeKind === SyntaxKind.Parameter || @@ -1165,13 +1176,17 @@ namespace ts.Completions { containingNodeKind === SyntaxKind.ExportSpecifier || containingNodeKind === SyntaxKind.NamespaceImport; + case SyntaxKind.GetKeyword: + case SyntaxKind.SetKeyword: + if (isFromClassElementDeclaration(contextToken)) { + return false; + } + // falls through case SyntaxKind.ClassKeyword: case SyntaxKind.EnumKeyword: case SyntaxKind.InterfaceKeyword: case SyntaxKind.FunctionKeyword: case SyntaxKind.VarKeyword: - case SyntaxKind.GetKeyword: - case SyntaxKind.SetKeyword: case SyntaxKind.ImportKeyword: case SyntaxKind.LetKeyword: case SyntaxKind.ConstKeyword: @@ -1180,6 +1195,13 @@ namespace ts.Completions { return true; } + // If the previous token is keyword correspoding to class member completion keyword + // there will be completion available here + if (isClassMemberCompletionKeywordText(contextToken.getText()) && + isFromClassElementDeclaration(contextToken)) { + return false; + } + // Previous token may have been a keyword that was converted to an identifier. switch (contextToken.getText()) { case "abstract": @@ -1373,8 +1395,8 @@ namespace ts.Completions { }); } - const classMemberKeywordCompletions = filter(keywordCompletions, entry => { - switch (stringToToken(entry.name)) { + function isClassMemberCompletionKeyword(kind: SyntaxKind) { + switch (kind) { case SyntaxKind.PublicKeyword: case SyntaxKind.ProtectedKeyword: case SyntaxKind.PrivateKeyword: @@ -1386,7 +1408,14 @@ namespace ts.Completions { case SyntaxKind.SetKeyword: return true; } - }); + } + + function isClassMemberCompletionKeywordText(text: string) { + return isClassMemberCompletionKeyword(stringToToken(text)); + } + + const classMemberKeywordCompletions = filter(keywordCompletions, entry => + isClassMemberCompletionKeywordText(entry.name)); function isEqualityExpression(node: Node): node is BinaryExpression { return isBinaryExpression(node) && isEqualityOperatorKind(node.operatorToken.kind); diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts index 5bf3e37f28a..4268a52c419 100644 --- a/src/services/symbolDisplay.ts +++ b/src/services/symbolDisplay.ts @@ -200,27 +200,33 @@ namespace ts.SymbolDisplay { (location.kind === SyntaxKind.ConstructorKeyword && location.parent.kind === SyntaxKind.Constructor)) { // At constructor keyword of constructor declaration // get the signature from the declaration and write it const functionDeclaration = location.parent; - const allSignatures = functionDeclaration.kind === SyntaxKind.Constructor ? type.getNonNullableType().getConstructSignatures() : type.getNonNullableType().getCallSignatures(); - if (!typeChecker.isImplementationOfOverload(functionDeclaration)) { - signature = typeChecker.getSignatureFromDeclaration(functionDeclaration); - } - else { - signature = allSignatures[0]; - } + // Use function declaration to write the signatures only if the symbol corresponding to this declaration + const locationIsSymbolDeclaration = findDeclaration(symbol, declaration => + declaration === (location.kind === SyntaxKind.ConstructorKeyword ? functionDeclaration.parent : functionDeclaration)); - if (functionDeclaration.kind === SyntaxKind.Constructor) { - // show (constructor) Type(...) signature - symbolKind = ScriptElementKind.constructorImplementationElement; - addPrefixForAnyFunctionOrVar(type.symbol, symbolKind); - } - else { - // (function/method) symbol(..signature) - addPrefixForAnyFunctionOrVar(functionDeclaration.kind === SyntaxKind.CallSignature && - !(type.symbol.flags & SymbolFlags.TypeLiteral || type.symbol.flags & SymbolFlags.ObjectLiteral) ? type.symbol : symbol, symbolKind); - } + if (locationIsSymbolDeclaration) { + const allSignatures = functionDeclaration.kind === SyntaxKind.Constructor ? type.getNonNullableType().getConstructSignatures() : type.getNonNullableType().getCallSignatures(); + if (!typeChecker.isImplementationOfOverload(functionDeclaration)) { + signature = typeChecker.getSignatureFromDeclaration(functionDeclaration); + } + else { + signature = allSignatures[0]; + } - addSignatureDisplayParts(signature, allSignatures); - hasAddedSymbolInfo = true; + if (functionDeclaration.kind === SyntaxKind.Constructor) { + // show (constructor) Type(...) signature + symbolKind = ScriptElementKind.constructorImplementationElement; + addPrefixForAnyFunctionOrVar(type.symbol, symbolKind); + } + else { + // (function/method) symbol(..signature) + addPrefixForAnyFunctionOrVar(functionDeclaration.kind === SyntaxKind.CallSignature && + !(type.symbol.flags & SymbolFlags.TypeLiteral || type.symbol.flags & SymbolFlags.ObjectLiteral) ? type.symbol : symbol, symbolKind); + } + + addSignatureDisplayParts(signature, allSignatures); + hasAddedSymbolInfo = true; + } } } } diff --git a/tests/cases/fourslash/completionEntryForClassMembers.ts b/tests/cases/fourslash/completionEntryForClassMembers.ts index da1051e784a..c52977a0002 100644 --- a/tests/cases/fourslash/completionEntryForClassMembers.ts +++ b/tests/cases/fourslash/completionEntryForClassMembers.ts @@ -1,6 +1,9 @@ /// ////abstract class B { +//// private privateMethod() { } +//// protected protectedMethod() { }; +//// static staticMethod() { } //// abstract getValue(): number; //// /*abstractClass*/ ////} @@ -69,25 +72,7 @@ //// static identi/*classThatStartedWritingIdentifierAfterStaticModifier*/ ////} -const allowedKeywords = [ - "public", - "private", - "protected", - "static", - "abstract", - "readonly", - "get", - "set", - "constructor" -]; - -const allowedKeywordCount = allowedKeywords.length; -function verifyAllowedKeyWords() { - for (const keyword of allowedKeywords) { - verify.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword"); - } -} - +const allowedKeywordCount = verify.allowedClassElementKeywords.length; const nonClassElementMarkers = [ "InsideMethod" ]; @@ -104,7 +89,7 @@ const onlyClassElementKeywordLocations = [ ]; for (const marker of onlyClassElementKeywordLocations) { goTo.marker(marker); - verifyAllowedKeyWords(); + verify.completionListContainsClassElementKeywords(); verify.completionListCount(allowedKeywordCount); } @@ -113,9 +98,8 @@ const classElementCompletionLocations = [ "classThatIsEmptyAndExtendingAnotherClass", "classThatHasAlreadyImplementedAnotherClassMethod", "classThatHasAlreadyImplementedAnotherClassMethodAfterMethod", - // TODO should we give completion for these keywords - //"classThatHasWrittenPublicKeyword", - //"classElementContainingStatic", + "classThatHasWrittenPublicKeyword", + "classElementContainingStatic", "classThatStartedWritingIdentifier", "propDeclarationWithoutSemicolon", "propDeclarationWithSemicolon", @@ -126,17 +110,30 @@ const classElementCompletionLocations = [ "methodImplementation", "accessorSignatureWithoutSemicolon", "accessorSignatureImplementation", - // TODO should we give completion for these keywords - //"classThatHasWrittenGetKeyword", - //"classThatHasWrittenSetKeyword", - //"classThatStartedWritingIdentifierOfGetAccessor", - //"classThatStartedWritingIdentifierOfSetAccessor", - //"classThatStartedWritingIdentifierAfterModifier", - //"classThatStartedWritingIdentifierAfterStaticModifier" + "classThatHasWrittenGetKeyword", + "classThatHasWrittenSetKeyword", + "classThatStartedWritingIdentifierOfGetAccessor", + "classThatStartedWritingIdentifierOfSetAccessor", + "classThatStartedWritingIdentifierAfterModifier", + "classThatStartedWritingIdentifierAfterStaticModifier" +]; + +const validMembersOfBase = [ + ["getValue", "(method) B.getValue(): number"], + ["protectedMethod", "(method) B.protectedMethod(): void"] +]; +const invalidMembersOfBase = [ + ["privateMethod", "(method) B.privateMethod(): void"], + ["staticMethod", "(method) B.staticMethod(): void"] ]; for (const marker of classElementCompletionLocations) { goTo.marker(marker); - verify.completionListContains("getValue", "(method) B.getValue(): number", /*documentation*/ undefined, "method"); - verifyAllowedKeyWords(); - verify.completionListCount(allowedKeywordCount + 1); + for (const [validMemberOfBaseSymbol, validMemberOfBaseText] of validMembersOfBase) { + verify.completionListContains(validMemberOfBaseSymbol, validMemberOfBaseText, /*documentation*/ undefined, "method"); + } + for (const [invalidMemberOfBaseSymbol, invalidMemberOfBaseText] of invalidMembersOfBase) { + verify.not.completionListContains(invalidMemberOfBaseSymbol, invalidMemberOfBaseText, /*documentation*/ undefined, "method"); + } + verify.completionListContainsClassElementKeywords(); + verify.completionListCount(allowedKeywordCount + validMembersOfBase.length); } \ No newline at end of file diff --git a/tests/cases/fourslash/completionListBuilderLocations_properties.ts b/tests/cases/fourslash/completionListBuilderLocations_properties.ts index 806d8c1de4f..2cdc3b7e7ba 100644 --- a/tests/cases/fourslash/completionListBuilderLocations_properties.ts +++ b/tests/cases/fourslash/completionListBuilderLocations_properties.ts @@ -10,4 +10,4 @@ //// public static a/*property2*/ ////} -goTo.eachMarker(() => verify.completionListIsEmpty()); +goTo.eachMarker(() => verify.completionListContainsClassElementKeywords()); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 374dfd33c7d..31d8b8b4ef2 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -133,11 +133,13 @@ declare namespace FourSlashInterface { class verifyNegatable { private negative; not: verifyNegatable; + allowedClassElementKeywords: string[]; constructor(negative?: boolean); completionListCount(expectedCount: number): void; completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number): void; completionListItemsCountIsGreaterThan(count: number): void; completionListIsEmpty(): void; + completionListContainsClassElementKeywords(): void; completionListAllowsNewIdentifier(): void; signatureHelpPresent(): void; errorExistsBetweenMarkers(startMarker: string, endMarker: string): void; diff --git a/tests/cases/fourslash/referencesForClassMembersExtendingGenericClass.ts b/tests/cases/fourslash/referencesForClassMembersExtendingGenericClass.ts index 3e453663f63..448c2627531 100644 --- a/tests/cases/fourslash/referencesForClassMembersExtendingGenericClass.ts +++ b/tests/cases/fourslash/referencesForClassMembersExtendingGenericClass.ts @@ -26,7 +26,7 @@ const methods = ranges.get("method"); const [m0, m1, m2] = methods; verify.referenceGroups(m0, [{ definition: "(method) Base.method(a?: T, b?: U): this", ranges: methods }]); verify.referenceGroups(m1, [ - { definition: "(method) Base.method(): void", ranges: [m0] }, + { definition: "(method) Base.method(a?: T, b?: U): this", ranges: [m0] }, { definition: "(method) MyClass.method(): void", ranges: [m1, m2] } ]); verify.referenceGroups(m2, [