Skip to content

fix: Don't trigger two flychecks when saving files that are part of targets #20635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 14, 2025
Merged

fix: Don't trigger two flychecks when saving files that are part of targets #20635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b394f5d
Skip flycheck for workspace if it is already being checked
iorizu Sep 8, 2025
8497cba
Minor refactoring
iorizu Sep 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/rust-analyzer/src/flycheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ pub(crate) enum FlycheckConfig {
}

impl FlycheckConfig {
pub(crate) fn invocation_strategy_once(&self) -> bool {
pub(crate) fn invocation_strategy(&self) -> InvocationStrategy {
match self {
FlycheckConfig::CargoCommand { .. } => false,
FlycheckConfig::CargoCommand { .. } => InvocationStrategy::PerWorkspace,
FlycheckConfig::CustomCommand { invocation_strategy, .. } => {
*invocation_strategy == InvocationStrategy::Once
invocation_strategy.clone()
}
}
}
Expand Down Expand Up @@ -529,7 +529,7 @@ impl FlycheckActor {
if let Some(command_handle) = self.command_handle.take() {
tracing::debug!(
command = ?command_handle,
"did cancel flycheck"
"did cancel flycheck"
);
command_handle.cancel();
self.command_receiver.take();
Expand Down
275 changes: 159 additions & 116 deletions crates/rust-analyzer/src/handlers/notification.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
//! This module is responsible for implementing handlers for Language Server
//! Protocol. This module specifically handles notifications.

use std::ops::{Deref, Not as _};
use std::{
ops::{Deref, Not as _},
panic::UnwindSafe,
};

use ide_db::base_db::salsa::Cancelled;
use itertools::Itertools;
use lsp_types::{
CancelParams, DidChangeConfigurationParams, DidChangeTextDocumentParams,
Expand All @@ -16,7 +18,7 @@ use vfs::{AbsPathBuf, ChangeKind, VfsPath};

use crate::{
config::{Config, ConfigChange},
flycheck::Target,
flycheck::{InvocationStrategy, Target},
global_state::{FetchWorkspaceRequest, GlobalState},
lsp::{from_proto, utils::apply_document_changes},
lsp_ext::{self, RunFlycheckParams},
Expand Down Expand Up @@ -301,124 +303,165 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
let file_id = state.vfs.read().0.file_id(&vfs_path);
if let Some((file_id, vfs::FileExcluded::No)) = file_id {
let world = state.snapshot();
let invocation_strategy_once = state.config.flycheck(None).invocation_strategy_once();
let invocation_strategy = state.config.flycheck(None).invocation_strategy();
let may_flycheck_workspace = state.config.flycheck_workspace(None);
let mut updated = false;
let task = move || -> std::result::Result<(), Cancelled> {
if invocation_strategy_once {
let saved_file = vfs_path.as_path().map(|p| p.to_owned());
world.flycheck[0].restart_workspace(saved_file);
}

let target = TargetSpec::for_file(&world, file_id)?.and_then(|it| {
let tgt_kind = it.target_kind();
let (tgt_name, root, package) = match it {
TargetSpec::Cargo(c) => (c.target, c.workspace_root, c.package),
_ => return None,
};

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 Some((None, root, package)),
};

Some((Some(tgt), root, package))
});
tracing::debug!(?target, "flycheck target");
// we have a specific non-library target, attempt to only check that target, nothing
// else will be affected
if let Some((target, root, package)) = target {
// trigger a package check if we have a non-library target as that can't affect
// anything else in the workspace OR if we're not allowed to check the workspace as
// the user opted into package checks then
let package_check_allowed = target.is_some() || !may_flycheck_workspace;
if package_check_allowed {
let workspace = world.workspaces.iter().position(|ws| match &ws.kind {
project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
| project_model::ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _, _)),
..
} => *cargo.workspace_root() == root,
_ => false,
});
if let Some(idx) = workspace {
world.flycheck[idx].restart_for_package(package, target);
}
let task: Box<dyn FnOnce() -> ide::Cancellable<()> + Send + UnwindSafe> =
match invocation_strategy {
InvocationStrategy::Once => {
Box::new(move || {
// FIXME: Because triomphe::Arc's auto UnwindSafe impl requires that the inner type
// be UnwindSafe, and FlycheckHandle is not UnwindSafe, `word.flycheck` cannot
// be captured directly. std::sync::Arc has an UnwindSafe impl that only requires
// that the inner type be RefUnwindSafe, so if we were using that one we wouldn't
// have this problem. Remove the line below when triomphe::Arc has an UnwindSafe impl
// like std::sync::Arc's.
let world = world;
stdx::always!(
world.flycheck.len() == 1,
"should have exactly one flycheck handle when invocation strategy is once"
);
let saved_file = vfs_path.as_path().map(ToOwned::to_owned);
world.flycheck[0].restart_workspace(saved_file);
Ok(())
})
}
}

if !may_flycheck_workspace {
return Ok(());
}

// Trigger flychecks for all workspaces that depend on the saved file
// Crates containing or depending on the saved file
let crate_ids = world
.analysis
.crates_for(file_id)?
.into_iter()
.flat_map(|id| world.analysis.transitive_rev_deps(id))
.flatten()
.unique()
.collect::<Vec<_>>();
tracing::debug!(?crate_ids, "flycheck crate ids");
let crate_root_paths: Vec<_> = crate_ids
.iter()
.filter_map(|&crate_id| {
world
.analysis
.crate_root(crate_id)
.map(|file_id| {
world.file_id_to_file_path(file_id).as_path().map(ToOwned::to_owned)
})
.transpose()
})
.collect::<ide::Cancellable<_>>()?;
let crate_root_paths: Vec<_> = crate_root_paths.iter().map(Deref::deref).collect();
tracing::debug!(?crate_root_paths, "flycheck crate roots");

// Find all workspaces that have at least one target containing the saved file
let workspace_ids =
world.workspaces.iter().enumerate().filter(|(_, ws)| match &ws.kind {
project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
| project_model::ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _, _)),
..
} => cargo.packages().any(|pkg| {
cargo[pkg]
.targets
InvocationStrategy::PerWorkspace => {
Box::new(move || {
let target = TargetSpec::for_file(&world, file_id)?.and_then(|it| {
let tgt_kind = it.target_kind();
let (tgt_name, root, package) = match it {
TargetSpec::Cargo(c) => (c.target, c.workspace_root, c.package),
_ => return None,
};

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 Some((None, root, package)),
};

Some((Some(tgt), root, package))
});
tracing::debug!(?target, "flycheck target");
// we have a specific non-library target, attempt to only check that target, nothing
// else will be affected
let mut package_workspace_idx = None;
if let Some((target, root, package)) = target {
// trigger a package check if we have a non-library target as that can't affect
// anything else in the workspace OR if we're not allowed to check the workspace as
// the user opted into package checks then
let package_check_allowed = target.is_some() || !may_flycheck_workspace;
if package_check_allowed {
package_workspace_idx =
world.workspaces.iter().position(|ws| match &ws.kind {
project_model::ProjectWorkspaceKind::Cargo {
cargo,
..
}
| project_model::ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _, _)),
..
} => *cargo.workspace_root() == root,
_ => false,
});
if let Some(idx) = package_workspace_idx {
world.flycheck[idx].restart_for_package(package, target);
}
}
}

if !may_flycheck_workspace {
return Ok(());
}

// 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();
tracing::debug!(?crate_ids, "flycheck crate ids");
let crate_root_paths: Vec<_> = crate_ids
.iter()
.any(|&it| crate_root_paths.contains(&cargo[it].root.as_path()))
}),
project_model::ProjectWorkspaceKind::Json(project) => project
.crates()
.any(|(_, krate)| crate_root_paths.contains(&krate.root_module.as_path())),
project_model::ProjectWorkspaceKind::DetachedFile { .. } => false,
});

let saved_file = vfs_path.as_path().map(|p| p.to_owned());

// Find and trigger corresponding flychecks
'flychecks: for flycheck in world.flycheck.iter() {
for (id, _) in workspace_ids.clone() {
if id == flycheck.id() {
updated = true;
flycheck.restart_workspace(saved_file.clone());
continue 'flychecks;
}
}
}
// No specific flycheck was triggered, so let's trigger all of them.
if !updated {
for flycheck in world.flycheck.iter() {
flycheck.restart_workspace(saved_file.clone());
.filter_map(|&crate_id| {
world
.analysis
.crate_root(crate_id)
.map(|file_id| {
world
.file_id_to_file_path(file_id)
.as_path()
.map(ToOwned::to_owned)
})
.transpose()
})
.collect::<ide::Cancellable<_>>()?;
let crate_root_paths: Vec<_> =
crate_root_paths.iter().map(Deref::deref).collect();
tracing::debug!(?crate_root_paths, "flycheck crate roots");

// Find all workspaces that have at least one target containing the saved file
let workspace_ids =
world.workspaces.iter().enumerate().filter(|&(idx, ws)| {
let ws_contains_file = match &ws.kind {
project_model::ProjectWorkspaceKind::Cargo {
cargo, ..
}
| project_model::ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _, _)),
..
} => cargo.packages().any(|pkg| {
cargo[pkg].targets.iter().any(|&it| {
crate_root_paths.contains(&cargo[it].root.as_path())
})
}),
project_model::ProjectWorkspaceKind::Json(project) => {
project.crates().any(|(_, krate)| {
crate_root_paths.contains(&krate.root_module.as_path())
})
}
project_model::ProjectWorkspaceKind::DetachedFile {
..
} => false,
};
let is_pkg_ws = match package_workspace_idx {
Some(pkg_idx) => pkg_idx == idx,
None => false,
};
ws_contains_file && !is_pkg_ws
});

