Skip to content

Commit

Permalink
Add completions from the 'this' type (#21231)
Browse files Browse the repository at this point in the history
* Add completions from the 'this' type

* Code review
  • Loading branch information
Andy authored Jan 17, 2018
1 parent 48ac301 commit 8ed885d
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 25 deletions.
20 changes: 14 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ namespace ts {
getAccessibleSymbolChain,
getTypePredicateOfSignature,
resolveExternalModuleSymbol,
tryGetThisTypeAt: node => {
node = getParseTreeNode(node);
return node && tryGetThisTypeAt(node);
},
};

const tupleTypes: GenericType[] = [];
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
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
51 changes: 36 additions & 15 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
};
}

Expand All @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
}
});
Expand Down Expand Up @@ -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:
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 8ed885d

Please sign in to comment.