From 6f3648c00b2eea88096551a490ba74f922f12815 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:08:36 +0000 Subject: [PATCH 1/4] Initial plan From 1c94fdaa6ef8ec6cd2e3af3b10dd50f92a9314e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:40:17 +0000 Subject: [PATCH 2/4] Fix signature help assertion failure by replacing hard assert with graceful check Replace debug.Assert in getContainingArgumentInfo with a graceful nil return when a child node's range is not contained by its parent's range. This can happen due to reparsed nodes (e.g. JSDoc-derived nodes in JS files) whose positions may not align with their parents. Add fourslash test covering signature help with various JSDoc patterns (@param, @this, @type, @template) in JavaScript files. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/650815a7-829e-4de2-b0fe-1a4bba1f1b28 Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com> --- .../tests/signatureHelpJSDocReparsed_test.go | 46 ++++++ internal/ls/signaturehelp.go | 9 +- .../signatureHelpJSDocReparsed.baseline | 142 ++++++++++++++++++ 3 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 internal/fourslash/tests/signatureHelpJSDocReparsed_test.go create mode 100644 testdata/baselines/reference/fourslash/signatureHelp/signatureHelpJSDocReparsed.baseline diff --git a/internal/fourslash/tests/signatureHelpJSDocReparsed_test.go b/internal/fourslash/tests/signatureHelpJSDocReparsed_test.go new file mode 100644 index 0000000000..d607868875 --- /dev/null +++ b/internal/fourslash/tests/signatureHelpJSDocReparsed_test.go @@ -0,0 +1,46 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/testutil" +) + +func TestSignatureHelpJSDocReparsed(t *testing.T) { + t.Parallel() + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = `// @allowJs: true +// @checkJs: true +// @Filename: test.js +/** + * @param {string} x + */ +function foo(x) { + foo(/*1*/); +} +/** + * @this {string} + */ +function bar() { + bar(/*2*/); +} +/** + * @type {function(string): void} + */ +function qux(x) { + qux(/*3*/); +} +/** + * @template T + * @param {T} x + * @returns {T} + */ +function identity(x) { + identity(/*4*/); +} +` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + f.VerifyBaselineSignatureHelp(t) +} diff --git a/internal/ls/signaturehelp.go b/internal/ls/signaturehelp.go index 9d989ddafb..7266d193af 100644 --- a/internal/ls/signaturehelp.go +++ b/internal/ls/signaturehelp.go @@ -775,9 +775,12 @@ func containsPrecedingToken(startingToken *ast.Node, sourceFile *ast.SourceFile, func getContainingArgumentInfo(node *ast.Node, sourceFile *ast.SourceFile, checker *checker.Checker, isManuallyInvoked bool, position int) *argumentListInfo { for n := node; !ast.IsSourceFile(n) && (isManuallyInvoked || !ast.IsBlock(n)); n = n.Parent { - // If the node is not a subspan of its parent, this is a big problem. - // There have been crashes that might be caused by this violation. - debug.Assert(RangeContainsRange(n.Parent.Loc, n.Loc), "Not a subspan. Child: ", n.KindString(), ", parent: ", n.Parent.KindString()) + // If the node is not a subspan of its parent, bail out. + // This can happen due to reparsed nodes (e.g. JSDoc-derived nodes in JS files) + // whose positions may not align with their parents. + if !RangeContainsRange(n.Parent.Loc, n.Loc) { + return nil + } argumentInfo := getImmediatelyContainingArgumentOrContextualParameterInfo(n, position, sourceFile, checker) if argumentInfo != nil { return argumentInfo diff --git a/testdata/baselines/reference/fourslash/signatureHelp/signatureHelpJSDocReparsed.baseline b/testdata/baselines/reference/fourslash/signatureHelp/signatureHelpJSDocReparsed.baseline new file mode 100644 index 0000000000..54c37caedb --- /dev/null +++ b/testdata/baselines/reference/fourslash/signatureHelp/signatureHelpJSDocReparsed.baseline @@ -0,0 +1,142 @@ +// === SignatureHelp === +=== /test.js === +// /** +// * @param {string} x +// */ +// function foo(x) { +// foo(); +// ^ +// | ---------------------------------------------------------------------- +// | foo(**x: string**): void +// | ---------------------------------------------------------------------- +// } +// /** +// * @this {string} +// */ +// function bar() { +// bar(); +// ^ +// | ---------------------------------------------------------------------- +// | bar(): void +// | ---------------------------------------------------------------------- +// } +// /** +// * @type {function(string): void} +// */ +// function qux(x) { +// qux(); +// ^ +// | ---------------------------------------------------------------------- +// | qux(**x: any**): void +// | ---------------------------------------------------------------------- +// } +// /** +// * @template T +// * @param {T} x +// * @returns {T} +// */ +// function identity(x) { +// identity(); +// ^ +// | ---------------------------------------------------------------------- +// | identity(**x: unknown**): unknown +// | ---------------------------------------------------------------------- +// } +// +[ + { + "marker": { + "Position": 55, + "LSPosition": { + "line": 4, + "character": 8 + }, + "Name": "1", + "Data": {} + }, + "item": { + "signatures": [ + { + "label": "foo(x: string): void", + "parameters": [ + { + "label": "x: string" + } + ], + "activeParameter": 0 + } + ], + "activeSignature": 0 + } + }, + { + "marker": { + "Position": 111, + "LSPosition": { + "line": 10, + "character": 8 + }, + "Name": "2", + "Data": {} + }, + "item": { + "signatures": [ + { + "label": "bar(): void", + "parameters": [] + } + ], + "activeSignature": 0 + } + }, + { + "marker": { + "Position": 184, + "LSPosition": { + "line": 16, + "character": 8 + }, + "Name": "3", + "Data": {} + }, + "item": { + "signatures": [ + { + "label": "qux(x: any): void", + "parameters": [ + { + "label": "x: any" + } + ], + "activeParameter": 0 + } + ], + "activeSignature": 0 + } + }, + { + "marker": { + "Position": 280, + "LSPosition": { + "line": 24, + "character": 13 + }, + "Name": "4", + "Data": {} + }, + "item": { + "signatures": [ + { + "label": "identity(x: unknown): unknown", + "parameters": [ + { + "label": "x: unknown" + } + ], + "activeParameter": 0 + } + ], + "activeSignature": 0 + } + } +] \ No newline at end of file From 5132cb67866654ea2a7ee5128f2aeb053f8d2960 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 17:52:38 +0000 Subject: [PATCH 3/4] Fix signature help assertion failure by marking all DeepCloneReparse descendants as reparsed The root cause is that DeepCloneReparse only marked the root of a cloned subtree with NodeFlagsReparsed, leaving descendants unmarked. While FindPrecedingToken skips reparsed roots (and thus their subtrees), other traversal patterns walking the parent chain could encounter nodes with JSDoc-derived positions inside parents with source-derived positions, violating the range containment invariant. Fix by introducing setParentAndReparsedInChildren which sets both parent pointers AND NodeFlagsReparsed on all descendants during DeepCloneReparse. This ensures all JSDoc-derived cloned nodes are properly flagged, making the range containment assertion in getContainingArgumentInfo safe. The debug.Assert is kept intact as it correctly enforces that no node in the parent chain has a range outside its parent's range. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/9e893ee8-48fb-4d85-847e-57e7ae8a8752 Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com> --- internal/ast/deepclone.go | 3 +-- internal/ast/utilities.go | 19 +++++++++++++++++++ internal/ls/signaturehelp.go | 9 +++------ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/ast/deepclone.go b/internal/ast/deepclone.go index 48e9b4893d..c7d88032e3 100644 --- a/internal/ast/deepclone.go +++ b/internal/ast/deepclone.go @@ -75,8 +75,7 @@ func (f *NodeFactory) DeepCloneNode(node *Node) *Node { func (f *NodeFactory) DeepCloneReparse(node *Node) *Node { if node != nil { node = getDeepCloneVisitor(f, false /*syntheticLocation*/).VisitNode(node) - SetParentInChildren(node) - node.Flags |= NodeFlagsReparsed + setParentAndReparsedInChildren(node) } return node } diff --git a/internal/ast/utilities.go b/internal/ast/utilities.go index 686d093210..08a5e5b8eb 100644 --- a/internal/ast/utilities.go +++ b/internal/ast/utilities.go @@ -931,6 +931,25 @@ func SetParentInChildren(node *Node) { fn(node) } +// setParentAndReparsedInChildren is like SetParentInChildren but also marks +// all nodes with NodeFlagsReparsed. Used by DeepCloneReparse to ensure all +// cloned descendants from JSDoc are properly flagged as reparsed, so +// traversal code (e.g. FindPrecedingToken) correctly skips them. +func setParentAndReparsedInChildren(node *Node) { + node.Flags |= NodeFlagsReparsed + var visit func(*Node) bool + visit = func(child *Node) bool { + child.Parent = node + child.Flags |= NodeFlagsReparsed + saveParent := node + node = child + child.ForEachChild(visit) + node = saveParent + return false + } + node.ForEachChild(visit) +} + // This should never be called outside the parser func SetImportsOfSourceFile(node *SourceFile, imports []*LiteralLikeNode) { node.imports = imports diff --git a/internal/ls/signaturehelp.go b/internal/ls/signaturehelp.go index 7266d193af..9d989ddafb 100644 --- a/internal/ls/signaturehelp.go +++ b/internal/ls/signaturehelp.go @@ -775,12 +775,9 @@ func containsPrecedingToken(startingToken *ast.Node, sourceFile *ast.SourceFile, func getContainingArgumentInfo(node *ast.Node, sourceFile *ast.SourceFile, checker *checker.Checker, isManuallyInvoked bool, position int) *argumentListInfo { for n := node; !ast.IsSourceFile(n) && (isManuallyInvoked || !ast.IsBlock(n)); n = n.Parent { - // If the node is not a subspan of its parent, bail out. - // This can happen due to reparsed nodes (e.g. JSDoc-derived nodes in JS files) - // whose positions may not align with their parents. - if !RangeContainsRange(n.Parent.Loc, n.Loc) { - return nil - } + // If the node is not a subspan of its parent, this is a big problem. + // There have been crashes that might be caused by this violation. + debug.Assert(RangeContainsRange(n.Parent.Loc, n.Loc), "Not a subspan. Child: ", n.KindString(), ", parent: ", n.Parent.KindString()) argumentInfo := getImmediatelyContainingArgumentOrContextualParameterInfo(n, position, sourceFile, checker) if argumentInfo != nil { return argumentInfo From 8fdb10dc32e0c003b2244e7b132001cdca0c8a4a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 18:00:13 +0000 Subject: [PATCH 4/4] Refactor setParentAndReparsedInChildren to pass parent explicitly Address code review feedback: pass parent as an explicit parameter to the visit function instead of relying on closure variable reassignment. Also clarify in the comment that the root node is marked as well. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/9e893ee8-48fb-4d85-847e-57e7ae8a8752 Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com> --- internal/ast/utilities.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/internal/ast/utilities.go b/internal/ast/utilities.go index 08a5e5b8eb..8df011de7a 100644 --- a/internal/ast/utilities.go +++ b/internal/ast/utilities.go @@ -932,22 +932,24 @@ func SetParentInChildren(node *Node) { } // setParentAndReparsedInChildren is like SetParentInChildren but also marks -// all nodes with NodeFlagsReparsed. Used by DeepCloneReparse to ensure all -// cloned descendants from JSDoc are properly flagged as reparsed, so -// traversal code (e.g. FindPrecedingToken) correctly skips them. +// all nodes (including the root) with NodeFlagsReparsed. Used by +// DeepCloneReparse to ensure all cloned nodes from JSDoc are properly +// flagged as reparsed, so traversal code (e.g. FindPrecedingToken) +// correctly skips them. func setParentAndReparsedInChildren(node *Node) { node.Flags |= NodeFlagsReparsed - var visit func(*Node) bool - visit = func(child *Node) bool { - child.Parent = node + var visit func(child *Node, parent *Node) bool + visit = func(child *Node, parent *Node) bool { + child.Parent = parent child.Flags |= NodeFlagsReparsed - saveParent := node - node = child - child.ForEachChild(visit) - node = saveParent + child.ForEachChild(func(grandchild *Node) bool { + return visit(grandchild, child) + }) return false } - node.ForEachChild(visit) + node.ForEachChild(func(child *Node) bool { + return visit(child, node) + }) } // This should never be called outside the parser