Add completions from the 'this' type (#21231)

Andy · web-flow · commit 8ed885db3e46 · 2018-01-17T12:05:31.000-08:00
* Add completions from the 'this' type

* Code review
diff --git 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
@@ -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
@@ -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
@@ -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<SourceFile>,
     ): 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
@@ -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
@@ -0,0 +1,29 @@
+/// <reference path="fourslash.ts" />
+
+////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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -2921,6 +2921,8 @@ namespace ts {`
`2921`	`2921`	`/* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node \| undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] \| undefined;`
`2922`	`2922`	`/* @internal */ getTypePredicateOfSignature(signature: Signature): TypePredicate;`
`2923`	`2923`	`/* @internal */ resolveExternalModuleSymbol(symbol: Symbol): Symbol;`
	`2924`	+ /** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */
	`2925`	`+ /* @internal */ tryGetThisTypeAt(node: Node): Type \| undefined;`
`2924`	`2926`	`}`
`2925`	`2927`
`2926`	`2928`	`/* @internal */`
Original file line number	Diff line number	Diff line change
@@ -3152,8 +3152,9 @@ Actual: ${stringify(fullActual)}`);
`3152`	`3152`	`assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId));`
`3153`	`3153`	`}`
`3154`	`3154`
`3155`		`- assert.equal(item.hasAction, hasAction);`
	`3155`	`+ assert.equal(item.hasAction, hasAction, "hasAction");`
`3156`	`3156`	`assert.equal(item.isRecommended, options && options.isRecommended, "isRecommended");`
	`3157`	`+ assert.equal(item.insertText, options && options.insertText, "insertText");`
`3157`	`3158`	`}`
`3158`	`3159`
`3159`	`3160`	`private findFile(indexOrName: string \| number) {`
`@@ -4615,6 +4616,7 @@ namespace FourSlashInterface {`
`4615`	`4616`	`export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {`
`4616`	`4617`	`sourceDisplay: string;`
`4617`	`4618`	`isRecommended?: true;`
	`4619`	`+ insertText?: string;`
`4618`	`4620`	`}`
`4619`	`4621`
`4620`	`4622`	`export interface NewContentOptions {`