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: unable to filter irrelevant diagnostics #36427

Open
myitcv opened this issue Jan 7, 2020 · 7 comments
Open

x/tools/gopls: unable to filter irrelevant diagnostics #36427

myitcv opened this issue Jan 7, 2020 · 7 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jan 7, 2020

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

$ go version
go version devel +693748e9fa Mon Jan 6 11:46:56 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200103221440-774c71fcf114
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20200103221440-774c71fcf114

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-build656642862=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Continuing discussion from #36243 (comment)

Consider the following setup:

-- go.mod --
module mod.com

go 1.14
-- main.go --
package main
-- p/p.go --
package p

asdf

Furthermore, that we want to work on the main package at the module root.

If we open govim and open main.go we see the following diagnostics:

p/p.go|3 col 1| expected declaration, found asdf

i.e. nothing to do with the main package.

Based on the diagnostics we get back from gopls, we (in the editor) have no way of filtering to only show the diagnostics that are relevant to the package we are working on (i.e. the package and its transitive dependencies).

What did you expect to see?

Expected to have some way to filter the diagnostics based on my current file, i.e. context.

What did you see instead?

All diagnostics.


cc @stamblerre

@gopherbot gopherbot added this to the Unreleased milestone Jan 7, 2020
@gopherbot gopherbot added the Tools label Jan 7, 2020
@pjweinbgo

This comment has been minimized.

Copy link
Contributor

@pjweinbgo pjweinbgo commented Jan 7, 2020

This may be a vim issue rather than one with gopls. The gopls syntax error message says that it is a diagnostic for p.go in its "uri" field. (at least when I try your example)

gopls looks at p.go because it runs go/packages.Load ./... (that is, on the whole module) to report errors that would stop the module from compiling, I think.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Jan 7, 2020

This may be a vim issue rather than one with gopls

I don't think so.

I raised this issue because the diagnostics returned by gopls don't carry with them any dependency information.

This isn't about me being able to filter to show only the main.go diagnostics; it's about being able to filter the list of diagnostics to only show the diagnostics for package main and its transitive dependencies.

And by extension we might also want to show diagnostic errors for reverse dependencies too, filtering out anything else.

But without this information being sent by gopls we can't do any filtering in govim/Vim/elsewhere.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Jan 7, 2020

Can you share the gopls log that you're seeing for this example? The diagnostics I get are:

[Trace - 14:48:07.106 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///[redacted]/p/p.go","diagnostics":[{"range":{"start":{"line":2,"character":0},"end":{"line":2,"character":0}},"severity":1,"source":"syntax","message":"expected declaration, found asdf"}]}

which make it clear that the error is not in main.go.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Jan 7, 2020

The diagnostics I get are:

That's exactly what I see as well.

which make it clear that the error is not in main.go.

Indeed.

But we can't programmatically filter out that diagnostic in govim/Vim because we can't know that it's not in the transitive dependencies of package main.

What I'm imagining here is three modes of showing diagnostics in the quickfix window in Vim:

  1. show all diagnostics sent by gopls
  2. show diagnostics for the current file's package and its transitive dependencies
  3. show diagnostics for the current file's package, its transitive dependencies, and reverse dependencies of the current file's package

Because if there are errors, like in package p, that are not relevant to the file/package I'm working on (main in this example) then I can't easily tell when I'm "done". I often use "zero diagnostics" as an indicator that I'm good to test (the current package).

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Jan 7, 2020

How would this work in the context of LSP publishDiagnostics message? A diagnostic has to be associated with a specific file, so I'm not sure how it would be possible to show diagnostics in a way that makes it clear that they apply to a reverse dependency or something like that.

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Jan 8, 2020
@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Jan 8, 2020

How would this work in the context of LSP publishDiagnostics message

That's exactly the reason for me raising this issue. gopls (now) always sends diagnostics for the entire workspace: given the current LSP spec, we currently have no means by which we can filter diagnostics that are shown to the user based on some context i.e. visible file(s).

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Jan 8, 2020

This sounds like a specifically terminal-based editor issue. In VS Code, all diagnostics are associated with a file and diagnostics are shown for the whole workspace, so you are able to see which diagnostics belong to which file by looking in the file explorer tab on the left or through the "Problems" pane. I expect most people keep working until their entire workspace builds. In Vim, I see how this might be an issue since you don't have similar navigation tools. I would recommend looking into how other LSP clients handle this.

The closest thing that LSP has for this is the "Related Information" in the diagnostic, but that's not exactly what you mean (I think). To me, it sounds like you are looking for additional configuration in gopls that changes the scope of the diagnostics sent. It's similar to what people have requested for diagnostics that are only sent on save. In those cases, we have suggested that the configuration ought to live client-side. I understand why it would require a lot of additional work in govim to figure out dependencies and reverse dependencies for packages, but I'm not convinced that this use case is significant enough to justify additional complexity in gopls. If this is an issue that users report naturally, I'd be happy to consider it again, but the example you've given seems artificial to me.

@stamblerre stamblerre modified the milestones: gopls v1.0, gopls unplanned Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.