Add completions from the 'this' type

Andy Hanson · Andy Hanson · commit 312976fef970 · 2018-01-17T09:09:10.000-08:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -295,6 +295,15 @@ namespace ts {
             getAccessibleSymbolChain,
             getTypePredicateOfSignature,
             resolveExternalModuleSymbol,
+            tryGetThisTypeAt: node => {
+                node = getParseTreeNode(node);
+                return node && tryGetThisTypeAt(node, /*isForTest*/ true);
+            },
+            isMemberSymbol: symbol =>
+                symbol.flags & SymbolFlags.ClassMember
+                    && symbol !== argumentsSymbol
+                    && symbol !== undefinedSymbol
+                    && !(symbol.parent && symbol.parent.flags & SymbolFlags.Module),
         };
 
         const tupleTypes: GenericType[] = [];
@@ -13221,13 +13230,24 @@ namespace ts {
         }
 
         function checkThisExpression(node: Node): Type {
+            const type = tryGetThisTypeAt(node, /*isForTest*/ false);
+            if (type) 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 tryGetThisTypeAt(node: Node, isForTest: boolean): Type | undefined {
             // Stop at the first arrow function so that we can
             // tell whether 'this' needs to be captured.
             let container = getThisContainer(node, /* includeArrowFunctions */ true);
             let needToCaptureLexicalThis = false;
 
             if (container.kind === SyntaxKind.Constructor) {
-                checkThisBeforeSuper(node, container, Diagnostics.super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class);
+                if (!isForTest) checkThisBeforeSuper(node, container, Diagnostics.super_must_be_called_before_accessing_this_in_the_constructor_of_a_derived_class);
             }
 
             // Now skip arrow functions to get the "real" owner of 'this'.
@@ -13238,36 +13258,14 @@ namespace ts {
                 needToCaptureLexicalThis = (languageVersion < ScriptTarget.ES2015);
             }
 
-            switch (container.kind) {
-                case SyntaxKind.ModuleDeclaration:
-                    error(node, Diagnostics.this_cannot_be_referenced_in_a_module_or_namespace_body);
-                    // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
-                    break;
-                case SyntaxKind.EnumDeclaration:
-                    error(node, Diagnostics.this_cannot_be_referenced_in_current_location);
-                    // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
-                    break;
-                case SyntaxKind.Constructor:
-                    if (isInConstructorArgumentInitializer(node, container)) {
-                        error(node, Diagnostics.this_cannot_be_referenced_in_constructor_arguments);
-                        // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
-                    }
-                    break;
-                case SyntaxKind.PropertyDeclaration:
-                case SyntaxKind.PropertySignature:
-                    if (hasModifier(container, ModifierFlags.Static)) {
-                        error(node, Diagnostics.this_cannot_be_referenced_in_a_static_property_initializer);
-                        // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
-                    }
-                    break;
-                case SyntaxKind.ComputedPropertyName:
-                    error(node, Diagnostics.this_cannot_be_referenced_in_a_computed_property_name);
-                    break;
-            }
+            if (!isForTest) {
+                reportErrorsForThisExpression(node, container);
 
-            if (needToCaptureLexicalThis) {
-                captureLexicalThis(node, container);
+                if (needToCaptureLexicalThis) {
+                    captureLexicalThis(node, container);
+                }
             }
+
             if (isFunctionLike(container) &&
                 (!isInParameterInitializerBeforeContainingFunction(node) || getThisParameter(container))) {
                 // Note: a parameter initializer should refer to class-this unless function-this is explicitly annotated.
@@ -13306,12 +13304,35 @@ 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);
+        function reportErrorsForThisExpression(node: Node, container: Node): void {
+            switch (container.kind) {
+                case SyntaxKind.ModuleDeclaration:
+                    error(node, Diagnostics.this_cannot_be_referenced_in_a_module_or_namespace_body);
+                    // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
+                    break;
+                case SyntaxKind.EnumDeclaration:
+                    error(node, Diagnostics.this_cannot_be_referenced_in_current_location);
+                    // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
+                    break;
+                case SyntaxKind.Constructor:
+                    if (isInConstructorArgumentInitializer(node, container)) {
+                        error(node, Diagnostics.this_cannot_be_referenced_in_constructor_arguments);
+                        // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
+                    }
+                    break;
+                case SyntaxKind.PropertyDeclaration:
+                case SyntaxKind.PropertySignature:
+                    if (hasModifier(container, ModifierFlags.Static)) {
+                        error(node, Diagnostics.this_cannot_be_referenced_in_a_static_property_initializer);
+                        // do not return here so in case if lexical this is captured - it will be reflected in flags on NodeLinks
+                    }
+                    break;
+                case SyntaxKind.ComputedPropertyName:
+                    error(node, Diagnostics.this_cannot_be_referenced_in_a_computed_property_name);
+                    break;
             }
-            return anyType;
         }
 
         function getTypeForThisExpressionFromJSDoc(node: Node) {
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2919,6 +2919,9 @@ 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 */ isMemberSymbol(symbol: Symbol): boolean;
     }
 
     /* @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
@@ -167,7 +167,6 @@ namespace ts.Completions {
             return undefined;
         }
         const { name, needsConvertPropertyAccess } = info;
-        Debug.assert(!(needsConvertPropertyAccess && !propertyAccessToConvert));
         if (needsConvertPropertyAccess && !includeInsertTextCompletions) {
             return undefined;
         }
@@ -186,14 +185,24 @@ 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 !== undefined),
             isRecommended: trueOrUndefined(isRecommendedCompletionMatch(symbol, recommendedCompletion, typeChecker)),
+            ...getInsertTextAndReplacementSpan(),
         };
+
+        function getInsertTextAndReplacementSpan(): { insertText?: string, replacementSpan?: TextSpan } {
+            if (kind === CompletionKind.Global) {
+                if (typeChecker.isMemberSymbol(symbol)) {
+                    return { insertText: needsConvertPropertyAccess ? `this["${name}"]` : `this.${name}` };
+                }
+            }
+            if (needsConvertPropertyAccess) {
+                // TODO: GH#20619 Use configured quote style
+                const replacementSpan = createTextSpanFromBounds(findChildOfKind(propertyAccessToConvert!, SyntaxKind.DotToken, sourceFile)!.getStart(sourceFile), propertyAccessToConvert!.name.end);
+                return { insertText: `["${name}"]`, replacementSpan };
+            }
+            return {};
+        }
     }
 
 
@@ -1097,6 +1106,15 @@ 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) {
+                    symbols.push(...getPropertiesForCompletion(thisType, typeChecker, /*isForAccess*/ true));
+                }
+            }
+
             if (options.includeExternalModuleExports) {
                 getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", target);
             }
@@ -2052,13 +2070,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
`@@ -2919,6 +2919,9 @@ namespace ts {`
`2919`	`2919`	`/* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node \| undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] \| undefined;`
`2920`	`2920`	`/* @internal */ getTypePredicateOfSignature(signature: Signature): TypePredicate;`
`2921`	`2921`	`/* @internal */ resolveExternalModuleSymbol(symbol: Symbol): Symbol;`
	`2922`	+ /** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */
	`2923`	`+ /* @internal */ tryGetThisTypeAt(node: Node): Type \| undefined;`
	`2924`	`+ /* @internal */ isMemberSymbol(symbol: Symbol): boolean;`
`2922`	`2925`	`}`
`2923`	`2926`
`2924`	`2927`	`/* @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 {`