Reminder issue, following up on #63641: it appears that gopls was not finding the new slices package in GOROOT until the ztdlib.go index was updated. But according to @heschi this index is meant to just be a cache. We should debug why dynamic scanning of GOROOT was not working.
Comment From: seankhliao
I'm not sure if it's goimports that's the problem. What I get is: completions are offered for non stdlib, but if I reject the completions and save, it pulls in stdlib.
https://github.com/golang/go/assets/11343221/99dc9d09-1405-402b-9e1b-528d0819d696
Comment From: seankhliao
For me this was only a problem when the go directive in go.mod was a tip version (currently go 1.22)
I think gopls 0.14.0 is doing something else where it just isn't offering any completions for me at all until I save/run goimports. Once the imports are added, completions work.
Comment From: danp
Trying to trace through what happens when I type slic<> and get completions in a module.
At first I was using a main module that had some required modules. Pretty sure that got me stdlib slices early due to transitive imports. For example, if something in my module (or my module directly) imported sort, that got me slices since sort imports slices. That's happening around here via AllMetadata:
https://github.com/golang/tools/blob/fcb8d5b1a810c653611b491448c7df4d1945c6cb/gopls/internal/lsp/source/completion/completion.go#L1690
Then I changed to an empty module (go mod init using tip + main.go with package main and empty func main). That means imports.GetAllCandidates called further down in unimportedPackages needs to find slices.
That ultimately calls getCandidatePkgs. getCandidatePkgs does start by scanning the stdlib cache (discussed in #63641) which for me does include slices. Since I'm interested in how slices might be found if the stdlib cache is out of date, I removed info about slices from the stdlib package var map.
At that point, trying slic<> stopped getting me a stdlib slices suggestion, I was getting golang.org/x/exp/slices.
Here's what I think is happening.
imports.GetAllCandidates calls getCandidatePkgs with a static rootFound callback always returning true:
https://github.com/golang/tools/blob/fcb8d5b1a810c653611b491448c7df4d1945c6cb/internal/imports/fix.go#L731-L733
rootFound is called when a new root dir (eg GOROOT, GOPATH, module, module cache, etc) is found and it indicates whether the root should be walked for packages.
While GetAllCandidates has a static true rootFound, getCandidatePkgs wraps it further:
https://github.com/golang/tools/blob/fcb8d5b1a810c653611b491448c7df4d1945c6cb/internal/imports/fix.go#L656-L658
In module mode ModuleResolver.scan ends up being called which does:
https://github.com/golang/tools/blob/fcb8d5b1a810c653611b491448c7df4d1945c6cb/internal/imports/mod.go#L502-L504
r.roots for me contains my GOROOT/src, my main module, and my GOMODCACHE. After this filtering it does not have GOROOT/src anymore.
If I change rootFound in getCandidatePkgs to always return true (ignoring whether the root type is a GOROOT), my slic<> completion starts working again.
Comment From: findleyr
@danp thanks very much for that analysis! Sounds about right, and this comment that you found seems to suggest that skipping GOROOT was intentional:
// Exclude goroot results -- getting them is relatively expensive, not cached,
// and generally redundant with the in-memory version.
Aside: this is very complicated logic, which has proven to be error-prone and hard to maintain. I've been collecting goimports issues under the gopls/goimports label in the hopes of making a project out of this technical debt.
Comment From: pjweinb
This is one of the cases where it is not clear how imports should find the right answer. Searching through the source of the standard library is expensive. The existing code contains the state of the standard library when gopls was compiled, but occasionally, as here, that's obsolete. The expense of searching may be worth it if the search succeeds, but searching on every typo would be quite costly. We'll figure it out in the long run, but not immediately.
Comment From: danp
Would it be workable to generate something like the bundled stdlib cache on demand and cache it keyed on the Go version in use or similar? I forget if the bundled stdlib cache describes what Go version was used to generate it. If so that could be factored in too (ie if the bundled cache was generated with Go 1.23 and that's the version in use then no need to rebuild it).
Comment From: pjweinb
That's one of the possibilities. (It depends on exactly what 'on demand' means.) An alternative would be to store the version-dependent caches on disk and think up a policy for when they are generated and updated. "We'll figure it out in the long run, but not immediately."
Comment From: adonovan
@pjweinb You've recently been working on using internal/stdlib to improve unimported completions. Would that fix this issue too?
Comment From: pjweinb
I don't think so. Unimported completions look in the internally cached version of stdlib, created when gopls is compiled. If a new gopls is released after a stdlib change, it will have anything new. gopls does check Go versions. The hard case is using a development version of Go with a released version of Gopls. (And although this has affected me, I don't think it's worth doing anything to fix it.)