Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #17302 - mladedav:dm/fix-clear, r=Veykril
fix diagnostics clearing when flychecks run per-workspace
This might be causing #17300 or it's a different bug with the same functionality.
I wonder if the decision to clear diagnostics should stay in the main loop or maybe the flycheck itself should track it and tell the mainloop?
I have used a hash map but we could just as well use a vector since the IDs are `usizes` in some given range starting at 0. It would be probably faster but this just felt a bit cleaner and it allows us to change the ID to newtype later and we can just use a hasher that returns the underlying integer.
| -rw-r--r-- | crates/flycheck/src/lib.rs | 27 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/global_state.rs | 2 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/main_loop.rs | 14 |
3 files changed, 30 insertions, 13 deletions
diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 6d5ca8321e..afdc3e389b 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -163,6 +163,9 @@ pub enum Message { /// Request adding a diagnostic with fixes included to a file AddDiagnostic { id: usize, workspace_root: AbsPathBuf, diagnostic: Diagnostic }, + /// Request clearing all previous diagnostics + ClearDiagnostics { id: usize }, + /// Request check progress notification to client Progress { /// Flycheck instance ID @@ -180,6 +183,9 @@ impl fmt::Debug for Message { .field("workspace_root", workspace_root) .field("diagnostic_code", &diagnostic.code.as_ref().map(|it| &it.code)) .finish(), + Message::ClearDiagnostics { id } => { + f.debug_struct("ClearDiagnostics").field("id", id).finish() + } Message::Progress { id, progress } => { f.debug_struct("Progress").field("id", id).field("progress", progress).finish() } @@ -220,6 +226,8 @@ struct FlycheckActor { command_handle: Option<CommandHandle<CargoCheckMessage>>, /// The receiver side of the channel mentioned above. command_receiver: Option<Receiver<CargoCheckMessage>>, + + status: FlycheckStatus, } enum Event { @@ -227,6 +235,13 @@ enum Event { CheckEvent(Option<CargoCheckMessage>), } +#[derive(PartialEq)] +enum FlycheckStatus { + Started, + DiagnosticSent, + Finished, +} + const SAVED_FILE_PLACEHOLDER: &str = "$saved_file"; impl FlycheckActor { @@ -248,6 +263,7 @@ impl FlycheckActor { manifest_path, command_handle: None, command_receiver: None, + status: FlycheckStatus::Finished, } } @@ -298,12 +314,14 @@ 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: {} error={}", formatted_command, error ))); + self.status = FlycheckStatus::Finished; } } } @@ -323,7 +341,11 @@ impl FlycheckActor { error ); } + if self.status == FlycheckStatus::Started { + self.send(Message::ClearDiagnostics { id: self.id }); + } self.report_progress(Progress::DidFinish(res)); + self.status = FlycheckStatus::Finished; } Event::CheckEvent(Some(message)) => match message { CargoCheckMessage::CompilerArtifact(msg) => { @@ -341,11 +363,15 @@ impl FlycheckActor { message = msg.message, "diagnostic received" ); + if self.status == FlycheckStatus::Started { + self.send(Message::ClearDiagnostics { id: self.id }); + } self.send(Message::AddDiagnostic { id: self.id, workspace_root: self.root.clone(), diagnostic: msg, }); + self.status = FlycheckStatus::DiagnosticSent; } }, } @@ -362,6 +388,7 @@ impl FlycheckActor { ); command_handle.cancel(); self.report_progress(Progress::DidCancel); + self.status = FlycheckStatus::Finished; } } diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 79b87ecd58..f64e66183d 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -87,7 +87,6 @@ pub(crate) struct GlobalState { pub(crate) flycheck_sender: Sender<flycheck::Message>, pub(crate) flycheck_receiver: Receiver<flycheck::Message>, pub(crate) last_flycheck_error: Option<String>, - pub(crate) diagnostics_received: bool, // Test explorer pub(crate) test_run_session: Option<Vec<flycheck::CargoTestHandle>>, @@ -225,7 +224,6 @@ impl GlobalState { flycheck_sender, flycheck_receiver, last_flycheck_error: None, - diagnostics_received: false, test_run_session: None, test_run_sender, diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 7acd302867..193b3fdd4a 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -804,10 +804,6 @@ impl GlobalState { fn handle_flycheck_msg(&mut self, message: flycheck::Message) { match message { flycheck::Message::AddDiagnostic { id, workspace_root, diagnostic } => { - if !self.diagnostics_received { - self.diagnostics.clear_check(id); - self.diagnostics_received = true; - } let snap = self.snapshot(); let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp( &self.config.diagnostics_map(), @@ -833,12 +829,11 @@ impl GlobalState { } } + flycheck::Message::ClearDiagnostics { id } => self.diagnostics.clear_check(id), + flycheck::Message::Progress { id, progress } => { let (state, message) = match progress { - flycheck::Progress::DidStart => { - self.diagnostics_received = false; - (Progress::Begin, None) - } + flycheck::Progress::DidStart => (Progress::Begin, None), flycheck::Progress::DidCheckCrate(target) => (Progress::Report, Some(target)), flycheck::Progress::DidCancel => { self.last_flycheck_error = None; @@ -852,9 +847,6 @@ impl GlobalState { flycheck::Progress::DidFinish(result) => { self.last_flycheck_error = result.err().map(|err| format!("cargo check failed to start: {err}")); - if !self.diagnostics_received { - self.diagnostics.clear_check(id); - } (Progress::End, None) } }; |