[LSP Spec]: In response of signatureHelp, activeParameter starts from 1

[jinzhongjia]

I recently discovered a problem when using basedpyright on neovim
But this problem should not require code to reproduce. You only need to find a python method with parameters to verify it.

main

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#signatureHelp

According to doc, activeParameter should start from 0, but now it is start from 1

I'm not sure if this is really a problem, so I'll ask and if there is no problem please close the issue.

[jinzhongjia]

I tested it further and found that the signaturehelp of list.sort() is incorrect. It starts from 1
But the signaturehelp of print() is correct, it starts from 0

This behavior is strange

[KotlinIsland]

is activeSignature related to overloads?

could you please provide steps on how to reproduce this? something like:

1. enter

   print()
   list.sort()

2. check language server logs for signatureHelp response

[DetachHead]

yeah i don't really understand what signatureHelp is and the language server documentation doesn't have any screenshots so i will need some more information. preferably a screenshot of what exactly is incorrect with list.sort() compared to print()

[jinzhongjia]

I will try to check the logs later

[KotlinIsland]

when you say that it "starts from 1", what exactly do you mean? how do you know that it is starting from 1? where you can see this information?

[jinzhongjia]

Please ignore my initial report
I'm trying to write a neovim plugin and then use signaturehelp to display only the currently inferred parameter types.

As a test, I directly printed the response information of signaturehelp (which has been automatically converted to Lua's table by neovim's lsp function)

I noticed that activeParameter starts from 1, but actually according to the specification it should start from 0
This is different from other servers

Here is a link to the canonical documentation for the response: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#signatureHelp

for list.sort():

{
    activeParameter = 3,
    signatures = { {
        documentation = {
            kind = "markdown",
            value =
            "Sort the list in ascending order and return None.\n\nThe sort is in-place (i.e. the list itself is modified) and stable (i.e. the\norder of two equal elements is maintained).\n\nIf a key function is given, apply it once to each list item and sort them,\nascending or descending, according to their function values.\n\nThe reverse flag can be set to sort in descending order."
        },
        label = "(*, key: None = None, reverse: bool = False) -> None",
        parameters = { {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 1, 2 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 4, 20 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 22, 43 }
        } }
    }, {
        documentation = {
            kind = "markdown",
            value =
            "Sort the list in ascending order and return None.\n\nThe sort is in-place (i.e. the list itself is modified) and stable (i.e. the\norder of two equal elements is maintained).\n\nIf a key function is given, apply it once to each list item and sort them,\nascending or descending, according to their function values.\n\nThe reverse flag can be set to sort in descending order."
        },
        label = "(*, key: (int) -> SupportsRichComparison, reverse: bool = False) -> None",
        parameters = { {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 1, 2 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 4, 40 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 42, 63 }
        } }
    } }
}

for print():

{
    activeParameter = 5,
    activeSignature = 0,
    signatures = { {
        activeParameter = 0,
        documentation = {
            kind = "markdown",
            value =
            "Prints the values to a stream, or to sys.stdout by default.\n\nsep  \n  string inserted between values, default a space.  \nend  \n  string appended after the last value, default a newline.  \nfile  \n  a file-like object (stream); defaults to the current sys.stdout.  \nflush  \n  whether to forcibly flush the stream."
        },
        label =
        '(*values: object, sep: str | None = " ", end: str | None = "\\n", file: SupportsWrite[str] | None = None, flush: Literal[False] = False) -> None',
        parameters = { {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 1, 16 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 18, 39 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 41, 63 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 65, 103 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 105, 134 }
        } }
    }, {
        activeParameter = 0,
        documentation = {
            kind = "markdown",
            value =
            "Prints the values to a stream, or to sys.stdout by default.\n\nsep  \n  string inserted between values, default a space.  \nend  \n  string appended after the last value, default a newline.  \nfile  \n  a file-like object (stream); defaults to the current sys.stdout.  \nflush  \n  whether to forcibly flush the stream."
        },
        label =
        '(*values: object, sep: str | None = " ", end: str | None = "\\n", file: _SupportsWriteAndFlush[str] | None = None, flush: bool) -> None',
        parameters = { {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 1, 16 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 18, 39 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 41, 63 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 65, 112 }
        }, {
            documentation = {
                kind = "markdown",
                value = ""
            },
            label = { 114, 125 }
        } }
    } }
}

