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

"Go: Add Import" does not work with gopls v0.6.11 #1549

Closed
hyangah opened this issue Jun 8, 2021 · 6 comments
Closed

"Go: Add Import" does not work with gopls v0.6.11 #1549

hyangah opened this issue Jun 8, 2021 · 6 comments
Assignees

Comments

@hyangah
Copy link
Contributor

hyangah commented Jun 8, 2021

https://go-review.googlesource.com/c/vscode-go/+/322169 uses gopls as a backend for Go: Add Import" command, and the gopls.list_known_packagescommand is available only after gopls v0.7.0 (not yet released). The idea was that first the extension tries to run gopls command, and if the command fails (because gopls is old ), the extension tries the oldimplbased backend. But it turned outgopls` v0.6.11 doesn't seem to return an error but an empty packages list. As a result, the command fails to show any candidates packages. @marwan-at-work

The following is the log with 'v0.6.11'.

[Trace - 10:50:04.526 AM] Received response 'initialize - (0)' in 2ms.
Result: {"capabilities":{"textDocumentSync":{"openClose":true,"change":2,"save":{}},"completionProvider":{"triggerCharacters":["."],"completionItem":{}},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor.extract","refactor.rewrite","source.fixAll","source.organizeImports"]},"codeLensProvider":{},"documentLinkProvider":{},"workspaceSymbolProvider":true,"documentFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"renameProvider":{"prepareProvider":true},"foldingRangeProvider":true,"executeCommandProvider":{"commands":["gopls.add_dependency","gopls.add_import","gopls.apply_fix","gopls.check_upgrades","gopls.gc_details","gopls.generate","gopls.generate_gopls_mod","gopls.go_get_package","gopls.list_known_packages","gopls.regenerate_cgo","gopls.remove_dependency","gopls.run_tests","gopls.start_debugging","gopls.test","gopls.tidy","gopls.toggle_gc_details","gopls.update_go_sum","gopls.upgrade_dependency","gopls.vendor","gopls.workspace_metadata"]},"callHierarchyProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":"workspace/didChangeWorkspaceFolders"}}},"serverInfo":{"name":"gopls","version":"{\"path\":\"golang.org/x/tools/gopls\",\"version\":\"v0.6.11\",\"sum\":\"h1:7S2k0xuVYc3secjy2uz0n+fGYxGJU6gXsLOmQ/r1HoI=\",\"deps\":[{\"path\":\"github.com/BurntSushi/toml\",\"version\":\"v0.3.1\",\"sum\":\"h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=\"},{\"path\":\"github.com/google/go-cmp\",\"version\":\"v0.5.5\",\"sum\":\"h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=\"},{\"path\":\"github.com/sergi/go-diff\",\"version\":\"v1.1.0\",\"sum\":\"h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=\"},{\"path\":\"golang.org/x/mod\",\"version\":\"v0.4.2\",\"sum\":\"h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=\"},{\"path\":\"golang.org/x/sync\",\"version\":\"v0.0.0-20210220032951-036812b2e83c\",\"sum\":\"h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=\"},{\"path\":\"golang.org/x/sys\",\"version\":\"v0.0.0-20210403161142-5e06dd20ab57\",\"sum\":\"h1:F5Gozwx4I1xtr/sr/8CFbb57iKi3297KFs0QDbGN60A=\"},{\"path\":\"golang.org/x/tools\",\"version\":\"v0.1.1-0.20210504170620-03ebc2c9fca8\",\"sum\":\"h1:rTLms91GhM16y4sUcNGLdel0jJ8jXdQeXuN+7evgYiQ=\"},{\"path\":\"golang.org/x/xerrors\",\"version\":\"v0.0.0-20200804184101-5ec99f83aff1\",\"sum\":\"h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=\"},{\"path\":\"honnef.co/go/tools\",\"version\":\"v0.1.3\",\"sum\":\"h1:qTakTkI6ni6LFD5sBwwsdSO+AQqbSIxOauHTTQKZ/7o=\"},{\"path\":\"mvdan.cc/gofumpt\",\"version\":\"v0.1.1\",\"sum\":\"h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=\"},{\"path\":\"mvdan.cc/xurls/v2\",\"version\":\"v2.2.0\",\"sum\":\"h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=\"}]}"}}
...

