Fix go-to-definition for binding elements in ObjectBindingPattern by Copilot · Pull Request #2842 · microsoft/typescript-go

Copilot · 2026-02-19T19:56:51Z

Add handling for BindingElement within ObjectBindingPattern in getDeclarationsFromLocation in internal/ls/definition.go
Create a test file for the new behavior (goToDefinitionObjectBindingPattern_test.go)
Generate and accept the baseline file for the new test
Update submodule test baselines that are now correctly fixed (4 tests now match TypeScript reference)
Restore SkipIfFailing in gen test files; keep tests out of failingTests.txt to enable them
Address reviewer feedback:
- Move block comment inside the if block, add blank lines for readability
- Add DotDotDotToken == nil guard to skip rest elements
- Add newline after backtick, add semicolons to interface property in test
- Add renaming test cases (markers 5-8): renamed property, local alias, nested destructuring, empty binding pattern
- Replace hardcoded marker names with f.MarkerNames()...
- Regenerate baseline with all 8 markers
Add separate TestGoToDefinitionObjectBindingPatternRest test covering ...rest spread binding (verifies rest elements navigate to the binding element itself, not a type property)

Original prompt

This section details on the original issue you should resolve

<issue_title>Cannot "go to definition" of a playwright fixture</issue_title>
<issue_description>## Steps to reproduce
Clone the repository at https://github.com/david-guevara-gorillalogic/tsgo-playwright-issue.git

This an standard playwright repository with a custom fixture:
export const test = base.extend<{
  helper: () => Promise<void>;
}>({
  helper: async ({ page }, use) => {
    await use(async () => {
      await page.goto("https://playwright.dev/");
      await expect(page).toHaveTitle(/Playwright/);
    });
  },
});
Open a test file that uses that custom fixture:
test("has title", async ({ helper }) => {
  await helper();
});
Move your cursor over the helper fixture and try to "go to definition".

Your editor won't go to the definition inside the fixtures file, at most it will move to the parameter { helper }.
Behavior with typescript@6.0

The editor will go to the definition inside the fixture file.

Behavior with tsgo

The editor won't go to the definition inside the fixtures file, at most it will move to the parameter { helper }.

Notes

I tested this with neovim and VSCode and was able to reproduce the issue in both.</issue_description>

Comments on the Issue (you are @copilot in this section)
@DanielRosenwasser I do get references in VS Code, but only because of the default `editor.gotoLocation.alternativeDefinitionCommand`.
Basically I think tsgo sees this as a definition, and the editor instead decides to retry as find-all-references.

Here's a more minimal repro:
interface SomeType {
    targetProperty: number
}

function foo(callback: (p: SomeType) => void) {
}

foo(({ /*1*/targetProperty }) => {
    /*4*/targetProperty
})

let { /*2*/targetProperty }: SomeType = { /*3*/targetProperty: 42 }
A test can just baseline go-to-definition on all markers. A fix should witness a change in go-to-definition on targets /*1*/ and /*2*/.</comment_new>

Fixes Cannot "go to definition" of a playwright fixture #2839

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

DanielRosenwasser · 2026-02-19T22:24:50Z

