Unnamed repository; edit this file 'description' to name the repository.
fix: Make flycheck clearing dependency-aware
| -rw-r--r-- | crates/project-model/src/build_dependencies.rs | 11 | ||||
| -rw-r--r-- | crates/project-model/src/cargo_workspace.rs | 57 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/diagnostics.rs | 23 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/flycheck.rs | 110 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/global_state.rs | 23 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/handlers/notification.rs | 10 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/main_loop.rs | 19 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/target_spec.rs | 3 |
8 files changed, 213 insertions, 43 deletions
diff --git a/crates/project-model/src/build_dependencies.rs b/crates/project-model/src/build_dependencies.rs index 203173c11b..5eda5af3ac 100644 --- a/crates/project-model/src/build_dependencies.rs +++ b/crates/project-model/src/build_dependencies.rs @@ -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; diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs index e613fd590c..adc0cc5094 100644 --- a/crates/project-model/src/cargo_workspace.rs +++ b/crates/project-model/src/cargo_workspace.rs @@ -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, diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs index ee50237c40..4bfad98b39 100644 --- a/crates/rust-analyzer/src/diagnostics.rs +++ b/crates/rust-analyzer/src/diagnostics.rs @@ -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); diff --git a/crates/rust-analyzer/src/flycheck.rs b/crates/rust-analyzer/src/flycheck.rs index 315c45d5b6..cded34be14 100644 --- a/crates/rust-analyzer/src/flycheck.rs +++ b/crates/rust-analyzer/src/flycheck.rs @@ -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 { diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 89d6fb8edc..ce6644f725 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -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) } diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index 68c91a6539..87be09dcbd 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -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, + ); } } } diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index c6762f3183..3e80e8b7bd 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -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), - } => self.diagnostics.clear_check_older_than(id, generation), + kind: ClearDiagnosticsKind::All(ClearScope::Workspace), + } => self.diagnostics.clear_check(id), FlycheckMessage::ClearDiagnostics { id, - kind: ClearDiagnosticsKind::Package(package_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::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), diff --git a/crates/rust-analyzer/src/target_spec.rs b/crates/rust-analyzer/src/target_spec.rs index 7132e09146..e532d15553 100644 --- a/crates/rust-analyzer/src/target_spec.rs +++ b/crates/rust-analyzer/src/target_spec.rs @@ -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, |