Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions internal/fourslash/tests/signatureHelpApplicableRange_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package fourslash_test

import (
"testing"

"github.com/microsoft/typescript-go/internal/fourslash"
"github.com/microsoft/typescript-go/internal/testutil"
)

// Test case 1: Basic applicable range test
// This test verifies the applicable range for signature help
// According to the issue, signature help should be provided:
// - Inside the parentheses (markers 1, 2)
// - NOT on the call target before the opening paren (marker a, b, c would test this)
// - NOT after the closing paren including whitespace (markers 3, 4)
func TestSignatureHelpApplicableRangeBasic(t *testing.T) {
t.Parallel()

defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `let obj = {
foo(s: string): string {
return s;
}
};

let s =/*a*/ /*b*/obj/*c*/./*d*/foo/*e*/(/*1*/"Hello, world!"/*2*/)/*3*/
/*4*/;`
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)

// Use VerifyBaselineSignatureHelp to check behavior at all markers
f.VerifyBaselineSignatureHelp(t)
}

// Test case 2: Nested calls - outer should take precedence
func TestSignatureHelpNestedCalls(t *testing.T) {
t.Parallel()

defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `function foo(s: string) { return s; }
function bar(s: string) { return s; }
let s = foo(/*a*//*b*/bar/*c*/(/*d*/"hello"/*e*/)/*f*/);`
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)

// Use VerifyBaselineSignatureHelp to check behavior at all markers
f.VerifyBaselineSignatureHelp(t)
}
32 changes: 27 additions & 5 deletions internal/ls/signaturehelp.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,16 +614,37 @@ func containsPrecedingToken(startingToken *ast.Node, sourceFile *ast.SourceFile,
}

func getContainingArgumentInfo(node *ast.Node, sourceFile *ast.SourceFile, checker *checker.Checker, isManuallyInvoked bool, position int) *argumentListInfo {
var lastArgumentInfo *argumentListInfo
for n := node; !ast.IsSourceFile(n) && (isManuallyInvoked || !ast.IsBlock(n)); n = n.Parent {
// If the node is not a subspan of its parent, this is a big problem.
// There have been crashes that might be caused by this violation.
debug.Assert(RangeContainsRange(n.Parent.Loc, n.Loc), fmt.Sprintf("Not a subspan. Child: %s, parent: %s", n.KindString(), n.Parent.KindString()))
argumentInfo := getImmediatelyContainingArgumentOrContextualParameterInfo(n, position, sourceFile, checker)
if argumentInfo != nil {
return argumentInfo
// For contextual invocations (e.g., arrow functions with contextual types),
// always return immediately without checking the position.
// This ensures that when inside a callback's parameter list, we show the callback's
// signature, not the outer call's signature.
if argumentInfo.invocation.contextualInvocation != nil {
return argumentInfo
}

// For regular call expressions, check if the position is actually within the applicable span.
// This ensures that for nested calls, the outer call takes precedence
// when the position is outside the inner call's argument list.
if argumentInfo.argumentsSpan.Contains(position) {
return argumentInfo
}
// Remember this argument info in case we don't find an outer call
if lastArgumentInfo == nil {
lastArgumentInfo = argumentInfo
}
// Continue looking for an outer call if position is outside this call's applicable span
}
}
return nil
// If we didn't find a call that contains the position, return the last call we found.
// This handles cases where the cursor is at the edge of a call (e.g., right after a parameter).
return lastArgumentInfo
Comment on lines +645 to +647
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we need this.

@copilot is there a test file that shows where this helps? What happens if you remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback logic (lines 645-647) is needed to handle edge cases where the cursor is inside a call's parentheses but outside the computed applicable span. This happens in several scenarios:

  1. Empty argument lists: foo(/*cursor*/) - the cursor is inside the parentheses but there are no arguments, so the applicable span may not include this position
  2. After the last argument: foo(arg/*cursor*/) - when trivia/whitespace follows the last argument

Without this fallback, 14 existing tests fail (e.g., TestSignatureHelpJSDocTags where markers like Foo.method1(/*11*/) return null instead of showing signature help).

The fallback ensures that if we find a call while walking up the tree but the position is slightly outside its span (and there's no outer call), we still show that call's signature help rather than returning null. This preserves the existing behavior for non-nested calls while still fixing the nested call precedence issue.

}

func getImmediatelyContainingArgumentOrContextualParameterInfo(node *ast.Node, position int, sourceFile *ast.SourceFile, checker *checker.Checker) *argumentListInfo {
Expand Down Expand Up @@ -895,14 +916,15 @@ func getArgumentOrParameterListInfo(node *ast.Node, sourceFile *ast.SourceFile,
}

func getApplicableSpanForArguments(argumentList *ast.NodeList, node *ast.Node, sourceFile *ast.SourceFile) core.TextRange {
// We use full start and skip trivia on the end because we want to include trivia on
// both sides. For example,
// The applicable span starts at the beginning of the argument list (including leading trivia)
// and extends to the end of the argument list plus any trailing trivia.
// For example,
//
// foo( /*comment */ a, b, c /*comment*/ )
// | |
//
// The applicable span is from the first bar to the second bar (inclusive,
// but not including parentheses)
// but not including parentheses).
if argumentList == nil && node != nil {
// If the user has just opened a list, and there are no arguments.
// For example, foo( )
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
// === SignatureHelp ===
=== /signatureHelpApplicableRangeBasic.ts ===
// let obj = {
// foo(s: string): string {
// return s;
// }
// };
//
// let s = obj.foo("Hello, world!")
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*a*/.
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*b*/.
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*c*/.
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*d*/.
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*e*/.
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | foo(**s: string**): string
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | foo(**s: string**): string
// | ----------------------------------------------------------------------
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*3*/.
// | ----------------------------------------------------------------------
// ;
// ^
// | ----------------------------------------------------------------------
// | No signaturehelp at /*4*/.
// | ----------------------------------------------------------------------
[
{
"marker": {
"Position": 76,
"LSPosition": {
"line": 6,
"character": 7
},
"Name": "a",
"Data": {}
},
"item": null
},
{
"marker": {
"Position": 77,
"LSPosition": {
"line": 6,
"character": 8
},
"Name": "b",
"Data": {}
},
"item": null
},
{
"marker": {
"Position": 80,
"LSPosition": {
"line": 6,
"character": 11
},
"Name": "c",
"Data": {}
},
"item": null
},
{
"marker": {
"Position": 81,
"LSPosition": {
"line": 6,
"character": 12
},
"Name": "d",
"Data": {}
},
"item": null
},
{
"marker": {
"Position": 84,
"LSPosition": {
"line": 6,
"character": 15
},
"Name": "e",
"Data": {}
},
"item": null
},
{
"marker": {
"Position": 85,
"LSPosition": {
"line": 6,
"character": 16
},
"Name": "1",
"Data": {}
},
"item": {
"signatures": [
{
"label": "foo(s: string): string",
"parameters": [
{
"label": "s: string"
}
]
}
],
"activeSignature": 0,
"activeParameter": 0
}
},
{
"marker": {
"Position": 100,
"LSPosition": {
"line": 6,
"character": 31
},
"Name": "2",
"Data": {}
},
"item": {
"signatures": [
{
"label": "foo(s: string): string",
"parameters": [
{
"label": "s: string"
}
]
}
],
"activeSignature": 0,
"activeParameter": 0
}
},
{
"marker": {
"Position": 101,
"LSPosition": {
"line": 6,
"character": 32
},
"Name": "3",
"Data": {}
},
"item": null
},
{
"marker": {
"Position": 106,
"LSPosition": {
"line": 7,
"character": 2
},
"Name": "4",
"Data": {}
},
"item": null
}
]
Loading
Loading