From 87afd974b8252cbea45465fc6db57871e554b095 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 15:43:18 +0000 Subject: [PATCH 1/7] Initial plan From 6fab600b41cfc0415de02981f3d2a57744e11cba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 16:10:24 +0000 Subject: [PATCH 2/7] Add handling for BindingPattern in Node.Text() method Fixes panic when calling Text() on binding patterns (object/array destructuring). Returns empty string since binding patterns don't have simple text representation. Added test case for findAllReferences with JSDoc parameter tags on destructured params. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- internal/ast/ast.go | 4 +++ .../findAllRefsJSDocDestructuredParam_test.go | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 internal/fourslash/tests/manual/findAllRefsJSDocDestructuredParam_test.go diff --git a/internal/ast/ast.go b/internal/ast/ast.go index 2971ef562d..7d68eb1bd3 100644 --- a/internal/ast/ast.go +++ b/internal/ast/ast.go @@ -334,6 +334,10 @@ func (n *Node) Text() string { return strings.Join(n.AsJSDocLinkCode().text, "") case KindJSDocLinkPlain: return strings.Join(n.AsJSDocLinkPlain().text, "") + case KindObjectBindingPattern, KindArrayBindingPattern: + // Binding patterns don't have a simple text representation + // Return empty string to avoid panics in comparisons + return "" } panic(fmt.Sprintf("Unhandled case in Node.Text: %T", n.data)) } diff --git a/internal/fourslash/tests/manual/findAllRefsJSDocDestructuredParam_test.go b/internal/fourslash/tests/manual/findAllRefsJSDocDestructuredParam_test.go new file mode 100644 index 0000000000..066c4d24fe --- /dev/null +++ b/internal/fourslash/tests/manual/findAllRefsJSDocDestructuredParam_test.go @@ -0,0 +1,25 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/testutil" +) + +// Regression test for panic: "Unhandled case in Node.Text: *ast.BindingPattern" +// This occurred when requesting findAllReferences with a JSDoc parameter tag +// that referenced a destructured parameter. +func TestFindAllRefsJSDocDestructuredParam(t *testing.T) { + t.Parallel() + + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = ` +/** + * @param /*1*/obj - object destructured parameter + */ +function f({ x, y }) {} +` + f := fourslash.NewFourslash(t, nil /*capabilities*/, content) + f.VerifyBaselineFindAllReferences(t, "1") +} From 29a1c2fc2a4661333e99c6a1ff50a47dd5b20ca4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 16:12:48 +0000 Subject: [PATCH 3/7] Accept baseline for findAllRefsJSDocDestructuredParam test Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- .../findAllRefsJSDocDestructuredParam.baseline.jsonc | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocDestructuredParam.baseline.jsonc diff --git a/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocDestructuredParam.baseline.jsonc b/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocDestructuredParam.baseline.jsonc new file mode 100644 index 0000000000..962605e2c1 --- /dev/null +++ b/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocDestructuredParam.baseline.jsonc @@ -0,0 +1,7 @@ +// === findAllReferences === +// === /findAllRefsJSDocDestructuredParam.ts === +// /** +// * @param /*FIND ALL REFS*/obj - object destructured parameter +// */ +// function f({ x, y }) {} +// \ No newline at end of file From e16624b9d244fcaf84a1672db0068f035fbd118a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 16:21:29 +0000 Subject: [PATCH 4/7] Add array binding pattern test case Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- ...AllRefsJSDocArrayDestructuredParam_test.go | 24 +++++++++++++++++++ ...JSDocArrayDestructuredParam.baseline.jsonc | 7 ++++++ 2 files changed, 31 insertions(+) create mode 100644 internal/fourslash/tests/manual/findAllRefsJSDocArrayDestructuredParam_test.go create mode 100644 testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocArrayDestructuredParam.baseline.jsonc diff --git a/internal/fourslash/tests/manual/findAllRefsJSDocArrayDestructuredParam_test.go b/internal/fourslash/tests/manual/findAllRefsJSDocArrayDestructuredParam_test.go new file mode 100644 index 0000000000..7dbcc3e67c --- /dev/null +++ b/internal/fourslash/tests/manual/findAllRefsJSDocArrayDestructuredParam_test.go @@ -0,0 +1,24 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/testutil" +) + +// Regression test for panic: "Unhandled case in Node.Text: *ast.BindingPattern" +// This tests array binding patterns specifically. +func TestFindAllRefsJSDocArrayDestructuredParam(t *testing.T) { + t.Parallel() + + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = ` +/** + * @param /*1*/arr - array destructured parameter + */ +function f([x, y]) {} +` + f := fourslash.NewFourslash(t, nil /*capabilities*/, content) + f.VerifyBaselineFindAllReferences(t, "1") +} diff --git a/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocArrayDestructuredParam.baseline.jsonc b/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocArrayDestructuredParam.baseline.jsonc new file mode 100644 index 0000000000..35e1555c2e --- /dev/null +++ b/testdata/baselines/reference/fourslash/findAllReferences/findAllRefsJSDocArrayDestructuredParam.baseline.jsonc @@ -0,0 +1,7 @@ +// === findAllReferences === +// === /findAllRefsJSDocArrayDestructuredParam.ts === +// /** +// * @param /*FIND ALL REFS*/arr - array destructured parameter +// */ +// function f([x, y]) {} +// \ No newline at end of file From 093a1e0c6fe36153afe1a57bc5f1224cada34121 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 16:59:55 +0000 Subject: [PATCH 5/7] Fix panic by checking for binding patterns before calling Text() The correct fix is to check if a parameter name is a binding pattern before calling Text() on it, rather than making Text() handle binding patterns. This follows the TypeScript reference implementation which checks !isBindingPattern(param.name) before accessing the name's text. Reverted the previous change to ast.go and fixed the caller in checker.go instead. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- internal/ast/ast.go | 4 ---- internal/checker/checker.go | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/ast/ast.go b/internal/ast/ast.go index 7d68eb1bd3..2971ef562d 100644 --- a/internal/ast/ast.go +++ b/internal/ast/ast.go @@ -334,10 +334,6 @@ func (n *Node) Text() string { return strings.Join(n.AsJSDocLinkCode().text, "") case KindJSDocLinkPlain: return strings.Join(n.AsJSDocLinkPlain().text, "") - case KindObjectBindingPattern, KindArrayBindingPattern: - // Binding patterns don't have a simple text representation - // Return empty string to avoid panics in comparisons - return "" } panic(fmt.Sprintf("Unhandled case in Node.Text: %T", n.data)) } diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 0ff050b0a7..5207318ad6 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -30398,7 +30398,8 @@ func (c *Checker) getSymbolAtLocation(node *ast.Node, ignoreErrors bool) *ast.Sy } else if ast.IsJSDocParameterTag(parent) && parent.Name() == node { if fn := ast.GetNodeAtPosition(ast.GetSourceFileOfNode(node), node.Pos(), false); fn != nil && ast.IsFunctionLike(fn) { for _, param := range fn.Parameters() { - if param.Name().Text() == node.Text() { + // Skip binding patterns - they don't have a simple name to compare + if !ast.IsBindingPattern(param.Name()) && param.Name().Text() == node.Text() { return c.getSymbolOfNode(param) } } From adaae6033a7a957a86c5c59f4b36ae4b91f5ab71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 18:22:22 +0000 Subject: [PATCH 6/7] Use positive IsIdentifier check instead of negative IsBindingPattern check Following the TypeScript reference more precisely - check if param.Name() is an Identifier rather than checking if it's NOT a binding pattern. This is more explicit and matches the pattern in utilities.ts:4703 where it checks `p.name.kind === SyntaxKind.Identifier`. Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com> --- internal/checker/checker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 5207318ad6..d31bd51fb3 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -30398,8 +30398,8 @@ func (c *Checker) getSymbolAtLocation(node *ast.Node, ignoreErrors bool) *ast.Sy } else if ast.IsJSDocParameterTag(parent) && parent.Name() == node { if fn := ast.GetNodeAtPosition(ast.GetSourceFileOfNode(node), node.Pos(), false); fn != nil && ast.IsFunctionLike(fn) { for _, param := range fn.Parameters() { - // Skip binding patterns - they don't have a simple name to compare - if !ast.IsBindingPattern(param.Name()) && param.Name().Text() == node.Text() { + // Only compare against identifier names, not binding patterns + if ast.IsIdentifier(param.Name()) && param.Name().Text() == node.Text() { return c.getSymbolOfNode(param) } } From 07dfd07eadeee07d60033465a7a18f04b54d94d1 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 29 Oct 2025 11:45:30 -0700 Subject: [PATCH 7/7] Remove rando comment --- internal/checker/checker.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index d31bd51fb3..b59a382777 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -30398,7 +30398,6 @@ func (c *Checker) getSymbolAtLocation(node *ast.Node, ignoreErrors bool) *ast.Sy } else if ast.IsJSDocParameterTag(parent) && parent.Name() == node { if fn := ast.GetNodeAtPosition(ast.GetSourceFileOfNode(node), node.Pos(), false); fn != nil && ast.IsFunctionLike(fn) { for _, param := range fn.Parameters() { - // Only compare against identifier names, not binding patterns if ast.IsIdentifier(param.Name()) && param.Name().Text() == node.Text() { return c.getSymbolOfNode(param) }