fix: Make flycheck clearing dependency-aware

This commit is contained in:
Shoyu Vanilla 2025-09-18 03:21:01 +09:00
parent b12a129347
commit de3ad58b73
8 changed files with 213 additions and 43 deletions

View file

@ -9,7 +9,7 @@
use std::{cell::RefCell, io, mem, process::Command};
use base_db::Env;
use cargo_metadata::{Message, camino::Utf8Path};
use cargo_metadata::{Message, PackageId, camino::Utf8Path};
use cfg::CfgAtom;
use itertools::Itertools;
use la_arena::ArenaMap;
@ -18,6 +18,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
use serde::Deserialize as _;
use stdx::never;
use toolchain::Tool;
use triomphe::Arc;
use crate::{
CargoConfig, CargoFeatures, CargoWorkspace, InvocationStrategy, ManifestPath, Package, Sysroot,
@ -284,7 +285,7 @@ impl WorkspaceBuildScripts {
// NB: Cargo.toml could have been modified between `cargo metadata` and
// `cargo check`. We shouldn't assume that package ids we see here are
// exactly those from `config`.
let mut by_id: FxHashMap<String, Package> = FxHashMap::default();
let mut by_id: FxHashMap<Arc<PackageId>, Package> = FxHashMap::default();
for package in workspace.packages() {
outputs.insert(package, BuildScriptOutput::default());
by_id.insert(workspace[package].id.clone(), package);
@ -323,7 +324,7 @@ impl WorkspaceBuildScripts {
// ideally this would be something like:
// with_output_for: impl FnMut(&str, dyn FnOnce(&mut BuildScriptOutput)),
// but owned trait objects aren't a thing
mut with_output_for: impl FnMut(&str, &mut dyn FnMut(&str, &mut BuildScriptOutput)),
mut with_output_for: impl FnMut(&PackageId, &mut dyn FnMut(&str, &mut BuildScriptOutput)),
progress: &dyn Fn(String),
) -> io::Result<Option<String>> {
let errors = RefCell::new(String::new());
@ -346,7 +347,7 @@ impl WorkspaceBuildScripts {
match message {
Message::BuildScriptExecuted(mut message) => {
with_output_for(&message.package_id.repr, &mut |name, data| {
with_output_for(&message.package_id, &mut |name, data| {
progress(format!("build script {name} run"));
let cfgs = {
let mut acc = Vec::new();
@ -377,7 +378,7 @@ impl WorkspaceBuildScripts {
});
}
Message::CompilerArtifact(message) => {
with_output_for(&message.package_id.repr, &mut |name, data| {
with_output_for(&message.package_id, &mut |name, data| {
progress(format!("proc-macro {name} built"));
if data.proc_macro_dylib_path == ProcMacroDylibPath::NotBuilt {
data.proc_macro_dylib_path = ProcMacroDylibPath::NotProcMacro;

View file

@ -5,7 +5,7 @@ use std::str::from_utf8;
use anyhow::Context;
use base_db::Env;
use cargo_metadata::{CargoOpt, MetadataCommand};
use cargo_metadata::{CargoOpt, MetadataCommand, PackageId};
use la_arena::{Arena, Idx};
use paths::{AbsPath, AbsPathBuf, Utf8Path, Utf8PathBuf};
use rustc_hash::{FxHashMap, FxHashSet};
@ -14,6 +14,7 @@ use serde_json::from_value;
use span::Edition;
use stdx::process::spawn_with_streaming_output;
use toolchain::Tool;
use triomphe::Arc;
use crate::cargo_config_file::make_lockfile_copy;
use crate::{CfgOverrides, InvocationStrategy};
@ -155,8 +156,8 @@ pub struct PackageData {
pub features: FxHashMap<String, Vec<String>>,
/// List of features enabled on this package
pub active_features: Vec<String>,
/// String representation of package id
pub id: String,
/// Package id
pub id: Arc<PackageId>,
/// Authors as given in the `Cargo.toml`
pub authors: Vec<String>,
/// Description as given in the `Cargo.toml`
@ -173,6 +174,10 @@ pub struct PackageData {
pub rust_version: Option<semver::Version>,
/// The contents of [package.metadata.rust-analyzer]
pub metadata: RustAnalyzerPackageMetaData,
/// If this package is a member of the workspace, store all direct and transitive
/// dependencies as long as they are workspace members, to track dependency relationships
/// between members.
pub all_member_deps: Option<FxHashSet<Package>>,
}
#[derive(Deserialize, Default, Debug, Clone, Eq, PartialEq)]
@ -334,6 +339,8 @@ impl CargoWorkspace {
let mut is_virtual_workspace = true;
let mut requires_rustc_private = false;
let mut members = FxHashSet::default();
meta.packages.sort_by(|a, b| a.id.cmp(&b.id));
for meta_pkg in meta.packages {
let cargo_metadata::Package {
@ -356,6 +363,7 @@ impl CargoWorkspace {
rust_version,
..
} = meta_pkg;
let id = Arc::new(id);
let meta = from_value::<PackageMetadata>(metadata).unwrap_or_default();
let edition = match edition {
cargo_metadata::Edition::E2015 => Edition::Edition2015,
@ -375,7 +383,7 @@ impl CargoWorkspace {
let manifest = ManifestPath::try_from(AbsPathBuf::assert(manifest_path)).unwrap();
is_virtual_workspace &= manifest != ws_manifest_path;
let pkg = packages.alloc(PackageData {
id: id.repr.clone(),
id: id.clone(),
name: name.to_string(),
version,
manifest: manifest.clone(),
@ -395,7 +403,11 @@ impl CargoWorkspace {
features: features.into_iter().collect(),
active_features: Vec::new(),
metadata: meta.rust_analyzer.unwrap_or_default(),
all_member_deps: None,
});
if is_member {
members.insert(pkg);
}
let pkg_data = &mut packages[pkg];
requires_rustc_private |= pkg_data.metadata.rustc_private;
pkg_by_id.insert(id, pkg);
@ -440,6 +452,43 @@ impl CargoWorkspace {
.extend(node.features.into_iter().map(|it| it.to_string()));
}
fn saturate_all_member_deps(
packages: &mut Arena<PackageData>,
to_visit: Package,
visited: &mut FxHashSet<Package>,
members: &FxHashSet<Package>,
) {
let pkg_data = &mut packages[to_visit];
if !visited.insert(to_visit) {
return;
}
let deps: Vec<_> = pkg_data
.dependencies
.iter()
.filter_map(|dep| {
let pkg = dep.pkg;
if members.contains(&pkg) { Some(pkg) } else { None }
})
.collect();
let mut all_member_deps = FxHashSet::from_iter(deps.iter().copied());
for dep in deps {
saturate_all_member_deps(packages, dep, visited, members);
if let Some(transitives) = &packages[dep].all_member_deps {
all_member_deps.extend(transitives);
}
}
packages[to_visit].all_member_deps = Some(all_member_deps);
}
let mut visited = FxHashSet::default();
for member in members.iter() {
saturate_all_member_deps(&mut packages, *member, &mut visited, &members);
}
CargoWorkspace {
packages,
targets,

View file

@ -120,6 +120,29 @@ impl DiagnosticCollection {
}
}
pub(crate) fn clear_check_older_than_for_package(
&mut self,
flycheck_id: usize,
package_id: Arc<PackageId>,
generation: DiagnosticsGeneration,
) {
let Some(check) = self.check.get_mut(flycheck_id) else {
return;
};
let package_id = Some(package_id);
let Some((_, checks)) = check
.per_package
.extract_if(|k, v| *k == package_id && v.generation < generation)
.next()
else {
return;
};
self.changes.extend(checks.per_file.into_keys());
if let Some(fixes) = Arc::make_mut(&mut self.check_fixes).get_mut(flycheck_id) {
fixes.remove(&package_id);
}
}
pub(crate) fn clear_native_for(&mut self, file_id: FileId) {
self.native_syntax.remove(&file_id);
self.native_semantic.remove(&file_id);

View file

@ -180,17 +180,27 @@ impl FlycheckHandle {
pub(crate) fn restart_workspace(&self, saved_file: Option<AbsPathBuf>) {
let generation = self.generation.fetch_add(1, Ordering::Relaxed) + 1;
self.sender
.send(StateChange::Restart { generation, package: None, saved_file, target: None })
.send(StateChange::Restart {
generation,
scope: FlycheckScope::Workspace,
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, target: Option<Target>) {
pub(crate) fn restart_for_package(
&self,
package: Arc<PackageId>,
target: Option<Target>,
workspace_deps: Option<FxHashSet<Arc<PackageId>>>,
) {
let generation = self.generation.fetch_add(1, Ordering::Relaxed) + 1;
self.sender
.send(StateChange::Restart {
generation,
package: Some(package),
scope: FlycheckScope::Package { package, workspace_deps },
saved_file: None,
target,
})
@ -213,8 +223,13 @@ impl FlycheckHandle {
#[derive(Debug)]
pub(crate) enum ClearDiagnosticsKind {
All,
OlderThan(DiagnosticsGeneration),
All(ClearScope),
OlderThan(DiagnosticsGeneration, ClearScope),
}
#[derive(Debug)]
pub(crate) enum ClearScope {
Workspace,
Package(Arc<PackageId>),
}
@ -275,10 +290,15 @@ pub(crate) enum Progress {
DidFailToRestart(String),
}
enum FlycheckScope {
Workspace,
Package { package: Arc<PackageId>, workspace_deps: Option<FxHashSet<Arc<PackageId>>> },
}
enum StateChange {
Restart {
generation: DiagnosticsGeneration,
package: Option<String>,
scope: FlycheckScope,
saved_file: Option<AbsPathBuf>,
target: Option<Target>,
},
@ -298,6 +318,7 @@ struct FlycheckActor {
/// or the project root of the project.
root: Arc<AbsPathBuf>,
sysroot_root: Option<AbsPathBuf>,
scope: FlycheckScope,
/// CargoHandle exists to wrap around the communication needed to be able to
/// run `cargo check` without blocking. Currently the Rust standard library
/// doesn't provide a way to read sub-process output without blocking, so we
@ -343,6 +364,7 @@ impl FlycheckActor {
config,
sysroot_root,
root: Arc::new(workspace_root),
scope: FlycheckScope::Workspace,
manifest_path,
command_handle: None,
command_receiver: None,
@ -376,7 +398,7 @@ impl FlycheckActor {
}
Event::RequestStateChange(StateChange::Restart {
generation,
package,
scope,
saved_file,
target,
}) => {
@ -389,11 +411,11 @@ impl FlycheckActor {
}
}
let command = self.check_command(&scope, saved_file.as_deref(), target);
self.scope = scope;
self.generation = generation;
let Some(command) =
self.check_command(package.as_deref(), saved_file.as_deref(), target)
else {
let Some(command) = command else {
continue;
};
@ -435,19 +457,55 @@ impl FlycheckActor {
tracing::trace!(flycheck_id = self.id, "clearing diagnostics");
// We finished without receiving any diagnostics.
// Clear everything for good measure
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::All,
});
match &self.scope {
FlycheckScope::Workspace => {
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::All(ClearScope::Workspace),
});
}
FlycheckScope::Package { package, workspace_deps } => {
for pkg in
std::iter::once(package).chain(workspace_deps.iter().flatten())
{
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::All(ClearScope::Package(
pkg.clone(),
)),
});
}
}
}
} else if res.is_ok() {
// We clear diagnostics for packages on
// `[CargoCheckMessage::CompilerArtifact]` but there seem to be setups where
// cargo may not report an artifact to our runner at all. To handle such
// cases, clear stale diagnostics when flycheck completes successfully.
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::OlderThan(self.generation),
});
match &self.scope {
FlycheckScope::Workspace => {
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::OlderThan(
self.generation,
ClearScope::Workspace,
),
});
}
FlycheckScope::Package { package, workspace_deps } => {
for pkg in
std::iter::once(package).chain(workspace_deps.iter().flatten())
{
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::OlderThan(
self.generation,
ClearScope::Package(pkg.clone()),
),
});
}
}
}
}
self.clear_diagnostics_state();
@ -475,7 +533,7 @@ impl FlycheckActor {
);
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::Package(package_id),
kind: ClearDiagnosticsKind::All(ClearScope::Package(package_id)),
});
}
}
@ -498,7 +556,9 @@ impl FlycheckActor {
);
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::Package(package_id.clone()),
kind: ClearDiagnosticsKind::All(ClearScope::Package(
package_id.clone(),
)),
});
}
} else if self.diagnostics_received
@ -507,7 +567,7 @@ impl FlycheckActor {
self.diagnostics_received = DiagnosticsReceived::YesAndClearedForAll;
self.send(FlycheckMessage::ClearDiagnostics {
id: self.id,
kind: ClearDiagnosticsKind::All,
kind: ClearDiagnosticsKind::All(ClearScope::Workspace),
});
}
self.send(FlycheckMessage::AddDiagnostic {
@ -548,7 +608,7 @@ impl FlycheckActor {
/// return None.
fn check_command(
&self,
package: Option<&str>,
scope: &FlycheckScope,
saved_file: Option<&AbsPath>,
target: Option<Target>,
) -> Option<Command> {
@ -564,9 +624,9 @@ impl FlycheckActor {
}
cmd.arg(command);
match package {
Some(pkg) => cmd.arg("-p").arg(pkg),
None => cmd.arg("--workspace"),
match scope {
FlycheckScope::Workspace => cmd.arg("--workspace"),
FlycheckScope::Package { package, .. } => cmd.arg("-p").arg(&package.repr),
};
if let Some(tgt) = target {

View file

@ -9,6 +9,7 @@ use std::{
time::{Duration, Instant},
};
use cargo_metadata::PackageId;
use crossbeam_channel::{Receiver, Sender, unbounded};
use hir::ChangeWithProcMacros;
use ide::{Analysis, AnalysisHost, Cancellable, FileId, SourceRootId};
@ -784,6 +785,7 @@ impl GlobalStateSnapshot {
cargo_toml: package_data.manifest.clone(),
crate_id,
package: cargo.package_flag(package_data),
package_id: package_data.id.clone(),
target: target_data.name.clone(),
target_kind: target_data.kind,
required_features: target_data.required_features.clone(),
@ -812,6 +814,27 @@ impl GlobalStateSnapshot {
None
}
pub(crate) fn all_workspace_dependencies_for_package(
&self,
package: &Arc<PackageId>,
) -> Option<FxHashSet<Arc<PackageId>>> {
for workspace in self.workspaces.iter() {
match &workspace.kind {
ProjectWorkspaceKind::Cargo { cargo, .. }
| ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, _, _)), .. } => {
let package = cargo.packages().find(|p| cargo[*p].id == *package)?;
return cargo[package]
.all_member_deps
.as_ref()
.map(|deps| deps.iter().map(|dep| cargo[*dep].id.clone()).collect());
}
_ => {}
}
}
None
}
pub(crate) fn file_exists(&self, file_id: FileId) -> bool {
self.vfs.read().0.exists(file_id)
}

View file

@ -331,7 +331,7 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
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),
TargetSpec::Cargo(c) => (c.target, c.workspace_root, c.package_id),
_ => return None,
};
@ -368,7 +368,13 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
_ => false,
});
if let Some(idx) = package_workspace_idx {
world.flycheck[idx].restart_for_package(package, target);
let workspace_deps =
world.all_workspace_dependencies_for_package(&package);
world.flycheck[idx].restart_for_package(
package,
target,
workspace_deps,
);
}
}
}

View file

@ -20,7 +20,7 @@ use crate::{
config::Config,
diagnostics::{DiagnosticsGeneration, NativeDiagnosticsFetchKind, fetch_native_diagnostics},
discover::{DiscoverArgument, DiscoverCommand, DiscoverProjectMessage},
flycheck::{self, ClearDiagnosticsKind, FlycheckMessage},
flycheck::{self, ClearDiagnosticsKind, ClearScope, FlycheckMessage},
global_state::{
FetchBuildDataResponse, FetchWorkspaceRequest, FetchWorkspaceResponse, GlobalState,
file_id_to_url, url_to_file_id,
@ -1042,17 +1042,22 @@ impl GlobalState {
};
}
}
FlycheckMessage::ClearDiagnostics { id, kind: ClearDiagnosticsKind::All } => {
self.diagnostics.clear_check(id)
}
FlycheckMessage::ClearDiagnostics {
id,
kind: ClearDiagnosticsKind::OlderThan(generation),
kind: ClearDiagnosticsKind::All(ClearScope::Workspace),
} => self.diagnostics.clear_check(id),
FlycheckMessage::ClearDiagnostics {
id,
kind: ClearDiagnosticsKind::All(ClearScope::Package(package_id)),
} => self.diagnostics.clear_check_for_package(id, package_id),
FlycheckMessage::ClearDiagnostics {
id,
kind: ClearDiagnosticsKind::OlderThan(generation, ClearScope::Workspace),
} => self.diagnostics.clear_check_older_than(id, generation),
FlycheckMessage::ClearDiagnostics {
id,
kind: ClearDiagnosticsKind::Package(package_id),
} => self.diagnostics.clear_check_for_package(id, package_id),
kind: ClearDiagnosticsKind::OlderThan(generation, ClearScope::Package(package_id)),
} => self.diagnostics.clear_check_older_than_for_package(id, package_id, generation),
FlycheckMessage::Progress { id, progress } => {
let (state, message) = match progress {
flycheck::Progress::DidStart => (Progress::Begin, None),

View file

@ -2,12 +2,14 @@
use std::mem;
use cargo_metadata::PackageId;
use cfg::{CfgAtom, CfgExpr};
use hir::sym;
use ide::{Cancellable, Crate, FileId, RunnableKind, TestId};
use project_model::project_json::Runnable;
use project_model::{CargoFeatures, ManifestPath, TargetKind};
use rustc_hash::FxHashSet;
use triomphe::Arc;
use vfs::AbsPathBuf;
use crate::global_state::GlobalStateSnapshot;
@ -52,6 +54,7 @@ pub(crate) struct CargoTargetSpec {
pub(crate) workspace_root: AbsPathBuf,
pub(crate) cargo_toml: ManifestPath,
pub(crate) package: String,
pub(crate) package_id: Arc<PackageId>,
pub(crate) target: String,
pub(crate) target_kind: TargetKind,
pub(crate) crate_id: Crate,