Skip to content

Commit 816cfe3

Browse files
Merge pull request #3615 from Microsoft/completionsInObjectPatterns
Enable completions in object binding patterns
2 parents 560dbd9 + e52a27b commit 816cfe3

15 files changed

+239
-33
lines changed

src/compiler/checker.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -12386,7 +12386,7 @@ namespace ts {
1238612386
return resolveExternalModuleName(node, <LiteralExpression>node);
1238712387
}
1238812388

12389-
// Intentional fall-through
12389+
// fall through
1239012390
case SyntaxKind.NumericLiteral:
1239112391
// index access
1239212392
if (node.parent.kind === SyntaxKind.ElementAccessExpression && (<ElementAccessExpression>node.parent).argumentExpression === node) {
@@ -12453,6 +12453,10 @@ namespace ts {
1245312453
return symbol && getTypeOfSymbol(symbol);
1245412454
}
1245512455

12456+
if (isBindingPattern(node)) {
12457+
return getTypeForVariableLikeDeclaration(<VariableLikeDeclaration>node.parent);
12458+
}
12459+
1245612460
if (isInRightSideOfImportOrExportAssignment(<Identifier>node)) {
1245712461
let symbol = getSymbolInfo(node);
1245812462
let declaredType = symbol && getDeclaredTypeOfSymbol(symbol);

src/harness/fourslash.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -660,8 +660,7 @@ module FourSlash {
660660
}
661661
errorMsg += "]\n";
662662

663-
Harness.IO.log(errorMsg);
664-
this.raiseError("Member list is not empty at Caret");
663+
this.raiseError("Member list is not empty at Caret: " + errorMsg);
665664

666665
}
667666
}

src/services/services.ts

+53-30
Original file line numberDiff line numberDiff line change
@@ -2983,21 +2983,32 @@ namespace ts {
29832983
}
29842984

