Skip to content

Fix token cache parent mismatch panic in signature help #1338

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.

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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
68 changes: 22 additions & 46 deletions internal/ls/signaturehelp.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,12 @@ func (l *LanguageService) GetSignatureHelpItems(
candidateInfo := getCandidateOrTypeInfo(argumentInfo, typeChecker, sourceFile, startingToken, onlyUseSyntacticOwners)
// cancellationToken.throwIfCancellationRequested();

// if (!candidateInfo) { !!!
// // We didn't have any sig help items produced by the TS compiler. If this is a JS
// // file, then see if we can figure out anything better.
// return isSourceFileJS(sourceFile) ? createJSSignatureHelpItems(argumentInfo, program, cancellationToken) : undefined;
// }
if candidateInfo == nil {
// We didn't have any sig help items produced by the TS compiler. If this is a JS
// file, then see if we can figure out anything better.
// return isSourceFileJS(sourceFile) ? createJSSignatureHelpItems(argumentInfo, program, cancellationToken) : undefined;
return nil
}

// return typeChecker.runWithCancellationToken(cancellationToken, typeChecker =>
if candidateInfo.candidateInfo != nil {
Expand Down Expand Up @@ -562,20 +563,10 @@ func isSyntacticOwner(startingToken *ast.Node, node *ast.Node, sourceFile *ast.S
if !ast.IsCallOrNewExpression(node) {
return false
}
invocationChildren := getTokensFromNode(node, sourceFile)
switch startingToken.Kind {
case ast.KindOpenParenToken:
return containsNode(invocationChildren, startingToken)
case 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:
return false
}

// Check if the startingToken is within the range of the call expression
// This replaces the old getTokensFromNode approach which was causing token cache mismatches
return startingToken.Pos() >= node.Pos() && startingToken.End() <= node.End()
}

func containsPrecedingToken(startingToken *ast.Node, sourceFile *ast.SourceFile, container *ast.Node) bool {
Expand Down Expand Up @@ -1054,24 +1045,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 {
Expand All @@ -1090,21 +1063,24 @@ func getTokenFromNodeList(nodeList *ast.NodeList, nodeListParent *ast.Node, sour
token := scanner.Token()
tokenFullStart := scanner.TokenFullStart()
tokenEnd := scanner.TokenEnd()
tokens = append(tokens, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, nodeListParent))
// Try to find existing token instead of creating a new one
existingToken := astnav.GetTokenAtPosition(sourceFile, tokenFullStart)
if existingToken != nil && ast.IsTokenKind(existingToken.Kind) &&
existingToken.Pos() == tokenFullStart && existingToken.End() == tokenEnd {
// Use the existing token which has the correct parent
tokens = append(tokens, existingToken)
} else {
// Fall back to creating token - this might still cause cache mismatches
// but it's better than always creating potentially wrong tokens
tokens = append(tokens, sourceFile.GetOrCreateToken(token, tokenFullStart, tokenEnd, nodeListParent))
}
left = tokenEnd
}
}
return tokens
}

func containsNode(nodes []*ast.Node, node *ast.Node) bool {
for i := range nodes {
if nodes[i] == node {
return true
}
}
return false
}


func getArgumentListInfoForTemplate(tagExpression *ast.TaggedTemplateExpression, argumentIndex *int, sourceFile *ast.SourceFile) *argumentListInfo {
// argumentCount is either 1 or (numSpans + 1) to account for the template strings array argument.
Expand Down
52 changes: 52 additions & 0 deletions internal/ls/signaturehelp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,58 @@ type verifySignatureHelpOptions struct {
// tags?: ReadonlyArray<JSDocTagInfo>;
}

func TestSignatureHelpTokenCachePanic(t *testing.T) {
t.Parallel()
if !bundled.Embedded {
t.Skip("bundled files are not embedded")
}

// This test reproduces the token cache mismatch panic in getTokensFromNode
// when isSyntacticOwner is called with different call expressions that overlap
// in token positions but have different parent nodes.
//
// The key is to have nested call expressions where the inner call tokens
// get cached with one parent, then accessed again with a different parent.
input := `declare const array: string[];
// This creates nested call structure that triggers the cache issue
array.filter((item) => item.includes("test")).map((x) => x.length);`

testData := fourslash.ParseTestData(t, input, "/mainFile.ts")
file := testData.Files[0].FileName()
ctx := projecttestutil.WithRequestID(t.Context())
languageService, done := createLanguageService(ctx, file, map[string]any{
file: testData.Files[0].Content,
})
defer done()

context := &lsproto.SignatureHelpContext{
TriggerKind: lsproto.SignatureHelpTriggerKindTriggerCharacter,
TriggerCharacter: ptrTo("("),
}
ptrTrue := ptrTo(true)
capabilities := &lsproto.SignatureHelpClientCapabilities{
SignatureInformation: &lsproto.ClientSignatureInformationOptions{
ActiveParameterSupport: ptrTrue,
NoActiveParameterSupport: ptrTrue,
ParameterInformation: &lsproto.ClientSignatureParameterInformationOptions{
LabelOffsetSupport: ptrTrue,
},
},
}
preferences := &ls.UserPreferences{}

// First signature help call - this should populate token cache via isSyntacticOwner
pos1 := lsproto.Position{Line: 2, Character: 14} // Inside .filter() call
result1 := languageService.ProvideSignatureHelp(ctx, ls.FileNameToDocumentURI(file), pos1, context, capabilities, preferences)
assert.Assert(t, result1 != nil, "First signature help call should succeed")

// Second signature help call in the chained .map() call
// This may trigger the token cache mismatch when isSyntacticOwner processes overlapping tokens
pos2 := lsproto.Position{Line: 2, Character: 58} // Inside .map() call
result2 := languageService.ProvideSignatureHelp(ctx, ls.FileNameToDocumentURI(file), pos2, context, capabilities, preferences)
assert.Assert(t, result2 != nil, "Second signature help call should succeed without panic")
}

func TestSignatureHelp(t *testing.T) {
t.Parallel()
if !bundled.Embedded {
Expand Down
12 changes: 9 additions & 3 deletions internal/ls/utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ func hasChildOfKind(containingNode *ast.Node, kind ast.Kind, sourceFile *ast.Sou
}

func findChildOfKind(containingNode *ast.Node, kind ast.Kind, sourceFile *ast.SourceFile) *ast.Node {
return findChild(containingNode, sourceFile, func(node *ast.Node) bool {
return node.Kind == kind
})
}

func findChild(containingNode *ast.Node, sourceFile *ast.SourceFile, predicate func(*ast.Node) bool) *ast.Node {
lastNodePos := containingNode.Pos()
scanner := scanner.GetScannerForSourceFile(sourceFile, lastNodePos)

Expand All @@ -158,14 +164,14 @@ func findChildOfKind(containingNode *ast.Node, kind ast.Kind, sourceFile *ast.So
tokenFullStart := scanner.TokenFullStart()
tokenEnd := scanner.TokenEnd()
token := sourceFile.GetOrCreateToken(tokenKind, tokenFullStart, tokenEnd, containingNode)
if tokenKind == kind {
if predicate(token) {
foundChild = token
return true
}
startPos = tokenEnd
scanner.Scan()
}
if node.Kind == kind {
if predicate(node) {
foundChild = node
return true
}
Expand All @@ -188,7 +194,7 @@ func findChildOfKind(containingNode *ast.Node, kind ast.Kind, sourceFile *ast.So
tokenFullStart := scanner.TokenFullStart()
tokenEnd := scanner.TokenEnd()
token := sourceFile.GetOrCreateToken(tokenKind, tokenFullStart, tokenEnd, containingNode)
if tokenKind == kind {
if predicate(token) {
return token
}
startPos = tokenEnd
Expand Down