fix: run flycheck without rev_deps when target is specified #17912
Conversation
rustbot
added
the
S-waiting-on-review
alibektas
changed the title
|
It has been mentioned that this particular case is not only limited to binary targets. I would be happy to add also other cases. |
There was a problem hiding this comment.
Okay two things, I think I was wrong with this requiring a queued task as we have already been doing all the work within the spawned task (and can still continue to do so), so we can undo that part. The other thing is that when we do have the binary case here, we should not only skip the reverse dependency stuff, we should also tell flycheck to pass --bin <bin-name> for its check, that will require some more changes in the flycheck restart logic
alibektas
force-pushed
the
cargo_check_on_binary
branch
from
9fbff41 to
ffc3bfe
Compare
crates/rust-analyzer/src/flycheck.rs
Outdated
| @@ -396,6 +400,10 @@ impl FlycheckActor { | |||
| None => cmd.arg("--workspace"), | |||
| }; | |||
|
|
|||
| if let Some(tgt) = bin_target { | |||
| cmd.arg("--bin").arg(tgt); | |||
There was a problem hiding this comment.
We probably should also pass the package here (non-optional). Two packages in a workspace can have the a binary with the same name. In that case, Cargo checks both of them (tested it).
There was a problem hiding this comment.
I think its fine to support either, the package is passed above if a package was set (which should be usually the case). Or does rustlings name all binaries the same? (I hope not)
There was a problem hiding this comment.
Not for Rustlings. But if we extend this to tests instead of only bins, a workspace could have multiple packages with the test integration_tests for example. I don't see why rust-analyzer should check all of them if only one changes.
There was a problem hiding this comment.
Fair, I'd still leave the logic as is though and just change the restart callers as this invocation isn't technically invalid.
| // Trigger flychecks for the only workspace which the binary crate belongs to | ||
| world.analysis.crates_for(file_id)?.into_iter().unique().collect::<Vec<_>>() |
There was a problem hiding this comment.
We should already have that information in the TargetSpec, it should contain the relevant crate id
There was a problem hiding this comment.
this is the only bit I couldn't really come up with a solution to because the package information for RustProjectJson is not really clear.
There was a problem hiding this comment.
No worries, #17946 the data was there but not exposed that's all
alibektas
changed the title
|
@bors r+ |
|
☀️ Test successful - checks-actions |
Since querying for a crate's target is a call to salsa and therefore blocking, flycheck task is now deferred out of main thread by using
GlobalStatesdeferred_task_queue. Fixes #17829 and rust-lang/rustlings#2071