Minor refactoring

This commit is contained in:
Ifeanyi Orizu 2025-09-08 16:09:59 -05:00
parent b394f5d408
commit 8497cba0e7
3 changed files with 166 additions and 136 deletions

View file

@ -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()
}
}
}
@ -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();

View file

@ -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,
@ -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},
@ -301,131 +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 workspace_check_triggered = false;
let task = move || -> std::result::Result<(), Cancelled> {
let saved_file = vfs_path.as_path().map(|p| p.to_owned());
if invocation_strategy_once {
world.flycheck[0].restart_workspace(saved_file.clone());
}
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 {
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 {
package_workspace_idx = Some(idx);
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(())
})
}
}
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,
};
if !may_flycheck_workspace {
return Ok(());
}
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)),
};
// 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");
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);
}
}
}
// Find all workspaces that have at least one target containing the saved file
let workspace_ids = world
.workspaces
.iter()
.enumerate()
.filter(|&(idx, _)| match package_workspace_idx {
Some(pkg_idx) => idx != pkg_idx,
None => true,
})
.filter(|&(_, ws)| match &ws.kind {
project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
| project_model::ProjectWorkspaceKind::DetachedFile {
cargo: Some((cargo, _, _)),
..
} => cargo.packages().any(|pkg| {
cargo[pkg]
.targets
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,
});
.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 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;
}
// 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(())
})
}
}
// 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(())
};
};
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:?}")

View file

@ -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) => {
@ -339,7 +339,7 @@ impl GlobalState {
Ok(workspace)
}
})
.collect::<Vec<_>>();
.collect();
let mut i = 0;
while i < workspaces.len() {
@ -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(),