Support flychecking only the current crate

[P1n3appl3]

The issue with checking all crates

Currently when rust-analyzer runs a flycheck it always runs on the entire workspace. In large workspaces (or in my case a large rust-project.json project) running a check build on many crates can significantly increase the time it takes to get results for the current crate. This is especially an issue in build systems that don't support cargo's functionality of streaming diagnostics over stdout as it goes, so if 1 crate in the workspace takes on the order of minutes to run a check build and the rest take milliseconds, you end up having to wait minutes every time a transitive dependency of that long-check-time crate changes to get diagnostics in your editor. Even if you do have streaming diagnostics, not knowing which crate has been modified means the build system can't schedule the check builds to prioritize the one you're actually editing, which causes the same problem of waiting for lots of other crates to check and harming your time-to-first-diagnostic.

Potential solution

As I understand it rust-analyzer has the required info (the path of the modified file) to only run a check build on the single crate that changed. In the case of a cargo command it could do this automatically by mapping the changed file to the changed crate and passing -p crate_name rather than --workspace to the flycheck subcommand invocation. For rust-project.json based build systems, if checkOnSave.overrideCommand were to expose some sort of variable expansions like $crate_name, $source_root, and/or $changed_file then it would be possible for GN/Bazel/Buck/etc. to use that info to only perform a check build on the modified crate.

Example: fuchsia's codebase

In fuchsia's codebase (where we use rust-project.json with GN) we already have support for this "map $changed_file to the changed target and only run a check build on that crate" functionality, but it can't be used from the editor because there's no way for rust-analyzer to pass the path of the modified file through to our check build wrapper script. Passing that info along makes the difference between "check on save" being viable or not for our developers due to the latency of checking potentially thousands of crates (including outliers that take minutes to check) vs just one.

Config/UI concerns

I realize that the vast majority of cargo workspaces aren't large enough that this is an issue, and it's possible that people have come to expect seeing errors in downstream crates from the same workspace when editing a dependency, so I think it'd make sense to still default to the "check all crates" behavior. For huge workspaces and monorepos like ours however, getting diagnostics on save just for the current crate, and having the ability to manually trigger workspace-wide checks in the editor (#12870) would be a preferable workflow.

The least intrusive way I can think of to add the functionality would be to introduce a boolean flag like checkOnSave.onlyCurrentCrate which makes cargo based flychecks automatically use -p crate_name instead of --workspace, and enables the expansion of those variables for checkOnSave.overrideCommand. Alternatively, if we're pretty confident that noone's overrideCommand contains the strings that we choose for variable expansion, we could just enable the expansion by default in that case.

I'd love some guidance on whether this is actually a good idea or if there are issues with trying to unambiguously map source files to crate roots/names, or other issues with how LSP diagnostics requests work. Also if there are complications with getting cargo commands to work with this functionality I'd be fine putting off that work and just implementing support for rust-project.json + custom flycheck commands, both because that's what we'd need for fuchsia and because rust-project.json based projects tend to be more monorepo-y so they'd care about this feature more.

I also haven't considered the non-flycheck diagnostics that rust-analyzer provides itself. Do those checks also currently run on the entire workspace (and if so is this affected by cachePriming.enable)? Would it make sense to tie the two such that checkOnSave.onlyCurrentCrate applies to both the flycheck and native rust-analyzer diagnostics?

[Veykril]

Having this sounds reasonable, I assume a config setting makes the most sense here yes. overrideCommand currently completely overrides things though, effectively ignoring all the other settings. We might want to look into being able to combine the various flags with the overrideCommand somehow, though that feels like a separate task.

That said, this shouldn't be too difficult to implement. We can easily map a file to the crates it belongs to, note the fact that a file can belong to multiple crates, not just one.

The native rust-analyzer diagnostics only run in the opened files I believe (or rather for those files that the client asks diagnostics for I think).

On another note, the main problem with only checking the one crate is that crates that depend on that crate won't get checked and might contain diagnostics, maybe it also makes sense to add another option (or combine them really) to check a crate and all its dependents.

[jhgg]

On another note, the main problem with only checking the one crate is that crates that depend on that crate won't get checked and might contain diagnostics, maybe it also makes sense to add another option (or combine them really) to check a crate and all its dependents.

I wonder if it's possible to split this into 2 cargo check invocations. The first one checking current crate for more responsive diagnostics, and once that completes doing a cargo check for dependent crates.

[jhgg]

Would be good to get some concrete guidance on this. I might want to try to potentially tackle this over the weekend. At work, we have a rather large workspace that's growing by the day, and this would significantly improve developer experience in terms of being able to save + see errors as fast as possible.

I think my plan might be to split the check into 2 phases:

1. Any crates that the file belongs to directly will be invoked w/ cargo check, diagnostics will be published immediately to the user after this phase completes.
2. If phase 1 is not successful, phase 2 is not initiated, as we cannot check dependencies when the dependent crates are broken. But, if phase 1 does succeed, we will re-run cargo check with the list of dependent crates to ensure they did not break. Diagnostics would then be updated to provide more errors if they exist.

---

There are however some hairy bits like, what if 2 files from 2 different crates change? For example, if I was to run a "rename function" refactor that might touch many crates, then hit "save all files" in the editor. I am unsure what to do about this... we could potentially track "dirty" files between cargo checks, and even involve a small delay incase many saves are coming in at once, in order to expand the crates checked in phases 1 & 2. Additionally, we could track "dirty" crates between when the check started, and when it finished, to see if we need to re-check because a save happening during phase 1 or 2 may have caused the result to be invalid.

[Veykril]

#12808 might be helpful, since that changed flychecking all workspaces to flychecking workspace whose files have changed.

I'd say if more than one crate (for a given workspace) has changed we should just fallback to checking that entire workspace. Doing more complex stuff there doesn't seem to be too beneficial to me.

Note that this feature will probably require a few more invasive refactors than #12808, since flycheck currently assumes per workspace checking to occur only.

[jhgg]

I'd say if more than one crate (for a given workspace) has changed we should just fallback to checking that entire workspace. Doing more complex stuff there doesn't seem to be too beneficial to me.

Yeah that is fair. Falling back to the more broad and slow for the rare case makes sense.

[P1n3appl3]

I've begun to work on implementation here, and put up a WIP PR with the initial changes: #12994

I'm working on the rust-project.json path (custom flycheck overrideCommand) first because that's what I use, and in that case I think it'd make sense for the "single crate flycheck" command to be an entirely different config setting. We could leave it as one command and expand variables to some special value for the full worksapce checks, but that seems pretty unwieldy so I'd only go with that option if adding the extra config is deemed unacceptable. For cargo it's probably possible to use one user provided cargo command + extra args and fill in the necessary -p <name> or --workspace internally.

@Veykril you brought up that we could potentially check the downstream deps from the changed file as well, and i think that's a good idea for cargo, but i'm less sure of whether it's a good idea for rust-project.json. For that to work you'd want the overrideCommand to support some sort of variable-length argument expansion like ./my-check-script $list-of-crates, and it may make more sense to leave deriving that list up to the specific build system that's being used to run the checkbuild anyways.

@jhgg if you were planning on implementing this as well feel free to email me to coordinate/pair so we don't duplicate effort. You seem interested in the cargo path so I'd be happy to just get the rust-project.json path working and let you handle cargo.

[jhgg]

@P1n3appl3 i was going to but I didn't start much yet! Glad to see your progress on this! I'll leave it to you!

[alper]

Is this why my code is blocked 10 seconds every time I save something? For me this is a recent development. I'm pretty sure it wasn't always so.

[bjorn3]

This happens asynchronously. Unless your system is overloaded, rust-analyzer running cargo check in the background will never block anything in the editor.