rust-analyzer icon indicating copy to clipboard operation
rust-analyzer copied to clipboard

rust-analyzer.check.workspace = false is ignored on server startup

Open BaxHugh opened this issue 1 year ago • 13 comments

"When rust-analyzer.check.workspace" : false is set I expect that cargo check should only be run on the rust crate which I have open in vs code. However, rust-analyzer runs cargo check across all workspace members [[edit] when the server is first started. After which saving a file shows that check is only run on the crate which is opened.] This is a problem as I am working in a mono repo which has many member crates: when I'm working in one, where VS Code is only open at the member crate's manifest directory, the rust-analyzer is slow [[edit] to start] as it is unnecessarily checking all the workspace crates.

I have reproduced this problem in a simple toy workspace.

rust-analyzer version: (eg. output of "rust-analyzer: Show RA Version" command, accessible in VSCode via Ctrl/⌘+Shift+P) rust-analyzer 0.3.2112-standalone

rustc version: (eg. output of rustc -V) rustc 1.77.0 (aedd173a2 2024-03-17)

editor or extension: (eg. VSCode, Vim, Emacs, etc. For VSCode users, specify your extension version; for users of other editors, provide the distribution if applicable) VSCode version 1.93.1 38c31bc77e0dd6ae88a4e9cc93428cc27a56ba40 x64 rust-analyzer extension: v0.3.2112

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME) in .vscode/settings.json for the workspace (user level settings.json file removed) "rust-analyzer.check.workspace": false,

code example to reproduce: To reproduce, please see the zipped demo project: ra-workspace-issue-demo.zip

This has the structure

./
├── bar
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
├── Cargo.lock
├── Cargo.toml
└── foo
    ├── Cargo.toml
    └── src
        └── lib.rs

i.e. a Cargo workspace with two workspace.member crates which do not depend on each other. In both lib.rs files, I have called a non existent function such that cargo check reports an error.

If I open, say, the foo crate: code ./foo in the problems panel, I can clearly see that rust-analyzer has reported problems in the foo crate which is not open in VS Code. (similarly I can confirm by introducing a build.rs file with panics, and the rust-analyzer output shows the error when it has built the crate it should not be running check on).

Image Screenshot showing that problems from both the foo and bar crate are being reported, despite only foo being open

[edit] Image Screenshot showing that after a save, only problems from the open crate are reported

BaxHugh avatar Sep 18 '24 17:09 BaxHugh

I cannot reproduce. Initially rust-analyzer indeed checks all packages, but when editing and saving it checks only one.

ChayimFriedman2 avatar Sep 18 '24 19:09 ChayimFriedman2

Thanks for the feedback @ChayimFriedman2. When you say initially rust-analyzer checks all packages. Do you agree that, that is not the desired behaviour? check.workspace = false specifies that the --workspace flag is not passed, but instead -p=crate is. If the -p argument is passed all along, then cargo check should never be run on anything but the single crate which is opened. That is how I understand it.

BaxHugh avatar Sep 18 '24 22:09 BaxHugh

@ChayimFriedman2 I can confirm that I see the same behaviour as you. The screenshot showing both problems is after first restarting the RA server. Once I save a file, the problems listed reduce to just the crate that is open.

I'll update the original post to reflect this.

However, RA checking the whole workspace on startup is still a problem for me, as there are a lot of crates and deps in the workspace, and starting the server takes a long time, after opening the project in VSCode.

BaxHugh avatar Sep 19 '24 08:09 BaxHugh

I would recommend using rust-analyzer.linkedProjects

alibektas avatar Oct 10 '24 20:10 alibektas

Recently, I'm also observing issues with rust-analyzer.check.workspace" : false. It just checks the current crate and all the dependent crates on it. In very deep crates, it takes quite some time to check things on each save. Before, it was just changing the current crate. Has something changed recently in the behavior of this setting?

afroozeh avatar Oct 10 '24 21:10 afroozeh

@afroozeh Yes, we did it exactly like you describe. See #17912

In very deep crates, it takes quite some time to check things on each save

If this wasn't the case before, then it indicates that we did something wrong.

alibektas avatar Oct 10 '24 21:10 alibektas

@alibektas I can confirm that it was not the case before, editing/save in that deep crate was immediate when rust-analyzer.check.workspace was false. Now, it seems like true or false value is meaningless because you're just checking all the dependent crates. And it has really degraded my developer experience using ~~RustRover~~ RustAnalyzer: each save takes about 5-10 seconds... Before I was invoking cargo check for the whole workspace on demand. I think, it would be good to have the option with these values instead of just true and false: "all", "dependent_crates", "current_crate"

Update: RustRover was a typo here, I meant RustAnalyzer.

afroozeh avatar Oct 10 '24 21:10 afroozeh

You may want to make sure that you have your rust-analyzer up-to-date. There was previously an issue with which crates we ran our flycheck on but #18197 hopefully resolved. If your rust-analyzer is indeed a more recent version, I will check if there are other cases where this issue persists but I highly doubt that.

alibektas avatar Oct 10 '24 22:10 alibektas

@alibektas my RustAnalyzer is up to date. if you're now running cargo check on the current crate and all dependent crates, it's going to be slow for live editing for very deep crates in the dependency graph. I want an option that runs cargo check just for a single crate, i.e., cargo check -p <current_crate>. Again to double check, Is this the case when rust-analyzer.check.workspace is false? Or false means check the current crate and all dependent crates?

afroozeh avatar Oct 10 '24 22:10 afroozeh

@alibektas my RustAnalyzer is up to date. if you're now running cargo check on the current crate and all dependent crates, it's going to be slow for live editing for very deep crates in the dependency graph. I want an option that runs cargo check just for a single crate, i.e., cargo check -p <current_crate>.

Let me get back to you with this in a few days. I am rewriting most of the cargo check logic.

Again to double check, Is this the case when rust-analyzer.check.workspace is false? Or false means check the current crate and all dependent crates?

true : Add --workspace to cargo check false : Add -p <package_name>

And pardon my ignorance but I thought that RustRover uses IntellijRust or is it now possible to use both of them?

alibektas avatar Oct 10 '24 23:10 alibektas

And pardon my ignorance but I thought that RustRover uses IntellijRust or is it now possible to use both of them?

Not without some active work: while it is possible to configure a classic, non-Fleet JetBrains IDE to use an arbitrary LSP server, I'm not sure why folks would want to point RustRover at rust-analyzer.

davidbarsky avatar Oct 11 '24 16:10 davidbarsky

I'm sorry, I meant RustAnalyzer, there is no RustRover, it was a typo 😅 (I updated the comment to avoid confusing for other readers)

afroozeh avatar Oct 11 '24 16:10 afroozeh

Let me get back to you with this in a few days. I am rewriting most of the cargo check logic.

alibektas Thank you. Please also make the semantic of the config also clear. As I said, it seems like when the flag is false, it checks the current crate and all dependent ones.

afroozeh avatar Oct 11 '24 16:10 afroozeh

@alibektas is there any update on this issue?

afroozeh avatar Nov 06 '24 12:11 afroozeh

@afroozeh hey sorry for the late reply. Honestly, I totally forgot that I wanted to resolve this issue but now the team lead is away and I definitely want to check with him first before I do it because something doesn't seem right. Sorry for the inconvenience! I made a separate issue you can check it out the updates

alibektas avatar Nov 12 '24 16:11 alibektas

I am not sure if this is related, but I have the following in my settings.json:

{
  "rust-analyzer.check.workspace": false
}

When I save a file, in a large project (e.g. packages/package-a/main.rs), it takes a few minutes to run the check.

However, if I run the following in a terminal for that same project, it takes one second or less:

cargo check -p package-a

TheColorRed avatar Nov 26 '24 16:11 TheColorRed

Did you run the commands one after the other?

ChayimFriedman2 avatar Nov 26 '24 16:11 ChayimFriedman2

I did it in different tests:

  1. Save same file two times in a row (both times ran 1+ minutes)
  2. Run save (ran 1+ minutes) then run in terminal (ran under 1 second)
  3. Run save in the terminal two times in a row (both ran under 1 second)

So, for me, no matter how many times I run it via file save it takes lots of time as if it is running caro check --workspace instead of cargo check -p ...

TheColorRed avatar Nov 26 '24 16:11 TheColorRed

The current "rust-analyzer.check.workspace": false is broken in the sense that it does not do cargo check -p. To me it seems like it checks the current package and all dependent ones, and that's why it takes a long time in a large project. This was working fine a couple of months ago, so something has changed. @alibektas Is there any plan to fix this? You created a new issue but that seems to be just there without any activity.

afroozeh avatar Nov 26 '24 16:11 afroozeh

I am waiting for the project lead to be online again which will happen either tomorrow or the day after. After that it will be resolved in no time. Once again terribly sorry for the inconvenience.

alibektas avatar Nov 26 '24 16:11 alibektas

Ah it's probably https://github.com/rust-lang/rust-analyzer/pull/18197, you don't have a target set (like the default). @alibektas I get a headache every time I try to think if we should have && or ||, but are you sure && is the correct call?

ChayimFriedman2 avatar Nov 26 '24 16:11 ChayimFriedman2

Ah it's probably https://github.com/rust-lang/rust-analyzer/pull/18197, you don't have a target set (like the default). @alibektas I get a headache every time I try to think if we should have && or ||, but are you sure && is the correct call?

I was sure back then can't say I am sure now. Let me have a look again.

alibektas avatar Nov 26 '24 16:11 alibektas

Thank you! appreciate your work on this project!

Ah it's probably https://github.com/rust-lang/rust-analyzer/pull/18197, you don't have a target set (like the default).

How can I set a target and check if it works with that?

afroozeh avatar Nov 26 '24 16:11 afroozeh

@afroozeh Save a bin file (or test (integration)/example/bench).

ChayimFriedman2 avatar Nov 26 '24 17:11 ChayimFriedman2

They are all [lib] targets already, so not sure if that's related.

afroozeh avatar Nov 26 '24 17:11 afroozeh

@afroozeh Yeah, [lib] is where this doesn't work. Add a [bin] target and save it.

ChayimFriedman2 avatar Nov 26 '24 17:11 ChayimFriedman2

Given that this is not the same problem as OP's, I opened a new issue about this: #18562, please use it for discussions.

ChayimFriedman2 avatar Nov 26 '24 17:11 ChayimFriedman2

@ChayimFriedman2 I can't just add [bin] there... it's a large project with a lot of inner lib crates that depend on each other. I guess you already know what's the problem. It should not check the dependent crates, or you should introduce a separate config to just check the current crate.

afroozeh avatar Nov 26 '24 17:11 afroozeh

@afroozeh Please discuss this in the new issue.

ChayimFriedman2 avatar Nov 26 '24 17:11 ChayimFriedman2

The current "rust-analyzer.check.workspace": false is broken in the sense that it does not do cargo check -p. To me it seems like it checks the current package and all dependent ones, and that's why it takes a long time in a large project. This was working fine a couple of months ago, so something has changed. @alibektas Is there any plan to fix this? You created a new issue but that seems to be just there without any activity.

I still have the same issue on v0.3.2220. @afroozeh @TheColorRed Maybe someone also tried to update to the latest release and check this again?

mhnap avatar Dec 20 '24 13:12 mhnap