rust-analyzer: Seemingly incompatible design between rust-project.json and checkOnSave.overrideCommand

Hey all!

I have a couple of questions:

Is it expected that in a code base where with Cargo you would have many Cargo.toml files spread across multiple folders, you only use one rust-project.json?
Is there a way to give some info on what file we want to “cargo check” (i.e. what file triggered the invocation of checkOnSave.overrideCommand)? For example, passing it as an argument to checkOnSave.overrideCommand.

My personal investigation has reached “yes, you should have only one rust-project.json” for 1.:

in rust-project.json you reference deps with their index in the array for the same rust-project.json file, and I don’t know how this could be compatible with multiple rust-project.jsons
trying to run rust-analyzer with multiple rust-project.jsons doesn’t work - e.g. cross rust-project.json goto def doesn’t work

and “no” for 2.:

it seems that nothing dynamic is passed to the executable when it’s run (in the source code) - only the presupplied args and the environment from checkOnSave.extraEnv are passed, which both appear to be static

This combination is very unfortunate, because now, there is no way for the tool that’s going to be used instead of cargo check to know what it should be building - in a huge repo, this is not viable.

I assume that when we’re using cargo this “what should I build” issue is resolved by the fact that cargo check is run in the root of the workspace, which is determined by a Cargo.toml file. However, in the rust-project.json, this is no longer a solution, because we can have only one rust-project.json file, hence in order to be correct, the only thing checkOnSave.overrideCommand could do is build everything in the workspace.

My proposed solutions are to do one of

“fix” 2. so that we pass some info to checkOnSave.overrideCommand (seems to be much easier)
“fix” 1. to allow autodiscovery and combining of rust-project.json files - seems to be much more difficult to implement, but perhaps it’s a better approach in the long run, so that the assumptions made for cargo based projects are also valid for rust-project.json based ones?

I would be happy to implement some version of “fix” 2., if we could come to an agreement on what that would be.

Context: I’m trying to implement diagnostics support for rust-analyzer+rules_rust, which relies on checkOnSave.overrideCommand/cargo check.

About this issue

Original URL
State: open
Created 2 years ago
Comments: 24 (14 by maintainers)

Most upvoted comments

The agreement seems to be that the $saved_file thing might be accepted, as a stopgap solution, but there is no one working on that currently, afaik.

googleson78 on Mar 3, 2023

Does rust-analyzer not use some of the diagnostics that cargo/rustc report in order to drive things, e.g. if cargo/rustc reports something like “unknown identifier”, does rust-analyzer not then try to suggest an “import missing identifier” action?

Rustc actually creates these suggestions on it’s own. It exposes them in a machine readable way when using the json error format to allow other tools like cargo fix and rust-analyzer to apply them verbatim. AFAIK rust-analyzer never feeds error messages generated by rustc back into any analysis or generates it’s own suggestions based on rustc error messages.

bjorn3 on Jan 13, 2023

d) none of the above - run once in the root dir and also provide information about the file being checked (e.g via an interpolation var), so that my check command (in this case bazel) can somehow figure out what targets it needs to build/check In the terminology of the PR you have, this sounds like once_per_root+something gets interpolated.

This doesn’t really make sense though, what would r-a pass to bazel? If we only invoke the command once there is no dynamic data at play here, at which point you can just encode whatever your check command needs in the additional args.

Maybe I’m missing something about per workspace in the root with the manifest path if the variable was used but I only really want to check the current file that the user is looking at, which I can approximate by checking all the targets/crates that that file is a part of. I don’t want to run the check command for every “workspace” that is available.

Ah, so you want the file path of whatever file was saved. That would be something we could add as an interpolation var for checkOnSave. To clarify https://github.com/rust-lang/rust-analyzer/pull/13128 technically only adds two things (it’s independent from interpolation in a sense), per_workspace invocations and once_in_root. Usages of interpolated variables then may change additional things. So for this use case the idea would be for you to use once_in_root and an interpolation variable that expands to the file path being saved.

I guess the per workspace in the root with the manifest path if the variable was used option would work if we could somehow teach rust-analyzer about custom manifest files (in my case that would be BUILD files that include specific strings in them, e.g. rust_library, rust_binary or rust_test). But again, maybe I’m misunderstanding what a “manifest file” is supposed to be.

A manifest file would be the Cargo.toml of the relevant workspace, I don’t really know rust-project.json too well but given it is there to not be dependent on cargo I would imagine the term manifest to not mean anything there, so the var wouldn’t make sense for rust-project.json projects.

Thanks for clarifying these things, this is really helpful in figuring things out here 😃

Veykril on Oct 19, 2022