x/tools/gopls/internal/cmd: `gopls check` doesn't wait for didOpen finish before it makes diagnostics request

podtserkovskiy commented 4 months ago

gopls version

0.15.3

gopls -v version
Build info
----------
gopls, built in GOPATH mode (unknown)
    @
go: go1.22.3 X:nocoverageredesign

go env

go env                                                                                        х INT
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/michaelpo/.cache/go-build'
GOENV='/home/michaelpo/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/michaelpo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/michaelpo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/golang'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.2 (Red Hat 1.22.2-1.el9)'
GCCGO='gccgo'
GOAMD64='v2'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2398423546=/tmp/go-build -gno-record-gcc-switches'



### What did you do?

I work on integration of a third-party build system (Buck2) and gopls.
I want to write e2e tests with my `GOPACKAGESDRIVER`.

The command `gopls check` would work good for a simple smoke test.

However, when I run it on a broken file, it reposts no issues.
This is happening because of lack of the synchronisation between [`conn.openFile`](https://github.com/golang/tools/blob/f2d2ebe43e72e46b6ae6140c48f124b74309ecdd/gopls/internal/cmd/check.go#L68C3-L68C39) and [`conn.diagnoseFiles`](https://github.com/golang/tools/blob/f2d2ebe43e72e46b6ae6140c48f124b74309ecdd/gopls/internal/cmd/check.go#L74) calls. 

The issue is that `conn.openFile` calls `Server.DidOpen` internally and `Server.DidOpen` processes files asynchronously.

When I put `time.Sleep(...)` before `conn.diagnoseFiles` the command reports all the errors correctly.

When I test this command without custom `GOPACKAGESDRIVER` it works with no extra `time.Sleep` because `go list` returns results faster than my driver and by the time `conn.diagnoseFiles` is called all the data is ready.

I'm happy to make a PR, but I'm not familiar with gopls codebase. 
Could you give some hints how to approach this issue nicer than `time.Sleep`? 
Is there a way to query another method of the server to check if the file has been processed before querying `conn.diagnoseFiles`?

### What did you see happen?

~

### What did you expect to see?

~

### Editor and settings

_No response_

### Logs

_No response_

gabyhelp commented 4 months ago

Similar Issues

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

findleyr commented 4 months ago

Thanks, this is a bug: the check subcommand is meant to wait for all outstanding diagnostics.

Separately, we also want to prioritize support for pull diagnostics, so we will aim to address this in the v0.17 release.

podtserkovskiy commented 3 months ago

@findleyr In the meantime, could you suggest another command that can be used as a smoke integration test of gopls and a custom GOPACKAGESDRIVER? Any command that traverses imports and validates all packages has been found should work for this purpose

findleyr commented 3 months ago

@podtserkovskiy you may have luck with golang.org/x/tools/go/packages/gopackages.

For example:

$ gopackages -json -mode=syntax . | jq .Errors

golang / go

x/tools/gopls/internal/cmd: `gopls check` doesn't wait for didOpen finish before it makes diagnostics request #68075

gopls version

go env