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

workspace-lsp-roots is not used when opening a new document under the same language server #12222

Closed
sysheap opened this issue Dec 9, 2024 · 2 comments · Fixed by #12223
Closed
Labels
C-bug Category: This is a bug

Comments

@sysheap
Copy link
Contributor

sysheap commented Dec 9, 2024

Summary

I have set up a POC which illustrates the bug.
The POC can be found here: https://github.com/hieronymusma/poc-helix-workspace-lsp-roots/

My crate has the following structure
workspace-crate (actual rust workspace crate)
- crate1 (part of rust workspace crate, host target)
- crate2 (not part of the rust workspace crate, different target)

Because crate2 has a different target than the workspace crate, I need to setup workspace-lsp-roots (to ["./", "./crate2"]).

Now I get the following behavior:

If I first open crate2/main.rs and then crate1/main.rs everything works fine. Rust-analyzer starts and annotates for me the variables which are unused (that's how I test if it works or not).

However, If I switch it around and first open crate1/main.rs and then crate2/main.rs I loose the annotations of crate2.

I dug into the source code, and it seems that the root_dirs argument of find_lsp_workspace is the problem. If I'm using the problematic order (first crate then crate2), root_dirs is always an empty array. The value is taken from editor::Config::workspace_lsp_roots which is always an empty Vector. It never gets set.

I already have a working fix. However, I'm not familiar with the codebase at all. So I'm not sure if my fix is correct. I will link it under this issue in a couple of minutes. Maybe we can discuss the technical sides of the PR inside the PR then.

Thank you already for your time!

Reproduction Steps

  1. git clone https://github.com/hieronymusma/poc-helix-workspace-lsp-roots/
  2. cd poc-helix-workspace-lsp-roots
  3. rustup component add rust-analyzer

Test case which works fine:
Open helix and first open crate2/src/main.rs via the file picker. Wait until rust-analyzer has done it's work, and then you will see a yellow annotation, that the info variable is unused. After that, open crate1/src/main.rs, wait again for the rust-analyzer, and then you will see that the variable unused is annotated.

Test case which doesn't work:
Now do the opposite. Open helix and first open crate1/src/main.rs. You will also see the yellow annotations. Then open crate2/src/main.rs. Wait a while and nothing happens. rust-analyzer is not running on this file.

Helix log

I compared both logs and the following line appears in the working version but not in the not-working version. That's also how I found the right spot in the source code.

difference.log
2024-12-09T17:30:31.640 helix_lsp::transport [INFO] rust-analyzer -> {"jsonrpc":"2.0","method":"workspace/didChangeWorkspaceFolders","params":{"event":{"added":[{"name":"poc-helix-workspace-lsp-roots","uri":"file:///home/hieronymusma/src/poc-helix-workspace-lsp-roots"}],"removed":[]}}}

Platform

Linux

Terminal Emulator

GNOME Terminal 3.52.0 using VTE 0.76.0 +BIDI +GNUTLS +ICU +SYSTEMD

Installation Method

snap

Helix Version

helix 24.7 (079f544)

@sysheap sysheap added the C-bug Category: This is a bug label Dec 9, 2024
@sysheap
Copy link
Contributor Author

sysheap commented Dec 15, 2024

Nobody interested in merging this?

@TornaxO7
Copy link
Contributor

I would rather think that the maintainers are quite busy at the moment, so don't worry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants