Skip to content

Signature help is wrong for nested calls, has weird applicable ranges #1419

@DanielRosenwasser

Description

@DanielRosenwasser

Here's an example file that exhibits new weirdness on signature help:

let obj = {
    foo(s: string): string {
        return s;
    }
};

let s =[| obj.foo("Hello, world!")  
  |];

Note that the spaces/newlines after [| and before |]` are intentional for the point I'm making.

If you request signature help at every position within the range, you will get different results compared to the old language service. Previously, signature help for foo would only be provided inside of the parentheses.

In the native port, signature help is now provided on parts of the call target, along with all whitespace following the parentheses.

Specifically, the actual positions you will get signature help on only fall within the following range:

let s = o[|bj.foo("Hello, world!")  
  |];
signature-help-weird-1.webm

I think that it is fine for us to provide signature help on a call target like obj.foo, but it is not including the correct start.

We should not include the whitespace after the call at all.

In other words someone should start with this test case:

let obj = {
    foo(s: string): string {
        return s;
    }
};

let s =/*a*/ [|obj.foo("Hello, world!"|])/*b*/  
  /*c*/;

Given the above test, signature help should be provided for every position in the range, but not for markers a, b, or c.

This is similar to what the Go LS does in that the position after the ) is not included in signature help. On the other hand, it does differ in that we would return signature help on all of obj.foo (Go only provides signature help on foo).


Speaking of which, we should have another test case on this as well:

function someFunc() {
    return {
        foo() {
            return "hello!"
        }
    }
}

[|someFunc(|][|).foo(|][|).toUpperCase(|]);

The test should verify that signature help for all positions:

  • Range 1: Label = someFunc
  • Range 2: Label = foo
  • Range 3: Label = toUpperCase

With all of that considered, there's a regression in signature help where inner calls take precedence over outer calls

signature-help-weird-2.webm
function foo(x: any) {
    return x;
}

function bar(x: any) {
    return x;
}

foo(/*a*/ /*b*/bar/*c*/(/*d*/)/*e*/ /*f*/)

In my opinion, signature help should work in the following way:

  • Markers a, b, c, e, f: Label = foo
  • Marker d: Label = bar

In other words, if there is an outer call, I think it takes precedence over signature help from the call target.

The Go LS actually prioritizes the call target rather than the outer call, and I actually don't think that behavior is usually desirable. Doing so means you need to move your cursor to the end of the call or add a space before the call target.

Metadata

Metadata

Assignees

Labels

Domain: EditorRelated to the LSP server, editor experience

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions