Skip to content

Commit

Permalink
Auto merge of rust-lang#17912 - alibektas:cargo_check_on_binary, r=Ve…
Browse files Browse the repository at this point in the history
…ykril

fix: run flycheck without rev_deps when target is specified

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 `GlobalState`s `deferred_task_queue`. Fixes rust-lang#17829  and rust-lang/rustlings#2071
  • Loading branch information
bors committed Aug 23, 2024
2 parents 1b0e158 + 0251cfa commit b88a4f0
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 22 deletions.
39 changes: 29 additions & 10 deletions src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ pub(crate) struct CargoOptions {
pub(crate) target_dir: Option<Utf8PathBuf>,
}

#[derive(Clone)]
pub(crate) enum Target {
Bin(String),
Example(String),
Benchmark(String),
Test(String),
}

impl CargoOptions {
pub(crate) fn apply_on_command(&self, cmd: &mut Command) {
for target in &self.target_triples {
Expand Down Expand Up @@ -119,13 +127,13 @@ impl FlycheckHandle {

/// Schedule a re-start of the cargo check worker to do a workspace wide check.
pub(crate) fn restart_workspace(&self, saved_file: Option<AbsPathBuf>) {
self.sender.send(StateChange::Restart { package: None, saved_file }).unwrap();
self.sender.send(StateChange::Restart { package: None, saved_file, target: None }).unwrap();
}

/// Schedule a re-start of the cargo check worker to do a package wide check.
pub(crate) fn restart_for_package(&self, package: String) {
pub(crate) fn restart_for_package(&self, package: String, target: Option<Target>) {
self.sender
.send(StateChange::Restart { package: Some(package), saved_file: None })
.send(StateChange::Restart { package: Some(package), saved_file: None, target })
.unwrap();
}

Expand Down Expand Up @@ -183,7 +191,7 @@ pub(crate) enum Progress {
}

enum StateChange {
Restart { package: Option<String>, saved_file: Option<AbsPathBuf> },
Restart { package: Option<String>, saved_file: Option<AbsPathBuf>, target: Option<Target> },
Cancel,
}

Expand Down Expand Up @@ -271,7 +279,7 @@ impl FlycheckActor {
tracing::debug!(flycheck_id = self.id, "flycheck cancelled");
self.cancel_check_process();
}
Event::RequestStateChange(StateChange::Restart { package, saved_file }) => {
Event::RequestStateChange(StateChange::Restart { package, saved_file, target }) => {
// Cancel the previously spawned process
self.cancel_check_process();
while let Ok(restart) = inbox.recv_timeout(Duration::from_millis(50)) {
Expand All @@ -281,11 +289,12 @@ impl FlycheckActor {
}
}

let command =
match self.check_command(package.as_deref(), saved_file.as_deref()) {
Some(c) => c,
None => continue,
};
let Some(command) =
self.check_command(package.as_deref(), saved_file.as_deref(), target)
else {
continue;
};

let formatted_command = format!("{command:?}");

tracing::debug!(?command, "will restart flycheck");
Expand Down Expand Up @@ -381,6 +390,7 @@ impl FlycheckActor {
&self,
package: Option<&str>,
saved_file: Option<&AbsPath>,
target: Option<Target>,
) -> Option<Command> {
match &self.config {
FlycheckConfig::CargoCommand { command, options, ansi_color_output } => {
Expand All @@ -396,6 +406,15 @@ impl FlycheckActor {
None => cmd.arg("--workspace"),
};

if let Some(tgt) = target {
match tgt {
Target::Bin(tgt) => cmd.arg("--bin").arg(tgt),
Target::Example(tgt) => cmd.arg("--example").arg(tgt),
Target::Test(tgt) => cmd.arg("--test").arg(tgt),
Target::Benchmark(tgt) => cmd.arg("--bench").arg(tgt),
};
}

cmd.arg(if *ansi_color_output {
"--message-format=json-diagnostic-rendered-ansi"
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ use vfs::{AbsPathBuf, ChangeKind, VfsPath};

use crate::{
config::{Config, ConfigChange},
flycheck::Target,
global_state::{FetchWorkspaceRequest, GlobalState},
lsp::{from_proto, utils::apply_document_changes},
lsp_ext::{self, RunFlycheckParams},
mem_docs::DocumentData,
reload,
target_spec::TargetSpec,
};

pub(crate) fn handle_cancel(state: &mut GlobalState, params: CancelParams) -> anyhow::Result<()> {
Expand Down Expand Up @@ -287,16 +289,40 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
let world = state.snapshot();
let mut updated = false;
let task = move || -> std::result::Result<(), ide::Cancelled> {
// Trigger flychecks for all workspaces that depend on the saved file
// Crates containing or depending on the saved file
let crate_ids: Vec<_> = world
.analysis
.crates_for(file_id)?
.into_iter()
.flat_map(|id| world.analysis.transitive_rev_deps(id))
.flatten()
.unique()
.collect();
// Is the target binary? If so we let flycheck run only for the workspace that contains the crate.
let target = TargetSpec::for_file(&world, file_id)?.and_then(|x| {
let tgt_kind = x.target_kind();
let tgt_name = match x {
TargetSpec::Cargo(c) => c.target,
TargetSpec::ProjectJson(p) => p.label,
};

let tgt = match tgt_kind {
project_model::TargetKind::Bin => Target::Bin(tgt_name),
project_model::TargetKind::Example => Target::Example(tgt_name),
project_model::TargetKind::Test => Target::Test(tgt_name),
project_model::TargetKind::Bench => Target::Benchmark(tgt_name),
_ => return None,
};

Some(tgt)
});

let crate_ids = if target.is_some() {
// Trigger flychecks for the only workspace which the binary crate belongs to
world.analysis.crates_for(file_id)?.into_iter().unique().collect::<Vec<_>>()
} else {
// Trigger flychecks for all workspaces that depend on the saved file
// Crates containing or depending on the saved file
world
.analysis
.crates_for(file_id)?
.into_iter()
.flat_map(|id| world.analysis.transitive_rev_deps(id))
.flatten()
.unique()
.collect::<Vec<_>>()
};

let crate_root_paths: Vec<_> = crate_ids
.iter()
Expand Down Expand Up @@ -346,8 +372,10 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
for (id, package) in workspace_ids.clone() {
if id == flycheck.id() {
updated = true;
match package.filter(|_| !world.config.flycheck_workspace()) {
Some(package) => flycheck.restart_for_package(package),
match package
.filter(|_| !world.config.flycheck_workspace() || target.is_some())
{
Some(package) => flycheck.restart_for_package(package, target.clone()),
None => flycheck.restart_workspace(saved_file.clone()),
}
continue;
Expand Down

0 comments on commit b88a4f0

Please sign in to comment.