29852985
function tryGetGlobalSymbols(): boolean {
2986-
let containingObjectLiteral = getContainingObjectLiteralApplicableForCompletion(contextToken);
2987-
if (containingObjectLiteral) {
2986+
let objectLikeContainer = tryGetObjectLikeCompletionContainer(contextToken);
2987+
if (objectLikeContainer) {
29882988
// Object literal expression, look up possible property names from contextual type
29892989
isMemberCompletion = true;
29902990
isNewIdentifierLocation = true;
29912991

2992-
let contextualType = typeChecker.getContextualType(containingObjectLiteral);
2993-
if (!contextualType) {
2992+
let typeForObject: Type;
2993+
let existingMembers: Declaration[];
2994+
2995+
if (objectLikeContainer.kind === SyntaxKind.ObjectLiteralExpression) {
2996+
typeForObject = typeChecker.getContextualType(<ObjectLiteralExpression>objectLikeContainer);
2997+
existingMembers = (<ObjectLiteralExpression>objectLikeContainer).properties;
2998+
}
2999+
else {
3000+
typeForObject = typeChecker.getTypeAtLocation(objectLikeContainer);
3001+
existingMembers = (<BindingPattern>objectLikeContainer).elements;
3002+
}
3003+
3004+
if (!typeForObject) {
29943005
return false;
29953006
}
29963007

2997-
let contextualTypeMembers = typeChecker.getPropertiesOfType(contextualType);
2998-
if (contextualTypeMembers && contextualTypeMembers.length > 0) {
3008+
let typeMembers = typeChecker.getPropertiesOfType(typeForObject);
3009+
if (typeMembers && typeMembers.length > 0) {
29993010
// Add filtered items to the completion list
3000-
symbols = filterContextualMembersList(contextualTypeMembers, containingObjectLiteral.properties);
3011+
symbols = filterObjectMembersList(typeMembers, existingMembers);
30013012
}
30023013
}
30033014
else if (getAncestor(contextToken, SyntaxKind.ImportClause)) {
@@ -3185,17 +3196,18 @@ namespace ts {
31853196
return false;
31863197
}
31873198

3188-
function getContainingObjectLiteralApplicableForCompletion(previousToken: Node): ObjectLiteralExpression {
3189-
// The locations in an object literal expression that are applicable for completion are property name definition locations.
3190-
3191-
if (previousToken) {
3192-
let parent = previousToken.parent;
3193-
3194-
switch (previousToken.kind) {
3199+
/**
3200+
* Returns the immediate owning object literal or binding pattern of a context token,
3201+
* on the condition that one exists and that the context implies completion should be given.
3202+
*/
3203+
function tryGetObjectLikeCompletionContainer(contextToken: Node): ObjectLiteralExpression | BindingPattern {
3204+
if (contextToken) {
3205+
switch (contextToken.kind) {
31953206
case SyntaxKind.OpenBraceToken: // let x = { |
31963207
case SyntaxKind.CommaToken: // let x = { a: 0, |
3197-
if (parent && parent.kind === SyntaxKind.ObjectLiteralExpression) {
3198-
return <ObjectLiteralExpression>parent;
3208+
let parent = contextToken.parent;
3209+
if (parent && (parent.kind === SyntaxKind.ObjectLiteralExpression || parent.kind === SyntaxKind.ObjectBindingPattern)) {
3210+
return <ObjectLiteralExpression | BindingPattern>parent;
31993211
}
32003212
break;
32013213
}
@@ -3234,8 +3246,7 @@ namespace ts {
32343246
containingNodeKind === SyntaxKind.ClassDeclaration || // class A<T, |
32353247
containingNodeKind === SyntaxKind.FunctionDeclaration || // function A<T, |
32363248
containingNodeKind === SyntaxKind.InterfaceDeclaration || // interface A<T, |
3237-
containingNodeKind === SyntaxKind.ArrayBindingPattern || // var [x, y|
3238-
containingNodeKind === SyntaxKind.ObjectBindingPattern; // function func({ x, y|
3249+
containingNodeKind === SyntaxKind.ArrayBindingPattern; // var [x, y|
32393250

32403251
case SyntaxKind.DotToken:
32413252
return containingNodeKind === SyntaxKind.ArrayBindingPattern; // var [.|
@@ -3253,8 +3264,7 @@ namespace ts {
32533264
case SyntaxKind.OpenBraceToken:
32543265
return containingNodeKind === SyntaxKind.EnumDeclaration || // enum a { |
32553266
containingNodeKind === SyntaxKind.InterfaceDeclaration || // interface a { |
3256-
containingNodeKind === SyntaxKind.TypeLiteral || // let x : { |
3257-
containingNodeKind === SyntaxKind.ObjectBindingPattern; // function func({ x|
3267+
containingNodeKind === SyntaxKind.TypeLiteral; // let x : { |
32583268

32593269
case SyntaxKind.SemicolonToken:
32603270
return containingNodeKind === SyntaxKind.PropertySignature &&
@@ -3346,26 +3356,39 @@ namespace ts {
33463356
return filter(exports, e => !lookUp(exisingImports, e.name));
33473357
}
33483358

3349-
function filterContextualMembersList(contextualMemberSymbols: Symbol[], existingMembers: Declaration[]): Symbol[] {
3359+
function filterObjectMembersList(contextualMemberSymbols: Symbol[], existingMembers: Declaration[]): Symbol[] {
33503360
if (!existingMembers || existingMembers.length === 0) {
33513361
return contextualMemberSymbols;
33523362
}
33533363

33543364
let existingMemberNames: Map<boolean> = {};
3355-
forEach(existingMembers, m => {
3356-
if (m.kind !== SyntaxKind.PropertyAssignment && m.kind !== SyntaxKind.ShorthandPropertyAssignment) {
3357-
// Ignore omitted expressions for missing members in the object literal
3358-
return;
3365+
for (let m of existingMembers) {
3366+
// Ignore omitted expressions for missing members
3367+
if (m.kind !== SyntaxKind.PropertyAssignment &&
3368+
m.kind !== SyntaxKind.ShorthandPropertyAssignment &&
3369+
m.kind !== SyntaxKind.BindingElement) {
3370+
continue;
33593371
}
33603372

3373+
// If this is the current item we are editing right now, do not filter it out
33613374
if (m.getStart() <= position && position <= m.getEnd()) {
3362-
// If this is the current item we are editing right now, do not filter it out
3363-
return;
3375+
continue;
33643376
}
33653377

3366-
// TODO(jfreeman): Account for computed property name
3367-
existingMemberNames[(<Identifier>m.name).text] = true;
3368-
});
3378+
let existingName: string;
3379+
3380+
if (m.kind === SyntaxKind.BindingElement && (<BindingElement>m).propertyName) {
3381+
existingName = (<BindingElement>m).propertyName.text;
3382+
}
3383+
else {
3384+
// TODO(jfreeman): Account for computed property name
3385+
// NOTE: if one only performs this step when m.name is an identifier,
3386+
// things like '__proto__' are not filtered out.
3387+
existingName = (<Identifier>m.name).text;
3388+
}
3389+
3390+
existingMemberNames[existingName] = true;
3391+
}
33693392

33703393
let filteredMembers: Symbol[] = [];
33713394
forEach(contextualMemberSymbols, s => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var foo: I;
9+
////var { /**/ } = foo;
10+
11+
goTo.marker();
12+
verify.completionListContains("property1");
13+
verify.completionListContains("property2");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var foo: I;
9+
////var { property1, /**/ } = foo;
10+
11+
goTo.marker();
12+
verify.completionListContains("property2");
13+
verify.not.completionListContains("property1");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var foo: I;
9+
////var { property1: /**/ } = foo;
10+
11+
goTo.marker();
12+
verify.completionListAllowsNewIdentifier();
13+
verify.completionListIsEmpty();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var foo: I;
9+
////var { prope/**/ } = foo;
10+
11+
goTo.marker();
12+
verify.completionListContains("property1");
13+
verify.completionListContains("property2");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var foo: I;
9+
////var { property1/**/ } = foo;
10+
11+
goTo.marker();
12+
verify.completionListContains("property1");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var foo: I;
9+
////var { property1, property2, /**/ } = foo;
10+
11+
goTo.marker();
12+
verify.completionListIsEmpty();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// propertyOfI_1: number;
5+
//// propertyOfI_2: string;
6+
////}
7+
////interface J {
8+
//// property1: I;
9+
//// property2: string;
10+
////}
11+
////
12+
////var foo: J;
13+
////var { property1: { /**/ } } = foo;
14+
15+
goTo.marker();
16+
verify.completionListContains("propertyOfI_1");
17+
verify.completionListContains("propertyOfI_2");
18+
verify.not.completionListContains("property2");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// propertyOfI_1: number;
5+
//// propertyOfI_2: string;
6+
////}
7+
////interface J {
8+
//// property1: I;
9+
//// property2: string;
10+
////}
11+
////
12+
////var foo: J;
13+
////var { property1: { propertyOfI_1, /**/ } } = foo;
14+
15+
goTo.marker();
16+
verify.completionListContains("propertyOfI_2");
17+
verify.not.completionListContains("propertyOfI_1");
18+
verify.not.completionListContains("property2");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// propertyOfI_1: number;
5+
//// propertyOfI_2: string;
6+
////}
7+
////interface J {
8+
//// property1: I;
9+
//// property2: string;
10+
////}
11+
////
12+
////var foo: J;
13+
////var { property1: { propertyOfI_1, }, /**/ } = foo;
14+
15+
goTo.marker();
16+
verify.completionListContains("property2");
17+
verify.not.completionListContains("property1");
18+
verify.not.completionListContains("propertyOfI_2");
19+
verify.not.completionListContains("propertyOfI_1");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// propertyOfI_1: number;
5+
//// propertyOfI_2: string;
6+
////}
7+
////interface J {
8+
//// property1: I;
9+
//// property2: string;
10+
////}
11+
////
12+
////var foo: J[];
13+
////var [{ property1: { propertyOfI_1, }, /*1*/ }, { /*2*/ }] = foo;
14+
15+
goTo.marker("1");
16+
verify.completionListContains("property2");
17+
verify.not.completionListContains("property1");
18+
verify.not.completionListContains("propertyOfI_2");
19+
verify.not.completionListContains("propertyOfI_1");
20+
21+
goTo.marker("2");
22+
verify.completionListContains("property1");
23+
verify.completionListContains("property2");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////var { property1: prop1, /**/ }: I;
9+
10+
goTo.marker("");
11+
verify.completionListContains("property2");
12+
verify.not.completionListContains("property1");
13+
verify.not.completionListContains("prop1");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface I {
4+
//// property1: number;
5+
//// property2: string;
6+
////}
7+
////
8+
////function f({ property1, /**/ }: I): void {
9+
////}
10+
11+
goTo.marker("");
12+
verify.completionListContains("property2");
13+
verify.not.completionListContains("property1");

0 commit comments

Comments
 (0)