Skip to content

Commit 0711420

Browse files
gabrittogabritto
authored
Get node's children with forEachChild + scanner in signature help (#1584)
1 parent 73637ac commit 0711420

File tree

4 files changed

+159
-58
lines changed

4 files changed

+159
-58
lines changed

‎internal/fourslash/fourslash.go‎

Lines changed: 78 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -544,12 +544,7 @@ func (f *FourslashTest) VerifyCompletions(t *testing.T, markerInput MarkerInput,
544544
}
545545

546546
func (f *FourslashTest) verifyCompletionsWorker(t *testing.T, expected *CompletionsExpectedList) {
547-
var prefix string
548-
if f.lastKnownMarkerName != nil {
549-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
550-
} else {
551-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
552-
}
547+
prefix := f.getCurrentPositionPrefix()
553548
params := &lsproto.CompletionParams{
554549
TextDocument: lsproto.TextDocumentIdentifier{
555550
Uri: ls.FileNameToDocumentURI(f.activeFilename),
@@ -564,12 +559,11 @@ func (f *FourslashTest) verifyCompletionsWorker(t *testing.T, expected *Completi
564559
if !resultOk {
565560
t.Fatalf(prefix+"Unexpected response type for completion request: %T", resMsg.AsResponse().Result)
566561
}
567-
f.verifyCompletionsResult(t, f.currentCaretPosition, result.List, expected, prefix)
562+
f.verifyCompletionsResult(t, result.List, expected, prefix)
568563
}
569564

570565
func (f *FourslashTest) verifyCompletionsResult(
571566
t *testing.T,
572-
position lsproto.Position,
573567
actual *lsproto.CompletionList,
574568
expected *CompletionsExpectedList,
575569
prefix string,
@@ -932,17 +926,11 @@ func (f *FourslashTest) VerifyBaselineHover(t *testing.T) {
932926
}
933927

934928
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentHoverInfo, params)
935-
var prefix string
936-
if f.lastKnownMarkerName != nil {
937-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
938-
} else {
939-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
940-
}
941929
if resMsg == nil {
942-
t.Fatalf(prefix+"Nil response received for quick info request", f.lastKnownMarkerName)
930+
t.Fatalf(f.getCurrentPositionPrefix()+"Nil response received for quick info request", f.lastKnownMarkerName)
943931
}
944932
if !resultOk {
945-
t.Fatalf(prefix+"Unexpected response type for quick info request: %T", resMsg.AsResponse().Result)
933+
t.Fatalf(f.getCurrentPositionPrefix()+"Unexpected response type for quick info request: %T", resMsg.AsResponse().Result)
946934
}
947935

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

10271015
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentSignatureHelpInfo, params)
1028-
var prefix string
1029-
if f.lastKnownMarkerName != nil {
1030-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
1031-
} else {
1032-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
1033-
}
10341016
if resMsg == nil {
1035-
t.Fatalf(prefix+"Nil response received for signature help request", f.lastKnownMarkerName)
1017+
t.Fatalf(f.getCurrentPositionPrefix()+"Nil response received for signature help request", f.lastKnownMarkerName)
10361018
}
10371019
if !resultOk {
1038-
t.Fatalf(prefix+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
1020+
t.Fatalf(f.getCurrentPositionPrefix()+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
10391021
}
10401022

10411023
return markerAndItem[*lsproto.SignatureHelp]{Marker: marker, Item: result.SignatureHelp}, true
@@ -1319,8 +1301,7 @@ func (f *FourslashTest) getScriptInfo(fileName string) *scriptInfo {
13191301
func (f *FourslashTest) VerifyQuickInfoAt(t *testing.T, marker string, expectedText string, expectedDocumentation string) {
13201302
f.GoToMarker(t, marker)
13211303
hover := f.getQuickInfoAtCurrentPosition(t)
1322-
1323-
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, fmt.Sprintf("At marker '%s': ", marker))
1304+
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, f.getCurrentPositionPrefix())
13241305
}
13251306

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

13931374
func (f *FourslashTest) VerifyQuickInfoIs(t *testing.T, expectedText string, expectedDocumentation string) {
13941375
hover := f.getQuickInfoAtCurrentPosition(t)
1395-
var prefix string
1376+
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, f.getCurrentPositionPrefix())
1377+
}
1378+
1379+
type SignatureHelpCase struct {
1380+
Context *lsproto.SignatureHelpContext
1381+
MarkerInput MarkerInput
1382+
Expected *lsproto.SignatureHelp
1383+
}
1384+
1385+
func (f *FourslashTest) VerifySignatureHelp(t *testing.T, signatureHelpCases ...*SignatureHelpCase) {
1386+
for _, option := range signatureHelpCases {
1387+
switch marker := option.MarkerInput.(type) {
1388+
case string:
1389+
f.GoToMarker(t, marker)
1390+
f.verifySignatureHelp(t, option.Context, option.Expected)
1391+
case *Marker:
1392+
f.goToMarker(t, marker)
1393+
f.verifySignatureHelp(t, option.Context, option.Expected)
1394+
case []string:
1395+
for _, markerName := range marker {
1396+
f.GoToMarker(t, markerName)
1397+
f.verifySignatureHelp(t, option.Context, option.Expected)
1398+
}
1399+
case []*Marker:
1400+
for _, marker := range marker {
1401+
f.goToMarker(t, marker)
1402+
f.verifySignatureHelp(t, option.Context, option.Expected)
1403+
}
1404+
case nil:
1405+
f.verifySignatureHelp(t, option.Context, option.Expected)
1406+
default:
1407+
t.Fatalf("Invalid marker input type: %T. Expected string, *Marker, []string, or []*Marker.", option.MarkerInput)
1408+
}
1409+
}
1410+
}
1411+
1412+
func (f *FourslashTest) verifySignatureHelp(
1413+
t *testing.T,
1414+
context *lsproto.SignatureHelpContext,
1415+
expected *lsproto.SignatureHelp,
1416+
) {
1417+
prefix := f.getCurrentPositionPrefix()
1418+
params := &lsproto.SignatureHelpParams{
1419+
TextDocument: lsproto.TextDocumentIdentifier{
1420+
Uri: ls.FileNameToDocumentURI(f.activeFilename),
1421+
},
1422+
Position: f.currentCaretPosition,
1423+
Context: context,
1424+
}
1425+
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentSignatureHelpInfo, params)
1426+
if resMsg == nil {
1427+
t.Fatalf(prefix+"Nil response received for signature help request", f.lastKnownMarkerName)
1428+
}
1429+
if !resultOk {
1430+
t.Fatalf(prefix+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
1431+
}
1432+
f.verifySignatureHelpResult(t, result.SignatureHelp, expected, prefix)
1433+
}
1434+
1435+
func (f *FourslashTest) verifySignatureHelpResult(
1436+
t *testing.T,
1437+
actual *lsproto.SignatureHelp,
1438+
expected *lsproto.SignatureHelp,
1439+
prefix string,
1440+
) {
1441+
assertDeepEqual(t, actual, expected, prefix+" SignatureHelp mismatch")
1442+
}
1443+
1444+
func (f *FourslashTest) getCurrentPositionPrefix() string {
13961445
if f.lastKnownMarkerName != nil {
1397-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
1398-
} else {
1399-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
1446+
return fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
14001447
}
1401-
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, prefix)
1448+
return fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
14021449
}

‎internal/fourslash/tests/signatureHelpTokenCrash_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package fourslash_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/microsoft/typescript-go/internal/fourslash"
7+
. "github.com/microsoft/typescript-go/internal/fourslash/tests/util"
8+
"github.com/microsoft/typescript-go/internal/lsp/lsproto"
9+
"github.com/microsoft/typescript-go/internal/testutil"
10+
)
11+
12+
func TestSignatureHelpTokenCrash(t *testing.T) {
13+
t.Parallel()
14+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
15+
const content = `
16+
function foo(a: any, b: any) {
17+
18+
}
19+
20+
foo((/*1*/
21+
22+
/** This is a JSDoc comment */
23+
foo/** More comments*/((/*2*/
24+
`
25+
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
26+
f.VerifySignatureHelp(t, &fourslash.SignatureHelpCase{
27+
MarkerInput: "1",
28+
Expected: nil,
29+
Context: &lsproto.SignatureHelpContext{
30+
IsRetrigger: false,
31+
TriggerCharacter: PtrTo("("),
32+
TriggerKind: lsproto.SignatureHelpTriggerKindTriggerCharacter,
33+
},
34+
})
35+
f.VerifySignatureHelp(t, &fourslash.SignatureHelpCase{
36+
MarkerInput: "2",
37+
Expected: nil,
38+
Context: &lsproto.SignatureHelpContext{
39+
IsRetrigger: false,
40+
TriggerCharacter: PtrTo("("),
41+
TriggerKind: lsproto.SignatureHelpTriggerKindTriggerCharacter,
42+
},
43+
})
44+
}

