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

Error/Warnings on save take a long time to disappear on crates with big dependents

Open Strackeror opened this issue 1 year ago • 6 comments

So, in the project I'm working on, we have a big workspace with a graph of about a dozen crates. Generally this works fine, but I've realized that sometimes the inline errors/warnings take a lot longer to appear than others.

Investigating that, I realized this is mostly a problem in the crates that are depended on by a lot of other crates, and especially in the hodgepodge 'utils' crate that literally every other crate in the workspace uses. I also realized that, weirdly, while errors appear very quickly after saving a file in the utils crate, they take a lot longer to disappear.

What seems to be happening is that when you save a file, errors/warnings appear progressively, or as soon as the file's crate is checked. But the full graph of dependent crates needs to be checked for the errors to disappear.

I made a minimal way to reproduce this here, where we have a main crate that artificially takes long to compile (with a proc macro that waits for 10s). that depends on a 'dep' crate. example.zip Editing the 'dep' create has that weird dev experience where errors appear super quick, but you need 10 full seconds to confirm they are gone. I've tried in both vscode and helix to confirm this wasn't specific to either.

Is this a bug ? Is there a workaround ?

Strackeror avatar Oct 11 '24 09:10 Strackeror

I expect those to be cargo check errors, not r-a errors, and the process finishes only after cargo finishes checking the entire workspace, and only after that r-a can retrieve the diagnostics.

Therefore, there is not much we can do here. Perhaps we can spawn a cargo process for every crate instead of one for the entire workspace, but that both will be slower and may interfere with cargo's caching.

ChayimFriedman2 avatar Oct 11 '24 11:10 ChayimFriedman2

Yeah, they're definitely cargo check errors. Warnings are definitely showing up before the full check is done, when warnings are new, so it looks like there's a bit more to it than that, but I don't know the codebase.

I wonder if there could be an option to sometimes just... not check dependents ? When I'm looking at a file, the full workspace check is just not relevant at all to me, I just want errors for my file. When I run cargo check -p dep on the example project, it's evidently not taking the 10s for checking the root crate, and I do get the warnings for the file i'm working on.

Strackeror avatar Oct 11 '24 11:10 Strackeror

When there are new errors cargo reports them immediately, but r-a don't remove new errors until the process exited and it knows for sure there won't be new errors. This is intended to improve experience - if we remove errors then add them back it will be a blink.

Maybe Cargo can report somehow it finishes checking a crate, but that's a change on Cargo's side.

I wonder if there could be an option to sometimes just... not check dependents ?

This already exists: it is called "rust-analyzer.check.workspace": false.

ChayimFriedman2 avatar Oct 11 '24 12:10 ChayimFriedman2

Okay, so, "rust-analyzer.check.workspace": false doesn't work in this case, or there's a problem with the option on my end, because the issue is exactly the same with the config key set.

I've also been exploring the code in flycheck.rs and main_loop.rs, and yeah I see why the issue is arising. The good news is, cargo check and RA do report when they're done with a crate. I think it should be possible to make use of that to clear up diagnostics in a more granular way, I'll try and see if I can make a PR for the improvement myself ! Is there any issue with that ?

Strackeror avatar Oct 11 '24 13:10 Strackeror

I've also been exploring the code in flycheck.rs and main_loop.rs, and yeah I see why the issue is arising. The good news is, cargo check and RA do report when they're done with a crate. I think it should be possible to make use of that to clear up diagnostics in a more granular way, I'll try and see if I can make a PR for the improvement myself ! Is there any issue with that ?

I think there's a lot of low-hanging potential improvements to the flycheck crate, but there also some subtle interactions as well. It might be a somewhat in-depth review, if you're okay with that!

davidbarsky avatar Oct 11 '24 13:10 davidbarsky

I think there's a lot of low-hanging potential improvements to the flycheck crate, but there also some subtle interactions as well. It might be a somewhat in-depth review, if you're okay with that!

I see no problem with that ! It doesn't look like a huge change, but it's definitely not a trivial one either.

Strackeror avatar Oct 11 '24 14:10 Strackeror

Closed by https://github.com/rust-lang/rust-analyzer/pull/18729 (thanks to @Strackeror for driving this forward with their initial PR though!)

Veykril avatar Dec 20 '24 14:12 Veykril

Which release of RA should contain the PR that closed this issue? I am still seeing errors stick around, but I am not sure if that's because of some bad interaction with custom build commands / the rustc workspace, or because I simply don't have this update yet.

RalfJung avatar Jan 02 '25 19:01 RalfJung

Yep I made a mistake, we still batch the clear commands til the end https://github.com/rust-lang/rust-analyzer/pull/18826 Verified it that it actually works as intended now

Veykril avatar Jan 03 '25 12:01 Veykril