x/tools/gopls: GOPACKAGESDRIVER calls happen at intervals that make the results incorrect

[JamyDev]

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

$ go version
1.20.3

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="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT="nocoverageredesign,nounified"
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go-code/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go-code"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/.cache/bazel/_bazel_jamy/b97476d719d716accead0f2d5b93104f/external/go_sdk_linux_amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/.cache/bazel/_bazel_jamy/b97476d719d716accead0f2d5b93104f/external/go_sdk_linux_amd64/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3 X:nounified,nocoverageredesign"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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 -fdebug-prefix-map=/tmp/go-build2135155060=/tmp/go-build -gno-record-gcc-switches"

What did you do?

With GOPACKAGESDRIVER enabled, adding an import (whether by inline writing code and accepting an import suggestion; or manually adding it to the imports section) will trigger a query to the packages driver. However, this trigger will happen prior to the file being saved, yet the packagesdriver will only get a filename to load packages for.

This triggers a bit of a race condition in build systems like Bazel:

1. Current imports in file.go are a, b, c
2. I add import d
3. GoPLS immediately tries to look up package info from the driver using the filename ($ pkgdrvr file=file.go)
4. Since the file isn't saved at this point in time, the packagesdriver will only see imports a, b, c and thus only return info for those 3
5. GoPLS gets incomplete results back and flags import d as bad.
6. Even after save, there is no re-trigger of the packagesdriver, which means this bad imports stays bad until after the imports are changed again; or the language server is restarted.

What did you expect to see?

Since the packagesdriver works with files on disk (in the general case) we should not query the packagesdriver until the IDE has committed the changes on disk.

Alternatively we can extend the packages driver call to query directly for the information of the imported package, rather than indirectly querying for the file the import is added to.

What did you see instead?

Package import resolution fails until language server restart.

I will try to get a project where we can repro this situation in.

[hyangah]

Can you share which gopackagesdriver you are using.
The default gopackage driver go list handles this case by using overlay.
https://pkg.go.dev/golang.org/x/tools/go/packages#Config

Is it possible to make your gopackagesdriver recognize the overlay info?

@golang/tools-team I think we need documentation on gopackagesdriver #34341

[JamyDev]

We use the rules_go one. I'll have a look at overlay info and see how to support it in the rules_go driver

[findleyr]

We discussed this a bit more in our team meeting, and decided it would probably be very difficult to support this in the bazel driver (based on our understanding of bazel).

So we'll leave this open: it would be nice to fix, but unlikely to get prioritized any time soon. The work to implement this would be approximately:

• accumulate invalidations in cache.snapshot.clone while the user types.
• only invalidate and reload packages when the user saves all files

...but this will be very tricky to implement, and even after implemented may have to a subtly confusing UX.