x/tools/gopls: define which LSP features return errors

[myitcv]

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

$ go version
go version devel +0ac8739ad5 Mon Nov 18 15:11:03 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20191118222007-07fc4c7f2b98
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20191118222007-07fc4c7f2b98

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=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
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-build491500556=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Previously we have discussed what Formatting/CodeAction should do in the presence of parse errors.

Currently, Formatting/CodeAction return an error if the code in question does not parse. However, clients have no way of distinguishing a parse error from a "real" error (i.e. anything else that would point to a more significant problem)

Therefore clients like govim end up simply ignoring return errors, reason being:

• the user is already aware of parse errors via diagnostics
• having an error thrown on format-on-save will prevent the user from ever saving the file until the parse error is fixed (which the user might not want to do just yet)

This approach of ignoring all errors has the unfortunate side effect of "real" errors getting missed. In govim we have had ~2/3 instances of non-parse errors getting missed in this way and real bugs slipping through the net.

We have at least two options as previously discussed:

1. do not return parse errors for either Formatting/CodeAction and instead rely on diagnostics which already tell the user about this
2. return structured errors so that the user can distinguish and ignore parse errors

Thoughts?

---

cc @stamblerre

[stamblerre]

I'm going to co-opt this issue to continue the general discussion about which LSP requests should return errors, and which should just fail silently. The guiding principle that I'm following right now is: An action triggered by the user should always get a complete response or an error, whereas an action triggered by the editor should get a partial response. As an example:

• textDocument/documentLink returns a partial response, because the editor triggers it when file contents change.
• textDocument/references returns a complete response or an error, because it's triggered by the user.

I'd like to complete this list for all of the different features, and formatting and code actions are corner cases, since they can be triggered by the editor automatically or by the user directly.

[stamblerre]

I think this is mostly resolved. We don't return errors for requests that the client will send frequently (on file change), but we do for user-triggered requests.