Skip to content

Commit d352e3b

Browse files
yuitweswigham
authored andcommitted
[Master] fix 16407 - LS in binding element of object binding pattern (#16534)
* wip-try get symbol of bindingelement in objectBindingPattern first * Add fourslash tests * Update .types baselines * Update .symbols baselines * Revert checker changes * Actually lookup type for binding property name definition * More succinct check, clarify yui's comment
1 parent c92deef commit d352e3b

7 files changed

+81
-5
lines changed

src/services/goToDefinition.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,28 @@ namespace ts.GoToDefinition {
7676
declaration => createDefinitionInfo(declaration, shorthandSymbolKind, shorthandSymbolName, shorthandContainerName));
7777
}
7878

79+
// If the node is the name of a BindingElement within an ObjectBindingPattern instead of just returning the
80+
// declaration the symbol (which is itself), we should try to get to the original type of the ObjectBindingPattern
81+
// and return the property declaration for the referenced property.
82+
// For example:
83+
// import('./foo').then(({ b/*goto*/ar }) => undefined); => should get use to the declaration in file "./foo"
84+
//
85+
// function bar<T>(onfulfilled: (value: T) => void) { //....}
86+
// interface Test {
87+
// pr/*destination*/op1: number
88+
// }
89+
// bar<Test>(({pr/*goto*/op1})=>{});
90+
if (isPropertyName(node) && isBindingElement(node.parent) && isObjectBindingPattern(node.parent.parent) &&
91+
(node === (node.parent.propertyName || node.parent.name))) {
92+
const type = typeChecker.getTypeAtLocation(node.parent.parent);
93+
if (type) {
94+
const propSymbols = getPropertySymbolsFromType(type, node);
95+
if (propSymbols) {
96+
return flatMap(propSymbols, propSymbol => getDefinitionFromSymbol(typeChecker, propSymbol, node));
97+
}
98+
}
99+
}
100+
79101
// If the current location we want to find its definition is in an object literal, try to get the contextual type for the
80102
// object literal, lookup the property symbol in the contextual type, and use this for goto-definition.
81103
// For example

src/services/services.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,12 +2138,17 @@ namespace ts {
21382138
export function getPropertySymbolsFromContextualType(typeChecker: TypeChecker, node: ObjectLiteralElement): Symbol[] {
21392139
const objectLiteral = <ObjectLiteralExpression | JsxAttributes>node.parent;
21402140
const contextualType = typeChecker.getContextualType(objectLiteral);
2141-
const name = unescapeLeadingUnderscores(getTextOfPropertyName(node.name));
2142-
if (name && contextualType) {
2141+
return getPropertySymbolsFromType(contextualType, node.name);
2142+
}
2143+
2144+
/* @internal */
2145+
export function getPropertySymbolsFromType(type: Type, propName: PropertyName) {
2146+
const name = unescapeLeadingUnderscores(getTextOfPropertyName(propName));
2147+
if (name && type) {
21432148
const result: Symbol[] = [];
2144-
const symbol = contextualType.getProperty(name);
2145-
if (contextualType.flags & TypeFlags.Union) {
2146-
forEach((<UnionType>contextualType).types, t => {
2149+
const symbol = type.getProperty(name);
2150+
if (type.flags & TypeFlags.Union) {
2151+
forEach((<UnionType>type).types, t => {
21472152
const symbol = t.getProperty(name);
21482153
if (symbol) {
21492154
result.push(symbol);
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @Filename: foo.ts
4+
//// export function [|bar|]() { return "bar"; }
5+
6+
//// import('./foo').then(({ [|bar|] }) => undefined);
7+
8+
const [r0, r1] = test.ranges();
9+
// This is because bindingElement at r1 are both name and value
10+
verify.referencesOf(r0, [r1, r0, r1, r0]);
11+
verify.referencesOf(r1, [r0, r1, r1, r0]);
12+
verify.renameLocations(r0, [r0, r1]);
13+
verify.renameLocations(r1, [r1, r0, r0, r1]);
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @Filename: foo.ts
4+
//// export function /*Destination*/bar() { return "bar"; }
5+
6+
//// import('./foo').then(({ ba/*1*/r }) => undefined);
7+
8+
verify.goToDefinition("1", "Destination");
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @Filename: foo.ts
4+
//// export function /*Destination*/bar() { return "bar"; }
5+
6+
//// import('./foo').then(({ ba/*1*/r }) => undefined);
7+
8+
verify.goToDefinition("1", "Destination");
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// function bar<T>(onfulfilled: (value: T) => void) {
4+
//// return undefined;
5+
//// }
6+
7+
//// interface Test {
8+
//// /*destination*/prop2: number
9+
//// }
10+
//// bar<Test>(({pr/*goto*/op2})=>{});
11+
12+
verify.goToDefinition("goto", "destination");
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// var p0 = ({a/*1*/a}) => {console.log(aa)};
4+
//// function f2({ a/*a1*/1, b/*b1*/1 }: { /*a1_dest*/a1: number, /*b1_dest*/b1: number } = { a1: 0, b1: 0 }) {}
5+
6+
verify.goToDefinition("1", []);
7+
verify.goToDefinition("a1", "a1_dest");
8+
verify.goToDefinition("b1", "b1_dest");

0 commit comments

Comments
 (0)