‎internal/ls/signaturehelp.go‎

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -597,19 +597,14 @@ func getCandidateOrTypeInfo(info *argumentListInfo, c *checker.Checker, sourceFi
597597
return nil // return Debug.assertNever(invocation);
598598
}
599599

600-
func isSyntacticOwner(startingToken *ast.Node, node *ast.Node, sourceFile *ast.SourceFile) bool { // !!! not tested
600+
func isSyntacticOwner(startingToken *ast.Node, node *ast.CallLikeExpression, sourceFile *ast.SourceFile) bool { // !!! not tested
601601
if !ast.IsCallOrNewExpression(node) {
602602
return false
603603
}
604-
invocationChildren := getTokensFromNode(node, sourceFile)
604+
invocationChildren := getChildrenFromNonJSDocNode(node, sourceFile)
605605
switch startingToken.Kind {
606-
case ast.KindOpenParenToken:
607-
return containsNode(invocationChildren, startingToken)
608-
case ast.KindCommaToken:
606+
case ast.KindOpenParenToken, ast.KindCommaToken:
609607
return containsNode(invocationChildren, startingToken)
610-
// !!!
611-
// const containingList = findContainingList(startingToken);
612-
// return !!containingList && contains(invocationChildren, containingList);
613608
case ast.KindLessThanToken:
614609
return containsPrecedingToken(startingToken, sourceFile, node.AsCallExpression().Expression)
615610
default:
@@ -1079,25 +1074,6 @@ func countBinaryExpressionParameters(b *ast.BinaryExpression) int {
10791074
return 2
10801075
}
10811076

1082-
func getTokensFromNode(node *ast.Node, sourceFile *ast.SourceFile) []*ast.Node {
1083-
if node == nil {
1084-
return nil
1085-
}
1086-
var children []*ast.Node
1087-
current := node
1088-
left := node.Pos()
1089-
scanner := scanner.GetScannerForSourceFile(sourceFile, left)
1090-
for left < current.End() {
1091-
token := scanner.Token()
1092-
tokenFullStart := scanner.TokenFullStart()
1093-
tokenEnd := scanner.TokenEnd()
1094-
children = append(children, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, current))
1095-
left = tokenEnd
1096-
scanner.Scan()
1097-
}
1098-
return children
1099-
}
1100-
11011077
func getTokenFromNodeList(nodeList *ast.NodeList, nodeListParent *ast.Node, sourceFile *ast.SourceFile) []*ast.Node {
11021078
if nodeList == nil || nodeListParent == nil {
11031079
return nil

‎internal/ls/utilities.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,3 +1627,37 @@ func getLeadingCommentRangesOfNode(node *ast.Node, file *ast.SourceFile) iter.Se
16271627
}
16281628
return scanner.GetLeadingCommentRanges(&ast.NodeFactory{}, file.Text(), node.Pos())
16291629
}
1630+
1631+
// Equivalent to Strada's `node.getChildren()` for non-JSDoc nodes.
1632+
func getChildrenFromNonJSDocNode(node *ast.Node, sourceFile *ast.SourceFile) []*ast.Node {
1633+
var childNodes []*ast.Node
1634+
node.ForEachChild(func(child *ast.Node) bool {
1635+
childNodes = append(childNodes, child)
1636+
return false
1637+
})
1638+
var children []*ast.Node
1639+
pos := node.Pos()
1640+
for _, child := range childNodes {
1641+
scanner := scanner.GetScannerForSourceFile(sourceFile, pos)
1642+
for pos < child.Pos() {
1643+
token := scanner.Token()
1644+
tokenFullStart := scanner.TokenFullStart()
1645+
tokenEnd := scanner.TokenEnd()
1646+
children = append(children, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, node))
1647+
pos = tokenEnd
1648+
scanner.Scan()
1649+
}
1650+
children = append(children, child)
1651+
pos = child.End()
1652+
}
1653+
scanner := scanner.GetScannerForSourceFile(sourceFile, pos)
1654+
for pos < node.End() {
1655+
token := scanner.Token()
1656+
tokenFullStart := scanner.TokenFullStart()
1657+
tokenEnd := scanner.TokenEnd()
1658+
children = append(children, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, node))
1659+
pos = tokenEnd
1660+
scanner.Scan()
1661+
}
1662+
return children
1663+
}

0 commit comments

Comments
 (0)