Skip to content

Commit ccc9c25

Browse files
iorizuiorizu
committed
Minor refactoring
1 parent b394f5d commit ccc9c25

File tree

3 files changed

+171
-140
lines changed

3 files changed

+171
-140
lines changed

‎crates/rust-analyzer/src/flycheck.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,11 @@ pub(crate) enum FlycheckConfig {
104104
}
105105

106106
impl FlycheckConfig {
107-
pub(crate) fn invocation_strategy_once(&self) -> bool {
107+
pub(crate) fn invocation_strategy(&self) -> InvocationStrategy {
108108
match self {
109-
FlycheckConfig::CargoCommand { .. } => false,
109+
FlycheckConfig::CargoCommand { .. } => InvocationStrategy::PerWorkspace,
110110
FlycheckConfig::CustomCommand { invocation_strategy, .. } => {
111-
*invocation_strategy == InvocationStrategy::Once
111+
invocation_strategy.clone()
112112
}
113113
}
114114
}
@@ -529,7 +529,7 @@ impl FlycheckActor {
529529
if let Some(command_handle) = self.command_handle.take() {
530530
tracing::debug!(
531531
command = ?command_handle,
532-
"did cancel flycheck"
532+
"did cancel flycheck"
533533
);
534534
command_handle.cancel();
535535
self.command_receiver.take();

‎crates/rust-analyzer/src/handlers/notification.rs

Lines changed: 160 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
//! This module is responsible for implementing handlers for Language Server
22
//! Protocol. This module specifically handles notifications.
33
4-
use std::ops::{Deref, Not as _};
4+
use std::{
5+
ops::{Deref, Not as _},
6+
panic::UnwindSafe,
7+
};
58

6-
use ide_db::base_db::salsa::Cancelled;
79
use itertools::Itertools;
810
use lsp_types::{
911
CancelParams, DidChangeConfigurationParams, DidChangeTextDocumentParams,
@@ -16,7 +18,7 @@ use vfs::{AbsPathBuf, ChangeKind, VfsPath};
1618

1719
use crate::{
1820
config::{Config, ConfigChange},
19-
flycheck::Target,
21+
flycheck::{InvocationStrategy, Target},
2022
global_state::{FetchWorkspaceRequest, GlobalState},
2123
lsp::{from_proto, utils::apply_document_changes},
2224
lsp_ext::{self, RunFlycheckParams},
@@ -301,131 +303,166 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
301303
let file_id = state.vfs.read().0.file_id(&vfs_path);
302304
if let Some((file_id, vfs::FileExcluded::No)) = file_id {
303305
let world = state.snapshot();
304-
let invocation_strategy_once = state.config.flycheck(None).invocation_strategy_once();
306+
let invocation_strategy = state.config.flycheck(None).invocation_strategy();
305307
let may_flycheck_workspace = state.config.flycheck_workspace(None);
306-
let mut workspace_check_triggered = false;
307-
let task = move || -> std::result::Result<(), Cancelled> {
308-
let saved_file = vfs_path.as_path().map(|p| p.to_owned());
309-
if invocation_strategy_once {
310-
world.flycheck[0].restart_workspace(saved_file.clone());
311-
}
312308

313-
let target = TargetSpec::for_file(&world, file_id)?.and_then(|it| {
314-
let tgt_kind = it.target_kind();
315-
let (tgt_name, root, package) = match it {
316-
TargetSpec::Cargo(c) => (c.target, c.workspace_root, c.package),
317-
_ => return None,
318-
};
319-
320-
let tgt = match tgt_kind {
321-
project_model::TargetKind::Bin => Target::Bin(tgt_name),
322-
project_model::TargetKind::Example => Target::Example(tgt_name),
323-
project_model::TargetKind::Test => Target::Test(tgt_name),
324-
project_model::TargetKind::Bench => Target::Benchmark(tgt_name),
325-
_ => return Some((None, root, package)),
326-
};
327-
328-
Some((Some(tgt), root, package))
329-
});
330-
tracing::debug!(?target, "flycheck target");
331-
// we have a specific non-library target, attempt to only check that target, nothing
332-
// else will be affected
333-
let mut package_workspace_idx = None;
334-
if let Some((target, root, package)) = target {
335-
// trigger a package check if we have a non-library target as that can't affect
336-
// anything else in the workspace OR if we're not allowed to check the workspace as
337-
// the user opted into package checks then
338-
let package_check_allowed = target.is_some() || !may_flycheck_workspace;
339-
if package_check_allowed {
340-
let workspace = world.workspaces.iter().position(|ws| match &ws.kind {
341-
project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
342-
| project_model::ProjectWorkspaceKind::DetachedFile {
343-
cargo: Some((cargo, _, _)),
344-
..
345-
} => *cargo.workspace_root() == root,
346-
_ => false,
347-
});
348-
if let Some(idx) = workspace {
349-
package_workspace_idx = Some(idx);
350-
world.flycheck[idx].restart_for_package(package, target);
351-
}
309+
let task: Box<dyn FnOnce() -> ide::Cancellable<()> + Send + UnwindSafe> =
310+
match invocation_strategy {
311+
InvocationStrategy::Once => {
312+
Box::new(move || {
313+
// FIXME: Because triomphe::Arc's auto UnwindSafe impl requires that the inner type
314+
// be UnwindSafe, and FlycheckHandle is not UnwindSafe, `word.flycheck` cannot
315+
// be captured directly. std::sync::Arc has an UnwindSafe impl that only requires
316+
// that the inner type be RefUnwindSafe, so if we were using that one we wouldn't
317+
// have this problem. Remove the line below when triomphe::Arc has an UnwindSafe impl
318+
// like std::sync::Arc's.
319+
let world = world;
320+
stdx::always!(
321+
world.flycheck.len() == 1,
322+
"should only exactly one flycheck handle when invocation strategy is once"
323+
);
324+
let saved_file = vfs_path.as_path().map(ToOwned::to_owned);
325+
world.flycheck[0].restart_workspace(saved_file);
326+
Ok(())
327+
})
352328
}
353-
}
354-
355-
if !may_flycheck_workspace {
356-
return Ok(());
357-
}
358-
359-
// Trigger flychecks for all workspaces that depend on the saved file
360-
// Crates containing or depending on the saved file
361-
let crate_ids = world
362-
.analysis
363-
.crates_for(file_id)?
364-
.into_iter()
365-
.flat_map(|id| world.analysis.transitive_rev_deps(id))
366-
.flatten()
367-
.unique()
368-
.collect::<Vec<_>>();
369-
tracing::debug!(?crate_ids, "flycheck crate ids");
370-
let crate_root_paths: Vec<_> = crate_ids
371-
.iter()
372-
.filter_map(|&crate_id| {
373-
world
374-
.analysis
375-
.crate_root(crate_id)
376-
.map(|file_id| {
377-
world.file_id_to_file_path(file_id).as_path().map(ToOwned::to_owned)
378-
})
379-
.transpose()
380-
})
381-
.collect::<ide::Cancellable<_>>()?;
382-
let crate_root_paths: Vec<_> = crate_root_paths.iter().map(Deref::deref).collect();
383-
tracing::debug!(?crate_root_paths, "flycheck crate roots");
384-
385-
// Find all workspaces that have at least one target containing the saved file
386-
let workspace_ids = world
387-
.workspaces
388-
.iter()
389-
.enumerate()
390-
.filter(|&(idx, _)| match package_workspace_idx {
391-
Some(pkg_idx) => idx != pkg_idx,
392-
None => true,
393-
})
394-
.filter(|&(_, ws)| match &ws.kind {
395-
project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
396-
| project_model::ProjectWorkspaceKind::DetachedFile {
397-
cargo: Some((cargo, _, _)),
398-
..
399-
} => cargo.packages().any(|pkg| {
400-
cargo[pkg]
401-
.targets
329+
InvocationStrategy::PerWorkspace => {
330+
Box::new(move || {
331+
let target = TargetSpec::for_file(&world, file_id)?.and_then(|it| {
332+
let tgt_kind = it.target_kind();
333+
let (tgt_name, root, package) = match it {
334+
TargetSpec::Cargo(c) => (c.target, c.workspace_root, c.package),
335+
_ => return None,
336+
};
337+
338+
let tgt = match tgt_kind {
339+
project_model::TargetKind::Bin => Target::Bin(tgt_name),
340+
project_model::TargetKind::Example => Target::Example(tgt_name),
341+
project_model::TargetKind::Test => Target::Test(tgt_name),
342+
project_model::TargetKind::Bench => Target::Benchmark(tgt_name),
343+
_ => return Some((None, root, package)),
344+
};
345+
346+
Some((Some(tgt), root, package))
347+
});
348+
tracing::debug!(?target, "flycheck target");
349+
// we have a specific non-library target, attempt to only check that target, nothing
350+
// else will be affected
351+
let mut package_workspace_idx = None;
352+
if let Some((target, root, package)) = target {
353+
// trigger a package check if we have a non-library target as that can't affect
354+
// anything else in the workspace OR if we're not allowed to check the workspace as
355+
// the user opted into package checks then
356+
let package_check_allowed = target.is_some() || !may_flycheck_workspace;
357+
if package_check_allowed {
358+
let workspace =
359+
world.workspaces.iter().position(|ws| match &ws.kind {
360+
project_model::ProjectWorkspaceKind::Cargo {
361+
cargo,
362+
..
363+
}
364+
| project_model::ProjectWorkspaceKind::DetachedFile {
365+
cargo: Some((cargo, _, _)),
366+
..
367+
} => *cargo.workspace_root() == root,
368+
_ => false,
369+
});
370+
if let Some(idx) = workspace {
371+
package_workspace_idx = Some(idx);
372+
world.flycheck[idx].restart_for_package(package, target);
373+
}
374+
}
375+
}
376+
377+
if !may_flycheck_workspace {
378+
return Ok(());
379+
}
380+
381+
// Trigger flychecks for all workspaces that depend on the saved file
382+
// Crates containing or depending on the saved file
383+
let crate_ids: Vec<_> = world
384+
.analysis
385+
.crates_for(file_id)?
386+
.into_iter()
387+
.flat_map(|id| world.analysis.transitive_rev_deps(id))
388+
.flatten()
389+
.unique()
390+
.collect();
391+
tracing::debug!(?crate_ids, "flycheck crate ids");
392+
let crate_root_paths: Vec<_> = crate_ids
402393
.iter()
403-
.any(|&it| crate_root_paths.contains(&cargo[it].root.as_path()))
404-
}),
405-
project_model::ProjectWorkspaceKind::Json(project) => project
406-
.crates()
407-
.any(|(_, krate)| crate_root_paths.contains(&krate.root_module.as_path())),
408-
project_model::ProjectWorkspaceKind::DetachedFile { .. } => false,
409-
});
410-
411-
// Find and trigger corresponding flychecks
412-
'flychecks: for flycheck in world.flycheck.iter() {
413-
for (id, _) in workspace_ids.clone() {
414-
if id == flycheck.id() {
415-
workspace_check_triggered = true;
416-
flycheck.restart_workspace(saved_file.clone());
417-
continue 'flychecks;
418-
}
419-
}
420-
}
421-
// No specific flycheck was triggered, so let's trigger all of them.
422-
if !workspace_check_triggered && package_workspace_idx.is_none() {
423-
for flycheck in world.flycheck.iter() {
424-
flycheck.restart_workspace(saved_file.clone());
394+
.filter_map(|&crate_id| {
395+
world
396+
.analysis
397+
.crate_root(crate_id)
398+
.map(|file_id| {
399+
world
400+
.file_id_to_file_path(file_id)
401+
.as_path()
402+
.map(ToOwned::to_owned)
403+
})
404+
.transpose()
405+
})
406+
.collect::<ide::Cancellable<_>>()?;
407+
let crate_root_paths: Vec<_> =
408+
crate_root_paths.iter().map(Deref::deref).collect();
409+
tracing::debug!(?crate_root_paths, "flycheck crate roots");
410+
411+
// Find all workspaces that have at least one target containing the saved file
412+
let workspace_ids =
413+
world.workspaces.iter().enumerate().filter(|&(idx, ws)| {
414+
let ws_contains_file = match &ws.kind {
415+
project_model::ProjectWorkspaceKind::Cargo {
416+
cargo, ..
417+
}
418+
| project_model::ProjectWorkspaceKind::DetachedFile {
419+
cargo: Some((cargo, _, _)),
420+
..
421+
} => cargo.packages().any(|pkg| {
422+
cargo[pkg].targets.iter().any(|&it| {
423+
crate_root_paths.contains(&cargo[it].root.as_path())
424+
})
425+
}),
426+
project_model::ProjectWorkspaceKind::Json(project) => {
427+
project.crates().any(|(_, krate)| {
428+
crate_root_paths.contains(&krate.root_module.as_path())
429+
})
430+
}
431+
project_model::ProjectWorkspaceKind::DetachedFile {
432+
..
433+
} => false,
434+
};
435+
let is_pkg_ws = match package_workspace_idx {
436+
Some(pkg_idx) => pkg_idx == idx,
437+
None => false,
438+
};
439+
ws_contains_file && !is_pkg_ws
440+
});
441+
442+
let saved_file = vfs_path.as_path().map(ToOwned::to_owned);
443+
let mut workspace_check_triggered = false;
444+
// Find and trigger corresponding flychecks
445+
'flychecks: for flycheck in world.flycheck.iter() {
446+
for (id, _) in workspace_ids.clone() {
447+
if id == flycheck.id() {
448+
workspace_check_triggered = true;
449+
flycheck.restart_workspace(saved_file.clone());
450+
continue 'flychecks;
451+
}
452+
}
453+
}
454+
455+
// No specific flycheck was triggered, so let's trigger all of them.
456+
if !workspace_check_triggered && package_workspace_idx.is_none() {
457+
for flycheck in world.flycheck.iter() {
458+
flycheck.restart_workspace(saved_file.clone());
459+
}
460+
}
461+
Ok(())
462+
})
425463
}
426-
}
427-
Ok(())
428-
};
464+
};
465+
429466
state.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, move |_| {
430467
if let Err(e) = std::panic::catch_unwind(task) {
431468
tracing::error!("flycheck task panicked: {e:?}")

‎crates/rust-analyzer/src/reload.rs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ impl GlobalState {
318318
}
319319
}
320320

321-
let mut workspaces = linked_projects
321+
let mut workspaces: Vec<_> = linked_projects
322322
.iter()
323323
.map(|project| match project {
324324
LinkedProject::ProjectManifest(manifest) => {
@@ -339,7 +339,7 @@ impl GlobalState {
339339
Ok(workspace)
340340
}
341341
})
342-
.collect::<Vec<_>>();
342+
.collect();
343343

344344
let mut i = 0;
345345
while i < workspaces.len() {
@@ -848,23 +848,17 @@ impl GlobalState {
848848
fn reload_flycheck(&mut self) {
849849
let _p = tracing::info_span!("GlobalState::reload_flycheck").entered();
850850
let config = self.config.flycheck(None);
851-
let sender = self.flycheck_sender.clone();
852-
let invocation_strategy = match config {
853-
FlycheckConfig::CargoCommand { .. } => {
854-
crate::flycheck::InvocationStrategy::PerWorkspace
855-
}
856-
FlycheckConfig::CustomCommand { ref invocation_strategy, .. } => {
857-
invocation_strategy.clone()
858-
}
859-
};
860-
let next_gen = self.flycheck.iter().map(|f| f.generation() + 1).max().unwrap_or_default();
851+
let sender = &self.flycheck_sender;
852+
let invocation_strategy = config.invocation_strategy();
853+
let next_gen =
854+
self.flycheck.iter().map(FlycheckHandle::generation).max().unwrap_or_default() + 1;
861855

862856
self.flycheck = match invocation_strategy {
863857
crate::flycheck::InvocationStrategy::Once => {
864858
vec![FlycheckHandle::spawn(
865859
0,
866860
next_gen,
867-
sender,
861+
sender.clone(),
868862
config,
869863
None,
870864
self.config.root_path().clone(),

0 commit comments

Comments
 (0)