From b89c68280adb5280b0e1754fc509a10e5c589d15 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 18 Jul 2019 14:34:19 -0700 Subject: [PATCH 1/4] 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. --- src/harness/fourslash.ts | 67 +++++++------------ src/services/completions.ts | 12 ++-- .../fourslash/completionInFunctionLikeBody.ts | 2 +- ...FunctionLikeBody_includesPrimitiveTypes.ts | 16 +++++ .../completionListIsGlobalCompletion.ts | 2 +- tests/cases/fourslash/fourslash.ts | 1 - tests/cases/user/prettier/prettier | 2 +- 7 files changed, 52 insertions(+), 50 deletions(-) create mode 100644 tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index aa764e74cdc44..aa958636b1f23 100644 --- a/src/harness/fourslash.ts +++ b/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 = statementKeywordsWithTypes.filter(k => { @@ -4968,8 +4951,20 @@ namespace FourSlashInterface { "let", "package", "yield", + "any", "async", "await", + "boolean", + "keyof", + "never", + "readonly", + "number", + "object", + "string", + "symbol", + "unique", + "unknown", + "bigint", ].map(keywordEntry); export const undefinedVarEntry: ExpectedCompletionEntry = { @@ -5041,40 +5036,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); @@ -5121,17 +5099,24 @@ namespace FourSlashInterface { "let", "package", "yield", + "any", "async", "await", + "boolean", + "keyof", + "never", + "readonly", + "number", + "object", + "string", + "symbol", + "unique", + "unknown", + "bigint", ].map(keywordEntry); export const insideMethodInJsKeywords = getInJsKeywords(insideMethodKeywords); - export const globalKeywordsPlusUndefined: ReadonlyArray = (() => { - const i = ts.findIndex(globalKeywords, x => x.name === "unique"); - return [...globalKeywords.slice(0, i), keywordEntry("undefined"), ...globalKeywords.slice(i)]; - })(); - export const globals: ReadonlyArray = [ globalThisEntry, ...globalsVars, diff --git a/src/services/completions.ts b/src/services/completions.ts index b5c412a788ae4..cb591a2f642ea 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -2060,16 +2060,15 @@ 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 - || isTypeKeyword(kind) && kind !== SyntaxKind.UndefinedKeyword; + return isFunctionLikeBodyKeyword(kind) || kind === SyntaxKind.DeclareKeyword || kind === SyntaxKind.ModuleKeyword; + 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 +2131,10 @@ 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) + || isTypeKeyword(kind) && kind !== SyntaxKind.UndefinedKeyword; } function keywordForNode(node: Node): SyntaxKind { diff --git a/tests/cases/fourslash/completionInFunctionLikeBody.ts b/tests/cases/fourslash/completionInFunctionLikeBody.ts index cf3b32ccb39b4..bfc6b003ec51f 100644 --- a/tests/cases/fourslash/completionInFunctionLikeBody.ts +++ b/tests/cases/fourslash/completionInFunctionLikeBody.ts @@ -19,7 +19,7 @@ verify.completions( includes: ["async", "await"].map( name => ({ name, sortText: completion.SortText.GlobalsOrKeywords }) ), - excludes: ["public", "private", "protected", "constructor", "readonly", "static", "abstract", "get", "set"], + excludes: ["public", "private", "protected", "constructor", "static", "abstract", "get", "set"], }, { marker: ["3", "4"], exact: completion.classElementKeywords, isNewIdentifierLocation: true }, ); diff --git a/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts b/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts new file mode 100644 index 0000000000000..ff4b794347010 --- /dev/null +++ b/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts @@ -0,0 +1,16 @@ +/// + +//// class Foo { } +//// function test() { +//// new Foo; export const insideMethodKeywords: ReadonlyArray; export const insideMethodInJsKeywords: ReadonlyArray; - export const globalKeywordsPlusUndefined: ReadonlyArray; export const globalsVars: ReadonlyArray; export function globalsInsideFunction(plus: ReadonlyArray): ReadonlyArray; export function globalsInJsInsideFunction(plus: ReadonlyArray): ReadonlyArray; diff --git a/tests/cases/user/prettier/prettier b/tests/cases/user/prettier/prettier index 1e471a007968b..7f938c71ffda2 160000 --- a/tests/cases/user/prettier/prettier +++ b/tests/cases/user/prettier/prettier @@ -1 +1 @@ -Subproject commit 1e471a007968b7490563b91ed6909ae6046f3fe8 +Subproject commit 7f938c71ffda293eb1b69adf8bd12b7c11f9113b From d6a9432fd4aa0eee65b0694489d8a0d8377ae58c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 18 Jul 2019 15:45:38 -0700 Subject: [PATCH 2/4] Change keyword filter in filterGlobalCompletion Instead of changing FunctionLikeBodyKeywords --- src/harness/fourslash.ts | 24 ------------------- src/services/completions.ts | 10 ++++---- .../fourslash/completionInFunctionLikeBody.ts | 2 +- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index aa958636b1f23..f12506be2fb27 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -4951,20 +4951,8 @@ namespace FourSlashInterface { "let", "package", "yield", - "any", "async", "await", - "boolean", - "keyof", - "never", - "readonly", - "number", - "object", - "string", - "symbol", - "unique", - "unknown", - "bigint", ].map(keywordEntry); export const undefinedVarEntry: ExpectedCompletionEntry = { @@ -5099,20 +5087,8 @@ namespace FourSlashInterface { "let", "package", "yield", - "any", "async", "await", - "boolean", - "keyof", - "never", - "readonly", - "number", - "object", - "string", - "symbol", - "unique", - "unknown", - "bigint", ].map(keywordEntry); export const insideMethodInJsKeywords = getInJsKeywords(insideMethodKeywords); diff --git a/src/services/completions.ts b/src/services/completions.ts index cb591a2f642ea..4960940f9480e 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1182,7 +1182,7 @@ namespace ts.Completions { function filterGlobalCompletion(symbols: Symbol[]): void { const isTypeOnly = isTypeOnlyCompletion(); const allowTypes = isTypeOnly || !isContextTokenValueLocation(contextToken) && isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker); - if (isTypeOnly) { + if (allowTypes) { keywordFilters = isTypeAssertion() ? KeywordCompletionFilters.TypeAssertionKeywords : KeywordCompletionFilters.TypeKeywords; @@ -2060,7 +2060,10 @@ namespace ts.Completions { case KeywordCompletionFilters.None: return false; case KeywordCompletionFilters.All: - return isFunctionLikeBodyKeyword(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: @@ -2133,8 +2136,7 @@ namespace ts.Completions { function isFunctionLikeBodyKeyword(kind: SyntaxKind) { return kind === SyntaxKind.AsyncKeyword || kind === SyntaxKind.AwaitKeyword - || !isContextualKeyword(kind) && !isClassMemberCompletionKeyword(kind) - || isTypeKeyword(kind) && kind !== SyntaxKind.UndefinedKeyword; + || !isContextualKeyword(kind) && !isClassMemberCompletionKeyword(kind); } function keywordForNode(node: Node): SyntaxKind { diff --git a/tests/cases/fourslash/completionInFunctionLikeBody.ts b/tests/cases/fourslash/completionInFunctionLikeBody.ts index bfc6b003ec51f..cf3b32ccb39b4 100644 --- a/tests/cases/fourslash/completionInFunctionLikeBody.ts +++ b/tests/cases/fourslash/completionInFunctionLikeBody.ts @@ -19,7 +19,7 @@ verify.completions( includes: ["async", "await"].map( name => ({ name, sortText: completion.SortText.GlobalsOrKeywords }) ), - excludes: ["public", "private", "protected", "constructor", "static", "abstract", "get", "set"], + excludes: ["public", "private", "protected", "constructor", "readonly", "static", "abstract", "get", "set"], }, { marker: ["3", "4"], exact: completion.classElementKeywords, isNewIdentifierLocation: true }, ); From ca8c42e5ad5220f6508aae4bb191f156d33e3018 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 19 Jul 2019 11:22:32 -0700 Subject: [PATCH 3/4] Add more tests cases --- ...onInFunctionLikeBody_includesPrimitiveTypes.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts b/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts index ff4b794347010..bc94936ab4353 100644 --- a/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts +++ b/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts @@ -1,8 +1,15 @@ /// //// class Foo { } -//// function test() { -//// new Foo Date: Fri, 19 Jul 2019 14:30:02 -0700 Subject: [PATCH 4/4] Make type-only completions after < more common Because isPossiblyTypeArgumentPosition doesn't give false positives now that it uses type information. --- src/services/completions.ts | 28 ++++++++++--------- .../completionListInUnclosedTypeArguments.ts | 9 +++--- ...mpletionsIsPossiblyTypeArgumentPosition.ts | 17 +++++------ 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 4960940f9480e..e9a38fb1516db 100644 --- a/src/services/completions.ts +++ b/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,8 +1183,7 @@ namespace ts.Completions { function filterGlobalCompletion(symbols: Symbol[]): void { const isTypeOnly = isTypeOnlyCompletion(); - const allowTypes = isTypeOnly || !isContextTokenValueLocation(contextToken) && isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker); - if (allowTypes) { + if (isTypeOnly) { keywordFilters = isTypeAssertion() ? KeywordCompletionFilters.TypeAssertionKeywords : KeywordCompletionFilters.TypeKeywords; @@ -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) { diff --git a/tests/cases/fourslash/completionListInUnclosedTypeArguments.ts b/tests/cases/fourslash/completionListInUnclosedTypeArguments.ts index 31397518ed833..8a100a9616e3d 100644 --- a/tests/cases/fourslash/completionListInUnclosedTypeArguments.ts +++ b/tests/cases/fourslash/completionListInUnclosedTypeArguments.ts @@ -8,14 +8,14 @@ ////f ////f -////f(); +////f(); //// ////f2 ////f2 -////f2(); +////f2(); //// ////f2 { 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, }); }); diff --git a/tests/cases/fourslash/completionsIsPossiblyTypeArgumentPosition.ts b/tests/cases/fourslash/completionsIsPossiblyTypeArgumentPosition.ts index 1ea8416d70be7..704fe0d8347a0 100644 --- a/tests/cases/fourslash/completionsIsPossiblyTypeArgumentPosition.ts +++ b/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(): callAndConstruct; (): string; }; ////interface callAndConstruct {} ////new callAndConstruct