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: fails to find new std library APIs dynamically #63767

Open
findleyr opened this issue Oct 27, 2023 · 4 comments
Open

x/tools/gopls: fails to find new std library APIs dynamically #63767

findleyr opened this issue Oct 27, 2023 · 4 comments
Labels
gopls/imports gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

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.

@findleyr findleyr added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. gopls/imports labels Oct 27, 2023
@findleyr findleyr added this to the gopls/v0.15.0 milestone Oct 27, 2023
@findleyr findleyr changed the title x/tools/gopls: fails to find new std library APIs via dynamic means x/tools/gopls: fails to find new std library APIs dynamically Oct 27, 2023
@seankhliao
Copy link
Member

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.

recording.mp4

@seankhliao
Copy link
Member

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.

@danp
Copy link
Contributor

danp commented Oct 28, 2023

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.

@findleyr
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/imports gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants