Auto merge of rust-lang#17302 - mladedav:dm/fix-clear, r=Veykril

fix diagnostics clearing when flychecks run per-workspace

This might be causing rust-lang#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.

Author: bors

src/tools/rust-analyzer/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,13 +226,22 @@ struct FlycheckActor {
     command_handle: Option<CommandHandle<CargoCheckMessage>>,
     /// The receiver side of the channel mentioned above.
     command_receiver: Option<Receiver<CargoCheckMessage>>,
+
+    status: FlycheckStatus,
 }
 
 enum Event {
     RequestStateChange(StateChange),
     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;
         }
     }
 

src/tools/rust-analyzer/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,

src/tools/rust-analyzer/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)
                     }
                 };