Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
93 changes: 48 additions & 45 deletions crates/rust-analyzer/src/dispatch.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -37,49 +37,53 @@ 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 _pctx = stdx::panic_context::enter(panic_context);

let result = f(self.global_state, params);
let response = result_to_response::<R>(id, result);
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);
}

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)
if let Ok(response) = thread_result_to_response::<R>(req.id.clone(), result) {
self.global_state.respond(response);
}

self
}

/// Dispatches the request onto thread pool
Expand All @@ -92,7 +96,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,
};
Expand All @@ -104,8 +108,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),
}
}
});

Expand All @@ -124,7 +130,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,
Expand All @@ -134,12 +140,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(
Expand All @@ -157,7 +163,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,
Expand All @@ -166,53 +172,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> {
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/from_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 1 addition & 6 deletions crates/rust-analyzer/src/lsp_utils.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -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 {
Expand Down
35 changes: 18 additions & 17 deletions crates/rust-analyzer/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)?;
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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);
}
})
Expand Down