Skip to content

Support completions contextual types in more places #20768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
2 commits merged into from
Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 2 additions & 18 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13943,7 +13943,7 @@ namespace ts {
// the contextual type of an initializer expression is the type annotation of the containing declaration, if present.
function getContextualTypeForInitializerExpression(node: Expression): Type {
const declaration = <VariableLikeDeclaration>node.parent;
if (hasInitializer(declaration) && node === declaration.initializer || node.kind === SyntaxKind.EqualsToken) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does = go away? I guess that it's because in batch compilation, = doesn't have a type, and node is in fact only ever = when called from services.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this code was added in #20020 and this PR moves the special cases to completions.ts.

if (hasInitializer(declaration) && node === declaration.initializer) {
const typeNode = getEffectiveTypeAnnotationNode(declaration);
if (typeNode) {
return getTypeFromTypeNode(typeNode);
Expand Down Expand Up @@ -14075,12 +14075,6 @@ namespace ts {
case SyntaxKind.AmpersandAmpersandToken:
case SyntaxKind.CommaToken:
return node === right ? getContextualType(binaryExpression) : undefined;
case SyntaxKind.EqualsEqualsEqualsToken:
case SyntaxKind.EqualsEqualsToken:
case SyntaxKind.ExclamationEqualsEqualsToken:
case SyntaxKind.ExclamationEqualsToken:
// For completions after `x === `
return node === operatorToken ? getTypeOfExpression(binaryExpression.left) : undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really not used anywhere else besides services?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was also added by #20020.

default:
return undefined;
}
Expand Down Expand Up @@ -14296,12 +14290,8 @@ namespace ts {
return getContextualTypeForReturnExpression(node);
case SyntaxKind.YieldExpression:
return getContextualTypeForYieldOperand(<YieldExpression>parent);
case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
if (node.kind === SyntaxKind.NewKeyword) { // for completions after `new `
return getContextualType(parent as NewExpression);
}
// falls through
case SyntaxKind.CallExpression:
return getContextualTypeForArgument(<CallExpression | NewExpression>parent, node);
case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
Expand Down Expand Up @@ -14336,12 +14326,6 @@ namespace ts {
case SyntaxKind.JsxOpeningElement:
case SyntaxKind.JsxSelfClosingElement:
return getAttributesTypeFromJsxOpeningLikeElement(<JsxOpeningLikeElement>parent);
case SyntaxKind.CaseClause: {
if (node.kind === SyntaxKind.CaseKeyword) { // for completions after `case `
const switchStatement = (parent as CaseClause).parent.parent;
return getTypeOfExpression(switchStatement.expression);
}
}
}
return undefined;
}
Expand Down
16 changes: 9 additions & 7 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,12 +441,11 @@ namespace FourSlash {
this.goToPosition(marker.position);
}

public goToEachMarker(action: () => void) {
const markers = this.getMarkers();
public goToEachMarker(markers: ReadonlyArray<Marker>, action: (marker: FourSlash.Marker, index: number) => void) {
assert(markers.length);
for (const marker of markers) {
this.goToMarker(marker);
action();
for (let i = 0; i < markers.length; i++) {
this.goToMarker(markers[i]);
action(markers[i], i);
}
}

Expand Down Expand Up @@ -3764,8 +3763,11 @@ namespace FourSlashInterface {
this.state.goToMarker(name);
}

public eachMarker(action: () => void) {
this.state.goToEachMarker(action);
public eachMarker(markers: ReadonlyArray<string>, action: (marker: FourSlash.Marker, index: number) => void): void;
public eachMarker(action: (marker: FourSlash.Marker, index: number) => void): void;
public eachMarker(a: ReadonlyArray<string> | ((marker: FourSlash.Marker, index: number) => void), b?: (marker: FourSlash.Marker, index: number) => void): void {
const markers = typeof a === "function" ? this.state.getMarkers() : a.map(m => this.state.getMarkerByName(m));
this.state.goToEachMarker(markers, typeof a === "function" ? a : b);
}

public rangeStart(range: FourSlash.Range) {
Expand Down
85 changes: 58 additions & 27 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ namespace ts.Completions {

function getStringLiteralCompletionEntries(sourceFile: SourceFile, position: number, typeChecker: TypeChecker, compilerOptions: CompilerOptions, host: LanguageServiceHost, log: Log): CompletionInfo | undefined {
const node = findPrecedingToken(position, sourceFile);
if (!node || (node.kind !== SyntaxKind.StringLiteral && node.kind !== SyntaxKind.NoSubstitutionTemplateLiteral)) {
if (!node || !isStringLiteral(node) && !isNoSubstitutionTemplateLiteral(node)) {
return undefined;
}

Expand Down Expand Up @@ -277,21 +277,9 @@ namespace ts.Completions {
// import x = require("/*completion position*/");
// var y = require("/*completion position*/");
// export * from "/*completion position*/";
const entries = PathCompletions.getStringLiteralCompletionsFromModuleNames(<StringLiteral>node, compilerOptions, host, typeChecker);
const entries = PathCompletions.getStringLiteralCompletionsFromModuleNames(node, compilerOptions, host, typeChecker);
return pathCompletionsInfo(entries);
}
else if (isEqualityExpression(node.parent)) {
// Get completions from the type of the other operand
// i.e. switch (a) {
// case '/*completion position*/'
// }
return getStringLiteralCompletionEntriesFromType(typeChecker.getTypeAtLocation(node.parent.left === node ? node.parent.right : node.parent.left), typeChecker);
}
else if (isCaseOrDefaultClause(node.parent)) {
// Get completions from the type of the switch expression
// i.e. x === '/*completion position'
return getStringLiteralCompletionEntriesFromType(typeChecker.getTypeAtLocation((<SwitchStatement>node.parent.parent.parent).expression), typeChecker);
}
else {
const argumentInfo = SignatureHelp.getImmediatelyContainingArgumentInfo(node, position, sourceFile);
if (argumentInfo) {
Expand All @@ -303,7 +291,7 @@ namespace ts.Completions {

// Get completion for string literal from string literal type
// i.e. var x: "hi" | "hello" = "/*completion position*/"
return getStringLiteralCompletionEntriesFromType(typeChecker.getContextualType(<LiteralExpression>node), typeChecker);
return getStringLiteralCompletionEntriesFromType(getContextualTypeFromParent(node, typeChecker), typeChecker);
}
}

Expand Down Expand Up @@ -602,15 +590,57 @@ namespace ts.Completions {
}
type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag };

function getRecommendedCompletion(currentToken: Node, checker: TypeChecker/*, symbolToOriginInfoMap: SymbolOriginInfoMap*/): Symbol | undefined {
const ty = checker.getContextualType(currentToken as Expression);
function getRecommendedCompletion(currentToken: Node, checker: TypeChecker): Symbol | undefined {
const ty = getContextualType(currentToken, checker);
const symbol = ty && ty.symbol;
// Don't include make a recommended completion for an abstract class
return symbol && (symbol.flags & SymbolFlags.Enum || symbol.flags & SymbolFlags.Class && !isAbstractConstructorSymbol(symbol))
? getFirstSymbolInChain(symbol, currentToken, checker)
: undefined;
}

function getContextualType(currentToken: Node, checker: ts.TypeChecker): Type | undefined {
const { parent } = currentToken;
switch (currentToken.kind) {
case ts.SyntaxKind.Identifier:
return getContextualTypeFromParent(currentToken as ts.Identifier, checker);
case ts.SyntaxKind.EqualsToken:
return ts.isVariableDeclaration(parent) ? checker.getContextualType(parent.initializer) :
ts.isBinaryExpression(parent) ? checker.getTypeAtLocation(parent.left) : undefined;
case ts.SyntaxKind.NewKeyword:
return checker.getContextualType(parent as ts.Expression);
case ts.SyntaxKind.CaseKeyword:
return getSwitchedType(cast(currentToken.parent, isCaseClause), checker);
default:
return isEqualityOperatorKind(currentToken.kind) && ts.isBinaryExpression(parent) && isEqualityOperatorKind(parent.operatorToken.kind)
// completion at `x ===/**/` should be for the right side
? checker.getTypeAtLocation(parent.left)
: checker.getContextualType(currentToken as ts.Expression);
}
}

function getContextualTypeFromParent(node: ts.Expression, checker: ts.TypeChecker): Type | undefined {
const { parent } = node;
switch (parent.kind) {
case ts.SyntaxKind.NewExpression:
return checker.getContextualType(parent as ts.NewExpression);
case ts.SyntaxKind.BinaryExpression: {
const { left, operatorToken, right } = parent as ts.BinaryExpression;
return isEqualityOperatorKind(operatorToken.kind)
? checker.getTypeAtLocation(node === right ? left : right)
: checker.getContextualType(node);
}
case ts.SyntaxKind.CaseClause:
return (parent as ts.CaseClause).expression === node ? getSwitchedType(parent as ts.CaseClause, checker) : undefined;
default:
return checker.getContextualType(node);
}
}

function getSwitchedType(caseClause: ts.CaseClause, checker: ts.TypeChecker): ts.Type {
return checker.getTypeAtLocation(caseClause.parent.parent.expression);
}

function getFirstSymbolInChain(symbol: Symbol, enclosingDeclaration: Node, checker: TypeChecker): Symbol | undefined {
const chain = checker.getAccessibleSymbolChain(symbol, enclosingDeclaration, /*meaning*/ SymbolFlags.All, /*useOnlyExternalAliasing*/ false);
if (chain) return first(chain);
Expand Down Expand Up @@ -851,7 +881,7 @@ namespace ts.Completions {

log("getCompletionData: Semantic work: " + (timestamp() - semanticStart));

const recommendedCompletion = getRecommendedCompletion(previousToken, typeChecker);
const recommendedCompletion = previousToken && getRecommendedCompletion(previousToken, typeChecker);
return { symbols, isGlobalCompletion, isMemberCompletion, allowStringLiteral, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters, symbolToOriginInfoMap, recommendedCompletion, previousToken };

type JSDocTagWithTypeExpression = JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag;
Expand Down Expand Up @@ -2081,15 +2111,16 @@ namespace ts.Completions {
return isConstructorParameterCompletionKeyword(stringToToken(text));
}

function isEqualityExpression(node: Node): node is BinaryExpression {
return isBinaryExpression(node) && isEqualityOperatorKind(node.operatorToken.kind);
}

function isEqualityOperatorKind(kind: SyntaxKind) {
return kind === SyntaxKind.EqualsEqualsToken ||
kind === SyntaxKind.ExclamationEqualsToken ||
kind === SyntaxKind.EqualsEqualsEqualsToken ||
kind === SyntaxKind.ExclamationEqualsEqualsToken;
function isEqualityOperatorKind(kind: ts.SyntaxKind): kind is EqualityOperator {
switch (kind) {
case ts.SyntaxKind.EqualsEqualsEqualsToken:
case ts.SyntaxKind.EqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsEqualsToken:
case ts.SyntaxKind.ExclamationEqualsToken:
return true;
default:
return false;
}
}

/** Get the corresponding JSDocTag node if the position is in a jsDoc comment */
Expand Down
2 changes: 1 addition & 1 deletion src/services/pathCompletions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* @internal */
namespace ts.Completions.PathCompletions {
export function getStringLiteralCompletionsFromModuleNames(node: StringLiteral, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): CompletionEntry[] {
export function getStringLiteralCompletionsFromModuleNames(node: LiteralExpression, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): CompletionEntry[] {
const literalValue = normalizeSlashes(node.text);

const scriptPath = node.getSourceFile().path;
Expand Down
12 changes: 7 additions & 5 deletions tests/cases/fourslash/completionsRecommended_equals.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
/// <reference path="fourslash.ts" />

////enum E {}
////declare const e: E;
////e === /**/
////enum Enu {}
////declare const e: Enu;
////e === /*a*/;
////e === E/*b*/

goTo.marker();
verify.completionListContains("E", "enum E", "", "enum", undefined, undefined, { isRecommended: true });
goTo.eachMarker(["a", "b"], () => {
verify.completionListContains("Enu", "enum Enu", "", "enum", undefined, undefined, { isRecommended: true });
});
39 changes: 25 additions & 14 deletions tests/cases/fourslash/completionsRecommended_import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,36 @@
// @noLib: true

// @Filename: /a.ts
////export class C {}
////export function f(c: C) {}
////export class Cls {}
////export function f(c: Cls) {}

// @Filename: /b.ts
////import { f } from "./a";
// Here we will recommend a new import of 'C'
////f(new /*b*/);
// Here we will recommend a new import of 'Cls'
////f(new C/*b0*/);
////f(new /*b1*/);

// @Filename: /c.ts
////import * as a from "./a";
// Here we will recommend 'a' because it contains 'C'.
////a.f(new /*c*/);
////import * as alpha from "./a";
// Here we will recommend 'alpha' because it contains 'Cls'.
////alpha.f(new al/*c0*/);
////alpha.f(new /*c1*/);

goTo.marker("b");
verify.completionListContains({ name: "C", source: "/a" }, "class C", "", "class", undefined, /*hasAction*/ true, {
includeExternalModuleExports: true,
isRecommended: true,
sourceDisplay: "./a",
goTo.eachMarker(["b0", "b1"], (_, idx) => {
verify.completionListContains(
{ name: "Cls", source: "/a" },
idx === 0 ? "constructor Cls(): Cls" : "class Cls",
"",
"class",
undefined,
/*hasAction*/ true, {
includeExternalModuleExports: true,
isRecommended: true,
sourceDisplay: "./a",
});
});

goTo.eachMarker(["c0", "c1"], (_, idx) => {
verify.completionListContains("alpha", "import alpha", "", "alias", undefined, undefined, { isRecommended: true })
});

goTo.marker("c");
verify.completionListContains("a", "import a", "", "alias", undefined, undefined, { isRecommended: true });
45 changes: 32 additions & 13 deletions tests/cases/fourslash/completionsRecommended_local.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
/// <reference path="fourslash.ts" />

////enum E {}
////class C {}
////abstract class A {}
////const e: E = /*e*/
////const c: C = new /*c*/
////const a: A = new /*a*/
////enum Enu {}
////class Cls {}
////abstract class Abs {}
////const e: Enu = E/*e0*/;
////const e: Enu = /*e1*/;
////const c: Cls = new C/*c0*/;
////const c: Cls = new /*c1*/;
////const a: Abs = new A/*a0*/;
////const a: Abs = new /*a1*/;

goTo.marker("e");
verify.completionListContains("E", "enum E", "", "enum", undefined, undefined, { isRecommended: true });
// Also works on mutations
////let enu: Enu;
////enu = E/*let0*/;
////enu = E/*let1*/;

goTo.marker("c");
verify.completionListContains("C", "class C", "", "class", undefined, undefined, { isRecommended: true });
goTo.eachMarker(["e0"], () => {//, "e1", "let0", "let1"
verify.completionListContains("Enu", "enum Enu", "", "enum", undefined, undefined, { isRecommended: true });
});

goTo.marker("a");
// Not recommended, because it's an abstract class
verify.completionListContains("A", "class A", "", "class");
goTo.eachMarker(["c0", "c1"], (_, idx) => {
verify.completionListContains(
"Cls",
idx === 0 ? "constructor Cls(): Cls" : "class Cls",
"",
"class",
undefined,
undefined, {
isRecommended: true,
});
});

goTo.eachMarker(["a0", "a1"], (_, idx) => {
// Not recommended, because it's an abstract class
verify.completionListContains("Abs", idx == 0 ? "constructor Abs(): Abs" : "class Abs", "", "class");
});
40 changes: 23 additions & 17 deletions tests/cases/fourslash/completionsRecommended_namespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,37 @@
// @noLib: true

// @Filename: /a.ts
////export namespace N {
////export namespace Name {
//// export class C {}
////}
////export function f(c: N.C) {}
////f(new /*a*/);
////export function f(c: Name.C) {}
////f(new N/*a0*/);
////f(new /*a1*/);

// @Filename: /b.ts
////import { f } from "./a";
// Here we will recommend a new import of 'N'
////f(new /*b*/);
// Here we will recommend a new import of 'Name'
////f(new N/*b0*/);
////f(new /*b1*/);

// @Filename: /c.ts
////import * as a from "./a";
// Here we will recommend 'a' because it contains 'N' which contains 'C'.
////a.f(new /*c*/);
////import * as alpha from "./a";
// Here we will recommend 'a' because it contains 'Name' which contains 'C'.
////alpha.f(new a/*c0*/);
////alpha.f(new /*c1*/);

goTo.marker("a");
verify.completionListContains("N", "namespace N", "", "module", undefined, undefined, { isRecommended: true });
goTo.eachMarker(["a0", "a1"], () => {
verify.completionListContains("Name", "namespace Name", "", "module", undefined, undefined, { isRecommended: true });
});

goTo.marker("b");
verify.completionListContains({ name: "N", source: "/a" }, "namespace N", "", "module", undefined, /*hasAction*/ true, {
includeExternalModuleExports: true,
isRecommended: true,
sourceDisplay: "./a",
goTo.eachMarker(["b0", "b1"], () => {
verify.completionListContains({ name: "Name", source: "/a" }, "namespace Name", "", "module", undefined, /*hasAction*/ true, {
includeExternalModuleExports: true,
isRecommended: true,
sourceDisplay: "./a",
});
});

goTo.marker("c");
verify.completionListContains("a", "import a", "", "alias", undefined, undefined, { isRecommended: true });
goTo.eachMarker(["c0", "c1"], () => {
verify.completionListContains("alpha", "import alpha", "", "alias", undefined, undefined, { isRecommended: true });
});
Loading