internal/fourslash/tests/gen/gotoDefinitionInObjectBindingPattern1_test.go

 )

 func TestGotoDefinitionInObjectBindingPattern1(t *testing.T) {
-	fourslash.SkipIfFailing(t)


@copilot Instead of just deleting these, you need to modify the failingtests.txt file

Restored SkipIfFailing(t) to all three gen test files in f55d461. The tests are now enabled by keeping them out of failingTests.txt (rather than removing the SkipIfFailing calls).

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

Copilot

Pull request overview

This PR fixes go-to-definition functionality for binding elements in object binding patterns, enabling developers to navigate from destructured parameters to their type definitions. The issue was reported in the context of Playwright fixtures where go-to-definition failed to navigate to the fixture definition.

Changes:

Added new handling logic in getDeclarationsFromLocation for ObjectBindingPattern binding elements that navigates to the property declaration in the contextual type
Created a new test file with comprehensive test cases for the feature
Updated 4 existing test baselines that now correctly match TypeScript's reference behavior

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/ls/definition.go	Added logic to detect binding elements in object binding patterns and navigate to their property declarations in the contextual type using GetTypeAtLocation and GetPropertyOfType
internal/fourslash/tests/goToDefinitionObjectBindingPattern_test.go	New test file with test cases covering function parameters, variable declarations, and object literals with destructuring
testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionObjectBindingPattern.baseline.jsonc	New baseline file for the custom test, showing correct navigation to interface properties
testdata/baselines/reference/submodule/fourslash/goToDefinition/gotoDefinitionInObjectBindingPattern1.baseline.jsonc	Updated baseline - now correctly navigates to interface property instead of binding element
testdata/baselines/reference/submodule/fourslash/goToDefinition/gotoDefinitionInObjectBindingPattern1.baseline.jsonc.diff	Removed - test now matches TypeScript reference
testdata/baselines/reference/submodule/fourslash/goToDefinition/gotoDefinitionInObjectBindingPattern2.baseline.jsonc	Updated baseline - now correctly navigates to interface properties
testdata/baselines/reference/submodule/fourslash/goToDefinition/gotoDefinitionInObjectBindingPattern2.baseline.jsonc.diff	Removed - test now matches TypeScript reference
testdata/baselines/reference/submodule/fourslash/goToDefinition/goToDefinitionDynamicImport3.baseline.jsonc	Updated baseline - now correctly navigates to export declarations in dynamic imports
testdata/baselines/reference/submodule/fourslash/goToDefinition/goToDefinitionDynamicImport3.baseline.jsonc.diff	Removed - test now matches TypeScript reference
testdata/baselines/reference/submodule/fourslash/goToDefinition/goToDefinitionDynamicImport4.baseline.jsonc	Updated baseline - now correctly navigates to export declarations in dynamic imports
testdata/baselines/reference/submodule/fourslash/goToDefinition/goToDefinitionDynamicImport4.baseline.jsonc.diff	Removed - test now matches TypeScript reference

Copilot · 2026-02-20T00:41:07Z

internal/ls/definition.go

+	if ast.IsPropertyName(node) && ast.IsBindingElement(node.Parent) && ast.IsObjectBindingPattern(node.Parent.Parent) {
+		bindingEl := node.Parent.AsBindingElement()
+		if node == core.OrElse(bindingEl.PropertyName, node.Parent.Name()) {
+			if name, ok := ast.TryGetTextOfPropertyName(node); ok {
+				t := c.GetTypeAtLocation(node.Parent.Parent)
+				types := []*checker.Type{t}
+				if t.IsUnion() {
+					types = t.Types()
+				}
+				var result []*ast.Node
+				for _, unionType := range types {
+					if prop := c.GetPropertyOfType(unionType, name); prop != nil {
+						result = append(result, prop.Declarations...)
+					}
+				}
+				return result
+			}
+		}
+	}


The implementation should check if the BindingElement is a rest element (has DotDotDotToken) before attempting to navigate to property declarations. Rest elements like const { ...rest } = obj capture all remaining properties and should not navigate to a specific property in the type definition. Add a check: if bindingEl.DotDotDotToken != nil { return nil } or similar early in the block.

Done in d4ec7c7 — added bindingEl.DotDotDotToken == nil guard so rest elements like { ...rest } skip the property lookup.

DanielRosenwasser

@copilot

DanielRosenwasser · 2026-02-20T00:38:47Z

internal/fourslash/tests/goToDefinitionObjectBindingPattern_test.go

+	const content = `interface SomeType {
+    [|/*targetPropertyDef*/targetProperty|]: number;


Suggested change

const content = `interface SomeType {

[|/*targetPropertyDef*/targetProperty|]: number;

const content = `

interface SomeType {

targetProperty: number;

Done in d4ec7c7 — added newline after backtick and semicolons in the interface.

DanielRosenwasser · 2026-02-20T00:42:15Z

internal/ls/definition.go

+	// If the node is the name of a BindingElement within an ObjectBindingPattern instead of just returning the
+	// declaration of the symbol (which is itself), we should try to get to the original type of the
+	// ObjectBindingPattern and return the property declaration for the referenced property.
+	// For example:
+	//      import('./foo').then(({ bar }) => undefined); => should navigate to the declaration in file "./foo"
+	//
+	//      function bar<T>(onfulfilled: (value: T) => void) { }
+	//      interface Test { prop1: number }
+	//      bar<Test>(({ prop1 }) => {});  => should navigate to prop1 in Test


Add a newline, move the comment into the if check as is done above.

Done in d4ec7c7 — added blank line before the if block and moved the comment inside it.

DanielRosenwasser · 2026-02-20T00:42:26Z

internal/ls/definition.go

+	}
 	node = getDeclarationNameForKeyword(node)


Suggested change

}

node = getDeclarationNameForKeyword(node)

}

node = getDeclarationNameForKeyword(node)

Done in d4ec7c7.

DanielRosenwasser · 2026-02-20T00:49:32Z

internal/ls/definition.go

+	//      bar<Test>(({ prop1 }) => {});  => should navigate to prop1 in Test
+	if ast.IsPropertyName(node) && ast.IsBindingElement(node.Parent) && ast.IsObjectBindingPattern(node.Parent.Parent) {
+		bindingEl := node.Parent.AsBindingElement()
+		if node == core.OrElse(bindingEl.PropertyName, node.Parent.Name()) {


I don't think I see any tests where destructuring re-names the property. Add the following to the newly-added test

let { /*5*/targetProperty: /*6*/alias_1 }: SomeType = { targetProperty: 42 }; let { x: { /*7*/targetProperty: /*8*/{} }: { x: SomeType } = { x: { targetProperty: 42 } };

I'd expect marker 8 not to go anywhere.

Done in d4ec7c7 — added markers 5–8. Results in baseline: 5 → SomeType.targetProperty, 6 → binding element declaration for alias_1, 7 → SomeType.targetProperty, 8 → no definition (empty ObjectBindingPattern).

DanielRosenwasser · 2026-02-20T00:51:08Z

internal/fourslash/tests/goToDefinitionObjectBindingPattern_test.go

+let { [|/*2*/targetProperty|] }: SomeType = { /*3*/targetProperty: 42 };`
+	f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
+	defer done()
+	f.VerifyBaselineGoToDefinition(t, true, "1", "2", "3", "4")


Don't hard-code these in - look at how other tests baseline with a call to Markers() or whatever it is.

Done in d4ec7c7 — using f.MarkerNames()... now.

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

DanielRosenwasser · 2026-02-20T06:18:23Z

internal/ls/definition.go

+		//      interface Test { prop1: number }
+		//      bar<Test>(({ prop1 }) => {});  => should navigate to prop1 in Test
+		bindingEl := node.Parent.AsBindingElement()
+		if bindingEl.DotDotDotToken == nil && node == core.OrElse(bindingEl.PropertyName, node.Parent.Name()) {


If we're checking for ...bindings, we should add another test. The newly-added one is getting a bit long, so add a separate test in the same file.

@copilot go

Done in 9e89742 — added TestGoToDefinitionObjectBindingPatternRest as a separate test in the same file. It verifies that let { ...rest }: SomeType = {...} navigates to the rest binding element declaration (not a type property), confirming the DotDotDotToken guard works correctly.

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

Initial plan

6e3523b

Copilot AI assigned Copilot and DanielRosenwasser Feb 19, 2026

Copilot started work on behalf of DanielRosenwasser February 19, 2026 19:56 View session

Fix go-to-definition for binding elements in ObjectBindingPattern

5848990

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix go to definition issue for playwright fixture~~ Fix go-to-definition for binding elements in ObjectBindingPattern Feb 19, 2026

Copilot finished work on behalf of DanielRosenwasser February 19, 2026 20:21

DanielRosenwasser reviewed Feb 19, 2026

View reviewed changes

Copilot started work on behalf of DanielRosenwasser February 19, 2026 22:25 View session

Restore SkipIfFailing in gen tests; remove from failingTests.txt instead

f55d461

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

Copilot finished work on behalf of DanielRosenwasser February 19, 2026 22:34

DanielRosenwasser marked this pull request as ready for review February 20, 2026 00:35

Copilot AI review requested due to automatic review settings February 20, 2026 00:35

Copilot started reviewing on behalf of DanielRosenwasser February 20, 2026 00:36 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

DanielRosenwasser requested changes Feb 20, 2026

View reviewed changes

Address reviewer feedback: refactor definition.go, expand test coverage

d4ec7c7

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

Copilot finished work on behalf of DanielRosenwasser February 20, 2026 01:09

DanielRosenwasser enabled auto-merge February 20, 2026 06:16

DanielRosenwasser reviewed Feb 20, 2026

View reviewed changes

Copilot started work on behalf of DanielRosenwasser February 20, 2026 06:20 View session

Add separate test for rest binding elements in ObjectBindingPattern

9e89742

Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>

Copilot finished work on behalf of DanielRosenwasser February 20, 2026 06:28

		const content = `interface SomeType {
		[\|/targetPropertyDef/targetProperty\|]: number;

Conversation

Copilot AI commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behavior with typescript@6.0

Behavior with tsgo

Notes

Comments on the Issue (you are @copilot in this section)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

DanielRosenwasser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Copilot AI commented Feb 19, 2026 •

edited

Loading

Behavior with `typescript@6.0`

Behavior with `tsgo`