mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12431 - Veykril:request-retry, r=Veykril

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

With this PR we now retry requests if they get cancelled due to document changes.

This fixes the completions problem we have where completions seem to randomly disappear, see https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Frust-analyzer/topic/Completions.20not.20always.20appearing

Fixes https://github.com/rust-lang/rust-analyzer/issues/10187
Fixes https://github.com/rust-lang/rust-analyzer/issues/7560
Fixes https://github.com/rust-lang/rust-analyzer/issues/12153
This commit is contained in:
bors 2022-06-01 10:18:50 +00:00
commit 4f5c7aafff
6 changed files with 70 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/rust-analyzer/src/bin/main.rs
View File

@ -157,7 +157,7 @@ fn run_server() -> Result<()> {
let (initialize_id, initialize_params) = connection.initialize_start()?; let (initialize_id, initialize_params) = connection.initialize_start()?;
tracing::info!("InitializeParams: {}", initialize_params); tracing::info!("InitializeParams: {}", initialize_params);
let 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 let root_path = match initialize_params
.root_uri .root_uri

93
crates/rust-analyzer/src/dispatch.rs
View File

@ -1,12 +1,12 @@
//! See [RequestDispatcher]. //! See [RequestDispatcher].
use std::{fmt, panic, thread}; use std::{fmt, panic, thread};
use ide::Cancelled;
use lsp_server::ExtractError; use lsp_server::ExtractError;
use serde::{de::DeserializeOwned, Serialize}; use serde::{de::DeserializeOwned, Serialize};
use crate::{ use crate::{
global_state::{GlobalState, GlobalStateSnapshot}, global_state::{GlobalState, GlobalStateSnapshot},
lsp_utils::is_cancelled,
main_loop::Task, main_loop::Task,
LspError, Result, LspError, Result,
}; };
@ -37,38 +37,40 @@ impl<'a> RequestDispatcher<'a> {
pub(crate) fn on_sync_mut<R>( pub(crate) fn on_sync_mut<R>(
&mut self, &mut self,
f: fn(&mut GlobalState, R::Params) -> Result<R::Result>, f: fn(&mut GlobalState, R::Params) -> Result<R::Result>,
) -> Result<&mut Self> ) -> &mut Self
where where
R: lsp_types::request::Request, R: lsp_types::request::Request,
R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug, R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug,
R::Result: Serialize, R::Result: Serialize,
{ {
let (id, params, panic_context) = match self.parse::<R>() { let (req, params, panic_context) = match self.parse::<R>() {
Some(it) => it, Some(it) => it,
None => return Ok(self), None => return self,
}; };
let _pctx = stdx::panic_context::enter(panic_context); let result = {
let _pctx = stdx::panic_context::enter(panic_context);
f(self.global_state, params)
};
if let Ok(response) = result_to_response::<R>(req.id.clone(), result) {
self.global_state.respond(response);
}
let result = f(self.global_state, params); self
let response = result_to_response::<R>(id, result);
self.global_state.respond(response);
Ok(self)
} }
/// Dispatches the request onto the current thread. /// Dispatches the request onto the current thread.
pub(crate) fn on_sync<R>( pub(crate) fn on_sync<R>(
&mut self, &mut self,
f: fn(GlobalStateSnapshot, R::Params) -> Result<R::Result>, f: fn(GlobalStateSnapshot, R::Params) -> Result<R::Result>,
) -> Result<&mut Self> ) -> &mut Self
where where
R: lsp_types::request::Request + 'static, R: lsp_types::request::Request,
R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug, R::Params: DeserializeOwned + panic::UnwindSafe + fmt::Debug,
R::Result: Serialize, R::Result: Serialize,
{ {
let (id, params, panic_context) = match self.parse::<R>() { let (req, params, panic_context) = match self.parse::<R>() {
Some(it) => it, Some(it) => it,
None => return Ok(self), None => return self,
}; };
let global_state_snapshot = self.global_state.snapshot(); let global_state_snapshot = self.global_state.snapshot();
@ -76,10 +78,12 @@ pub(crate) fn on_sync<R>(
let _pctx = stdx::panic_context::enter(panic_context); let _pctx = stdx::panic_context::enter(panic_context);
f(global_state_snapshot, params) f(global_state_snapshot, params)
}); });
let response = thread_result_to_response::<R>(id, result);
self.global_state.respond(response); if let Ok(response) = thread_result_to_response::<R>(req.id.clone(), result) {
Ok(self) self.global_state.respond(response);
}
self
} }
/// Dispatches the request onto thread pool /// Dispatches the request onto thread pool
@ -92,7 +96,7 @@ pub(crate) fn on<R>(
R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
R::Result: Serialize, R::Result: Serialize,
{ {
let (id, params, panic_context) = match self.parse::<R>() { let (req, params, panic_context) = match self.parse::<R>() {
Some(it) => it, Some(it) => it,
None => return self, None => return self,
}; };
@ -104,8 +108,10 @@ pub(crate) fn on<R>(
let _pctx = stdx::panic_context::enter(panic_context); let _pctx = stdx::panic_context::enter(panic_context);
f(world, params) f(world, params)
}); });
let response = thread_result_to_response::<R>(id, result); match thread_result_to_response::<R>(req.id.clone(), result) {
Task::Response(response) Ok(response) => Task::Response(response),
Err(_) => Task::Retry(req),
}
} }
}); });
@ -124,7 +130,7 @@ pub(crate) fn finish(&mut self) {
} }
} }
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 where
R: lsp_types::request::Request, R: lsp_types::request::Request,
R::Params: DeserializeOwned + fmt::Debug, R::Params: DeserializeOwned + fmt::Debug,
@ -134,12 +140,12 @@ fn parse<R>(&mut self) -> Option<(lsp_server::RequestId, R::Params, String)>
_ => return None, _ => return None,
}; };
let res = crate::from_json(R::METHOD, req.params); let res = crate::from_json(R::METHOD, &req.params);
match res { match res {
Ok(params) => { Ok(params) => {
let panic_context = let panic_context =
format!("\nversion: {}\nrequest: {} {:#?}", env!("REV"), R::METHOD, params); format!("\nversion: {}\nrequest: {} {:#?}", env!("REV"), R::METHOD, params);
Some((req.id, params, panic_context)) Some((req, params, panic_context))
} }
Err(err) => { Err(err) => {
let response = lsp_server::Response::new_err( let response = lsp_server::Response::new_err(
@ -157,7 +163,7 @@ fn parse<R>(&mut self) -> Option<(lsp_server::RequestId, R::Params, String)>
fn thread_result_to_response<R>( fn thread_result_to_response<R>(
id: lsp_server::RequestId, id: lsp_server::RequestId,
result: thread::Result<Result<R::Result>>, result: thread::Result<Result<R::Result>>,
) -> lsp_server::Response ) -> Result<lsp_server::Response, Cancelled>
where where
R: lsp_types::request::Request, R: lsp_types::request::Request,
R::Params: DeserializeOwned, R::Params: DeserializeOwned,
@ -166,19 +172,22 @@ fn thread_result_to_response<R>(
match result { match result {
Ok(result) => result_to_response::<R>(id, result), Ok(result) => result_to_response::<R>(id, result),
Err(panic) => { Err(panic) => {
let mut message = "request handler panicked".to_string();
let panic_message = panic let panic_message = panic
.downcast_ref::<String>() .downcast_ref::<String>()
.map(String::as_str) .map(String::as_str)
.or_else(|| panic.downcast_ref::<&str>().copied()); .or_else(|| panic.downcast_ref::<&str>().copied());
let mut message = "request handler panicked".to_string();
if let Some(panic_message) = panic_message { if let Some(panic_message) = panic_message {
message.push_str(": "); message.push_str(": ");
message.push_str(panic_message) 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,
))
} }
} }
} }
@ -186,33 +195,27 @@ fn thread_result_to_response<R>(
fn result_to_response<R>( fn result_to_response<R>(
id: lsp_server::RequestId, id: lsp_server::RequestId,
result: Result<R::Result>, result: Result<R::Result>,
) -> lsp_server::Response ) -> Result<lsp_server::Response, Cancelled>
where where
R: lsp_types::request::Request, R: lsp_types::request::Request,
R::Params: DeserializeOwned, R::Params: DeserializeOwned,
R::Result: Serialize, R::Result: Serialize,
{ {
match result { let res = match result {
Ok(resp) => lsp_server::Response::new_ok(id, &resp), Ok(resp) => lsp_server::Response::new_ok(id, &resp),
Err(e) => match e.downcast::<LspError>() { Err(e) => match e.downcast::<LspError>() {
Ok(lsp_error) => lsp_server::Response::new_err(id, lsp_error.code, lsp_error.message), Ok(lsp_error) => lsp_server::Response::new_err(id, lsp_error.code, lsp_error.message),
Err(e) => { Err(e) => match e.downcast::<Cancelled>() {
if is_cancelled(&*e) { Ok(cancelled) => return Err(*cancelled),
lsp_server::Response::new_err( Err(e) => lsp_server::Response::new_err(
id, id,
lsp_server::ErrorCode::ContentModified as i32, lsp_server::ErrorCode::InternalError as i32,
"content modified".to_string(), e.to_string(),
) ),
} else { },
lsp_server::Response::new_err(
id,
lsp_server::ErrorCode::InternalError as i32,
e.to_string(),
)
}
}
}, },
} };
Ok(res)
} }
pub(crate) struct NotificationDispatcher<'a> { pub(crate) struct NotificationDispatcher<'a> {

2
crates/rust-analyzer/src/from_proto.rs
View File

@ -91,7 +91,7 @@ pub(crate) fn annotation(
) -> Result<Annotation> { ) -> Result<Annotation> {
let data = let data =
code_lens.data.ok_or_else(|| invalid_params_error("code lens without data".to_string()))?; 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 { match resolve {
lsp_ext::CodeLensResolveData::Impls(params) => { lsp_ext::CodeLensResolveData::Impls(params) => {

2
crates/rust-analyzer/src/lib.rs
View File

@ -49,7 +49,7 @@ macro_rules! eprintln {
pub type Error = Box<dyn std::error::Error + Send + Sync>; pub type Error = Box<dyn std::error::Error + Send + Sync>;
pub type Result<T, E = Error> = std::result::Result<T, E>; 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()) let res = serde_json::from_value(json.clone())
.map_err(|e| format!("Failed to deserialize {}: {}; {}", what, e, json))?; .map_err(|e| format!("Failed to deserialize {}: {}; {}", what, e, json))?;
Ok(res) Ok(res)

7
crates/rust-analyzer/src/lsp_utils.rs
View File

@ -1,7 +1,6 @@
//! Utilities for LSP-related boilerplate code. //! 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 lsp_server::Notification;
use crate::{ use crate::{
@ -15,10 +14,6 @@ pub(crate) fn invalid_params_error(message: String) -> LspError {
LspError { code: lsp_server::ErrorCode::InvalidParams as i32, message } 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>( pub(crate) fn notification_is<N: lsp_types::notification::Notification>(
notification: &Notification, notification: &Notification,
) -> bool { ) -> bool {

35
crates/rust-analyzer/src/main_loop.rs
View File

@ -8,7 +8,7 @@
use always_assert::always; use always_assert::always;
use crossbeam_channel::{select, Receiver}; 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_server::{Connection, Notification, Request};
use lsp_types::notification::Notification as _; use lsp_types::notification::Notification as _;
use vfs::{ChangeKind, FileId}; use vfs::{ChangeKind, FileId};
@ -19,7 +19,7 @@
from_proto, from_proto,
global_state::{file_id_to_url, url_to_file_id, GlobalState}, global_state::{file_id_to_url, url_to_file_id, GlobalState},
handlers, lsp_ext, handlers, lsp_ext,
lsp_utils::{apply_document_changes, is_cancelled, notification_is, Progress}, lsp_utils::{apply_document_changes, notification_is, Progress},
mem_docs::DocumentData, mem_docs::DocumentData,
reload::{self, BuildDataProgress, ProjectWorkspaceProgress}, reload::{self, BuildDataProgress, ProjectWorkspaceProgress},
Result, Result,
@ -60,6 +60,7 @@ enum Event {
#[derive(Debug)] #[derive(Debug)]
pub(crate) enum Task { pub(crate) enum Task {
Response(lsp_server::Response), Response(lsp_server::Response),
Retry(lsp_server::Request),
Diagnostics(Vec<(FileId, Vec<lsp_types::Diagnostic>)>), Diagnostics(Vec<(FileId, Vec<lsp_types::Diagnostic>)>),
PrimeCaches(PrimeCachesProgress), PrimeCaches(PrimeCachesProgress),
FetchWorkspace(ProjectWorkspaceProgress), FetchWorkspace(ProjectWorkspaceProgress),
@ -196,7 +197,7 @@ fn handle_event(&mut self, event: Event) -> Result<()> {
let was_quiescent = self.is_quiescent(); let was_quiescent = self.is_quiescent();
match event { match event {
Event::Lsp(msg) => match msg { 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) => { lsp_server::Message::Notification(not) => {
self.on_notification(not)?; self.on_notification(not)?;
} }
@ -208,6 +209,7 @@ fn handle_event(&mut self, event: Event) -> Result<()> {
loop { loop {
match task { match task {
Task::Response(response) => self.respond(response), Task::Response(response) => self.respond(response),
Task::Retry(req) => self.on_request(loop_start, req),
Task::Diagnostics(diagnostics_per_file) => { Task::Diagnostics(diagnostics_per_file) => {
for (file_id, diagnostics) in diagnostics_per_file { for (file_id, diagnostics) in diagnostics_per_file {
self.diagnostics.set_native_diagnostics(file_id, diagnostics) self.diagnostics.set_native_diagnostics(file_id, diagnostics)
@ -553,7 +555,7 @@ fn handle_event(&mut self, event: Event) -> Result<()> {
Ok(()) 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); self.register_request(&req, request_received);
if self.shutdown_requested { if self.shutdown_requested {
@ -562,8 +564,7 @@ fn on_request(&mut self, request_received: Instant, req: Request) -> Result<()>
lsp_server::ErrorCode::InvalidRequest as i32, lsp_server::ErrorCode::InvalidRequest as i32,
"Shutdown already requested.".to_owned(), "Shutdown already requested.".to_owned(),
)); ));
return;
return Ok(());
} }
// Avoid flashing a bunch of unresolved references during initial load. // Avoid flashing a bunch of unresolved references during initial load.
@ -573,21 +574,21 @@ fn on_request(&mut self, request_received: Instant, req: Request) -> Result<()>
lsp_server::ErrorCode::ContentModified as i32, lsp_server::ErrorCode::ContentModified as i32,
"waiting for cargo metadata or cargo check".to_owned(), "waiting for cargo metadata or cargo check".to_owned(),
)); ));
return Ok(()); return;
} }
RequestDispatcher { req: Some(req), global_state: self } RequestDispatcher { req: Some(req), global_state: self }
.on_sync_mut::<lsp_types::request::Shutdown>(|s, ()| { .on_sync_mut::<lsp_types::request::Shutdown>(|s, ()| {
s.shutdown_requested = true; s.shutdown_requested = true;
Ok(()) Ok(())
})? })
.on_sync_mut::<lsp_ext::ReloadWorkspace>(handlers::handle_workspace_reload)? .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::MemoryUsage>(handlers::handle_memory_usage)
.on_sync_mut::<lsp_ext::ShuffleCrateGraph>(handlers::handle_shuffle_crate_graph)? .on_sync_mut::<lsp_ext::ShuffleCrateGraph>(handlers::handle_shuffle_crate_graph)
.on_sync::<lsp_ext::JoinLines>(handlers::handle_join_lines)? .on_sync::<lsp_ext::JoinLines>(handlers::handle_join_lines)
.on_sync::<lsp_ext::OnEnter>(handlers::handle_on_enter)? .on_sync::<lsp_ext::OnEnter>(handlers::handle_on_enter)
.on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)? .on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)
.on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)? .on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)
.on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status) .on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status)
.on::<lsp_ext::SyntaxTree>(handlers::handle_syntax_tree) .on::<lsp_ext::SyntaxTree>(handlers::handle_syntax_tree)
.on::<lsp_ext::ViewHir>(handlers::handle_view_hir) .on::<lsp_ext::ViewHir>(handlers::handle_view_hir)
@ -644,8 +645,8 @@ fn on_request(&mut self, request_received: Instant, req: Request) -> Result<()>
.on::<lsp_types::request::WillRenameFiles>(handlers::handle_will_rename_files) .on::<lsp_types::request::WillRenameFiles>(handlers::handle_will_rename_files)
.on::<lsp_ext::Ssr>(handlers::handle_ssr) .on::<lsp_ext::Ssr>(handlers::handle_ssr)
.finish(); .finish();
Ok(())
} }
fn on_notification(&mut self, not: Notification) -> Result<()> { fn on_notification(&mut self, not: Notification) -> Result<()> {
NotificationDispatcher { not: Some(not), global_state: self } NotificationDispatcher { not: Some(not), global_state: self }
.on::<lsp_types::notification::Cancel>(|this, params| { .on::<lsp_types::notification::Cancel>(|this, params| {
@ -792,7 +793,7 @@ fn update_diagnostics(&mut self) {
.filter_map(|file_id| { .filter_map(|file_id| {
handlers::publish_diagnostics(&snapshot, file_id) handlers::publish_diagnostics(&snapshot, file_id)
.map_err(|err| { .map_err(|err| {
if !is_cancelled(&*err) { if err.is::<Cancelled>() {
tracing::error!("failed to compute diagnostics: {:?}", err); tracing::error!("failed to compute diagnostics: {:?}", err);
} }
}) })