diff --git a/internal/fourslash/fourslash.go b/internal/fourslash/fourslash.go index 06427dbbc4..8b4526610f 100644 --- a/internal/fourslash/fourslash.go +++ b/internal/fourslash/fourslash.go @@ -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), @@ -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, @@ -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 @@ -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 @@ -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 { @@ -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) } diff --git a/internal/fourslash/tests/signatureHelpTokenCrash_test.go b/internal/fourslash/tests/signatureHelpTokenCrash_test.go new file mode 100644 index 0000000000..84d1d98772 --- /dev/null +++ b/internal/fourslash/tests/signatureHelpTokenCrash_test.go @@ -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, + }, + }) +} diff --git a/internal/ls/signaturehelp.go b/internal/ls/signaturehelp.go index 94984a9910..e7ff15b03b 100644 --- a/internal/ls/signaturehelp.go +++ b/internal/ls/signaturehelp.go @@ -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: @@ -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 diff --git a/internal/ls/utilities.go b/internal/ls/utilities.go index ab58122018..aa8a9f0be6 100644 --- a/internal/ls/utilities.go +++ b/internal/ls/utilities.go @@ -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() + } + return children +}