[Trace - 10:50:15.550 AM] Sending request 'workspace/executeCommand - (14)'.
Params: {"command":"gopls.list_known_packages","arguments":[{"URI":"file:///Users/hakim/projects/demo/main.go"}]}


[Trace - 10:50:15.550 AM] Received request 'window/workDoneProgress/create - (5)'.
Params: {"token":"8674665223082153551"}


[Trace - 10:50:15.550 AM] Sending response 'window/workDoneProgress/create - (5)' in 0ms.
Result: 


[Trace - 10:50:15.551 AM] Received notification '$/progress'.
Params: {"token":"8674665223082153551","value":{"kind":"begin","title":"Listing packages","cancellable":true,"message":"Running..."}}


[Trace - 10:50:15.551 AM] Received notification '$/progress'.
Params: {"token":"8674665223082153551","value":{"kind":"end","message":"completed"}}


[Trace - 10:50:15.551 AM] Received response 'workspace/executeCommand - (14)' in 1ms.
Result: {"Packages":[]}

I was about to propose to check the initialize response and see if the gopls.list_known_package is available for use. But, unfortunately, it turned out the command is available in v0.6.11. But I guess v0.6.11 was released with partial implementation.

gopls v0.7.0 release is very close so I think the easiest workaround here is to tell users to update gopls.
Another option (for users who can't update to gopls v0.7.0+ right now) is to treat the empty result as an error and trigger fallback.

p.s. @stamblerre @findleyr is there any way to prevent this type of partial feature rollout in the future? (e.g. requires clients to opt in during the partial implementation period, etc).

@gopherbot gopherbot added this to the Untriaged milestone Jun 8, 2021
@findleyr
Copy link
Contributor

findleyr commented Jun 8, 2021

Oh no, this is an unfortunate sequence of events. I added the stub command and then Marwan filled it in.

For commands in particular we could have some kind of marker saying "this command is not ready yet", but that might be overfitting the solution.

Is it worth patching VS Code to hard-code the gopls version for these commands to 0.7.0?

@hyangah hyangah self-assigned this Jun 8, 2021
@findleyr findleyr modified the milestones: Untriaged, On Deck Jun 9, 2021
@findleyr findleyr added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 9, 2021
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/326712 mentions this issue: src/goImport: avoid using partially implemented gopls features

@hyangah
Copy link
Contributor Author

hyangah commented Jun 10, 2021

The situation was more complicated than I initially thought. I tried to summarize
the issue in the cl/326712 description. As gopls 0.7.0 is picked up more widely,
I hope eventually the problem goes away.

Unfortunately, gopls's version info is not a reliable feature
selector because if gopls is the latest unstable version, or
built for development, it doesn't have meaningful version info.
(For unstable, we always get a pseudoversion prefixed with v0.0.0).

I tried to heuristically determine whether the commands are bogus
by looking at the results. Again, that's not reliable either.
In particular, gopls.add_import returns success in v0.6.11 and
v0.7.0.

Thus, in this CL, we just check whether the version string is
exactly v0.6.11 and the language server name is gopls, and
sanitize the command list. Users with v0.6.11 prereleases or
dev or unstable version built before the implementation was checked
int may still see the problem, but that's a rare case, and
it's time for them to update their gopls.

@hyangah
Copy link
Contributor Author

hyangah commented Jun 15, 2021

v0.26.0 will go out without this fix.
Do we still need decision? Or should we just ask users to update to the latest gopls because "Go: Add Import" is, we think, a rarely used feature?

cc @stamblerre @marwan-at-work @findleyr

@marwan-at-work
Copy link
Contributor

I think until gopls is 1.0, it's been pretty common, at least personally, to frequently upgrade gopls so I'm in favor of that option and I agree that Add Import is not as common as other features.

Thank you!

@hyangah
Copy link
Contributor Author

hyangah commented Jun 22, 2021

We decided not to fix this issue.
Users who encounter this issue need to update their gopls.

@hyangah hyangah closed this as completed Jun 22, 2021
@hyangah hyangah added WillNotFix and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 22, 2021
@golang golang locked and limited conversation to collaborators Jun 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants