Skip to content

Commit 31653de

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request microsoft#16566 from Microsoft/keywordFilters
Fix the completion for parameters
2 parents 4b3e661 + 4ce8af3 commit 31653de

7 files changed

Lines changed: 269 additions & 36 deletions

‎src/harness/fourslash.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ namespace FourSlash {
694694

695695
public verifyCompletionListItemsCountIsGreaterThan(count: number, negative: boolean) {
696696
const completions = this.getCompletionListAtCaret();
697-
const itemsCount = completions.entries.length;
697+
const itemsCount = completions ? completions.entries.length : 0;
698698

699699
if (negative) {
700700
if (itemsCount > count) {
@@ -3521,6 +3521,12 @@ namespace FourSlashInterface {
35213521
"constructor",
35223522
"async"
35233523
];
3524+
public allowedConstructorParameterKeywords = [
3525+
"public",
3526+
"private",
3527+
"protected",
3528+
"readonly",
3529+
];
35243530

35253531
constructor(protected state: FourSlash.TestState, private negative = false) {
35263532
if (!negative) {
@@ -3563,6 +3569,12 @@ namespace FourSlashInterface {
35633569
}
35643570
}
35653571

3572+
public completionListContainsConstructorParameterKeywords() {
3573+
for (const keyword of this.allowedConstructorParameterKeywords) {
3574+
this.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword");
3575+
}
3576+
}
3577+
35663578
public completionListIsGlobal(expected: boolean) {
35673579
this.state.verifyCompletionListIsGlobal(expected);
35683580
}

‎src/services/completions.ts‎

Lines changed: 143 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
namespace ts.Completions {
55
export type Log = (message: string) => void;
66

7+
const enum KeywordCompletionFilters {
8+
None,
9+
ClassElementKeywords, // Keywords at class keyword
10+
ConstructorParameterKeywords, // Keywords at constructor parameter
11+
}
12+
713
export function getCompletionsAtPosition(host: LanguageServiceHost, typeChecker: TypeChecker, log: Log, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number): CompletionInfo | undefined {
814
if (isInReferenceComment(sourceFile, position)) {
915
return PathCompletions.getTripleSlashReferenceCompletion(sourceFile, position, compilerOptions, host);
@@ -18,7 +24,7 @@ namespace ts.Completions {
1824
return undefined;
1925
}
2026

21-
const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, hasFilteredClassMemberKeywords } = completionData;
27+
const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, keywordFilters } = completionData;
2228

2329
if (sourceFile.languageVariant === LanguageVariant.JSX &&
2430
location && location.parent && location.parent.kind === SyntaxKind.JsxClosingElement) {
@@ -54,21 +60,20 @@ namespace ts.Completions {
5460
addRange(entries, getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target));
5561
}
5662
else {
57-
if (!symbols || symbols.length === 0) {
58-
if (!hasFilteredClassMemberKeywords) {
63+
if ((!symbols || symbols.length === 0) && keywordFilters === KeywordCompletionFilters.None) {
5964
return undefined;
60-
}
6165
}
6266

6367
getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log);
6468
}
6569

66-
if (hasFilteredClassMemberKeywords) {
67-
addRange(entries, classMemberKeywordCompletions);
68-
}
69-
// Add keywords if this is not a member completion list
70-
else if (!isMemberCompletion) {
71-
addRange(entries, keywordCompletions);
70+
// TODO add filter for keyword based on type/value/namespace and also location
71+
72+
// Add all keywords if
73+
// - this is not a member completion list (all the keywords)
74+
// - other filters are enabled in required scenario so add those keywords
75+
if (keywordFilters !== KeywordCompletionFilters.None || !isMemberCompletion) {
76+
addRange(entries, getKeywordCompletions(keywordFilters));
7277
}
7378

7479
return { isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation: isNewIdentifierLocation, entries };
@@ -317,7 +322,10 @@ namespace ts.Completions {
317322
}
318323

319324
// Didn't find a symbol with this name. See if we can find a keyword instead.
320-
const keywordCompletion = forEach(keywordCompletions, c => c.name === entryName);
325+
const keywordCompletion = forEach(
326+
getKeywordCompletions(KeywordCompletionFilters.None),
327+
c => c.name === entryName
328+
);
321329
if (keywordCompletion) {
322330
return {
323331
name: entryName,
@@ -356,7 +364,7 @@ namespace ts.Completions {
356364
location: Node;
357365
isRightOfDot: boolean;
358366
request?: Request;
359-
hasFilteredClassMemberKeywords: boolean;
367+
keywordFilters: KeywordCompletionFilters;
360368
}
361369
type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag };
362370

@@ -432,7 +440,7 @@ namespace ts.Completions {
432440
}
433441

434442
if (request) {
435-
return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, hasFilteredClassMemberKeywords: false };
443+
return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, keywordFilters: KeywordCompletionFilters.None };
436444
}
437445

438446
if (!insideJsDocTagTypeExpression) {
@@ -531,7 +539,7 @@ namespace ts.Completions {
531539
let isGlobalCompletion = false;
532540
let isMemberCompletion: boolean;
533541
let isNewIdentifierLocation: boolean;
534-
let hasFilteredClassMemberKeywords = false;
542+
let keywordFilters = KeywordCompletionFilters.None;
535543
let symbols: Symbol[] = [];
536544

537545
if (isRightOfDot) {
@@ -569,7 +577,7 @@ namespace ts.Completions {
569577

570578
log("getCompletionData: Semantic work: " + (timestamp() - semanticStart));
571579

572-
return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, hasFilteredClassMemberKeywords };
580+
return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters };
573581

574582
type JSDocTagWithTypeExpression = JSDocAugmentsTag | JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag;
575583

@@ -664,6 +672,16 @@ namespace ts.Completions {
664672
return tryGetImportOrExportClauseCompletionSymbols(namedImportsOrExports);
665673
}
666674

675+
if (tryGetConstructorLikeCompletionContainer(contextToken)) {
676+
// no members, only keywords
677+
isMemberCompletion = false;
678+
// Declaring new property/method/accessor
679+
isNewIdentifierLocation = true;
680+
// Has keywords for constructor parameter
681+
keywordFilters = KeywordCompletionFilters.ConstructorParameterKeywords;
682+
return true;
683+
}
684+
667685
if (classLikeContainer = tryGetClassLikeCompletionContainer(contextToken)) {
668686
// cursor inside class declaration
669687
getGetClassLikeCompletionSymbols(classLikeContainer);
@@ -1046,7 +1064,7 @@ namespace ts.Completions {
10461064
// Declaring new property/method/accessor
10471065
isNewIdentifierLocation = true;
10481066
// Has keywords for class elements
1049-
hasFilteredClassMemberKeywords = true;
1067+
keywordFilters = KeywordCompletionFilters.ClassElementKeywords;
10501068

10511069
const baseTypeNode = getClassExtendsHeritageClauseElement(classLikeDeclaration);
10521070
const implementsTypeNodes = getClassImplementsHeritageClauseElements(classLikeDeclaration);
@@ -1136,6 +1154,16 @@ namespace ts.Completions {
11361154
return isClassElement(node.parent) && isClassLike(node.parent.parent);
11371155
}
11381156

1157+
function isParameterOfConstructorDeclaration(node: Node) {
1158+
return isParameter(node) && isConstructorDeclaration(node.parent);
1159+
}
1160+
1161+
function isConstructorParameterCompletion(node: Node) {
1162+
return node.parent &&
1163+
isParameterOfConstructorDeclaration(node.parent) &&
1164+
(isConstructorParameterCompletionKeyword(node.kind) || isDeclarationName(node));
1165+
}
1166+
11391167
/**
11401168
* Returns the immediate owning class declaration of a context token,
11411169
* on the condition that one exists and that the context implies completion should be given.
@@ -1149,8 +1177,14 @@ namespace ts.Completions {
11491177
}
11501178
break;
11511179

1152-
// class c {getValue(): number; | }
1180+
// class c {getValue(): number, | }
11531181
case SyntaxKind.CommaToken:
1182+
if (isClassLike(contextToken.parent)) {
1183+
return contextToken.parent;
1184+
}
1185+
break;
1186+
1187+
// class c {getValue(): number; | }
11541188
case SyntaxKind.SemicolonToken:
11551189
// class c { method() { } | }
11561190
case SyntaxKind.CloseBraceToken:
@@ -1175,6 +1209,26 @@ namespace ts.Completions {
11751209
return undefined;
11761210
}
11771211

1212+
/**
1213+
* Returns the immediate owning class declaration of a context token,
1214+
* on the condition that one exists and that the context implies completion should be given.
1215+
*/
1216+
function tryGetConstructorLikeCompletionContainer(contextToken: Node): ConstructorDeclaration {
1217+
if (contextToken) {
1218+
switch (contextToken.kind) {
1219+
case SyntaxKind.OpenParenToken:
1220+
case SyntaxKind.CommaToken:
1221+
return isConstructorDeclaration(contextToken.parent) && contextToken.parent;
1222+
1223+
default:
1224+
if (isConstructorParameterCompletion(contextToken)) {
1225+
return contextToken.parent.parent as ConstructorDeclaration;
1226+
}
1227+
}
1228+
}
1229+
return undefined;
1230+
}
1231+
11781232
function tryGetContainingJsxElement(contextToken: Node): JsxOpeningLikeElement {
11791233
if (contextToken) {
11801234
const parent = contextToken.parent;
@@ -1250,11 +1304,14 @@ namespace ts.Completions {
12501304
containingNodeKind === SyntaxKind.VariableStatement ||
12511305
containingNodeKind === SyntaxKind.EnumDeclaration || // enum a { foo, |
12521306
isFunctionLikeButNotConstructor(containingNodeKind) ||
1253-
containingNodeKind === SyntaxKind.ClassDeclaration || // class A<T, |
1254-
containingNodeKind === SyntaxKind.ClassExpression || // var C = class D<T, |
12551307
containingNodeKind === SyntaxKind.InterfaceDeclaration || // interface A<T, |
12561308
containingNodeKind === SyntaxKind.ArrayBindingPattern || // var [x, y|
1257-
containingNodeKind === SyntaxKind.TypeAliasDeclaration; // type Map, K, |
1309+
containingNodeKind === SyntaxKind.TypeAliasDeclaration || // type Map, K, |
1310+
// class A<T, |
1311+
// var C = class D<T, |
1312+
(isClassLike(contextToken.parent) &&
1313+
contextToken.parent.typeParameters &&
1314+
contextToken.parent.typeParameters.end >= contextToken.pos);
12581315

12591316
case SyntaxKind.DotToken:
12601317
return containingNodeKind === SyntaxKind.ArrayBindingPattern; // var [.|
@@ -1298,7 +1355,7 @@ namespace ts.Completions {
12981355
case SyntaxKind.PublicKeyword:
12991356
case SyntaxKind.PrivateKeyword:
13001357
case SyntaxKind.ProtectedKeyword:
1301-
return containingNodeKind === SyntaxKind.Parameter;
1358+
return containingNodeKind === SyntaxKind.Parameter && !isConstructorDeclaration(contextToken.parent.parent);
13021359

13031360
case SyntaxKind.AsKeyword:
13041361
return containingNodeKind === SyntaxKind.ImportSpecifier ||
@@ -1331,6 +1388,18 @@ namespace ts.Completions {
13311388
return false;
13321389
}
13331390

1391+
if (isConstructorParameterCompletion(contextToken)) {
1392+
// constructor parameter completion is available only if
1393+
// - its modifier of the constructor parameter or
1394+
// - its name of the parameter and not being edited
1395+
// eg. constructor(a |<- this shouldnt show completion
1396+
if (!isIdentifier(contextToken) ||
1397+
isConstructorParameterCompletionKeywordText(contextToken.getText()) ||
1398+
isCurrentlyEditingNode(contextToken)) {
1399+
return false;
1400+
}
1401+
}
1402+
13341403
// Previous token may have been a keyword that was converted to an identifier.
13351404
switch (contextToken.getText()) {
13361405
case "abstract":
@@ -1351,7 +1420,7 @@ namespace ts.Completions {
13511420
return true;
13521421
}
13531422

1354-
return false;
1423+
return isDeclarationName(contextToken) && !isJsxAttribute(contextToken.parent);
13551424
}
13561425

13571426
function isFunctionLikeButNotConstructor(kind: SyntaxKind) {
@@ -1574,14 +1643,45 @@ namespace ts.Completions {
15741643
}
15751644

15761645
// A cache of completion entries for keywords, these do not change between sessions
1577-
const keywordCompletions: CompletionEntry[] = [];
1578-
for (let i = SyntaxKind.FirstKeyword; i <= SyntaxKind.LastKeyword; i++) {
1579-
keywordCompletions.push({
1580-
name: tokenToString(i),
1581-
kind: ScriptElementKind.keyword,
1582-
kindModifiers: ScriptElementKindModifier.none,
1583-
sortText: "0"
1584-
});
1646+
const _keywordCompletions: CompletionEntry[][] = [];
1647+
function getKeywordCompletions(keywordFilter: KeywordCompletionFilters): CompletionEntry[] {
1648+
const completions = _keywordCompletions[keywordFilter];
1649+
if (completions) {
1650+
return completions;
1651+
}
1652+
return _keywordCompletions[keywordFilter] = generateKeywordCompletions(keywordFilter);
1653+
1654+
type FilterKeywordCompletions = (entryName: string) => boolean;
1655+
function generateKeywordCompletions(keywordFilter: KeywordCompletionFilters) {
1656+
switch (keywordFilter) {
1657+
case KeywordCompletionFilters.None:
1658+
return getAllKeywordCompletions();
1659+
case KeywordCompletionFilters.ClassElementKeywords:
1660+
return getFilteredKeywordCompletions(isClassMemberCompletionKeywordText);
1661+
case KeywordCompletionFilters.ConstructorParameterKeywords:
1662+
return getFilteredKeywordCompletions(isConstructorParameterCompletionKeywordText);
1663+
}
1664+
}
1665+
1666+
function getAllKeywordCompletions() {
1667+
const allKeywordsCompletions: CompletionEntry[] = [];
1668+
for (let i = SyntaxKind.FirstKeyword; i <= SyntaxKind.LastKeyword; i++) {
1669+
allKeywordsCompletions.push({
1670+
name: tokenToString(i),
1671+
kind: ScriptElementKind.keyword,
1672+
kindModifiers: ScriptElementKindModifier.none,
1673+
sortText: "0"
1674+
});
1675+
}
1676+
return allKeywordsCompletions;
1677+
}
1678+
1679+
function getFilteredKeywordCompletions(filterFn: FilterKeywordCompletions) {
1680+
return filter(
1681+
getKeywordCompletions(KeywordCompletionFilters.None),
1682+
entry => filterFn(entry.name)
1683+
);
1684+
}
15851685
}
15861686

15871687
function isClassMemberCompletionKeyword(kind: SyntaxKind) {
@@ -1604,8 +1704,19 @@ namespace ts.Completions {
16041704
return isClassMemberCompletionKeyword(stringToToken(text));
16051705
}
16061706

1607-
const classMemberKeywordCompletions = filter(keywordCompletions, entry =>
1608-
isClassMemberCompletionKeywordText(entry.name));
1707+
function isConstructorParameterCompletionKeyword(kind: SyntaxKind) {
1708+
switch (kind) {
1709+
case SyntaxKind.PublicKeyword:
1710+
case SyntaxKind.PrivateKeyword:
1711+
case SyntaxKind.ProtectedKeyword:
1712+
case SyntaxKind.ReadonlyKeyword:
1713+
return true;
1714+
}
1715+
}
1716+
1717+
function isConstructorParameterCompletionKeywordText(text: string) {
1718+
return isConstructorParameterCompletionKeyword(stringToToken(text));
1719+
}
16091720

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

‎tests/cases/fourslash/completionEntryForClassMembers.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@
112112
////class N extends B {
113113
//// async /*classThatHasWrittenAsyncKeyword*/
114114
////}
115+
////class O extends B {
116+
//// constructor(public a) {
117+
//// },
118+
//// /*classElementAfterConstructorSeparatedByComma*/
119+
////}
115120

116121
const allowedKeywordCount = verify.allowedClassElementKeywords.length;
117122
type CompletionInfo = [string, string];
@@ -220,7 +225,8 @@ const classInstanceElementLocations = [
220225
"classThatStartedWritingIdentifierOfGetAccessor",
221226
"classThatStartedWritingIdentifierOfSetAccessor",
222227
"classThatStartedWritingIdentifierAfterModifier",
223-
"classThatHasWrittenAsyncKeyword"
228+
"classThatHasWrittenAsyncKeyword",
229+
"classElementAfterConstructorSeparatedByComma"
224230
];
225231
verifyClassElementLocations(instanceMemberInfo, classInstanceElementLocations);
226232

0 commit comments

Comments
 (0)