x/tools/gopls: signatureHelp should return position info for signatures

[myitcv]

What version of Go are you using (go version)?

$ go version
go version devel +385b2e0cac Fri May 24 21:34:53 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190521203540-521d6ed310dd

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/myitcv/gostuff/src/github.com/myitcv/govim/cmd/govim/.bin"
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build579955727=/tmp/go-build -gno-record-gcc-switches"

The signature help call from client to server (https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#signature-help-request-leftwards_arrow_with_hook) returns a list of signatures:

type SignatureHelp struct {
	Signatures []SignatureInformation `json:"signatures"`
	ActiveSignature float64 `json:"activeSignature"`
	ActiveParameter float64 `json:"activeParameter"`
}

type SignatureInformation struct {
	Label string `json:"label"`
	Documentation string `json:"documentation,omitempty"` 
	Parameters []ParameterInformation `json:"parameters,omitempty"`
}

type ParameterInformation struct {
	Label string `json:"label"` 
	Documentation string `json:"documentation,omitempty"` 
}

But what's strange is that the signature information does not include position information, i.e. the start (and end, where it's valid/available) of where the call signature is valid: think the starting ( (to the end )).

This position information is valuable to my mind because it tells the client where to place, for example, a popup window.

---

cc @stamblerre @ianthehat

[stamblerre]

According to the LSP spec, the ParameterInformation struct can include position information instead of a label. If we used that, would it be enough to solve this problem?

[myitcv]

Oh wow, I totally missed that comment, thanks.

Yes, if this is position information like other places in the spec, should be fine!

[stamblerre]

@pjweinb: Would it be possible for us to make this change in the protocol generator?

[pjweinb]

I'll look at it when I get back, but seems likely.

…

On Mon, Jun 10, 2019 at 6:23 PM Rebecca Stambler ***@***.***> wrote: @pjweinb <https://github.com/pjweinb>: Would it be possible for us to make this change in the protocol generator? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#32468?email_source=notifications&email_token=ABJIAIZM2K4AYCQ5KC754ILPZ3H4XA5CNFSM4HVIA6Q2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXLMV5Q#issuecomment-500615926>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABJIAI7FBDA2LK235VPDF7TPZ3H4XANCNFSM4HVIA6QQ> .

[pjweinb]

@stamblerre: could you add a [2]float64 field to ParameterInformation in lsp/source/signature_help.go? It does't matter what you name it, but once it exists (and gets filled in), we can change line 57 in signature_help.go to use the new field in place of p.Label.

[myitcv]

@stamblerre @pjweinb - is there anything I can do to help move this along?

[stamblerre]

I'm sorry, this is a slightly lower priority issue for us right now. I will have to make the changes Peter requested before we can change the generator, but I haven't had a chance to get to this yet.

[oblitum]

For what is worth, I guess it's the same issue, this was fixed on the bingo server long ago saibing/bingo#20.

[oblitum]

In specific there was a mention to labelOffsetSupport: saibing/bingo#20 (comment).

[oblitum]

And the advent of the gopls server at the time caused a regression for bingo: saibing/bingo#135 (comment).

[stamblerre]

Thanks for reporting this. It's a slightly different issue because it's a bug in signature help, whereas this issue is to track making use of the labelOffsetSupport capability, as you said. I filed #36766 to track the bug you mentioned.