From 8ed885db3e462ec5ddeebfd136945e9a9cc0ac8a Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 17 Jan 2018 12:05:31 -0800 Subject: [PATCH] Add completions from the 'this' type (#21231) * Add completions from the 'this' type * Code review --- src/compiler/checker.ts | 20 +++++--- src/compiler/types.ts | 2 + src/harness/fourslash.ts | 4 +- src/services/completions.ts | 51 +++++++++++++------ .../cases/fourslash/completionListInScope.ts | 4 +- tests/cases/fourslash/completionsThisType.ts | 29 +++++++++++ tests/cases/fourslash/fourslash.ts | 8 ++- 7 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 tests/cases/fourslash/completionsThisType.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index b3f0678813151..39679675a316c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -295,6 +295,10 @@ namespace ts { getAccessibleSymbolChain, getTypePredicateOfSignature, resolveExternalModuleSymbol, + tryGetThisTypeAt: node => { + node = getParseTreeNode(node); + return node && tryGetThisTypeAt(node); + }, }; const tupleTypes: GenericType[] = []; @@ -13268,6 +13272,16 @@ namespace ts { if (needToCaptureLexicalThis) { captureLexicalThis(node, container); } + + const type = tryGetThisTypeAt(node, container); + if (!type && noImplicitThis) { + // With noImplicitThis, functions may not reference 'this' if it has type 'any' + error(node, Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation); + } + return type || anyType; + } + + function tryGetThisTypeAt(node: Node, container = getThisContainer(node, /*includeArrowFunctions*/ false)): Type | undefined { if (isFunctionLike(container) && (!isInParameterInitializerBeforeContainingFunction(node) || getThisParameter(container))) { // Note: a parameter initializer should refer to class-this unless function-this is explicitly annotated. @@ -13306,12 +13320,6 @@ namespace ts { return type; } } - - if (noImplicitThis) { - // With noImplicitThis, functions may not reference 'this' if it has type 'any' - error(node, Diagnostics.this_implicitly_has_type_any_because_it_does_not_have_a_type_annotation); - } - return anyType; } function getTypeForThisExpressionFromJSDoc(node: Node) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 9d970b88197e3..703b04699b8ac 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2921,6 +2921,8 @@ namespace ts { /* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] | undefined; /* @internal */ getTypePredicateOfSignature(signature: Signature): TypePredicate; /* @internal */ resolveExternalModuleSymbol(symbol: Symbol): Symbol; + /** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */ + /* @internal */ tryGetThisTypeAt(node: Node): Type | undefined; } /* @internal */ diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 4c0c72a200f3f..84bf3bb175201 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -3152,8 +3152,9 @@ Actual: ${stringify(fullActual)}`); assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId)); } - assert.equal(item.hasAction, hasAction); + assert.equal(item.hasAction, hasAction, "hasAction"); assert.equal(item.isRecommended, options && options.isRecommended, "isRecommended"); + assert.equal(item.insertText, options && options.insertText, "insertText"); } private findFile(indexOrName: string | number) { @@ -4615,6 +4616,7 @@ namespace FourSlashInterface { export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions { sourceDisplay: string; isRecommended?: true; + insertText?: string; } export interface NewContentOptions { diff --git a/src/services/completions.ts b/src/services/completions.ts index 2afac10a94435..36a10a1b57862 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4,7 +4,9 @@ namespace ts.Completions { export type Log = (message: string) => void; - interface SymbolOriginInfo { + type SymbolOriginInfo = { type: "this-type" } | SymbolOriginInfoExport; + interface SymbolOriginInfoExport { + type: "export"; moduleSymbol: Symbol; isDefaultExport: boolean; } @@ -170,11 +172,21 @@ namespace ts.Completions { return undefined; } const { name, needsConvertPropertyAccess } = info; - Debug.assert(!(needsConvertPropertyAccess && !propertyAccessToConvert)); if (needsConvertPropertyAccess && !includeInsertTextCompletions) { return undefined; } + let insertText: string | undefined; + let replacementSpan: TextSpan | undefined; + if (kind === CompletionKind.Global && origin && origin.type === "this-type") { + insertText = needsConvertPropertyAccess ? `this["${name}"]` : `this.${name}`; + } + else if (needsConvertPropertyAccess) { + // TODO: GH#20619 Use configured quote style + insertText = `["${name}"]`; + replacementSpan = createTextSpanFromBounds(findChildOfKind(propertyAccessToConvert!, SyntaxKind.DotToken, sourceFile)!.getStart(sourceFile), propertyAccessToConvert!.name.end); + } + // TODO(drosen): Right now we just permit *all* semantic meanings when calling // 'getSymbolKind' which is permissible given that it is backwards compatible; but // really we should consider passing the meaning for the node so that we don't report @@ -189,13 +201,10 @@ namespace ts.Completions { kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), sortText: "0", source: getSourceFromOrigin(origin), - // TODO: GH#20619 Use configured quote style - insertText: needsConvertPropertyAccess ? `["${name}"]` : undefined, - replacementSpan: needsConvertPropertyAccess - ? createTextSpanFromBounds(findChildOfKind(propertyAccessToConvert, SyntaxKind.DotToken, sourceFile)!.getStart(sourceFile), propertyAccessToConvert.name.end) - : undefined, - hasAction: trueOrUndefined(needsConvertPropertyAccess || origin !== undefined), + hasAction: trueOrUndefined(!!origin && origin.type === "export"), isRecommended: trueOrUndefined(isRecommendedCompletionMatch(symbol, recommendedCompletion, typeChecker)), + insertText, + replacementSpan, }; } @@ -210,7 +219,7 @@ namespace ts.Completions { } function getSourceFromOrigin(origin: SymbolOriginInfo | undefined): string | undefined { - return origin && stripQuotes(origin.moduleSymbol.name); + return origin && origin.type === "export" ? stripQuotes(origin.moduleSymbol.name) : undefined; } function getCompletionEntriesFromSymbols( @@ -504,7 +513,7 @@ namespace ts.Completions { } function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string { - return origin && origin.isDefaultExport && symbol.escapedName === InternalSymbolName.Default + return origin && origin.type === "export" && origin.isDefaultExport && symbol.escapedName === InternalSymbolName.Default // Name of "export default foo;" is "foo". Name of "export default 0" is the filename converted to camelCase. ? firstDefined(symbol.declarations, d => isExportAssignment(d) && isIdentifier(d.expression) ? d.expression.text : undefined) || codefix.moduleSymbolToValidIdentifier(origin.moduleSymbol, target) @@ -590,13 +599,13 @@ namespace ts.Completions { allSourceFiles: ReadonlyArray, ): CodeActionsAndSourceDisplay { const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; - return symbolOriginInfo + return symbolOriginInfo && symbolOriginInfo.type === "export" ? getCodeActionsAndSourceDisplayForImport(symbolOriginInfo, symbol, program, checker, host, compilerOptions, sourceFile, previousToken, formatContext, getCanonicalFileName, allSourceFiles) : { codeActions: undefined, sourceDisplay: undefined }; } function getCodeActionsAndSourceDisplayForImport( - symbolOriginInfo: SymbolOriginInfo, + symbolOriginInfo: SymbolOriginInfoExport, symbol: Symbol, program: Program, checker: TypeChecker, @@ -1117,6 +1126,18 @@ namespace ts.Completions { const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias; symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings); + + // Need to insert 'this.' before properties of `this` type, so only do that if `includeInsertTextCompletions` + if (options.includeInsertTextCompletions && scopeNode.kind !== SyntaxKind.SourceFile) { + const thisType = typeChecker.tryGetThisTypeAt(scopeNode); + if (thisType) { + for (const symbol of getPropertiesForCompletion(thisType, typeChecker, /*isForAccess*/ true)) { + symbolToOriginInfoMap[getSymbolId(symbol)] = { type: "this-type" }; + symbols.push(symbol); + } + } + } + if (options.includeExternalModuleExports) { getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", target); } @@ -1230,10 +1251,10 @@ namespace ts.Completions { symbol = getLocalSymbolForExportDefault(symbol) || symbol; } - const origin: SymbolOriginInfo = { moduleSymbol, isDefaultExport }; + const origin: SymbolOriginInfo = { type: "export", moduleSymbol, isDefaultExport }; if (stringContainsCharactersInOrder(getSymbolName(symbol, origin, target).toLowerCase(), tokenTextLowerCase)) { symbols.push(symbol); - symbolToOriginInfoMap[getSymbolId(symbol)] = { moduleSymbol, isDefaultExport }; + symbolToOriginInfoMap[getSymbolId(symbol)] = origin; } } }); @@ -2072,13 +2093,13 @@ namespace ts.Completions { if (isIdentifierText(name, target)) return validIdentiferResult; switch (kind) { case CompletionKind.None: - case CompletionKind.Global: case CompletionKind.MemberLike: return undefined; case CompletionKind.ObjectPropertyDeclaration: // TODO: GH#18169 return { name: JSON.stringify(name), needsConvertPropertyAccess: false }; case CompletionKind.PropertyAccess: + case CompletionKind.Global: // Don't add a completion for a name starting with a space. See https://github.com/Microsoft/TypeScript/pull/20547 return name.charCodeAt(0) === CharacterCodes.space ? undefined : { name, needsConvertPropertyAccess: true }; case CompletionKind.String: diff --git a/tests/cases/fourslash/completionListInScope.ts b/tests/cases/fourslash/completionListInScope.ts index 8773f6d4588ed..70f82b80f5f35 100644 --- a/tests/cases/fourslash/completionListInScope.ts +++ b/tests/cases/fourslash/completionListInScope.ts @@ -13,7 +13,7 @@ //// interface localInterface {} //// export interface exportedInterface {} //// -//// module localModule { +//// module localModule { //// export var x = 0; //// } //// export module exportedModule { @@ -38,7 +38,7 @@ //// interface localInterface2 {} //// export interface exportedInterface2 {} //// -//// module localModule2 { +//// module localModule2 { //// export var x = 0; //// } //// export module exportedModule2 { diff --git a/tests/cases/fourslash/completionsThisType.ts b/tests/cases/fourslash/completionsThisType.ts new file mode 100644 index 0000000000000..58325182a22d1 --- /dev/null +++ b/tests/cases/fourslash/completionsThisType.ts @@ -0,0 +1,29 @@ +/// + +////class C { +//// "foo bar": number; +//// xyz() { +//// /**/ +//// } +////} +//// +////function f(this: { x: number }) { /*f*/ } + +goTo.marker(""); + +verify.completionListContains("xyz", "(method) C.xyz(): void", "", "method", undefined, undefined, { + includeInsertTextCompletions: true, + insertText: "this.xyz", +}); + +verify.completionListContains("foo bar", '(property) C["foo bar"]: number', "", "property", undefined, undefined, { + includeInsertTextCompletions: true, + insertText: 'this["foo bar"]', +}); + +goTo.marker("f"); + +verify.completionListContains("x", "(property) x: number", "", "property", undefined, undefined, { + includeInsertTextCompletions: true, + insertText: "this.x", +}); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 97379957cbf17..0bf62eaef9240 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -151,7 +151,13 @@ declare namespace FourSlashInterface { kind?: string | { kind?: string, kindModifiers?: string }, spanIndex?: number, hasAction?: boolean, - options?: { includeExternalModuleExports?: boolean, sourceDisplay?: string, isRecommended?: true }, + options?: { + includeExternalModuleExports?: boolean, + includeInsertTextCompletions?: boolean, + sourceDisplay?: string, + isRecommended?: true, + insertText?: string, + }, ): void; completionListItemsCountIsGreaterThan(count: number): void; completionListIsEmpty(): void;