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

Gopackagesdriver: tell language server to fall back when bazel query fails #3334

Closed
ian-h-chamberlain opened this issue Oct 28, 2022 · 2 comments · Fixed by #3338
Closed

Comments

@ian-h-chamberlain
Copy link
Contributor

What version of rules_go are you using?

0.35.0

What version of gazelle are you using?

N/A

What version of Bazel are you using?

Bazelisk version: development
Build label: 5.3.2
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Wed Oct 19 18:29:41 2022 (1666204181)
Build timestamp: 1666204181
Build timestamp as int: 1666204181

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

macOS 12.6, x86_64

Any other potentially useful information about your toolchain?

Using GOPACKAGESDRIVER with

Version: 1.72.2
Commit: d045a5eda657f4d7b676dedbfa7aab8207f8a075
Date: 2022-10-12T22:16:30.254Z
Electron: 19.0.17
Chromium: 102.0.5005.167
Node.js: 16.14.2
V8: 10.2.154.15-electron.0
OS: Darwin x64 21.6.0
Sandboxed: No

What did you do?

Set up a simple rules_go repo and add a file not yet tracked by Bazel (untracked-file branch)
https://github.com/ian-h-chamberlain/gopackagesdriver-repro/tree/untracked-file

What did you expect to see?

I don't necessarily expect completions, etc. to work on the new file, but I also don't expect completion to block for a very long time. In this particular case, xyz.go is not part of any Bazel target, but the default gopls package List implementation can handle it and provide completions where gopackagesdriver cannot.

What did you see instead?

Asking for completions takes quite a long time and never actually works correctly in xyz.go.

I believe this happens because gopackagesdriver responds to the language server that it can handle queries about the file, even though bazel query has errors:

{"NotHandled":false,"Sizes":{"WordSize":8,"MaxAlign":8},"Packages":[]}

I believe that simply by responding with "NotHandled":true when bazel query fails, the language server will fallback to its default implementation and completion, etc. will work mostly correctly.
The main branch includes a proof-of-concept gopackagesdriver.sh that responds this way for any file with xyz in the name:
https://github.com/ian-h-chamberlain/gopackagesdriver-repro/tree/main

Hopefully this is an easy fix that will make things work nicer when a file isn't yet included in Bazel rules so that gopls can fall back to its default behavior!

@fmeum
Copy link
Member

fmeum commented Oct 30, 2022

That sounds very reasonable. Given that you already have a PoC, do you want to turn it into a PR?

ian-h-chamberlain added a commit to ian-h-chamberlain/rules_go that referenced this issue Oct 31, 2022
Fixes bazel-contrib#3334 by telling gopls to fallback if the gopackagesdriver can't
`bazel query` about the requested file.
ian-h-chamberlain added a commit to ian-h-chamberlain/rules_go that referenced this issue Oct 31, 2022
Fixes bazel-contrib#3334 by telling gopls to fallback if the gopackagesdriver can't
`bazel query` about the requested file.
ian-h-chamberlain added a commit to ian-h-chamberlain/rules_go that referenced this issue Oct 31, 2022
Fixes bazel-contrib#3334 by telling gopls to fallback if the gopackagesdriver can't
`bazel query` about the requested file.
@ian-h-chamberlain
Copy link
Contributor Author

@fmeum sure, opened #3338 with what I think is a simple fix, although it's possible I'm missing some edge cases there

linzhp pushed a commit that referenced this issue Oct 31, 2022
Fixes #3334 by telling gopls to fallback if the gopackagesdriver can't
`bazel query` about the requested file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants