Skip to content

Commit 37a2cdd

Browse files
sheetalkamatsheetalkamat
committed
Give the class element completion on typing keywords like public, private, readonly
Also when name of the function is location, make sure we are actually looking at the same symbol before using the declaration to get signature to display
1 parent b3d7936 commit 37a2cdd

File tree

7 files changed

+111
-60
lines changed

7 files changed

+111
-60
lines changed

‎src/harness/fourslash.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3417,6 +3417,17 @@ namespace FourSlashInterface {
34173417

34183418
export class VerifyNegatable {
34193419
public not: VerifyNegatable;
3420+
public allowedClassElementKeywords = [
3421+
"public",
3422+
"private",
3423+
"protected",
3424+
"static",
3425+
"abstract",
3426+
"readonly",
3427+
"get",
3428+
"set",
3429+
"constructor"
3430+
];
34203431

34213432
constructor(protected state: FourSlash.TestState, private negative = false) {
34223433
if (!negative) {
@@ -3453,6 +3464,12 @@ namespace FourSlashInterface {
34533464
this.state.verifyCompletionListIsEmpty(this.negative);
34543465
}
34553466

3467+
public completionListContainsClassElementKeywords() {
3468+
for (const keyword of this.allowedClassElementKeywords) {
3469+
this.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword");
3470+
}
3471+
}
3472+
34563473
public completionListIsGlobal(expected: boolean) {
34573474
this.state.verifyCompletionListIsGlobal(expected);
34583475
}

‎src/services/completions.ts

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,10 @@ namespace ts.Completions {
988988
return undefined;
989989
}
990990

991+
function isFromClassElementDeclaration(node: Node) {
992+
return isClassElement(node.parent) && isClassLike(node.parent.parent);
993+
}
994+
991995
/**
992996
* Returns the immediate owning class declaration of a context token,
993997
* on the condition that one exists and that the context implies completion should be given.
@@ -1010,6 +1014,13 @@ namespace ts.Completions {
10101014
return location;
10111015
}
10121016
break;
1017+
1018+
default:
1019+
if (isFromClassElementDeclaration(contextToken) &&
1020+
(isClassMemberCompletionKeyword(contextToken.kind) ||
1021+
isClassMemberCompletionKeywordText(contextToken.getText()))) {
1022+
return contextToken.parent.parent as ClassLikeDeclaration;
1023+
}
10131024
}
10141025
}
10151026

@@ -1148,7 +1159,7 @@ namespace ts.Completions {
11481159
isFunction(containingNodeKind);
11491160

11501161
case SyntaxKind.StaticKeyword:
1151-
return containingNodeKind === SyntaxKind.PropertyDeclaration;
1162+
return containingNodeKind === SyntaxKind.PropertyDeclaration && !isClassLike(contextToken.parent.parent);
11521163

11531164
case SyntaxKind.DotDotDotToken:
11541165
return containingNodeKind === SyntaxKind.Parameter ||
@@ -1165,13 +1176,17 @@ namespace ts.Completions {
11651176
containingNodeKind === SyntaxKind.ExportSpecifier ||
11661177
containingNodeKind === SyntaxKind.NamespaceImport;
11671178

1179+
case SyntaxKind.GetKeyword:
1180+
case SyntaxKind.SetKeyword:
1181+
if (isFromClassElementDeclaration(contextToken)) {
1182+
return false;
1183+
}
1184+
// falls through
11681185
case SyntaxKind.ClassKeyword:
11691186
case SyntaxKind.EnumKeyword:
11701187
case SyntaxKind.InterfaceKeyword:
11711188
case SyntaxKind.FunctionKeyword:
11721189
case SyntaxKind.VarKeyword:
1173-
case SyntaxKind.GetKeyword:
1174-
case SyntaxKind.SetKeyword:
11751190
case SyntaxKind.ImportKeyword:
11761191
case SyntaxKind.LetKeyword:
11771192
case SyntaxKind.ConstKeyword:
@@ -1180,6 +1195,13 @@ namespace ts.Completions {
11801195
return true;
11811196
}
11821197

1198+
// If the previous token is keyword correspoding to class member completion keyword
1199+
// there will be completion available here
1200+
if (isClassMemberCompletionKeywordText(contextToken.getText()) &&
1201+
isFromClassElementDeclaration(contextToken)) {
1202+
return false;
1203+
}
1204+
11831205
// Previous token may have been a keyword that was converted to an identifier.
11841206
switch (contextToken.getText()) {
11851207
case "abstract":
@@ -1373,8 +1395,8 @@ namespace ts.Completions {
13731395
});
13741396
}
13751397

1376-
const classMemberKeywordCompletions = filter(keywordCompletions, entry => {
1377-
switch (stringToToken(entry.name)) {
1398+
function isClassMemberCompletionKeyword(kind: SyntaxKind) {
1399+
switch (kind) {
13781400
case SyntaxKind.PublicKeyword:
13791401
case SyntaxKind.ProtectedKeyword:
13801402
case SyntaxKind.PrivateKeyword:
@@ -1386,7 +1408,14 @@ namespace ts.Completions {
13861408
case SyntaxKind.SetKeyword:
13871409
return true;
13881410
}
1389-
});
1411+
}
1412+
1413+
function isClassMemberCompletionKeywordText(text: string) {
1414+
return isClassMemberCompletionKeyword(stringToToken(text));
1415+
}
1416+
1417+
const classMemberKeywordCompletions = filter(keywordCompletions, entry =>
1418+
isClassMemberCompletionKeywordText(entry.name));
13901419

13911420
function isEqualityExpression(node: Node): node is BinaryExpression {
13921421
return isBinaryExpression(node) && isEqualityOperatorKind(node.operatorToken.kind);

‎src/services/symbolDisplay.ts

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -200,27 +200,33 @@ namespace ts.SymbolDisplay {
200200
(location.kind === SyntaxKind.ConstructorKeyword && location.parent.kind === SyntaxKind.Constructor)) { // At constructor keyword of constructor declaration
201201
// get the signature from the declaration and write it
202202
const functionDeclaration = <FunctionLikeDeclaration>location.parent;
203-
const allSignatures = functionDeclaration.kind === SyntaxKind.Constructor ? type.getNonNullableType().getConstructSignatures() : type.getNonNullableType().getCallSignatures();
204-
if (!typeChecker.isImplementationOfOverload(functionDeclaration)) {
205-
signature = typeChecker.getSignatureFromDeclaration(functionDeclaration);
206-
}
207-
else {
208-
signature = allSignatures[0];
209-
}
203+
// Use function declaration to write the signatures only if the symbol corresponding to this declaration
204+
const locationIsSymbolDeclaration = findDeclaration(symbol, declaration =>
205+
declaration === (location.kind === SyntaxKind.ConstructorKeyword ? functionDeclaration.parent : functionDeclaration));
206+
207+
if (locationIsSymbolDeclaration) {
208+
const allSignatures = functionDeclaration.kind === SyntaxKind.Constructor ? type.getNonNullableType().getConstructSignatures() : type.getNonNullableType().getCallSignatures();
209+
if (!typeChecker.isImplementationOfOverload(functionDeclaration)) {
210+
signature = typeChecker.getSignatureFromDeclaration(functionDeclaration);
211+
}
212+
else {
213+
signature = allSignatures[0];
214+
}
210215

211-
if (functionDeclaration.kind === SyntaxKind.Constructor) {
212-
// show (constructor) Type(...) signature
213-
symbolKind = ScriptElementKind.constructorImplementationElement;
214-
addPrefixForAnyFunctionOrVar(type.symbol, symbolKind);
215-
}
216-
else {
217-
// (function/method) symbol(..signature)
218-
addPrefixForAnyFunctionOrVar(functionDeclaration.kind === SyntaxKind.CallSignature &&
219-
!(type.symbol.flags & SymbolFlags.TypeLiteral || type.symbol.flags & SymbolFlags.ObjectLiteral) ? type.symbol : symbol, symbolKind);
220-
}
216+
if (functionDeclaration.kind === SyntaxKind.Constructor) {
217+
// show (constructor) Type(...) signature
218+
symbolKind = ScriptElementKind.constructorImplementationElement;
219+
addPrefixForAnyFunctionOrVar(type.symbol, symbolKind);
220+
}
221+
else {
222+
// (function/method) symbol(..signature)
223+
addPrefixForAnyFunctionOrVar(functionDeclaration.kind === SyntaxKind.CallSignature &&
224+
!(type.symbol.flags & SymbolFlags.TypeLiteral || type.symbol.flags & SymbolFlags.ObjectLiteral) ? type.symbol : symbol, symbolKind);
225+
}
221226

222-
addSignatureDisplayParts(signature, allSignatures);
223-
hasAddedSymbolInfo = true;
227+
addSignatureDisplayParts(signature, allSignatures);
228+
hasAddedSymbolInfo = true;
229+
}
224230
}
225231
}
226232
}

‎tests/cases/fourslash/completionEntryForClassMembers.ts

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
///<reference path="fourslash.ts" />
22

33
////abstract class B {
4+
//// private privateMethod() { }
5+
//// protected protectedMethod() { };
6+
//// static staticMethod() { }
47
//// abstract getValue(): number;
58
//// /*abstractClass*/
69
////}
@@ -69,25 +72,7 @@
6972
//// static identi/*classThatStartedWritingIdentifierAfterStaticModifier*/
7073
////}
7174

72-
const allowedKeywords = [
73-
"public",
74-
"private",
75-
"protected",
76-
"static",
77-
"abstract",
78-
"readonly",
79-
"get",
80-
"set",
81-
"constructor"
82-
];
83-
84-
const allowedKeywordCount = allowedKeywords.length;
85-
function verifyAllowedKeyWords() {
86-
for (const keyword of allowedKeywords) {
87-
verify.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword");
88-
}
89-
}
90-
75+
const allowedKeywordCount = verify.allowedClassElementKeywords.length;
9176
const nonClassElementMarkers = [
9277
"InsideMethod"
9378
];
@@ -104,7 +89,7 @@ const onlyClassElementKeywordLocations = [
10489
];
10590
for (const marker of onlyClassElementKeywordLocations) {
10691
goTo.marker(marker);
107-
verifyAllowedKeyWords();
92+
verify.completionListContainsClassElementKeywords();
10893
verify.completionListCount(allowedKeywordCount);
10994
}
11095

@@ -113,9 +98,8 @@ const classElementCompletionLocations = [
11398
"classThatIsEmptyAndExtendingAnotherClass",
11499
"classThatHasAlreadyImplementedAnotherClassMethod",
115100
"classThatHasAlreadyImplementedAnotherClassMethodAfterMethod",
116-
// TODO should we give completion for these keywords
117-
//"classThatHasWrittenPublicKeyword",
118-
//"classElementContainingStatic",
101+
"classThatHasWrittenPublicKeyword",
102+
"classElementContainingStatic",
119103
"classThatStartedWritingIdentifier",
120104
"propDeclarationWithoutSemicolon",
121105
"propDeclarationWithSemicolon",
@@ -126,17 +110,30 @@ const classElementCompletionLocations = [
126110
"methodImplementation",
127111
"accessorSignatureWithoutSemicolon",
128112
"accessorSignatureImplementation",
129-
// TODO should we give completion for these keywords
130-
//"classThatHasWrittenGetKeyword",
131-
//"classThatHasWrittenSetKeyword",
132-
//"classThatStartedWritingIdentifierOfGetAccessor",
133-
//"classThatStartedWritingIdentifierOfSetAccessor",
134-
//"classThatStartedWritingIdentifierAfterModifier",
135-
//"classThatStartedWritingIdentifierAfterStaticModifier"
113+
"classThatHasWrittenGetKeyword",
114+
"classThatHasWrittenSetKeyword",
115+
"classThatStartedWritingIdentifierOfGetAccessor",
116+
"classThatStartedWritingIdentifierOfSetAccessor",
117+
"classThatStartedWritingIdentifierAfterModifier",
118+
"classThatStartedWritingIdentifierAfterStaticModifier"
119+
];
120+
121+
const validMembersOfBase = [
122+
["getValue", "(method) B.getValue(): number"],
123+
["protectedMethod", "(method) B.protectedMethod(): void"]
124+
];
125+
const invalidMembersOfBase = [
126+
["privateMethod", "(method) B.privateMethod(): void"],
127+
["staticMethod", "(method) B.staticMethod(): void"]
136128
];
137129
for (const marker of classElementCompletionLocations) {
138130
goTo.marker(marker);
139-
verify.completionListContains("getValue", "(method) B.getValue(): number", /*documentation*/ undefined, "method");
140-
verifyAllowedKeyWords();
141-
verify.completionListCount(allowedKeywordCount + 1);
131+
for (const [validMemberOfBaseSymbol, validMemberOfBaseText] of validMembersOfBase) {
132+
verify.completionListContains(validMemberOfBaseSymbol, validMemberOfBaseText, /*documentation*/ undefined, "method");
133+
}
134+
for (const [invalidMemberOfBaseSymbol, invalidMemberOfBaseText] of invalidMembersOfBase) {
135+
verify.not.completionListContains(invalidMemberOfBaseSymbol, invalidMemberOfBaseText, /*documentation*/ undefined, "method");
136+
}
137+
verify.completionListContainsClassElementKeywords();
138+
verify.completionListCount(allowedKeywordCount + validMembersOfBase.length);
142139
}

‎tests/cases/fourslash/completionListBuilderLocations_properties.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
//// public static a/*property2*/
1111
////}
1212

13-
goTo.eachMarker(() => verify.completionListIsEmpty());
13+
goTo.eachMarker(() => verify.completionListContainsClassElementKeywords());

‎tests/cases/fourslash/fourslash.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,13 @@ declare namespace FourSlashInterface {
133133
class verifyNegatable {
134134
private negative;
135135
not: verifyNegatable;
136+
allowedClassElementKeywords: string[];
136137
constructor(negative?: boolean);
137138
completionListCount(expectedCount: number): void;
138139
completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number): void;
139140
completionListItemsCountIsGreaterThan(count: number): void;
140141
completionListIsEmpty(): void;
142+
completionListContainsClassElementKeywords(): void;
141143
completionListAllowsNewIdentifier(): void;
142144
signatureHelpPresent(): void;
143145
errorExistsBetweenMarkers(startMarker: string, endMarker: string): void;

‎tests/cases/fourslash/referencesForClassMembersExtendingGenericClass.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const methods = ranges.get("method");
2626
const [m0, m1, m2] = methods;
2727
verify.referenceGroups(m0, [{ definition: "(method) Base<T>.method<U>(a?: T, b?: U): this", ranges: methods }]);
2828
verify.referenceGroups(m1, [
29-
{ definition: "(method) Base<T>.method(): void", ranges: [m0] },
29+
{ definition: "(method) Base<T>.method<U>(a?: T, b?: U): this", ranges: [m0] },
3030
{ definition: "(method) MyClass.method(): void", ranges: [m1, m2] }
3131
]);
3232
verify.referenceGroups(m2, [

0 commit comments

Comments
 (0)