Fix type keyword completions (#32474)

* Fix type keyword completions

1. In functions, type keywords were omitted.
2. In All context, no keywords were omitted.

(1) fixes #28737
(2) removes 17 keywords that should not be suggested, even at the
toplevel of a typescript file:

* private
* protected
* public
* static
* abstract
* as
* constructor
* get
* infer
* is
* namespace
* require
* set
* type
* from
* global
* of

I don't know whether we have a bug tracking this or not.

* Change keyword filter in filterGlobalCompletion

Instead of changing FunctionLikeBodyKeywords

* Add more tests cases

* Make type-only completions after < more common

Because isPossiblyTypeArgumentPosition doesn't give false positives now
that it uses type information.

Author: sandersn

src/harness/fourslash.ts

@@ -797,7 +797,7 @@ namespace FourSlash {
                     for (const include of toArray(options.includes)) {
                         const name = typeof include === "string" ? include : include.name;
                         const found = nameToEntries.get(name);
-                        if (!found) throw this.raiseError(`No completion ${name} found`);
+                        if (!found) throw this.raiseError(`Includes: completion '${name}' not found.`);
                         assert(found.length === 1); // Must use 'exact' for multiple completions with same name
                         this.verifyCompletionEntry(ts.first(found), include);
                     }
@@ -806,7 +806,7 @@ namespace FourSlash {
                     for (const exclude of toArray(options.excludes)) {
                         assert(typeof exclude === "string");
                         if (nameToEntries.has(exclude)) {
-                            this.raiseError(`Did not expect to get a completion named ${exclude}`);
+                            this.raiseError(`Excludes: unexpected completion '${exclude}' found.`);
                         }
                     }
                 }
@@ -4827,40 +4827,23 @@ namespace FourSlashInterface {
             "interface",
             "let",
             "package",
-            "private",
-            "protected",
-            "public",
-            "static",
             "yield",
-            "abstract",
-            "as",
             "any",
             "async",
             "await",
             "boolean",
-            "constructor",
             "declare",
-            "get",
-            "infer",
-            "is",
             "keyof",
             "module",
-            "namespace",
             "never",
             "readonly",
-            "require",
             "number",
             "object",
-            "set",
             "string",
             "symbol",
-            "type",
             "unique",
             "unknown",
-            "from",
-            "global",
             "bigint",
-            "of",
         ].map(keywordEntry);
 
         export const statementKeywords: ReadonlyArray<ExpectedCompletionEntryObject> = statementKeywordsWithTypes.filter(k => {
@@ -5041,40 +5024,23 @@ namespace FourSlashInterface {
             "interface",
             "let",
             "package",
-            "private",
-            "protected",
-            "public",
-            "static",
             "yield",
-            "abstract",
-            "as",
             "any",
             "async",
             "await",
             "boolean",
-            "constructor",
             "declare",
-            "get",
-            "infer",
-            "is",
             "keyof",
             "module",
-            "namespace",
             "never",
             "readonly",
-            "require",
             "number",
             "object",
-            "set",
             "string",
             "symbol",
-            "type",
             "unique",
             "unknown",
-            "from",
-            "global",
             "bigint",
-            "of",
         ].map(keywordEntry);
 
         export const globalInJsKeywords = getInJsKeywords(globalKeywords);
@@ -5127,11 +5093,6 @@ namespace FourSlashInterface {
 
         export const insideMethodInJsKeywords = getInJsKeywords(insideMethodKeywords);
 
-        export const globalKeywordsPlusUndefined: ReadonlyArray<ExpectedCompletionEntryObject> = (() => {
-            const i = ts.findIndex(globalKeywords, x => x.name === "unique");
-            return [...globalKeywords.slice(0, i), keywordEntry("undefined"), ...globalKeywords.slice(i)];
-        })();
-
         export const globals: ReadonlyArray<ExpectedCompletionEntryObject> = [
             globalThisEntry,
             ...globalsVars,

src/services/completions.ts

@@ -947,11 +947,13 @@ namespace ts.Completions {
             // Right of dot member completion list
             completionKind = CompletionKind.PropertyAccess;
 
-            // Since this is qualified name check its a type node location
+            // Since this is qualified name check it's a type node location
             const isImportType = isLiteralImportTypeNode(node);
-            const isTypeLocation = insideJsDocTagTypeExpression || (isImportType && !(node as ImportTypeNode).isTypeOf) || isPartOfTypeNode(node.parent);
+            const isTypeLocation = insideJsDocTagTypeExpression
+                || (isImportType && !(node as ImportTypeNode).isTypeOf)
+                || isPartOfTypeNode(node.parent)
+                || isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker);
             const isRhsOfImportDeclaration = isInRightSideOfInternalImportEqualsDeclaration(node);
-            const allowTypeOrValue = isRhsOfImportDeclaration || (!isTypeLocation && isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker));
             if (isEntityName(node) || isImportType) {
                 const isNamespaceName = isModuleDeclaration(node.parent);
                 if (isNamespaceName) isNewIdentifierLocation = true;
@@ -968,7 +970,7 @@ namespace ts.Completions {
                             isNamespaceName
                                 // At `namespace N.M/**/`, if this is the only declaration of `M`, don't include `M` as a completion.
                                 ? symbol => !!(symbol.flags & SymbolFlags.Namespace) && !symbol.declarations.every(d => d.parent === node.parent)
-                                : allowTypeOrValue ?
+                                : isRhsOfImportDeclaration ?
                                     // Any kind is allowed when dotting off namespace in internal import equals declaration
                                     symbol => isValidTypeAccess(symbol) || isValidValueAccess(symbol) :
                                     isTypeLocation ? isValidTypeAccess : isValidValueAccess;
@@ -1181,7 +1183,6 @@ namespace ts.Completions {
 
         function filterGlobalCompletion(symbols: Symbol[]): void {
             const isTypeOnly = isTypeOnlyCompletion();
-            const allowTypes = isTypeOnly || !isContextTokenValueLocation(contextToken) && isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker);
             if (isTypeOnly) {
                 keywordFilters = isTypeAssertion()
                     ? KeywordCompletionFilters.TypeAssertionKeywords
@@ -1202,12 +1203,9 @@ namespace ts.Completions {
                         return !!(symbol.flags & SymbolFlags.Namespace);
                     }
 
-                    if (allowTypes) {
-                        // Its a type, but you can reach it by namespace.type as well
-                        const symbolAllowedAsType = symbolCanBeReferencedAtTypeLocation(symbol);
-                        if (symbolAllowedAsType || isTypeOnly) {
-                            return symbolAllowedAsType;
-                        }
+                    if (isTypeOnly) {
+                        // It's a type, but you can reach it by namespace.type as well
+                        return symbolCanBeReferencedAtTypeLocation(symbol);
                     }
                 }
 
@@ -1221,7 +1219,11 @@ namespace ts.Completions {
         }
 
         function isTypeOnlyCompletion(): boolean {
-            return insideJsDocTagTypeExpression || !isContextTokenValueLocation(contextToken) && (isPartOfTypeNode(location) || isContextTokenTypeLocation(contextToken));
+            return insideJsDocTagTypeExpression
+                || !isContextTokenValueLocation(contextToken) &&
+                (isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker)
+                 || isPartOfTypeNode(location)
+                 || isContextTokenTypeLocation(contextToken));
         }
 
         function isContextTokenValueLocation(contextToken: Node) {
@@ -2060,16 +2062,18 @@ namespace ts.Completions {
                 case KeywordCompletionFilters.None:
                     return false;
                 case KeywordCompletionFilters.All:
-                    return kind === SyntaxKind.AsyncKeyword || SyntaxKind.AwaitKeyword || !isContextualKeyword(kind) && !isClassMemberCompletionKeyword(kind) || kind === SyntaxKind.DeclareKeyword || kind === SyntaxKind.ModuleKeyword
+                    return isFunctionLikeBodyKeyword(kind)
+                        || kind === SyntaxKind.DeclareKeyword
+                        || kind === SyntaxKind.ModuleKeyword
                         || isTypeKeyword(kind) && kind !== SyntaxKind.UndefinedKeyword;
+                case KeywordCompletionFilters.FunctionLikeBodyKeywords:
+                    return isFunctionLikeBodyKeyword(kind);
                 case KeywordCompletionFilters.ClassElementKeywords:
                     return isClassMemberCompletionKeyword(kind);
                 case KeywordCompletionFilters.InterfaceElementKeywords:
                     return isInterfaceOrTypeLiteralCompletionKeyword(kind);
                 case KeywordCompletionFilters.ConstructorParameterKeywords:
                     return isParameterPropertyModifier(kind);
-                case KeywordCompletionFilters.FunctionLikeBodyKeywords:
-                    return isFunctionLikeBodyKeyword(kind);
                 case KeywordCompletionFilters.TypeAssertionKeywords:
                     return isTypeKeyword(kind) || kind === SyntaxKind.ConstKeyword;
                 case KeywordCompletionFilters.TypeKeywords:
@@ -2132,7 +2136,9 @@ namespace ts.Completions {
     }
 
     function isFunctionLikeBodyKeyword(kind: SyntaxKind) {
-        return kind === SyntaxKind.AsyncKeyword || kind === SyntaxKind.AwaitKeyword || !isContextualKeyword(kind) && !isClassMemberCompletionKeyword(kind);
+        return kind === SyntaxKind.AsyncKeyword
+            || kind === SyntaxKind.AwaitKeyword
+            || !isContextualKeyword(kind) && !isClassMemberCompletionKeyword(kind);
     }
 
     function keywordForNode(node: Node): SyntaxKind {

tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts

@@ -0,0 +1,27 @@
+/// <reference path='fourslash.ts'/>
+
+//// class Foo<T> { }
+//// class Bar { }
+//// function includesTypes() {
+////     new Foo</*1*/
+//// }
+//// function excludesTypes1() {
+////     new Bar</*2*/
+//// }
+//// function excludesTypes2() {
+////     1</*3*/
+//// }
+
+verify.completions(
+    {
+        marker: ["1"],
+        includes: [
+            { name: "string", sortText: completion.SortText.GlobalsOrKeywords },
+            { name: "String", sortText: completion.SortText.GlobalsOrKeywords },
+        ],
+    },
+    {
+        marker: ["2", "3"],
+        excludes: ["string"]
+    }
+);

tests/cases/fourslash/completionListInUnclosedTypeArguments.ts

@@ -8,14 +8,14 @@
 ////f</*1b*/T/*2b*/y/*3b*/;
 ////f</*1c*/T/*2c*/y/*3c*/>
 ////f</*1d*/T/*2d*/y/*3d*/>
-////f</*1eTypeOnly*/T/*2eTypeOnly*/y/*3eTypeOnly*/>();
+////f</*1e*/T/*2e*/y/*3e*/>();
 ////
 ////f2</*1k*/T/*2k*/y/*3k*/,
 ////f2</*1l*/T/*2l*/y/*3l*/,{| "newId": true |}T{| "newId": true |}y{| "newId": true |}
 ////f2</*1m*/T/*2m*/y/*3m*/,{| "newId": true |}T{| "newId": true |}y{| "newId": true |};
 ////f2</*1n*/T/*2n*/y/*3n*/,{| "newId": true |}T{| "newId": true |}y{| "newId": true |}>
 ////f2</*1o*/T/*2o*/y/*3o*/,{| "newId": true |}T{| "newId": true |}y{| "newId": true |}>
-////f2</*1pTypeOnly*/T/*2pTypeOnly*/y/*3pTypeOnly*/,{| "newId": true, "typeOnly": true |}T{| "newId": true, "typeOnly": true |}y{| "newId": true, "typeOnly": true |}>();
+////f2</*1p*/T/*2p*/y/*3p*/,{| "newId": true, "typeOnly": true |}T{| "newId": true, "typeOnly": true |}y{| "newId": true, "typeOnly": true |}>();
 ////
 ////f2<typeof /*1uValueOnly*/x, {| "newId": true |}T{| "newId": true |}y{| "newId": true |}
 ////
@@ -25,12 +25,11 @@
 
 goTo.eachMarker(marker => {
     const markerName = test.markerName(marker) || "";
-    const typeOnly = markerName.endsWith("TypeOnly") || marker.data && marker.data.typeOnly;
     const valueOnly = markerName.endsWith("ValueOnly");
     verify.completions({
         marker,
-        includes: typeOnly ? "Type" : valueOnly ? "x" : ["Type", "x"],
-        excludes: typeOnly ? "x" : valueOnly ? "Type" : [],
+        includes: valueOnly ? "x" : "Type",
+        excludes: valueOnly ? "Type" : "x",
         isNewIdentifierLocation: marker.data && marker.data.newId || false,
     });
 });

tests/cases/fourslash/completionListIsGlobalCompletion.ts

@@ -48,5 +48,5 @@ verify.completions(
     { marker: "13", exact: globals, isGlobalCompletion: false },
     { marker: "15", exact: globals, isGlobalCompletion: true, isNewIdentifierLocation: true },
     { marker: "16", exact: [...x, completion.globalThisEntry, ...completion.globalsVars, completion.undefinedVarEntry], isGlobalCompletion: false },
-    { marker: "17", exact: completion.globalKeywordsPlusUndefined, isGlobalCompletion: false },
+    { marker: "17", exact: completion.globalKeywords, isGlobalCompletion: false },
 );

tests/cases/fourslash/completionsIsPossiblyTypeArgumentPosition.ts

@@ -9,25 +9,22 @@
 ////x + {| "valueOnly": true |}
 ////x < {| "valueOnly": true |}
 ////f < {| "valueOnly": true |}
-////g < {| "valueOnly": false |}
-////const something: C<{| "typeOnly": true |};
-////const something2: C<C<{| "typeOnly": true |};
-////new C<{| "valueOnly": false |};
-////new C<C<{| "valueOnly": false |};
+////g < /*g*/
+////const something: C</*something*/;
+////const something2: C<C</*something2*/;
+////new C</*C*/;
+////new C<C</*CC*/;
 ////
 ////declare const callAndConstruct: { new<T>(): callAndConstruct<T>; <T>(): string; };
 ////interface callAndConstruct<T> {}
 ////new callAndConstruct<callAndConstruct</*callAndConstruct*/
 
 for (const marker of test.markers()) {
-    if (marker.data && marker.data.typeOnly) {
-        verify.completions({ marker, includes: "T", excludes: "x" });
-    }
-    else if (marker.data && marker.data.valueOnly) {
+    if (marker.data && marker.data.valueOnly) {
         verify.completions({ marker, includes: "x", excludes: "T" });
     }
     else {
-        verify.completions({ marker, includes: ["x", "T"] });
+        verify.completions({ marker, includes: "T", excludes: "x" });
     }
 }
 

tests/cases/fourslash/fourslash.ts

@@ -685,7 +685,6 @@ declare namespace completion {
     export const globalInJsKeywords: ReadonlyArray<Entry>;
     export const insideMethodKeywords: ReadonlyArray<Entry>;
     export const insideMethodInJsKeywords: ReadonlyArray<Entry>;
-    export const globalKeywordsPlusUndefined: ReadonlyArray<Entry>;
     export const globalsVars: ReadonlyArray<Entry>;
     export function globalsInsideFunction(plus: ReadonlyArray<Entry>): ReadonlyArray<Entry>;
     export function globalsInJsInsideFunction(plus: ReadonlyArray<Entry>): ReadonlyArray<Entry>;