x/tools/gopls: cannot send false responses in capabilities #46052

glennsarti · 2021-05-08T13:08:54Z

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

$ go version
go version go1.15.8 windows/amd64

Does this issue reproduce with the latest release?

Probably

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

go env Output

$ go env
set GO111MODULE=

set GOARCH=amd64

set GOBIN=

set GOCACHE=C:\Users\glenn\AppData\Local\go-build

set GOENV=C:\Users\glenn\AppData\Roaming\go\env

set GOEXE=.exe

set GOFLAGS=

set GOHOSTARCH=amd64

set GOHOSTOS=windows

set GOINSECURE=

set GOMODCACHE=C:\Users\glenn\go\pkg\mod

set GONOPROXY=

set GONOSUMDB=

set GOOS=windows

set GOPATH=C:\Users\glenn\go

set GOPRIVATE=

set GOPROXY=https://proxy.golang.org,direct

set GOROOT=C:\Program Files\Go

set GOSUMDB=sum.golang.org

set GOTMPDIR=

set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64

set GCCGO=gccgo

set AR=ar

set CC=gcc

set CXX=g++

set CGO_ENABLED=1

set GOMOD=C:\Source\oss\puppet-editor-services-go\go.mod

set CGO_CFLAGS=-g -O2

set CGO_CPPFLAGS=

set CGO_CXXFLAGS=-g -O2

set CGO_FFLAGS=-g -O2

set CGO_LDFLAGS=-g -O2

set PKG_CONFIG=pkg-config

set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\glenn\AppData\Local\Temp\go-build835385066=/tmp/go-build -gno-record-gcc-switches

What did you do?

I'm working on implementing a Language Server in Go (but not for golang itself). I was trying send a response from Lang. Server to the Language client to indicate I did not support many of the providers, e.g. CodeLens however the JSON response was not showing the false entries. e.g.

When I use the following response in Go.

		Capabilities: protocol.ServerCapabilities {
			FoldingRangeProvider: true,
			TextDocumentSync: protocol.ParseTextDocumentSyncKind("Full"),
			DocumentLinkProvider: protocol.DocumentLinkOptions{
				ResolveProvider: false,
			},
		},
	}

When it is converted to JSON, the output just ignores it

{{"capabilities":{"textDocumentSync":1,"completionProvider":{},"signatureHelpProvider":{},"codeLensProvider":{},"documentLinkProvider":{},"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"foldingRangeProvider":true,"executeCommandProvider":{"commands":null},"workspace":{"workspaceFolders":{}}},"serverInfo":{"name":"Puppet Language Server in Go","version":"0.0.1"}}

Note the documentLinkProvider":{} . It should have { resolveProvider: false}

What did you expect to see?

It should out the response fully in JSON

What did you see instead?

It ignores bools with false

This is probably due to:
https://github.com/golang/tools/blob/master/internal/lsp/protocol/tsprotocol.go#L1739

ResolveProvider bool `json:"resolveProvider,omitempty"`

This could (probably) be solved using *bool but as this file is automatically generated, this seems like a fix that needs to be made in the tools.

Now, while this code is internal and my usecase is not normal, I did notice that gopls will set a false value and this will not be sent to the Language client. And would be considered a bug.

https://github.com/golang/tools/blob/3e17c62e37b686d70c24a370a51cd33f8aa52df2/internal/lsp/general.go#L150

The text was updated successfully, but these errors were encountered:

findleyr · 2021-05-10T17:12:46Z

Thanks for the report.

And would be considered a bug.

I'm not sure this is a bug. Per the spec, "A missing property should be interpreted as an absence of the capability.". Is there a case where the client would actually interpret incorrect capabilities as a result of the json:"omitempty" tag?

glennsarti · 2021-05-12T12:37:45Z

Hi @findleyr

Sure a couple of ideas come to mind....

"A missing property should be interpreted as an absence of the capability."

That referred the to the Client Capabilities from the client to the server. I couldn't find the same for the Server Capabilities.

On searching through the spec I couldn't see any booleans that defaulted to true if omitted. But I could've missed one...

That said, I can see that what I was actually seeing was the server capabilities for things like documentOnTypeFormattingProvider. There didn't appear to be a way to not have that in the JSON response. Same with executeCommandProvider and bits that used a struct.

https://github.com/golang/tools/blob/master/internal/lsp/protocol/tsprotocol.go#L3603

Unless I'm doing something completely wrong 🤷‍♂️

findleyr · 2021-05-12T13:28:47Z

That referred the to the Client Capabilities from the client to the server

Yes that part of the spec referenced client capabilities, but absent any commentary on server capabilities (which is directly below), I assume the same applies to server capabilities. The spec tends not to have a lot of details, unfortunately.

On searching through the spec I couldn't see any booleans that defaulted to true if omitted. But I could've missed one...

I don't think there can be any basic capabilities that can default to true, as per the wording missing configuration=>no capability.

I agree that it would be cleaner if these were all pointers, but I don't think it's a bug per-se. I think the zero-value configuration has the correct semantics when marshaled to JSON.

CC @pjweinb who knows more of the history here, and might have a different opinion.

pjweinb · 2021-05-12T15:24:00Z

i think @findelyr is right. The definition in the typescript code says that resolveProvider is optional. In typescript a logical test of an undefined value is treated as false. IsRetrigger (in SignatureHelpContext) is an example of a bool that is not optional.

…

On Wed, May 12, 2021 at 9:29 AM findleyr ***@***.***> wrote: That referred the to the Client Capabilities from the client to the server Yes that part of the spec referenced client capabilities, but absent any commentary on server capabilities (which is directly below), I assume the same applies to server capabilities. The spec tends not to have a lot of details, unfortunately. On searching through the spec I couldn't see any booleans that defaulted to true if omitted. But I could've missed one... I don't think there can be any basic capabilities that can default to true, as per the wording missing configuration=>no capability. I agree that it would be *cleaner* if these were all pointers, but I don't think it's a bug per-se. I think the zero-value configuration has the correct semantics when marshaled to JSON. CC @pjweinb <https://github.com/pjweinb> who knows more of the history here, and might have a different opinion. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#46052 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJIAI5PNFH2QPMASFHRFA3TNJ7CPANCNFSM44M53ZOQ> .

glennsarti · 2021-05-15T13:22:28Z

Thanks both of you!

I'm happy to close this issue and raise one about the non-nil-able resolvers (like completionProvider) which can't seem to be turned off.

stamblerre · 2021-05-17T16:41:14Z

Sure sounds good--please raise a new issue if needed.

gopherbot added the gopls label May 8, 2021

stamblerre added this to the Unreleased milestone May 10, 2021

findleyr changed the title ~~gopls cannot send false responses in capabilities~~ x/tools/gopls: cannot send false responses in capabilities May 10, 2021

gopherbot added the Tools label May 10, 2021

findleyr added the NeedsInvestigation label May 10, 2021

findleyr modified the milestones: Unreleased, Backlog May 10, 2021

findleyr added the WaitingForInfo label May 10, 2021

stamblerre modified the milestones: Backlog, gopls/unplanned May 12, 2021

stamblerre closed this as completed May 17, 2021

stamblerre removed this from the gopls/unplanned milestone May 17, 2021

golang locked and limited conversation to collaborators May 17, 2022

gopherbot added the FrozenDueToAge label May 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/gopls: cannot send false responses in capabilities #46052

x/tools/gopls: cannot send false responses in capabilities #46052

glennsarti commented May 8, 2021 •

edited

Loading

findleyr commented May 10, 2021

glennsarti commented May 12, 2021

findleyr commented May 12, 2021 •

edited

Loading

pjweinb commented May 12, 2021 via email

glennsarti commented May 15, 2021

stamblerre commented May 17, 2021

x/tools/gopls: cannot send false responses in capabilities #46052

x/tools/gopls: cannot send false responses in capabilities #46052

Comments

glennsarti commented May 8, 2021 • edited Loading

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

findleyr commented May 10, 2021

glennsarti commented May 12, 2021

findleyr commented May 12, 2021 • edited Loading

pjweinb commented May 12, 2021 via email

glennsarti commented May 15, 2021

stamblerre commented May 17, 2021

glennsarti commented May 8, 2021 •

edited

Loading

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

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

findleyr commented May 12, 2021 •

edited

Loading