[jinzhongjia]

I just requested the server to respond to the two functions signaturehelp of list.sort and print.

[jinzhongjia]

At present, this seems to be just a problem of non-standard implementation, not a bug.
I just think this may need to be reported. So far, this non-standard implementation has not caused any additional trouble.

[KotlinIsland]

thanks for the additional information

I noticed that activeParameter starts from 1

when you say that it starts from 1, how do you know this? in that data i can see activeParameter = 3 did your code sample have the third parameter selected and you were expecting activeParameter = 2?

did your code sample look like:

[].sort(1, 2, 3)

actually according to the specification it should start from 0

i can't see that referenced anywhere in the docs that you linked, i can see this though:

defaults to 0 if the active signature has parameters

although i am very sure that this value is zero-based, so i would assume it starts from 0

could you please elaborate what you mean by "it should start from 0", because i can't really figure out what your trying to explain.

as far as i can tell, given:

def f(a, b, c): ...

f(a, b<caret>, c)

then signatureHelp will return activeParameter = 1, which is zero based

additionally, the correct parameter is selected in various editors that we have tested this on

maybe if you provided an expected and actual result, so that we can better comprehend the situation. and include the source code that you are using when triggering this. screenshots would also help if possible

[KotlinIsland]

here, we can see that the second parameter key is selected (activeParameter=1), because the first parameter is a star parameter

[jinzhongjia]

This is just my test code,

[KotlinIsland]

how are you getting activeParameter=3 with that example? there are no parameters entered

[jinzhongjia]

Yes, this is exactly what I am confused about. I don't understand why this response is returned to me.

[jinzhongjia]

Maybe I need to double check the request information I sent

[KotlinIsland]

so the issue is not that it is starting from 1, the problem is that you are receiving activeParameter=3 when there are no parameters?

[jinzhongjia]

No, no, you can find that paramters in the response content is an array with only three elements, so the maximum activeParameter should be only 2

[jinzhongjia]

Just after you reminded me, I found that the activeParamter value returned is 3, which is not correct. I only noticed the problem that it is greater than 2.

[jinzhongjia]

I checked the initiated request and found that it was a regular request that met the spec's criteria

[jinzhongjia]

Maybe basedpyright should be asked to print the complete log
But I didn't find specific instructions in the help document. It only mentioned how to use vscode to debug basedpyright.

[jinzhongjia]

Regarding the definition of activeParamter, there are two in the specification

[jinzhongjia]

So it can be said that the current problem is why the outer activeParameter gives the length of the paramters array by default. I don’t understand the code of basedpyright, so I can only make this guess.

[KotlinIsland]

hmm, it would seem that SignatureHelp.activeParameter isn't used if SignatureInformation.activeParameter is supplied, so it's not too much of a problem if it has the wrong value i think, we can check with @DetachHead.

i just noticed in the data you provided that there is no activeSignature for list.sort, hmm

[jinzhongjia]

activeSignature is optional, defaults to 0 if not

[KotlinIsland]

i don't understand this "defaults to 0", if it's optional, then wouldn't it be undefined not 0?

[jinzhongjia]

[DetachHead]

so is the issue that there's two separate activeParameter keys, the outer one is always wrong but the inner one is always present and always correct? and since the spec says that the inner one should override the outer one this technically shouldn't be an issue?

if this is the case i'm curious as to why there are two different keys for the same thing in the first place. one of them should be deleted or at least deprecated. the current docs for this are extremely confusing.

[jinzhongjia]

Yes, this technically shouldn't be an issue, but I thought it was worth reporting.
If the external key is invalid, it may be represented by a negative number. Using the array length representation may be confusing.

I also think the current documentation is confusing and it's not stated clearly enough

Or it would be better to mark the issue as QA

[DetachHead]

i've opened microsoft/language-server-protocol#2080 to try and get some clarification, then i'll decide what to do about it on my end. will leave this issue open in the mean time