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

RLS linting reports incorrect/outdated information due to two separate processes #2239

Closed
matt961 opened this issue Jan 23, 2019 · 12 comments
Closed
Labels
bug LSP Any issue relating to LSP or tsserver

Comments

@matt961
Copy link

matt961 commented Jan 23, 2019

Information

VIM version

NVIM v0.3.1
Build type: Release

Operating System: Linux 4.4.0-17763-Microsoft # 253-Microsoft Mon Dec 31 17:49:00 PST 2018 x86_64 x86_64 x86_64 GNU/Linux

What went wrong

When working in a cargo workspace with two packages and editing files from both, adding a function to one package that is a lib, and then trying to get code completion for it in the main function of the other package, it reports that no function is found. Using ALE to stop the language servers and then editing the bad-lint reporting file lets the out-of-date RLS catch up to the most recent state of the other package that it isn't monitoring. Since I don't think its watching the other package's directory, it doesn't notice the file change and doesn't incrementally compile.

I'm just about certain that this is why it is printing out of date lints since this works fine with LanguageClient-neovim and I could only find one rls PID from the output of ps aux vs when using ALE I could find two separate PIDs.

Reproducing the bug

  1. Install ALE
  2. Set the toolchain used to stable
  3. Follow https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html all from the same nvim, so I'm editing the two packages in one instance of nvim
  4. When trying to use add_one::add_one; in the adder package, RLS throws an error saying that the function doesn't exist, but it resolves just fine when invoking cargo build.

:ALEInfo

Command History:
(started) ['/bin/bash', '-c', '''rls'' +''stable''']
(started) ['/bin/bash', '-c', '''rls'' +''stable''']

uhh ohh ^ two separate processes

ps aux when using ALE

*****     6632  0.6  1.3 436376 112516 ?       Ssl  23:20   0:03 /home/*****/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rls
*****     6784  0.3  0.0  19192  7852 pts/0    Ss   23:29   0:00 /bin/bash
*****     7085  8.2  1.2 308240 102800 ?       Ssl  23:30   0:01 /home/*****/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rls

'ps aux' when using LanguageClient-neovim

*****     7162  0.0  0.0  33544  3204 ?        Ssl  23:33   0:00 /home/*****/.vim/bundle/LanguageClient-neovim/bin/languageclient
*****     7164  0.8  0.1  44920 16488 ?        Ss   23:33   0:00 python3 -c import sys; sys.path.remove(""); import neovim; neovim.start_host() /home/*****/.vim/bundle/deoplete.nvim/rplugin/python3/deoplete
*****     7168  6.0  1.8 494232 151136 ?       Sl   23:33   0:02 /home/*****/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rls
@andreypopp
Copy link
Contributor

andreypopp commented Jan 23, 2019

Could be that ALE doesn't handle that case correctly and spawns two RLS instances.

This is the code which looks for the project root for RLS: https://github.com/w0rp/ale/blob/master/ale_linters/rust/rls.vim#L13-L17

As you can see it looks for Cargo.toml. In you case do you have multiple Cargo.toml — one for main and one for lib?

@andreypopp
Copy link
Contributor

Maybe the better heuristics would be to look for Cargo.lock.

But the correct fix would be to make ALE to use cargo to fid the workspace root (does cargo has this feature?).

@matt961
Copy link
Author

matt961 commented Jan 23, 2019

It doesn't appear to have a command for that unfortunately. I like your idea of looking for cargo.lock though... Pretty sure all workspaces must share the same lock file between it's packages.

@andreypopp
Copy link
Contributor

@matt961 could try to fix it and send a PR if fix is working?

@andreypopp
Copy link
Contributor

It doesn't appear to have a command for that unfortunately.

I'd suggest to make a feature request to cargo to add this or even ask them to provide cargo lsp command which would run configured LSP automatically.

@matt961
Copy link
Author

matt961 commented Jan 23, 2019

Tomorrow I will try a fix. Probably the cargo team would be open to a working dir command.

@busimus
Copy link

busimus commented Jan 23, 2019

I was about to post exactly the same bug report.
Changing this line to instead look for Cargo.lock does indeed fix the problem. I don't think it's the right solution, but it works for now.

@w0rp
Copy link
Member

w0rp commented Jan 23, 2019

Looking for Cargo.lock seems like the right solution.

@w0rp w0rp added bug LSP Any issue relating to LSP or tsserver labels Jan 23, 2019
@andreypopp
Copy link
Contributor

See rust-lang/cargo#4933, new cargo returns root in cargo metadata output.

Looking for Cargo.lock seems like the right solution.

It doesn't exist all the time probably, but a good fallback if cargo metadata can't return root.

@w0rp
Copy link
Member

w0rp commented Jan 23, 2019

The ever unstable Rust.

@matt961
Copy link
Author

matt961 commented Jan 23, 2019

hmm must have been looking at an outdated manual, didn't know about the metadata command. Someone else should do a fix if we're not using Cargo.lock, I'm not vimscript savvy.

By the looks of it, the cargo team won't change this functionality without good reason, it's been in place since rust 1.24 which is I believe over a year old.

@matt961
Copy link
Author

matt961 commented May 4, 2022

Closing because 3 years later, rust-analyzer is now the de facto language server for Rust 😉

@matt961 matt961 closed this as completed May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug LSP Any issue relating to LSP or tsserver
Projects
None yet
Development

No branches or pull requests

4 participants