Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: JSON-RPC spec violation #39719

Closed
uhthomas opened this issue Jun 19, 2020 · 8 comments
Closed

x/tools/gopls: JSON-RPC spec violation #39719

uhthomas opened this issue Jun 19, 2020 · 8 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@uhthomas
Copy link

uhthomas commented Jun 19, 2020

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

$ go version
go version go1.14.4 darwin/amd64

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=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/thomas/Library/Caches/go-build"
GOENV="/Users/thomas/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="code.cfops.it"
GONOSUMDB="code.cfops.it"
GOOS="darwin"
GOPATH="/Users/thomas/go"
GOPRIVATE="code.cfops.it"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.14.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/thomas/cloudflare/segments/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/d5/hqd0d3f17h7gmntg94k251z80000gn/T/go-build278475873=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I'm using gopls, along with sublimelsp. There was an issue where a single JSON-RPC error response will persist for the duration of the session. During this, it was discovered that gopls violates the JSON-RPC spec by sending a result in an error response.

I have a feeling this is actually a net/rpc/jsonrpc issue, as it's missing ,omitempty.

What did you expect to see?

The response to not contain a result in an error response.

What did you see instead?

{
  "jsonrpc": "2.0",
  "error": {
    "data": null,
    "code": 0,
    "message": "computing fix edits: /Users/me/main.go:9:21: expected ';', found Println (and 2 more errors)"
  },
  "id": 3,
  "result": null
}

gopls

$ gopls version
version master, built in $GOPATH mode
$ git rev-parse HEAD
6023b8da91ed4093207e7f49f7a48e0cde121f9b

Editor

Sublime Text Dev Channel, Build 4074

{
    "lsp_format_on_save": true,
    "lsp_code_actions_on_save": {
        "source.fixAll": true,
        "source.organizeImports": true
    },
    "show_symbol_action_links": true,
    "only_show_lsp_completions": true,
    "clients": {
        "gopls": {
            "enabled": true,
            "settings": {
                "gopls.usePlaceholders": true,
                "gopls.analyses": {
                  "fillreturns": true,
                  "nonewvars": true,
                  "undeclaredname": true,
                  "unusedparams": true
                },
                "gopls.completeUnimported": true,
                "gopls.staticcheck": true
            }
        }
    }
}

See the original issue for more information.

cc @rchl @rwols

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 19, 2020
@gopherbot
Copy link

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jun 19, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jun 19, 2020
@ianthehat
Copy link

I am confused, because gopls does not use net/rpc/jsonrpc, it uses x/tools/internal/jsonrpc2 where the wire structs do have omitempty on them, and I cannot reproduce it with a simple test.
I am wondering if there is a way it has a non nil raw message that still ends up as a null and thus does not trigger the omit empty logic

@uhthomas
Copy link
Author

uhthomas commented Jun 19, 2020

Ah yes, I thought it might be using that internal tools package. I made a quick example of how to reproduce this behaviour.

@uhthomas
Copy link
Author

I assume the fix is to check if result is nil in x/tools/internal/jsonrpc2.NewResponse. I can do this now if you like 😄

@gopherbot
Copy link

Change https://golang.org/cl/239097 mentions this issue: internal/jsonrpc2: don't set result in response if there is an error

@uhthomas
Copy link
Author

Off-topic, but I wonder if it might be a good idea not to marshal the data field for errors? The spec says it's optional, currently null is always encoded.

@ianthehat
Copy link

Probably, especially as we never fill it in.

@uhthomas
Copy link
Author

Created a new issue #39736

@golang golang locked and limited conversation to collaborators Jun 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants