fix: Fix completions disappearing when typing two keys in quick succession

Veykril · Veykril · commit e2da96757887 · 2022-06-01T00:20:47.000+02:00
diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs
@@ -157,7 +157,7 @@ fn run_server() -> Result<()> {
     let (initialize_id, initialize_params) = connection.initialize_start()?;
     tracing::info!("InitializeParams: {}", initialize_params);
     let initialize_params =
-        from_json::<lsp_types::InitializeParams>("InitializeParams", initialize_params)?;
+        from_json::<lsp_types::InitializeParams>("InitializeParams", &initialize_params)?;
 
     let root_path = match initialize_params
         .root_uri
diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs
@@ -1,12 +1,12 @@
 //! See [RequestDispatcher].
 use std::{fmt, panic, thread};
 
+use ide::Cancelled;
 use lsp_server::ExtractError;
 use serde::{de::DeserializeOwned, Serialize};
 
 use crate::{
     global_state::{GlobalState, GlobalStateSnapshot},
-    lsp_utils::is_cancelled,
     main_loop::Task,
     LspError, Result,
 };
@@ -37,49 +37,55 @@ impl<'a> RequestDispatcher<'a> {
     pub(crate) fn on_sync_mut<R>(
         &mut self,
         f: fn(&mut GlobalState, R::Params) -> Result<R::Result>,
-    ) -> Result<&mut Self>
+    ) -> &mut Self
     where
         R: lsp_types::request::Request,
         R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug,
         R::Result: Serialize,
     {
-        let (id, params, panic_context) = match self.parse::<R>() {
+        let (req, params, panic_context) = match self.parse::<R>() {
             Some(it) => it,
-            None => return Ok(self),
+            None => return self,
+        };
+        let result = {
+            let _pctx = stdx::panic_context::enter(panic_context);
+            f(self.global_state, params)
+        };
+        match result_to_response::<R>(req.id.clone(), result) {
+            Ok(response) => self.global_state.respond(response),
+            Err(_) => self.global_state.task_pool.handle.send_retry(req),
         };
-        let _pctx = stdx::panic_context::enter(panic_context);
-
-        let result = f(self.global_state, params);
-        let response = result_to_response::<R>(id, result);
 
-        self.global_state.respond(response);
-        Ok(self)
+        self
     }
 
     /// Dispatches the request onto the current thread.
     pub(crate) fn on_sync<R>(
         &mut self,
         f: fn(GlobalStateSnapshot, R::Params) -> Result<R::Result>,
-    ) -> Result<&mut Self>
+    ) -> &mut Self
     where
-        R: lsp_types::request::Request + 'static,
+        R: lsp_types::request::Request,
         R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug,
         R::Result: Serialize,
     {
-        let (id, params, panic_context) = match self.parse::<R>() {
+        let (req, params, panic_context) = match self.parse::<R>() {
             Some(it) => it,
-            None => return Ok(self),
+            None => return self,
         };
         let global_state_snapshot = self.global_state.snapshot();
 
         let result = panic::catch_unwind(move || {
             let _pctx = stdx::panic_context::enter(panic_context);
             f(global_state_snapshot, params)
         });
-        let response = thread_result_to_response::<R>(id, result);
 
-        self.global_state.respond(response);
-        Ok(self)
+        match thread_result_to_response::<R>(req.id.clone(), result) {
+            Ok(response) => self.global_state.respond(response),
+            Err(_) => self.global_state.task_pool.handle.send_retry(req),
+        };
+
+        self
     }
 
     /// Dispatches the request onto thread pool
@@ -92,7 +98,7 @@ impl<'a> RequestDispatcher<'a> {
         R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
         R::Result: Serialize,
     {
-        let (id, params, panic_context) = match self.parse::<R>() {
+        let (req, params, panic_context) = match self.parse::<R>() {
             Some(it) => it,
             None => return self,
         };
@@ -104,8 +110,10 @@ impl<'a> RequestDispatcher<'a> {
                     let _pctx = stdx::panic_context::enter(panic_context);
                     f(world, params)
                 });
-                let response = thread_result_to_response::<R>(id, result);
-                Task::Response(response)
+                match thread_result_to_response::<R>(req.id.clone(), result) {
+                    Ok(response) => Task::Response(response),
+                    Err(_) => Task::Retry(req),
+                }
             }
         });
 
@@ -124,7 +132,7 @@ impl<'a> RequestDispatcher<'a> {
         }
     }
 
-    fn parse<R>(&mut self) -> Option<(lsp_server::RequestId, R::Params, String)>
+    fn parse<R>(&mut self) -> Option<(lsp_server::Request, R::Params, String)>
     where
         R: lsp_types::request::Request,
         R::Params: DeserializeOwned + fmt::Debug,
@@ -134,12 +142,12 @@ impl<'a> RequestDispatcher<'a> {
             _ => return None,
         };
 
-        let res = crate::from_json(R::METHOD, req.params);
+        let res = crate::from_json(R::METHOD, &req.params);
         match res {
             Ok(params) => {
                 let panic_context =
                     format!("\nversion: {}\nrequest: {} {:#?}", env!("REV"), R::METHOD, params);
-                Some((req.id, params, panic_context))
+                Some((req, params, panic_context))
             }
             Err(err) => {
                 let response = lsp_server::Response::new_err(
@@ -157,7 +165,7 @@ impl<'a> RequestDispatcher<'a> {
 fn thread_result_to_response<R>(
     id: lsp_server::RequestId,
     result: thread::Result<Result<R::Result>>,
-) -> lsp_server::Response
+) -> Result<lsp_server::Response, Cancelled>
 where
     R: lsp_types::request::Request,
     R::Params: DeserializeOwned,
@@ -166,53 +174,50 @@ where
     match result {
         Ok(result) => result_to_response::<R>(id, result),
         Err(panic) => {
-            let mut message = "request handler panicked".to_string();
-
             let panic_message = panic
                 .downcast_ref::<String>()
                 .map(String::as_str)
                 .or_else(|| panic.downcast_ref::<&str>().copied());
 
+            let mut message = "request handler panicked".to_string();
             if let Some(panic_message) = panic_message {
                 message.push_str(": ");
                 message.push_str(panic_message)
             };
 
-            lsp_server::Response::new_err(id, lsp_server::ErrorCode::InternalError as i32, message)
+            Ok(lsp_server::Response::new_err(
+                id,
+                lsp_server::ErrorCode::InternalError as i32,
+                message,
+            ))
         }
     }
 }
 
 fn result_to_response<R>(
     id: lsp_server::RequestId,
     result: Result<R::Result>,
-) -> lsp_server::Response
+) -> Result<lsp_server::Response, Cancelled>
 where
     R: lsp_types::request::Request,
     R::Params: DeserializeOwned,
     R::Result: Serialize,
 {
-    match result {
+    let res = match result {
         Ok(resp) => lsp_server::Response::new_ok(id, &resp),
         Err(e) => match e.downcast::<LspError>() {
             Ok(lsp_error) => lsp_server::Response::new_err(id, lsp_error.code, lsp_error.message),
-            Err(e) => {
-                if is_cancelled(&*e) {
-                    lsp_server::Response::new_err(
-                        id,
-                        lsp_server::ErrorCode::ContentModified as i32,
-                        "content modified".to_string(),
-                    )
-                } else {
-                    lsp_server::Response::new_err(
-                        id,
-                        lsp_server::ErrorCode::InternalError as i32,
-                        e.to_string(),
-                    )
-                }
-            }
+            Err(e) => match e.downcast::<Cancelled>() {
+                Ok(cancelled) => return Err(*cancelled),
+                Err(e) => lsp_server::Response::new_err(
+                    id,
+                    lsp_server::ErrorCode::InternalError as i32,
+                    e.to_string(),
+                ),
+            },
         },
-    }
+    };
+    Ok(res)
 }
 
 pub(crate) struct NotificationDispatcher<'a> {
diff --git a/crates/rust-analyzer/src/from_proto.rs b/crates/rust-analyzer/src/from_proto.rs
@@ -91,7 +91,7 @@ pub(crate) fn annotation(
 ) -> Result<Annotation> {
     let data =
         code_lens.data.ok_or_else(|| invalid_params_error("code lens without data".to_string()))?;
-    let resolve = from_json::<lsp_ext::CodeLensResolveData>("CodeLensResolveData", data)?;
+    let resolve = from_json::<lsp_ext::CodeLensResolveData>("CodeLensResolveData", &data)?;
 
     match resolve {
         lsp_ext::CodeLensResolveData::Impls(params) => {
diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs
@@ -49,7 +49,7 @@ pub use crate::{caps::server_capabilities, main_loop::main_loop};
 pub type Error = Box<dyn std::error::Error + Send + Sync>;
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
-pub fn from_json<T: DeserializeOwned>(what: &'static str, json: serde_json::Value) -> Result<T> {
+pub fn from_json<T: DeserializeOwned>(what: &'static str, json: &serde_json::Value) -> Result<T> {
     let res = serde_json::from_value(json.clone())
         .map_err(|e| format!("Failed to deserialize {}: {}; {}", what, e, json))?;
     Ok(res)
diff --git a/crates/rust-analyzer/src/lsp_utils.rs b/crates/rust-analyzer/src/lsp_utils.rs
@@ -1,7 +1,6 @@
 //! Utilities for LSP-related boilerplate code.
-use std::{error::Error, ops::Range, sync::Arc};
+use std::{ops::Range, sync::Arc};
 
-use ide_db::base_db::Cancelled;
 use lsp_server::Notification;
 
 use crate::{
@@ -15,10 +14,6 @@ pub(crate) fn invalid_params_error(message: String) -> LspError {
     LspError { code: lsp_server::ErrorCode::InvalidParams as i32, message }
 }
 
-pub(crate) fn is_cancelled(e: &(dyn Error + 'static)) -> bool {
-    e.downcast_ref::<Cancelled>().is_some()
-}
-
 pub(crate) fn notification_is<N: lsp_types::notification::Notification>(
     notification: &Notification,
 ) -> bool {
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
@@ -8,7 +8,7 @@ use std::{
 
 use always_assert::always;
 use crossbeam_channel::{select, Receiver};
-use ide_db::base_db::{SourceDatabaseExt, VfsPath};
+use ide_db::base_db::{Cancelled, SourceDatabaseExt, VfsPath};
 use lsp_server::{Connection, Notification, Request};
 use lsp_types::notification::Notification as _;
 use vfs::{ChangeKind, FileId};
@@ -19,7 +19,7 @@ use crate::{
     from_proto,
     global_state::{file_id_to_url, url_to_file_id, GlobalState},
     handlers, lsp_ext,
-    lsp_utils::{apply_document_changes, is_cancelled, notification_is, Progress},
+    lsp_utils::{apply_document_changes, notification_is, Progress},
     mem_docs::DocumentData,
     reload::{self, BuildDataProgress, ProjectWorkspaceProgress},
     Result,
@@ -60,6 +60,7 @@ enum Event {
 #[derive(Debug)]
 pub(crate) enum Task {
     Response(lsp_server::Response),
+    Retry(lsp_server::Request),
     Diagnostics(Vec<(FileId, Vec<lsp_types::Diagnostic>)>),
     PrimeCaches(PrimeCachesProgress),
     FetchWorkspace(ProjectWorkspaceProgress),
@@ -172,13 +173,13 @@ impl GlobalState {
                 msg.ok().map(Event::Lsp),
 
             recv(self.task_pool.receiver) -> task =>
-                Some(Event::Task(task.unwrap())),
+                task.ok().map(Event::Task),
 
             recv(self.loader.receiver) -> task =>
-                Some(Event::Vfs(task.unwrap())),
+                task.ok().map(Event::Vfs),
 
             recv(self.flycheck_receiver) -> task =>
-                Some(Event::Flycheck(task.unwrap())),
+                task.ok().map(Event::Flycheck),
         }
     }
 
@@ -196,7 +197,7 @@ impl GlobalState {
         let was_quiescent = self.is_quiescent();
         match event {
             Event::Lsp(msg) => match msg {
-                lsp_server::Message::Request(req) => self.on_request(loop_start, req)?,
+                lsp_server::Message::Request(req) => self.on_request(loop_start, req),
                 lsp_server::Message::Notification(not) => {
                     self.on_notification(not)?;
                 }
@@ -208,6 +209,7 @@ impl GlobalState {
                 loop {
                     match task {
                         Task::Response(response) => self.respond(response),
+                        Task::Retry(req) => self.on_request(loop_start, req),
                         Task::Diagnostics(diagnostics_per_file) => {
                             for (file_id, diagnostics) in diagnostics_per_file {
                                 self.diagnostics.set_native_diagnostics(file_id, diagnostics)
@@ -553,7 +555,7 @@ impl GlobalState {
         Ok(())
     }
 
-    fn on_request(&mut self, request_received: Instant, req: Request) -> Result<()> {
+    fn on_request(&mut self, request_received: Instant, req: Request) {
         self.register_request(&req, request_received);
 
         if self.shutdown_requested {
@@ -562,8 +564,7 @@ impl GlobalState {
                 lsp_server::ErrorCode::InvalidRequest as i32,
                 "Shutdown already requested.".to_owned(),
             ));
-
-            return Ok(());
+            return;
         }
 
         // Avoid flashing a bunch of unresolved references during initial load.
@@ -573,21 +574,21 @@ impl GlobalState {
                 lsp_server::ErrorCode::ContentModified as i32,
                 "waiting for cargo metadata or cargo check".to_owned(),
             ));
-            return Ok(());
+            return;
         }
 
         RequestDispatcher { req: Some(req), global_state: self }
             .on_sync_mut::<lsp_types::request::Shutdown>(|s, ()| {
                 s.shutdown_requested = true;
                 Ok(())
-            })?
-            .on_sync_mut::<lsp_ext::ReloadWorkspace>(handlers::handle_workspace_reload)?
-            .on_sync_mut::<lsp_ext::MemoryUsage>(handlers::handle_memory_usage)?
-            .on_sync_mut::<lsp_ext::ShuffleCrateGraph>(handlers::handle_shuffle_crate_graph)?
-            .on_sync::<lsp_ext::JoinLines>(handlers::handle_join_lines)?
-            .on_sync::<lsp_ext::OnEnter>(handlers::handle_on_enter)?
-            .on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)?
-            .on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)?
+            })
+            .on_sync_mut::<lsp_ext::ReloadWorkspace>(handlers::handle_workspace_reload)
+            .on_sync_mut::<lsp_ext::MemoryUsage>(handlers::handle_memory_usage)
+            .on_sync_mut::<lsp_ext::ShuffleCrateGraph>(handlers::handle_shuffle_crate_graph)
+            .on_sync::<lsp_ext::JoinLines>(handlers::handle_join_lines)
+            .on_sync::<lsp_ext::OnEnter>(handlers::handle_on_enter)
+            .on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)
+            .on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)
             .on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status)
             .on::<lsp_ext::SyntaxTree>(handlers::handle_syntax_tree)
             .on::<lsp_ext::ViewHir>(handlers::handle_view_hir)
@@ -644,8 +645,8 @@ impl GlobalState {
             .on::<lsp_types::request::WillRenameFiles>(handlers::handle_will_rename_files)
             .on::<lsp_ext::Ssr>(handlers::handle_ssr)
             .finish();
-        Ok(())
     }
+
     fn on_notification(&mut self, not: Notification) -> Result<()> {
         NotificationDispatcher { not: Some(not), global_state: self }
             .on::<lsp_types::notification::Cancel>(|this, params| {
@@ -792,7 +793,7 @@ impl GlobalState {
                 .filter_map(|file_id| {
                     handlers::publish_diagnostics(&snapshot, file_id)
                         .map_err(|err| {
-                            if !is_cancelled(&*err) {
+                            if err.is::<Cancelled>() {
                                 tracing::error!("failed to compute diagnostics: {:?}", err);
                             }
                         })
diff --git a/crates/rust-analyzer/src/task_pool.rs b/crates/rust-analyzer/src/task_pool.rs
