Skip to content

Commit e06d14e

Browse files
Copilotjakebailey
andauthored
Fix signature help applicable span for nested calls (#1949)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
1 parent 9522325 commit e06d14e

3 files changed

Lines changed: 135 additions & 4 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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/testutil"
8+
)
9+
10+
func TestSignatureHelpApplicableRange(t *testing.T) {
11+
t.Parallel()
12+
13+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
14+
const content = `let obj = {
15+
foo(s: string): string {
16+
return s;
17+
}
18+
};
19+
20+
let s =/*a*/ obj.foo("Hello, world!")/*b*/
21+
/*c*/;`
22+
f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
23+
defer done()
24+
25+
// Markers a, b, c should NOT show signature help (outside the call)
26+
f.VerifyNoSignatureHelpForMarkers(t, "a", "b", "c")
27+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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/testutil"
8+
)
9+
10+
func TestSignatureHelpNestedCalls(t *testing.T) {
11+
t.Parallel()
12+
13+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
14+
const content = `function foo(s: string) { return s; }
15+
function bar(s: string) { return s; }
16+
let s = foo(/*a*/ /*b*/bar/*c*/(/*d*/"hello"/*e*/)/*f*/);`
17+
f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
18+
defer done()
19+
20+
// Markers a, b, c should show foo (outer call)
21+
f.GoToMarker(t, "a")
22+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "foo(s: string): string"})
23+
24+
f.GoToMarker(t, "b")
25+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "foo(s: string): string"})
26+
27+
f.GoToMarker(t, "c")
28+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "foo(s: string): string"})
29+
30+
// Markers d, e should show bar (inside inner call, including the end boundary)
31+
f.GoToMarker(t, "d")
32+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "bar(s: string): string"})
33+
34+
f.GoToMarker(t, "e")
35+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "bar(s: string): string"})
36+
37+
// Marker f should show foo (after the inner call closes)
38+
f.GoToMarker(t, "f")
39+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "foo(s: string): string"})
40+
}
41+
42+
func TestSignatureHelpEmptyInnerCall(t *testing.T) {
43+
t.Parallel()
44+
45+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
46+
const content = `function foo(s: string) { return s; }
47+
function bar(s: string) { return s; }
48+
let s = foo(bar(/*a*/));`
49+
f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
50+
defer done()
51+
52+
// Marker a should show bar even though the inner argument list is empty.
53+
f.GoToMarker(t, "a")
54+
f.VerifySignatureHelp(t, fourslash.VerifySignatureHelpOptions{Text: "bar(s: string): string"})
55+
}

internal/ls/signaturehelp.go

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,16 +774,46 @@ func containsPrecedingToken(startingToken *ast.Node, sourceFile *ast.SourceFile,
774774
}
775775

776776
func getContainingArgumentInfo(node *ast.Node, sourceFile *ast.SourceFile, checker *checker.Checker, isManuallyInvoked bool, position int) *argumentListInfo {
777+
var firstArgumentInfo *argumentListInfo
777778
for n := node; !ast.IsSourceFile(n) && (isManuallyInvoked || !ast.IsBlock(n)); n = n.Parent {
778779
// If the node is not a subspan of its parent, this is a big problem.
779780
// There have been crashes that might be caused by this violation.
780781
debug.Assert(RangeContainsRange(n.Parent.Loc, n.Loc), "Not a subspan. Child: ", n.KindString(), ", parent: ", n.Parent.KindString())
781782
argumentInfo := getImmediatelyContainingArgumentOrContextualParameterInfo(n, position, sourceFile, checker)
782783
if argumentInfo != nil {
783-
return argumentInfo
784+
// For contextual invocations (e.g., arrow functions with contextual types),
785+
// always return immediately without checking the position.
786+
// This ensures that when inside a callback's parameter list, we show the callback's
787+
// signature, not the outer call's signature.
788+
if argumentInfo.invocation.contextualInvocation != nil {
789+
return argumentInfo
790+
}
791+
792+
// Remember the first (innermost) argument info we find
793+
if firstArgumentInfo == nil {
794+
firstArgumentInfo = argumentInfo
795+
}
796+
797+
// If the position is at the end boundary of an argument list, keep the
798+
// innermost call. This covers cases like foo(bar("x"|)) where the cursor is
799+
// still inside the inner invocation, just before its closing paren.
800+
if argumentInfo.argumentsSpan.End() == position {
801+
return argumentInfo
802+
}
803+
804+
// If any call's span contains the position, return it.
805+
// We walk from inner to outer, so this naturally prefers the innermost call
806+
// when multiple calls contain the position.
807+
if argumentInfo.argumentsSpan.Contains(position) {
808+
return argumentInfo
809+
}
784810
}
785811
}
786-
return nil
812+
813+
// No call's span contains the position. Fall back to the innermost call we found.
814+
// This covers boundary positions that are still syntactically associated with that
815+
// invocation, such as being at the end of the argument list or on the close paren.
816+
return firstArgumentInfo
787817
}
788818

789819
func getImmediatelyContainingArgumentOrContextualParameterInfo(node *ast.Node, position int, sourceFile *ast.SourceFile, checker *checker.Checker) *argumentListInfo {
@@ -1062,18 +1092,37 @@ func getApplicableSpanForArguments(argumentList *ast.NodeList, node *ast.Node, s
10621092
// | |
10631093
//
10641094
// The applicable span is from the first bar to the second bar (inclusive,
1065-
// but not including parentheses)
1095+
// but not including parentheses).
10661096
if argumentList == nil && node != nil {
10671097
// If the user has just opened a list, and there are no arguments.
10681098
// For example, foo( )
10691099
// | |
1070-
return core.NewTextRange(node.End(), scanner.SkipTrivia(sourceFile.Text(), node.End()))
1100+
// The span should include positions inside the parentheses.
1101+
spanStart := node.End()
1102+
spanEnd := scanner.SkipTrivia(sourceFile.Text(), node.End())
1103+
spanEnd = ensureMinimumSpanSize(spanStart, spanEnd)
1104+
return core.NewTextRange(spanStart, spanEnd)
10711105
}
10721106
applicableSpanStart := argumentList.Pos()
10731107
applicableSpanEnd := scanner.SkipTrivia(sourceFile.Text(), argumentList.End())
1108+
1109+
// If the argument list is empty (Pos == End), extend the span to include at least
1110+
// one position. This handles foo(|) where the cursor is right after the opening paren.
1111+
applicableSpanEnd = ensureMinimumSpanSize(applicableSpanStart, applicableSpanEnd)
1112+
10741113
return core.NewTextRange(applicableSpanStart, applicableSpanEnd)
10751114
}
10761115

1116+
// ensureMinimumSpanSize ensures that a span includes at least one position.
1117+
// TextRange.Contains uses a half-open interval, so an empty span would not contain
1118+
// the cursor immediately after typing an opening paren in a call like foo(bar(|)).
1119+
func ensureMinimumSpanSize(start, end int) int {
1120+
if end <= start {
1121+
return start + 1
1122+
}
1123+
return end
1124+
}
1125+
10771126
type argumentOrParameterListAndIndex struct {
10781127
list *ast.NodeList
10791128
argumentIndex int

0 commit comments

Comments
 (0)