Skip to content

Commit fa2b7ff

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request microsoft#22421 from Microsoft/typesInTypeArguments
Allow types as well as values in possibly type argument location
2 parents ef64cde + 73947f8 commit fa2b7ff

File tree

5 files changed

+168
-7
lines changed

5 files changed

+168
-7
lines changed

‎src/harness/fourslash.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ namespace FourSlash {
435435
}
436436
}
437437

438-
private markerName(m: Marker): string {
438+
public markerName(m: Marker): string {
439439
return ts.forEachEntry(this.testData.markerPositions, (marker, name) => {
440440
if (marker === m) {
441441
return name;
@@ -3768,6 +3768,10 @@ namespace FourSlashInterface {
37683768
return this.state.getMarkerByName(name);
37693769
}
37703770

3771+
public markerName(m: FourSlash.Marker) {
3772+
return this.state.markerName(m);
3773+
}
3774+
37713775
public ranges(): FourSlash.Range[] {
37723776
return this.state.getRanges();
37733777
}
@@ -3810,6 +3814,7 @@ namespace FourSlashInterface {
38103814
this.state.goToEachMarker(markers, typeof a === "function" ? a : b);
38113815
}
38123816

3817+
38133818
public rangeStart(range: FourSlash.Range) {
38143819
this.state.goToRangeStart(range);
38153820
}

‎src/services/completions.ts

+11-6
Original file line numberDiff line numberDiff line change
@@ -994,6 +994,7 @@ namespace ts.Completions {
994994
// Since this is qualified name check its a type node location
995995
const isTypeLocation = insideJsDocTagTypeExpression || isPartOfTypeNode(node.parent);
996996
const isRhsOfImportDeclaration = isInRightSideOfInternalImportEqualsDeclaration(node);
997+
const allowTypeOrValue = isRhsOfImportDeclaration || (!isTypeLocation && isPossiblyTypeArgumentPosition(contextToken, sourceFile));
997998
if (isEntityName(node)) {
998999
let symbol = typeChecker.getSymbolAtLocation(node);
9991000
if (symbol) {
@@ -1004,7 +1005,7 @@ namespace ts.Completions {
10041005
const exportedSymbols = Debug.assertEachDefined(typeChecker.getExportsOfModule(symbol), "getExportsOfModule() should all be defined");
10051006
const isValidValueAccess = (symbol: Symbol) => typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name);
10061007
const isValidTypeAccess = (symbol: Symbol) => symbolCanBeReferencedAtTypeLocation(symbol);
1007-
const isValidAccess = isRhsOfImportDeclaration ?
1008+
const isValidAccess = allowTypeOrValue ?
10081009
// Any kind is allowed when dotting off namespace in internal import equals declaration
10091010
(symbol: Symbol) => isValidTypeAccess(symbol) || isValidValueAccess(symbol) :
10101011
isTypeLocation ? isValidTypeAccess : isValidValueAccess;
@@ -1173,8 +1174,9 @@ namespace ts.Completions {
11731174
}
11741175

11751176
function filterGlobalCompletion(symbols: Symbol[]): void {
1176-
const isTypeCompletion = insideJsDocTagTypeExpression || !isContextTokenValueLocation(contextToken) && (isPartOfTypeNode(location) || isContextTokenTypeLocation(contextToken));
1177-
if (isTypeCompletion) keywordFilters = KeywordCompletionFilters.TypeKeywords;
1177+
const isTypeOnlyCompletion = insideJsDocTagTypeExpression || !isContextTokenValueLocation(contextToken) && (isPartOfTypeNode(location) || isContextTokenTypeLocation(contextToken));
1178+
const allowTypes = isTypeOnlyCompletion || !isContextTokenValueLocation(contextToken) && isPossiblyTypeArgumentPosition(contextToken, sourceFile);
1179+
if (isTypeOnlyCompletion) keywordFilters = KeywordCompletionFilters.TypeKeywords;
11781180

11791181
filterMutate(symbols, symbol => {
11801182
if (!isSourceFile(location)) {
@@ -1190,9 +1192,12 @@ namespace ts.Completions {
11901192
return !!(symbol.flags & SymbolFlags.Namespace);
11911193
}
11921194

1193-
if (isTypeCompletion) {
1195+
if (allowTypes) {
11941196
// Its a type, but you can reach it by namespace.type as well
1195-
return symbolCanBeReferencedAtTypeLocation(symbol);
1197+
const symbolAllowedAsType = symbolCanBeReferencedAtTypeLocation(symbol);
1198+
if (symbolAllowedAsType || isTypeOnlyCompletion) {
1199+
return symbolAllowedAsType;
1200+
}
11961201
}
11971202
}
11981203

@@ -1204,7 +1209,7 @@ namespace ts.Completions {
12041209
function isContextTokenValueLocation(contextToken: Node) {
12051210
return contextToken &&
12061211
contextToken.kind === SyntaxKind.TypeOfKeyword &&
1207-
contextToken.parent.kind === SyntaxKind.TypeQuery;
1212+
(contextToken.parent.kind === SyntaxKind.TypeQuery || isTypeOfExpression(contextToken.parent));
12081213
}
12091214

12101215
function isContextTokenTypeLocation(contextToken: Node): boolean {

‎src/services/utilities.ts

+108
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,114 @@ namespace ts {
890890
return isTemplateLiteralKind(token.kind) && position > token.getStart(sourceFile);
891891
}
892892

893+
export function findPrecedingMatchingToken(token: Node, matchingTokenKind: SyntaxKind, sourceFile: SourceFile) {
894+
const tokenKind = token.kind;
895+
let remainingMatchingTokens = 0;
896+
while (true) {
897+
token = findPrecedingToken(token.getFullStart(), sourceFile);
898+
if (!token) {
899+
return undefined;
900+
}
901+
902+
if (token.kind === matchingTokenKind) {
903+
if (remainingMatchingTokens === 0) {
904+
return token;
905+
}
906+
907+
remainingMatchingTokens--;
908+
}
909+
else if (token.kind === tokenKind) {
910+
remainingMatchingTokens++;
911+
}
912+
}
913+
}
914+
915+
export function isPossiblyTypeArgumentPosition(token: Node, sourceFile: SourceFile) {
916+
// This function determines if the node could be type argument position
917+
// Since during editing, when type argument list is not complete,
918+
// the tree could be of any shape depending on the tokens parsed before current node,
919+
// scanning of the previous identifier followed by "<" before current node would give us better result
920+
// Note that we also balance out the already provided type arguments, arrays, object literals while doing so
921+
let remainingLessThanTokens = 0;
922+
while (token) {
923+
switch (token.kind) {
924+
case SyntaxKind.LessThanToken:
925+
// Found the beginning of the generic argument expression
926+
token = findPrecedingToken(token.getFullStart(), sourceFile);
927+
const tokenIsIdentifier = token && isIdentifier(token);
928+
if (!remainingLessThanTokens || !tokenIsIdentifier) {
929+
return tokenIsIdentifier;
930+
}
931+
remainingLessThanTokens--;
932+
break;
933+
934+
case SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
935+
remainingLessThanTokens = + 3;
936+
break;
937+
938+
case SyntaxKind.GreaterThanGreaterThanToken:
939+
remainingLessThanTokens = + 2;
940+
break;
941+
942+
case SyntaxKind.GreaterThanToken:
943+
remainingLessThanTokens++;
944+
break;
945+
946+
case SyntaxKind.CloseBraceToken:
947+
// This can be object type, skip untill we find the matching open brace token
948+
// Skip untill the matching open brace token
949+
token = findPrecedingMatchingToken(token, SyntaxKind.OpenBraceToken, sourceFile);
950+
if (!token) return false;
951+
break;
952+
953+
case SyntaxKind.CloseParenToken:
954+
// This can be object type, skip untill we find the matching open brace token
955+
// Skip untill the matching open brace token
956+
token = findPrecedingMatchingToken(token, SyntaxKind.OpenParenToken, sourceFile);
957+
if (!token) return false;
958+
break;
959+
960+
case SyntaxKind.CloseBracketToken:
961+
// This can be object type, skip untill we find the matching open brace token
962+
// Skip untill the matching open brace token
963+
token = findPrecedingMatchingToken(token, SyntaxKind.OpenBracketToken, sourceFile);
964+
if (!token) return false;
965+
break;
966+
967+
// Valid tokens in a type name. Skip.
968+
case SyntaxKind.CommaToken:
969+
case SyntaxKind.EqualsGreaterThanToken:
970+
971+
case SyntaxKind.Identifier:
972+
case SyntaxKind.StringLiteral:
973+
case SyntaxKind.NumericLiteral:
974+
case SyntaxKind.TrueKeyword:
975+
case SyntaxKind.FalseKeyword:
976+
977+
case SyntaxKind.TypeOfKeyword:
978+
case SyntaxKind.ExtendsKeyword:
979+
case SyntaxKind.KeyOfKeyword:
980+
case SyntaxKind.DotToken:
981+
case SyntaxKind.BarToken:
982+
case SyntaxKind.QuestionToken:
983+
case SyntaxKind.ColonToken:
984+
break;
985+
986+
default:
987+
if (isTypeNode(token)) {
988+
break;
989+
}
990+
991+
// Invalid token in type
992+
return false;
993+
}
994+
995+
token = findPrecedingToken(token.getFullStart(), sourceFile);
996+
}
997+
998+
return false;
999+
}
1000+
8931001
/**
8941002
* Returns true if the cursor at position in sourceFile is within a comment.
8951003
*

‎tests/cases/fourslash/completionListInUnclosedTypeArguments.ts

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////let x = 10;
4+
////type Type = void;
5+
////declare function f<T>(): void;
6+
////declare function f2<T, U>(): void;
7+
////f</*1a*/T/*2a*/y/*3a*/
8+
////f</*1b*/T/*2b*/y/*3b*/;
9+
////f</*1c*/T/*2c*/y/*3c*/>
10+
////f</*1d*/T/*2d*/y/*3d*/>
11+
////f</*1eTypeOnly*/T/*2eTypeOnly*/y/*3eTypeOnly*/>();
12+
////
13+
////f2</*1k*/T/*2k*/y/*3k*/,
14+
////f2</*1l*/T/*2l*/y/*3l*/,/*4l*/T/*5l*/y/*6l*/
15+
////f2</*1m*/T/*2m*/y/*3m*/,/*4m*/T/*5m*/y/*6m*/;
16+
////f2</*1n*/T/*2n*/y/*3n*/,/*4n*/T/*5n*/y/*6n*/>
17+
////f2</*1o*/T/*2o*/y/*3o*/,/*4o*/T/*5o*/y/*6o*/>
18+
////f2</*1pTypeOnly*/T/*2pTypeOnly*/y/*3pTypeOnly*/,/*4pTypeOnly*/T/*5pTypeOnly*/y/*6pTypeOnly*/>();
19+
////
20+
////f2<typeof /*1uValueOnly*/x, /*4u*/T/*5u*/y/*6u*/
21+
////
22+
////f2</*1x*/T/*2x*/y/*3x*/, () =>/*4x*/T/*5x*/y/*6x*/
23+
////f2<() =>/*1y*/T/*2y*/y/*3y*/, () =>/*4y*/T/*5y*/y/*6y*/
24+
////f2<any, () =>/*1z*/T/*2z*/y/*3z*/
25+
26+
27+
goTo.eachMarker((marker) => {
28+
const markerName = test.markerName(marker);
29+
if (markerName.endsWith("TypeOnly")) {
30+
verify.not.completionListContains("x");
31+
}
32+
else {
33+
verify.completionListContains("x");
34+
}
35+
36+
if (markerName.endsWith("ValueOnly")) {
37+
verify.not.completionListContains("Type");
38+
}
39+
else {
40+
verify.completionListContains("Type");
41+
}
42+
});

‎tests/cases/fourslash/fourslash.ts

+1
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ declare namespace FourSlashInterface {
113113
class test_ {
114114
markers(): Marker[];
115115
markerNames(): string[];
116+
markerName(m: Marker): string;
116117
marker(name?: string): Marker;
117118
ranges(): Range[];
118119
spans(): Array<{ start: number, length: number }>;

0 commit comments

Comments
 (0)