diff options
4 files changed, 140 insertions, 53 deletions
diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/diagnostics.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/diagnostics.rs index 22910ee4c68..e64a15ae041 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/diagnostics.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/diagnostics.rs @@ -3,6 +3,7 @@ pub(crate) mod to_proto; use std::mem; +use cargo_metadata::PackageId; use ide::FileId; use ide_db::FxHashMap; use itertools::Itertools; @@ -13,7 +14,8 @@ use triomphe::Arc; use crate::{global_state::GlobalStateSnapshot, lsp, lsp_ext, main_loop::DiagnosticsTaskKind}; -pub(crate) type CheckFixes = Arc<IntMap<usize, IntMap<FileId, Vec<Fix>>>>; +pub(crate) type CheckFixes = + Arc<IntMap<usize, FxHashMap<Option<Arc<PackageId>>, IntMap<FileId, Vec<Fix>>>>>; #[derive(Debug, Default, Clone)] pub struct DiagnosticsMapConfig { @@ -31,7 +33,10 @@ pub(crate) struct DiagnosticCollection { pub(crate) native_syntax: IntMap<FileId, (DiagnosticsGeneration, Vec<lsp_types::Diagnostic>)>, pub(crate) native_semantic: IntMap<FileId, (DiagnosticsGeneration, Vec<lsp_types::Diagnostic>)>, // FIXME: should be Vec<flycheck::Diagnostic> - pub(crate) check: IntMap<usize, IntMap<FileId, Vec<lsp_types::Diagnostic>>>, + pub(crate) check: IntMap< + usize, + FxHashMap<Option<Arc<PackageId>>, IntMap<FileId, Vec<lsp_types::Diagnostic>>>, + >, pub(crate) check_fixes: CheckFixes, changes: IntSet<FileId>, /// Counter for supplying a new generation number for diagnostics. @@ -50,18 +55,19 @@ pub(crate) struct Fix { impl DiagnosticCollection { pub(crate) fn clear_check(&mut self, flycheck_id: usize) { - if let Some(it) = Arc::make_mut(&mut self.check_fixes).get_mut(&flycheck_id) { + if let Some(it) = self.check.get_mut(&flycheck_id) { it.clear(); } - if let Some(it) = self.check.get_mut(&flycheck_id) { - self.changes.extend(it.drain().map(|(key, _value)| key)); + if let Some(fixes) = Arc::make_mut(&mut self.check_fixes).get_mut(&flycheck_id) { + fixes.clear(); } } pub(crate) fn clear_check_all(&mut self) { Arc::make_mut(&mut self.check_fixes).clear(); - self.changes - .extend(self.check.values_mut().flat_map(|it| it.drain().map(|(key, _value)| key))) + self.changes.extend( + self.check.values_mut().flat_map(|it| it.drain().flat_map(|(_, v)| v.into_keys())), + ) } pub(crate) fn clear_native_for(&mut self, file_id: FileId) { @@ -70,14 +76,33 @@ impl DiagnosticCollection { self.changes.insert(file_id); } + pub(crate) fn clear_check_for_package( + &mut self, + flycheck_id: usize, + package_id: Arc<PackageId>, + ) { + let Some(check) = self.check.get_mut(&flycheck_id) else { + return; + }; + check.remove(&Some(package_id)); + } + pub(crate) fn add_check_diagnostic( &mut self, flycheck_id: usize, + package_id: &Option<Arc<PackageId>>, file_id: FileId, diagnostic: lsp_types::Diagnostic, fix: Option<Box<Fix>>, ) { - let diagnostics = self.check.entry(flycheck_id).or_default().entry(file_id).or_default(); + let diagnostics = self + .check + .entry(flycheck_id) + .or_default() + .entry(package_id.clone()) + .or_default() + .entry(file_id) + .or_default(); for existing_diagnostic in diagnostics.iter() { if are_diagnostics_equal(existing_diagnostic, &diagnostic) { return; @@ -86,7 +111,14 @@ impl DiagnosticCollection { if let Some(fix) = fix { let check_fixes = Arc::make_mut(&mut self.check_fixes); - check_fixes.entry(flycheck_id).or_default().entry(file_id).or_default().push(*fix); + check_fixes + .entry(flycheck_id) + .or_default() + .entry(package_id.clone()) + .or_default() + .entry(file_id) + .or_default() + .push(*fix); } diagnostics.push(diagnostic); self.changes.insert(file_id); @@ -135,7 +167,12 @@ impl DiagnosticCollection { ) -> impl Iterator<Item = &lsp_types::Diagnostic> { let native_syntax = self.native_syntax.get(&file_id).into_iter().flat_map(|(_, d)| d); let native_semantic = self.native_semantic.get(&file_id).into_iter().flat_map(|(_, d)| d); - let check = self.check.values().filter_map(move |it| it.get(&file_id)).flatten(); + let check = self + .check + .values() + .flat_map(|it| it.values()) + .filter_map(move |it| it.get(&file_id)) + .flatten(); native_syntax.chain(native_semantic).chain(check) } diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs index f2ad4f081c4..a9eb15ce339 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs @@ -1,8 +1,9 @@ //! Flycheck provides the functionality needed to run `cargo check` to provide //! LSP diagnostics based on the output of the command. -use std::{fmt, io, process::Command, time::Duration}; +use std::{fmt, io, mem, process::Command, time::Duration}; +use cargo_metadata::PackageId; use crossbeam_channel::{select_biased, unbounded, Receiver, Sender}; use paths::{AbsPath, AbsPathBuf, Utf8PathBuf}; use rustc_hash::FxHashMap; @@ -13,6 +14,7 @@ pub(crate) use cargo_metadata::diagnostic::{ Applicability, Diagnostic, DiagnosticCode, DiagnosticLevel, DiagnosticSpan, }; use toolchain::Tool; +use triomphe::Arc; use crate::command::{CommandHandle, ParseFromLine}; @@ -151,10 +153,19 @@ impl FlycheckHandle { pub(crate) enum FlycheckMessage { /// Request adding a diagnostic with fixes included to a file - AddDiagnostic { id: usize, workspace_root: AbsPathBuf, diagnostic: Diagnostic }, + AddDiagnostic { + id: usize, + workspace_root: Arc<AbsPathBuf>, + diagnostic: Diagnostic, + package_id: Option<Arc<PackageId>>, + }, - /// Request clearing all previous diagnostics - ClearDiagnostics { id: usize }, + /// Request clearing all outdated diagnostics. + ClearDiagnostics { + id: usize, + /// The package whose diagnostics to clear, or if unspecified, all diagnostics. + package_id: Option<Arc<PackageId>>, + }, /// Request check progress notification to client Progress { @@ -167,15 +178,18 @@ pub(crate) enum FlycheckMessage { impl fmt::Debug for FlycheckMessage { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic } => f + FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic, package_id } => f .debug_struct("AddDiagnostic") .field("id", id) .field("workspace_root", workspace_root) + .field("package_id", package_id) .field("diagnostic_code", &diagnostic.code.as_ref().map(|it| &it.code)) .finish(), - FlycheckMessage::ClearDiagnostics { id } => { - f.debug_struct("ClearDiagnostics").field("id", id).finish() - } + FlycheckMessage::ClearDiagnostics { id, package_id } => f + .debug_struct("ClearDiagnostics") + .field("id", id) + .field("package_id", package_id) + .finish(), FlycheckMessage::Progress { id, progress } => { f.debug_struct("Progress").field("id", id).field("progress", progress).finish() } @@ -201,12 +215,13 @@ enum StateChange { struct FlycheckActor { /// The workspace id of this flycheck instance. id: usize, + sender: Sender<FlycheckMessage>, config: FlycheckConfig, manifest_path: Option<AbsPathBuf>, /// Either the workspace root of the workspace we are flychecking, /// or the project root of the project. - root: AbsPathBuf, + root: Arc<AbsPathBuf>, sysroot_root: Option<AbsPathBuf>, /// CargoHandle exists to wrap around the communication needed to be able to /// run `cargo check` without blocking. Currently the Rust standard library @@ -216,8 +231,13 @@ struct FlycheckActor { command_handle: Option<CommandHandle<CargoCheckMessage>>, /// The receiver side of the channel mentioned above. command_receiver: Option<Receiver<CargoCheckMessage>>, + package_status: FxHashMap<Arc<PackageId>, DiagnosticReceived>, +} - status: FlycheckStatus, +#[derive(PartialEq, Eq, Copy, Clone, Debug)] +enum DiagnosticReceived { + Yes, + No, } #[allow(clippy::large_enum_variant)] @@ -226,13 +246,6 @@ enum Event { CheckEvent(Option<CargoCheckMessage>), } -#[derive(PartialEq)] -enum FlycheckStatus { - Started, - DiagnosticSent, - Finished, -} - pub(crate) const SAVED_FILE_PLACEHOLDER: &str = "$saved_file"; impl FlycheckActor { @@ -250,11 +263,11 @@ impl FlycheckActor { sender, config, sysroot_root, - root: workspace_root, + root: Arc::new(workspace_root), manifest_path, command_handle: None, command_receiver: None, - status: FlycheckStatus::Finished, + package_status: FxHashMap::default(), } } @@ -307,13 +320,11 @@ impl FlycheckActor { self.command_handle = Some(command_handle); self.command_receiver = Some(receiver); self.report_progress(Progress::DidStart); - self.status = FlycheckStatus::Started; } Err(error) => { self.report_progress(Progress::DidFailToRestart(format!( "Failed to run the following command: {formatted_command} error={error}" ))); - self.status = FlycheckStatus::Finished; } } } @@ -333,11 +344,25 @@ impl FlycheckActor { error ); } - if self.status == FlycheckStatus::Started { - self.send(FlycheckMessage::ClearDiagnostics { id: self.id }); + if self.package_status.is_empty() { + // We finished without receiving any diagnostics. + // That means all of them are stale. + self.send(FlycheckMessage::ClearDiagnostics { + id: self.id, + package_id: None, + }); + } else { + for (package_id, status) in mem::take(&mut self.package_status) { + if let DiagnosticReceived::No = status { + self.send(FlycheckMessage::ClearDiagnostics { + id: self.id, + package_id: Some(package_id), + }); + } + } } + self.report_progress(Progress::DidFinish(res)); - self.status = FlycheckStatus::Finished; } Event::CheckEvent(Some(message)) => match message { CargoCheckMessage::CompilerArtifact(msg) => { @@ -347,23 +372,32 @@ impl FlycheckActor { "artifact received" ); self.report_progress(Progress::DidCheckCrate(msg.target.name)); + self.package_status + .entry(Arc::new(msg.package_id)) + .or_insert(DiagnosticReceived::No); } - - CargoCheckMessage::Diagnostic(msg) => { + CargoCheckMessage::Diagnostic { diagnostic, package_id } => { tracing::trace!( flycheck_id = self.id, - message = msg.message, + message = diagnostic.message, "diagnostic received" ); - if self.status == FlycheckStatus::Started { - self.send(FlycheckMessage::ClearDiagnostics { id: self.id }); + if let Some(package_id) = &package_id { + if !self.package_status.contains_key(package_id) { + self.package_status + .insert(package_id.clone(), DiagnosticReceived::Yes); + self.send(FlycheckMessage::ClearDiagnostics { + id: self.id, + package_id: Some(package_id.clone()), + }); + } } self.send(FlycheckMessage::AddDiagnostic { id: self.id, + package_id, workspace_root: self.root.clone(), - diagnostic: msg, + diagnostic, }); - self.status = FlycheckStatus::DiagnosticSent; } }, } @@ -381,7 +415,7 @@ impl FlycheckActor { command_handle.cancel(); self.command_receiver.take(); self.report_progress(Progress::DidCancel); - self.status = FlycheckStatus::Finished; + self.package_status.clear(); } } @@ -401,7 +435,7 @@ impl FlycheckActor { cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(sysroot_root)); } cmd.arg(command); - cmd.current_dir(&self.root); + cmd.current_dir(&*self.root); match package { Some(pkg) => cmd.arg("-p").arg(pkg), @@ -443,11 +477,11 @@ impl FlycheckActor { match invocation_strategy { InvocationStrategy::Once => { - cmd.current_dir(&self.root); + cmd.current_dir(&*self.root); } InvocationStrategy::PerWorkspace => { // FIXME: cmd.current_dir(&affected_workspace); - cmd.current_dir(&self.root); + cmd.current_dir(&*self.root); } } @@ -487,7 +521,7 @@ impl FlycheckActor { #[allow(clippy::large_enum_variant)] enum CargoCheckMessage { CompilerArtifact(cargo_metadata::Artifact), - Diagnostic(Diagnostic), + Diagnostic { diagnostic: Diagnostic, package_id: Option<Arc<PackageId>> }, } impl ParseFromLine for CargoCheckMessage { @@ -502,11 +536,16 @@ impl ParseFromLine for CargoCheckMessage { Some(CargoCheckMessage::CompilerArtifact(artifact)) } cargo_metadata::Message::CompilerMessage(msg) => { - Some(CargoCheckMessage::Diagnostic(msg.message)) + Some(CargoCheckMessage::Diagnostic { + diagnostic: msg.message, + package_id: Some(Arc::new(msg.package_id)), + }) } _ => None, }, - JsonMessage::Rustc(message) => Some(CargoCheckMessage::Diagnostic(message)), + JsonMessage::Rustc(message) => { + Some(CargoCheckMessage::Diagnostic { diagnostic: message, package_id: None }) + } }; } diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs index b8c2124531a..8f2bf80ea26 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs @@ -1445,7 +1445,13 @@ pub(crate) fn handle_code_action( } // Fixes from `cargo check`. - for fix in snap.check_fixes.values().filter_map(|it| it.get(&frange.file_id)).flatten() { + for fix in snap + .check_fixes + .values() + .flat_map(|it| it.values()) + .filter_map(|it| it.get(&frange.file_id)) + .flatten() + { // FIXME: this mapping is awkward and shouldn't exist. Refactor // `snap.check_fixes` to not convert to LSP prematurely. let intersect_fix_range = fix diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs index a34f0a3c929..405860c5529 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs @@ -715,6 +715,7 @@ impl GlobalState { error!("FetchWorkspaceError: {e}"); } self.wants_to_switch = Some("fetched workspace".to_owned()); + self.diagnostics.clear_check_all(); (Progress::End, None) } }; @@ -956,7 +957,7 @@ impl GlobalState { fn handle_flycheck_msg(&mut self, message: FlycheckMessage) { match message { - FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic } => { + FlycheckMessage::AddDiagnostic { id, workspace_root, diagnostic, package_id } => { let snap = self.snapshot(); let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp( &self.config.diagnostics_map(None), @@ -968,6 +969,7 @@ impl GlobalState { match url_to_file_id(&self.vfs.read().0, &diag.url) { Ok(file_id) => self.diagnostics.add_check_diagnostic( id, + &package_id, file_id, diag.diagnostic, diag.fix, @@ -981,9 +983,12 @@ impl GlobalState { }; } } - - FlycheckMessage::ClearDiagnostics { id } => self.diagnostics.clear_check(id), - + FlycheckMessage::ClearDiagnostics { id, package_id: None } => { + self.diagnostics.clear_check(id) + } + FlycheckMessage::ClearDiagnostics { id, package_id: Some(package_id) } => { + self.diagnostics.clear_check_for_package(id, package_id) + } FlycheckMessage::Progress { id, progress } => { let (state, message) = match progress { flycheck::Progress::DidStart => (Progress::Begin, None), |