Skip to content

Commit

Permalink
Add completions from the 'this' type
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy Hanson committed Jan 17, 2018
1 parent 64b3086 commit 9550efd
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 21 deletions.
29 changes: 20 additions & 9 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,15 @@ namespace ts {
getAccessibleSymbolChain,
getTypePredicateOfSignature,
resolveExternalModuleSymbol,
tryGetThisTypeAt: node => {
node = getParseTreeNode(node);
return node && tryGetThisTypeAt(node);
},
isMemberSymbol: symbol =>
symbol.flags & SymbolFlags.ClassMember
&& symbol !== argumentsSymbol
&& symbol !== undefinedSymbol
&& !(symbol.parent && symbol.parent.flags & SymbolFlags.Module),
};

const tupleTypes: GenericType[] = [];
Expand Down Expand Up @@ -13268,6 +13277,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.
Expand Down Expand Up @@ -13302,16 +13321,8 @@ namespace ts {

if (isInJavaScriptFile(node)) {
const type = getTypeForThisExpressionFromJSDoc(container);
if (type && type !== unknownType) {
return type;
}
return type !== unknownType ? type : undefined;
}

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) {
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
4 changes: 3 additions & 1 deletion src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -4615,6 +4616,7 @@ namespace FourSlashInterface {
export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
sourceDisplay: string;
isRecommended?: true;
insertText?: string;
}

export interface NewContentOptions {
Expand Down
34 changes: 26 additions & 8 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ namespace ts.Completions {
return undefined;
}
const { name, needsConvertPropertyAccess } = info;
Debug.assert(!(needsConvertPropertyAccess && !propertyAccessToConvert));
if (needsConvertPropertyAccess && !includeInsertTextCompletions) {
return undefined;
}
Expand All @@ -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 {};
}
}


Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions tests/cases/fourslash/completionListInScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//// interface localInterface {}
//// export interface exportedInterface {}
////
//// module localModule {
//// module localModule {
//// export var x = 0;
//// }
//// export module exportedModule {
Expand All @@ -38,7 +38,7 @@
//// interface localInterface2 {}
//// export interface exportedInterface2 {}
////
//// module localModule2 {
//// module localModule2 {
//// export var x = 0;
//// }
//// export module exportedModule2 {
Expand Down
29 changes: 29 additions & 0 deletions tests/cases/fourslash/completionsThisType.ts
Original file line number Diff line number Diff line change
@@ -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",
});
8 changes: 7 additions & 1 deletion tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9550efd

Please sign in to comment.