let saved_file = vfs_path.as_path().map(ToOwned::to_owned);
let mut workspace_check_triggered = false;
// Find and trigger corresponding flychecks
'flychecks: for flycheck in world.flycheck.iter() {
for (id, _) in workspace_ids.clone() {
if id == flycheck.id() {
workspace_check_triggered = true;
flycheck.restart_workspace(saved_file.clone());
continue 'flychecks;
}
}
}

// No specific flycheck was triggered, so let's trigger all of them.
if !workspace_check_triggered && package_workspace_idx.is_none() {
for flycheck in world.flycheck.iter() {
flycheck.restart_workspace(saved_file.clone());
}
}
Ok(())
})
}
}
Ok(())
};
};

state.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, move |_| {
if let Err(e) = std::panic::catch_unwind(task) {
tracing::error!("flycheck task panicked: {e:?}")
Expand Down
20 changes: 7 additions & 13 deletions crates/rust-analyzer/src/reload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl GlobalState {
}
}

let mut workspaces = linked_projects
let mut workspaces: Vec<_> = linked_projects
.iter()
.map(|project| match project {
LinkedProject::ProjectManifest(manifest) => {
Expand All @@ -339,7 +339,7 @@ impl GlobalState {
Ok(workspace)
}
})
.collect::<Vec<_>>();
.collect();

let mut i = 0;
while i < workspaces.len() {
Expand Down Expand Up @@ -848,23 +848,17 @@ impl GlobalState {
fn reload_flycheck(&mut self) {
let _p = tracing::info_span!("GlobalState::reload_flycheck").entered();
let config = self.config.flycheck(None);
let sender = self.flycheck_sender.clone();
let invocation_strategy = match config {
FlycheckConfig::CargoCommand { .. } => {
crate::flycheck::InvocationStrategy::PerWorkspace
}
FlycheckConfig::CustomCommand { ref invocation_strategy, .. } => {
invocation_strategy.clone()
}
};
let next_gen = self.flycheck.iter().map(|f| f.generation() + 1).max().unwrap_or_default();
let sender = &self.flycheck_sender;
let invocation_strategy = config.invocation_strategy();
let next_gen =
self.flycheck.iter().map(FlycheckHandle::generation).max().unwrap_or_default() + 1;

self.flycheck = match invocation_strategy {
crate::flycheck::InvocationStrategy::Once => {
vec![FlycheckHandle::spawn(
0,
next_gen,
sender,
sender.clone(),
config,
None,
self.config.root_path().clone(),
Expand Down
Loading