From e543d8bc5a17bdee931ac1a0d2b9ddd32a7164a9 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 19 Jul 2019 15:22:04 -0700 Subject: [PATCH] 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. --- src/harness/fourslash.ts | 43 +------------------ src/services/completions.ts | 38 +++++++++------- ...FunctionLikeBody_includesPrimitiveTypes.ts | 27 ++++++++++++ .../completionListInUnclosedTypeArguments.ts | 9 ++-- .../completionListIsGlobalCompletion.ts | 2 +- ...mpletionsIsPossiblyTypeArgumentPosition.ts | 17 +++----- tests/cases/fourslash/fourslash.ts | 1 - tests/cases/user/prettier/prettier | 2 +- 8 files changed, 64 insertions(+), 75 deletions(-) create mode 100644 tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index aa764e74cdc..f12506be2fb 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 => { @@ -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 = (() => { - 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 b5c412a788a..e9a38fb1516 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,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 { diff --git a/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts b/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts new file mode 100644 index 00000000000..bc94936ab43 --- /dev/null +++ b/tests/cases/fourslash/completionInFunctionLikeBody_includesPrimitiveTypes.ts @@ -0,0 +1,27 @@ +/// + +//// class Foo { } +//// class Bar { } +//// function includesTypes() { +//// new Foo ////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/completionListIsGlobalCompletion.ts b/tests/cases/fourslash/completionListIsGlobalCompletion.ts index ea89155a771..91cd1a0f9c7 100644 --- a/tests/cases/fourslash/completionListIsGlobalCompletion.ts +++ b/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 }, ); diff --git a/tests/cases/fourslash/completionsIsPossiblyTypeArgumentPosition.ts b/tests/cases/fourslash/completionsIsPossiblyTypeArgumentPosition.ts index 1ea8416d70b..704fe0d8347 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; 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 1e471a00796..7f938c71ffd 160000 --- a/tests/cases/user/prettier/prettier +++ b/tests/cases/user/prettier/prettier @@ -1 +1 @@ -Subproject commit 1e471a007968b7490563b91ed6909ae6046f3fe8 +Subproject commit 7f938c71ffda293eb1b69adf8bd12b7c11f9113b