Skip to content

Get node's children with forEachChild + scanner in signature help #1584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 14, 2025
Merged

Get node's children with forEachChild + scanner in signature help #1584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f280652
fix signature help's use of token reading
gabritto Aug 14, 2025
d65240e
refactor position prefix into a method in fourslash
gabritto Aug 14, 2025
83ba066
Merge branch 'main' into gabritto/issue1298
gabritto Aug 14, 2025
4d032d3
rename and move getChildren function
gabritto Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 78 additions & 31 deletions internal/fourslash/fourslash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,12 +544,7 @@ func (f *FourslashTest) VerifyCompletions(t *testing.T, markerInput MarkerInput,
}

func (f *FourslashTest) verifyCompletionsWorker(t *testing.T, expected *CompletionsExpectedList) {
var prefix string
if f.lastKnownMarkerName != nil {
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
} else {
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
}
prefix := f.getCurrentPositionPrefix()
params := &lsproto.CompletionParams{
TextDocument: lsproto.TextDocumentIdentifier{
Uri: ls.FileNameToDocumentURI(f.activeFilename),
Expand All @@ -564,12 +559,11 @@ func (f *FourslashTest) verifyCompletionsWorker(t *testing.T, expected *Completi
if !resultOk {
t.Fatalf(prefix+"Unexpected response type for completion request: %T", resMsg.AsResponse().Result)
}
f.verifyCompletionsResult(t, f.currentCaretPosition, result.List, expected, prefix)
f.verifyCompletionsResult(t, result.List, expected, prefix)
}

func (f *FourslashTest) verifyCompletionsResult(
t *testing.T,
position lsproto.Position,
actual *lsproto.CompletionList,
expected *CompletionsExpectedList,
prefix string,
Expand Down Expand Up @@ -932,17 +926,11 @@ func (f *FourslashTest) VerifyBaselineHover(t *testing.T) {
}

resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentHoverInfo, params)
var prefix string
if f.lastKnownMarkerName != nil {
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
} else {
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
}
if resMsg == nil {
t.Fatalf(prefix+"Nil response received for quick info request", f.lastKnownMarkerName)
t.Fatalf(f.getCurrentPositionPrefix()+"Nil response received for quick info request", f.lastKnownMarkerName)
}
if !resultOk {
t.Fatalf(prefix+"Unexpected response type for quick info request: %T", resMsg.AsResponse().Result)
t.Fatalf(f.getCurrentPositionPrefix()+"Unexpected response type for quick info request: %T", resMsg.AsResponse().Result)
}

return markerAndItem[*lsproto.Hover]{Marker: marker, Item: result.Hover}, true
Expand Down Expand Up @@ -1025,17 +1013,11 @@ func (f *FourslashTest) VerifyBaselineSignatureHelp(t *testing.T) {
}

resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentSignatureHelpInfo, params)
var prefix string
if f.lastKnownMarkerName != nil {
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
} else {
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
}
if resMsg == nil {
t.Fatalf(prefix+"Nil response received for signature help request", f.lastKnownMarkerName)
t.Fatalf(f.getCurrentPositionPrefix()+"Nil response received for signature help request", f.lastKnownMarkerName)
}
if !resultOk {
t.Fatalf(prefix+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
t.Fatalf(f.getCurrentPositionPrefix()+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
}

return markerAndItem[*lsproto.SignatureHelp]{Marker: marker, Item: result.SignatureHelp}, true
Expand Down Expand Up @@ -1319,8 +1301,7 @@ func (f *FourslashTest) getScriptInfo(fileName string) *scriptInfo {
func (f *FourslashTest) VerifyQuickInfoAt(t *testing.T, marker string, expectedText string, expectedDocumentation string) {
f.GoToMarker(t, marker)
hover := f.getQuickInfoAtCurrentPosition(t)

f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, fmt.Sprintf("At marker '%s': ", marker))
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, f.getCurrentPositionPrefix())
}

func (f *FourslashTest) getQuickInfoAtCurrentPosition(t *testing.T) *lsproto.Hover {
Expand Down Expand Up @@ -1392,11 +1373,77 @@ func (f *FourslashTest) quickInfoIsEmpty(t *testing.T) (bool, *lsproto.Hover) {

func (f *FourslashTest) VerifyQuickInfoIs(t *testing.T, expectedText string, expectedDocumentation string) {
hover := f.getQuickInfoAtCurrentPosition(t)
var prefix string
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, f.getCurrentPositionPrefix())
}

type SignatureHelpCase struct {
Context *lsproto.SignatureHelpContext
MarkerInput MarkerInput
Expected *lsproto.SignatureHelp
}

func (f *FourslashTest) VerifySignatureHelp(t *testing.T, signatureHelpCases ...*SignatureHelpCase) {
for _, option := range signatureHelpCases {
switch marker := option.MarkerInput.(type) {
case string:
f.GoToMarker(t, marker)
f.verifySignatureHelp(t, option.Context, option.Expected)
case *Marker:
f.goToMarker(t, marker)
f.verifySignatureHelp(t, option.Context, option.Expected)
case []string:
for _, markerName := range marker {
f.GoToMarker(t, markerName)
f.verifySignatureHelp(t, option.Context, option.Expected)
}
case []*Marker:
for _, marker := range marker {
f.goToMarker(t, marker)
f.verifySignatureHelp(t, option.Context, option.Expected)
}
case nil:
f.verifySignatureHelp(t, option.Context, option.Expected)
default:
t.Fatalf("Invalid marker input type: %T. Expected string, *Marker, []string, or []*Marker.", option.MarkerInput)
}
}
}

func (f *FourslashTest) verifySignatureHelp(
t *testing.T,
context *lsproto.SignatureHelpContext,
expected *lsproto.SignatureHelp,
) {
prefix := f.getCurrentPositionPrefix()
params := &lsproto.SignatureHelpParams{
TextDocument: lsproto.TextDocumentIdentifier{
Uri: ls.FileNameToDocumentURI(f.activeFilename),
},
Position: f.currentCaretPosition,
Context: context,
}
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentSignatureHelpInfo, params)
if resMsg == nil {
t.Fatalf(prefix+"Nil response received for signature help request", f.lastKnownMarkerName)
}
if !resultOk {
t.Fatalf(prefix+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
}
f.verifySignatureHelpResult(t, result.SignatureHelp, expected, prefix)
}

func (f *FourslashTest) verifySignatureHelpResult(
t *testing.T,
actual *lsproto.SignatureHelp,
expected *lsproto.SignatureHelp,
prefix string,
) {
assertDeepEqual(t, actual, expected, prefix+" SignatureHelp mismatch")
}

func (f *FourslashTest) getCurrentPositionPrefix() string {
if f.lastKnownMarkerName != nil {
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
} else {
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
return fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
}
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, prefix)
return fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
}
44 changes: 44 additions & 0 deletions internal/fourslash/tests/signatureHelpTokenCrash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package fourslash_test

import (
"testing"

"github.com/microsoft/typescript-go/internal/fourslash"
. "github.com/microsoft/typescript-go/internal/fourslash/tests/util"
"github.com/microsoft/typescript-go/internal/lsp/lsproto"
"github.com/microsoft/typescript-go/internal/testutil"
)

func TestSignatureHelpTokenCrash(t *testing.T) {
t.Parallel()
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `
function foo(a: any, b: any) {

}

foo((/*1*/

/** This is a JSDoc comment */
foo/** More comments*/((/*2*/
`
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
f.VerifySignatureHelp(t, &fourslash.SignatureHelpCase{
MarkerInput: "1",
Expected: nil,
Context: &lsproto.SignatureHelpContext{
IsRetrigger: false,
TriggerCharacter: PtrTo("("),
TriggerKind: lsproto.SignatureHelpTriggerKindTriggerCharacter,
},
})
f.VerifySignatureHelp(t, &fourslash.SignatureHelpCase{
MarkerInput: "2",
Expected: nil,
Context: &lsproto.SignatureHelpContext{
IsRetrigger: false,
TriggerCharacter: PtrTo("("),
TriggerKind: lsproto.SignatureHelpTriggerKindTriggerCharacter,
},
})
}
30 changes: 3 additions & 27 deletions internal/ls/signaturehelp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,19 +597,14 @@ func getCandidateOrTypeInfo(info *argumentListInfo, c *checker.Checker, sourceFi
return nil // return Debug.assertNever(invocation);
}

func isSyntacticOwner(startingToken *ast.Node, node *ast.Node, sourceFile *ast.SourceFile) bool { // !!! not tested
func isSyntacticOwner(startingToken *ast.Node, node *ast.CallLikeExpression, sourceFile *ast.SourceFile) bool { // !!! not tested
if !ast.IsCallOrNewExpression(node) {
return false
}
invocationChildren := getTokensFromNode(node, sourceFile)
invocationChildren := getChildrenFromNonJSDocNode(node, sourceFile)
switch startingToken.Kind {
case ast.KindOpenParenToken:
return containsNode(invocationChildren, startingToken)
case ast.KindCommaToken:
case ast.KindOpenParenToken, ast.KindCommaToken:
return containsNode(invocationChildren, startingToken)
// !!!
// const containingList = findContainingList(startingToken);
// return !!containingList && contains(invocationChildren, containingList);
case ast.KindLessThanToken:
return containsPrecedingToken(startingToken, sourceFile, node.AsCallExpression().Expression)
default:
Expand Down Expand Up @@ -1079,25 +1074,6 @@ func countBinaryExpressionParameters(b *ast.BinaryExpression) int {
return 2
}

func getTokensFromNode(node *ast.Node, sourceFile *ast.SourceFile) []*ast.Node {
if node == nil {
return nil
}
var children []*ast.Node
current := node
left := node.Pos()
scanner := scanner.GetScannerForSourceFile(sourceFile, left)
for left < current.End() {
token := scanner.Token()
tokenFullStart := scanner.TokenFullStart()
tokenEnd := scanner.TokenEnd()
children = append(children, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, current))
left = tokenEnd
scanner.Scan()
}
return children
}

func getTokenFromNodeList(nodeList *ast.NodeList, nodeListParent *ast.Node, sourceFile *ast.SourceFile) []*ast.Node {
if nodeList == nil || nodeListParent == nil {
return nil
Expand Down
34 changes: 34 additions & 0 deletions internal/ls/utilities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1627,3 +1627,37 @@ func getLeadingCommentRangesOfNode(node *ast.Node, file *ast.SourceFile) iter.Se
}
return scanner.GetLeadingCommentRanges(&ast.NodeFactory{}, file.Text(), node.Pos())
}

// Equivalent to Strada's `node.getChildren()` for non-JSDoc nodes.
func getChildrenFromNonJSDocNode(node *ast.Node, sourceFile *ast.SourceFile) []*ast.Node {
var childNodes []*ast.Node
node.ForEachChild(func(child *ast.Node) bool {
childNodes = append(childNodes, child)
return false
})
var children []*ast.Node
pos := node.Pos()
for _, child := range childNodes {
scanner := scanner.GetScannerForSourceFile(sourceFile, pos)
for pos < child.Pos() {
token := scanner.Token()
tokenFullStart := scanner.TokenFullStart()
tokenEnd := scanner.TokenEnd()
children = append(children, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, node))
pos = tokenEnd
scanner.Scan()
}
children = append(children, child)
pos = child.End()
}
scanner := scanner.GetScannerForSourceFile(sourceFile, pos)
for pos < node.End() {
token := scanner.Token()
tokenFullStart := scanner.TokenFullStart()
tokenEnd := scanner.TokenEnd()
children = append(children, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, node))
pos = tokenEnd
scanner.Scan()
}
Comment on lines +1655 to +1661
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code seems to never be run; do we just not have an examples of trailing tokens at the moment? (The code seems correct, I think anyway)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it's just not tested right now, especially since it's only signature help using it and I don't know how good the signature help tests are in terms of coverage (even in Strada).

return children
}
Loading