Merge pull request #20689 from ShoyuVanilla/accurate-flycheck

fix: Make flycheck clearing dependency-aware

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;

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
+    /// Package id
-    pub id: String,
+    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,

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);

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,
+    All(ClearScope),
-    OlderThan(DiagnosticsGeneration),
+    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) =
+                    let Some(command) = command else {
-                        self.check_command(package.as_deref(), saved_file.as_deref(), target)
-                    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 {
+                        match &self.scope {
-                            id: self.id,
+                            FlycheckScope::Workspace => {
-                            kind: ClearDiagnosticsKind::All,
+                                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 {
+                        match &self.scope {
-                            id: self.id,
+                            FlycheckScope::Workspace => {
-                            kind: ClearDiagnosticsKind::OlderThan(self.generation),
+                                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 {
+                match scope {
-                    Some(pkg) => cmd.arg("-p").arg(pkg),
+                    FlycheckScope::Workspace => cmd.arg("--workspace"),
-                    None => cmd.arg("--workspace"),
+                    FlycheckScope::Package { package, .. } => cmd.arg("-p").arg(&package.repr),
                 };
 
                 if let Some(tgt) = target {

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)
     }

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,
+                                    );
                                 }
                             }
                         }

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),
+                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),
+                kind: ClearDiagnosticsKind::OlderThan(generation, ClearScope::Package(package_id)),
-            } => self.diagnostics.clear_check_for_package(id, 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),

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,