Skip to content

Commit 7e8d6cd

Browse files
gabrittoandrewbranch
gabritto
authored and
andrewbranch
committed
Get node's children with forEachChild + scanner in signature help (microsoft#1584)
1 parent fc34cd0 commit 7e8d6cd

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
@@ -526,12 +526,7 @@ func (f *FourslashTest) VerifyCompletions(t *testing.T, markerInput MarkerInput,
526526
}
527527

528528
func (f *FourslashTest) verifyCompletionsWorker(t *testing.T, expected *CompletionsExpectedList) {
529-
var prefix string
530-
if f.lastKnownMarkerName != nil {
531-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
532-
} else {
533-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
534-
}
529+
prefix := f.getCurrentPositionPrefix()
535530
params := &lsproto.CompletionParams{
536531
TextDocument: lsproto.TextDocumentIdentifier{
537532
Uri: ls.FileNameToDocumentURI(f.activeFilename),
@@ -546,12 +541,11 @@ func (f *FourslashTest) verifyCompletionsWorker(t *testing.T, expected *Completi
546541
if !resultOk {
547542
t.Fatalf(prefix+"Unexpected response type for completion request: %T", resMsg.AsResponse().Result)
548543
}
549-
f.verifyCompletionsResult(t, f.currentCaretPosition, result.List, expected, prefix)
544+
f.verifyCompletionsResult(t, result.List, expected, prefix)
550545
}
551546

552547
func (f *FourslashTest) verifyCompletionsResult(
553548
t *testing.T,
554-
position lsproto.Position,
555549
actual *lsproto.CompletionList,
556550
expected *CompletionsExpectedList,
557551
prefix string,
@@ -914,17 +908,11 @@ func (f *FourslashTest) VerifyBaselineHover(t *testing.T) {
914908
}
915909

916910
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentHoverInfo, params)
917-
var prefix string
918-
if f.lastKnownMarkerName != nil {
919-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
920-
} else {
921-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
922-
}
923911
if resMsg == nil {
924-
t.Fatalf(prefix+"Nil response received for quick info request", f.lastKnownMarkerName)
912+
t.Fatalf(f.getCurrentPositionPrefix()+"Nil response received for quick info request", f.lastKnownMarkerName)
925913
}
926914
if !resultOk {
927-
t.Fatalf(prefix+"Unexpected response type for quick info request: %T", resMsg.AsResponse().Result)
915+
t.Fatalf(f.getCurrentPositionPrefix()+"Unexpected response type for quick info request: %T", resMsg.AsResponse().Result)
928916
}
929917

930918
return markerAndItem[*lsproto.Hover]{Marker: marker, Item: result.Hover}, true
@@ -1007,17 +995,11 @@ func (f *FourslashTest) VerifyBaselineSignatureHelp(t *testing.T) {
1007995
}
1008996

1009997
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentSignatureHelpInfo, params)
1010-
var prefix string
1011-
if f.lastKnownMarkerName != nil {
1012-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
1013-
} else {
1014-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
1015-
}
1016998
if resMsg == nil {
1017-
t.Fatalf(prefix+"Nil response received for signature help request", f.lastKnownMarkerName)
999+
t.Fatalf(f.getCurrentPositionPrefix()+"Nil response received for signature help request", f.lastKnownMarkerName)
10181000
}
10191001
if !resultOk {
1020-
t.Fatalf(prefix+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
1002+
t.Fatalf(f.getCurrentPositionPrefix()+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
10211003
}
10221004

10231005
return markerAndItem[*lsproto.SignatureHelp]{Marker: marker, Item: result.SignatureHelp}, true
@@ -1301,8 +1283,7 @@ func (f *FourslashTest) getScriptInfo(fileName string) *scriptInfo {
13011283
func (f *FourslashTest) VerifyQuickInfoAt(t *testing.T, marker string, expectedText string, expectedDocumentation string) {
13021284
f.GoToMarker(t, marker)
13031285
hover := f.getQuickInfoAtCurrentPosition(t)
1304-
1305-
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, fmt.Sprintf("At marker '%s': ", marker))
1286+
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, f.getCurrentPositionPrefix())
13061287
}
13071288

13081289
func (f *FourslashTest) getQuickInfoAtCurrentPosition(t *testing.T) *lsproto.Hover {
@@ -1374,11 +1355,77 @@ func (f *FourslashTest) quickInfoIsEmpty(t *testing.T) (bool, *lsproto.Hover) {
13741355

13751356
func (f *FourslashTest) VerifyQuickInfoIs(t *testing.T, expectedText string, expectedDocumentation string) {
13761357
hover := f.getQuickInfoAtCurrentPosition(t)
1377-
var prefix string
1358+
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, f.getCurrentPositionPrefix())
1359+
}
1360+
1361+
type SignatureHelpCase struct {
1362+
Context *lsproto.SignatureHelpContext
1363+
MarkerInput MarkerInput
1364+
Expected *lsproto.SignatureHelp
1365+
}
1366+
1367+
func (f *FourslashTest) VerifySignatureHelp(t *testing.T, signatureHelpCases ...*SignatureHelpCase) {
1368+
for _, option := range signatureHelpCases {
1369+
switch marker := option.MarkerInput.(type) {
1370+
case string:
1371+
f.GoToMarker(t, marker)
1372+
f.verifySignatureHelp(t, option.Context, option.Expected)
1373+
case *Marker:
1374+
f.goToMarker(t, marker)
1375+
f.verifySignatureHelp(t, option.Context, option.Expected)
1376+
case []string:
1377+
for _, markerName := range marker {
1378+
f.GoToMarker(t, markerName)
1379+
f.verifySignatureHelp(t, option.Context, option.Expected)
1380+
}
1381+
case []*Marker:
1382+
for _, marker := range marker {
1383+
f.goToMarker(t, marker)
1384+
f.verifySignatureHelp(t, option.Context, option.Expected)
1385+
}
1386+
case nil:
1387+
f.verifySignatureHelp(t, option.Context, option.Expected)
1388+
default:
1389+
t.Fatalf("Invalid marker input type: %T. Expected string, *Marker, []string, or []*Marker.", option.MarkerInput)
1390+
}
1391+
}
1392+
}
1393+
1394+
func (f *FourslashTest) verifySignatureHelp(
1395+
t *testing.T,
1396+
context *lsproto.SignatureHelpContext,
1397+
expected *lsproto.SignatureHelp,
1398+
) {
1399+
prefix := f.getCurrentPositionPrefix()
1400+
params := &lsproto.SignatureHelpParams{
1401+
TextDocument: lsproto.TextDocumentIdentifier{
1402+
Uri: ls.FileNameToDocumentURI(f.activeFilename),
1403+
},
1404+
Position: f.currentCaretPosition,
1405+
Context: context,
1406+
}
1407+
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentSignatureHelpInfo, params)
1408+
if resMsg == nil {
1409+
t.Fatalf(prefix+"Nil response received for signature help request", f.lastKnownMarkerName)
1410+
}
1411+
if !resultOk {
1412+
t.Fatalf(prefix+"Unexpected response type for signature help request: %T", resMsg.AsResponse().Result)
1413+
}
1414+
f.verifySignatureHelpResult(t, result.SignatureHelp, expected, prefix)
1415+
}
1416+
1417+
func (f *FourslashTest) verifySignatureHelpResult(
1418+
t *testing.T,
1419+
actual *lsproto.SignatureHelp,
1420+
expected *lsproto.SignatureHelp,
1421+
prefix string,
1422+
) {
1423+
assertDeepEqual(t, actual, expected, prefix+" SignatureHelp mismatch")
1424+
}
1425+
1426+
func (f *FourslashTest) getCurrentPositionPrefix() string {
13781427
if f.lastKnownMarkerName != nil {
1379-
prefix = fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
1380-
} else {
1381-
prefix = fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
1428+
return fmt.Sprintf("At marker '%s': ", *f.lastKnownMarkerName)
13821429
}
1383-
f.verifyHoverContent(t, hover.Contents, expectedText, expectedDocumentation, prefix)
1430+
return fmt.Sprintf("At position (Ln %d, Col %d): ", f.currentCaretPosition.Line, f.currentCaretPosition.Character)
13841431
}

‎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)