diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/dispatch.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/dispatch.rs index 7adaef4ff6e..cf3b8d331de 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/dispatch.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/dispatch.rs @@ -95,45 +95,8 @@ impl RequestDispatcher<'_> { self } - /// Dispatches a non-latency-sensitive request onto the thread pool - /// without retrying it if it panics. - pub(crate) fn on_no_retry( - &mut self, - f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result, - ) -> &mut Self - where - R: lsp_types::request::Request + 'static, - R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, - R::Result: Serialize, - { - let (req, params, panic_context) = match self.parse::() { - Some(it) => it, - None => return self, - }; - - self.global_state.task_pool.handle.spawn(ThreadIntent::Worker, { - let world = self.global_state.snapshot(); - move || { - let result = panic::catch_unwind(move || { - let _pctx = stdx::panic_context::enter(panic_context); - f(world, params) - }); - match thread_result_to_response::(req.id.clone(), result) { - Ok(response) => Task::Response(response), - Err(_) => Task::Response(lsp_server::Response::new_err( - req.id, - lsp_server::ErrorCode::ContentModified as i32, - "content modified".to_owned(), - )), - } - } - }); - - self - } - /// Dispatches a non-latency-sensitive request onto the thread pool. - pub(crate) fn on( + pub(crate) fn on( &mut self, f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result, ) -> &mut Self @@ -142,11 +105,11 @@ impl RequestDispatcher<'_> { R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, R::Result: Serialize, { - self.on_with_thread_intent::(ThreadIntent::Worker, f) + self.on_with_thread_intent::(ThreadIntent::Worker, f) } /// Dispatches a latency-sensitive request onto the thread pool. - pub(crate) fn on_latency_sensitive( + pub(crate) fn on_latency_sensitive( &mut self, f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result, ) -> &mut Self @@ -155,7 +118,7 @@ impl RequestDispatcher<'_> { R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, R::Result: Serialize, { - self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) + self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) } /// Formatting requests should never block on waiting a for task thread to open up, editors will wait @@ -170,7 +133,7 @@ impl RequestDispatcher<'_> { R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, R::Result: Serialize, { - self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) + self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) } pub(crate) fn finish(&mut self) { @@ -185,7 +148,7 @@ impl RequestDispatcher<'_> { } } - fn on_with_thread_intent( + fn on_with_thread_intent( &mut self, intent: ThreadIntent, f: fn(GlobalStateSnapshot, R::Params) -> anyhow::Result, @@ -215,7 +178,12 @@ impl RequestDispatcher<'_> { }); match thread_result_to_response::(req.id.clone(), result) { Ok(response) => Task::Response(response), - Err(_) => Task::Retry(req), + Err(_cancelled) if ALLOW_RETRYING => Task::Retry(req), + Err(_cancelled) => Task::Response(lsp_server::Response::new_err( + req.id, + lsp_server::ErrorCode::ContentModified as i32, + "content modified".to_owned(), + )), } }); diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs index 7810979c8ae..f5b3ef51035 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs @@ -488,6 +488,10 @@ impl GlobalStateSnapshot { Ok(res) } + pub(crate) fn file_version(&self, file_id: FileId) -> Option { + Some(self.mem_docs.get(self.vfs_read().file_path(file_id))?.version) + } + pub(crate) fn url_file_version(&self, url: &Url) -> Option { let path = from_proto::vfs_path(url).ok()?; Some(self.mem_docs.get(&path)?.version) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs index b61a968af14..90851ac7c56 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/handlers/request.rs @@ -934,16 +934,18 @@ pub(crate) fn handle_related_tests( pub(crate) fn handle_completion( snap: GlobalStateSnapshot, - params: lsp_types::CompletionParams, + lsp_types::CompletionParams { text_document_position, context,.. }: lsp_types::CompletionParams, ) -> anyhow::Result> { let _p = tracing::span!(tracing::Level::INFO, "handle_completion").entered(); - let text_document_position = params.text_document_position.clone(); - let position = from_proto::file_position(&snap, params.text_document_position)?; + let mut position = from_proto::file_position(&snap, text_document_position.clone())?; + let line_index = snap.file_line_index(position.file_id)?; let completion_trigger_character = - params.context.and_then(|ctx| ctx.trigger_character).and_then(|s| s.chars().next()); + context.and_then(|ctx| ctx.trigger_character).and_then(|s| s.chars().next()); let source_root = snap.analysis.source_root(position.file_id)?; let completion_config = &snap.config.completion(Some(source_root)); + // FIXME: We should fix up the position when retrying the cancelled request instead + position.offset = position.offset.min(line_index.index.len()); let items = match snap.analysis.completions( completion_config, position, @@ -952,10 +954,14 @@ pub(crate) fn handle_completion( None => return Ok(None), Some(items) => items, }; - let line_index = snap.file_line_index(position.file_id)?; - let items = - to_proto::completion_items(&snap.config, &line_index, text_document_position, items); + let items = to_proto::completion_items( + &snap.config, + &line_index, + snap.file_version(position.file_id), + text_document_position, + items, + ); let completion_list = lsp_types::CompletionList { is_incomplete: true, items }; Ok(Some(completion_list.into())) @@ -974,16 +980,16 @@ pub(crate) fn handle_completion_resolve( .into()); } - let data = match original_completion.data.take() { - Some(it) => it, - None => return Ok(original_completion), - }; + let Some(data) = original_completion.data.take() else { return Ok(original_completion) }; let resolve_data: lsp_ext::CompletionResolveData = serde_json::from_value(data)?; let file_id = from_proto::file_id(&snap, &resolve_data.position.text_document.uri)?; let line_index = snap.file_line_index(file_id)?; - let offset = from_proto::offset(&line_index, resolve_data.position.position)?; + // FIXME: We should fix up the position when retrying the cancelled request instead + let Ok(offset) = from_proto::offset(&line_index, resolve_data.position.position) else { + return Ok(original_completion); + }; let source_root = snap.analysis.source_root(file_id)?; let additional_edits = snap @@ -1240,8 +1246,11 @@ pub(crate) fn handle_code_action( frange, )?; for (index, assist) in assists.into_iter().enumerate() { - let resolve_data = - if code_action_resolve_cap { Some((index, params.clone())) } else { None }; + let resolve_data = if code_action_resolve_cap { + Some((index, params.clone(), snap.file_version(file_id))) + } else { + None + }; let code_action = to_proto::code_action(&snap, assist, resolve_data)?; // Check if the client supports the necessary `ResourceOperation`s. @@ -1280,12 +1289,14 @@ pub(crate) fn handle_code_action_resolve( mut code_action: lsp_ext::CodeAction, ) -> anyhow::Result { let _p = tracing::span!(tracing::Level::INFO, "handle_code_action_resolve").entered(); - let params = match code_action.data.take() { - Some(it) => it, - None => return Err(invalid_params_error("code action without data".to_owned()).into()), + let Some(params) = code_action.data.take() else { + return Err(invalid_params_error("code action without data".to_owned()).into()); }; let file_id = from_proto::file_id(&snap, ¶ms.code_action_params.text_document.uri)?; + if snap.file_version(file_id) != params.version { + return Err(invalid_params_error("stale code action".to_owned()).into()); + } let line_index = snap.file_line_index(file_id)?; let range = from_proto::text_range(&line_index, params.code_action_params.range)?; let frange = FileRange { file_id, range }; @@ -1411,12 +1422,11 @@ pub(crate) fn handle_code_lens( pub(crate) fn handle_code_lens_resolve( snap: GlobalStateSnapshot, - code_lens: CodeLens, + mut code_lens: CodeLens, ) -> anyhow::Result { - if code_lens.data.is_none() { - return Ok(code_lens); - } - let Some(annotation) = from_proto::annotation(&snap, code_lens.clone())? else { + let Some(data) = code_lens.data.take() else { return Ok(code_lens) }; + let resolve = serde_json::from_value::(data)?; + let Some(annotation) = from_proto::annotation(&snap, code_lens.range, resolve)? else { return Ok(code_lens); }; let annotation = snap.analysis.resolve_annotation(annotation)?; @@ -1495,6 +1505,10 @@ pub(crate) fn handle_inlay_hints( )?; let line_index = snap.file_line_index(file_id)?; let source_root = snap.analysis.source_root(file_id)?; + let range = TextRange::new( + range.start().min(line_index.index.len()), + range.end().min(line_index.index.len()), + ); let inlay_hints_config = snap.config.inlay_hints(Some(source_root)); Ok(Some( @@ -1522,8 +1536,12 @@ pub(crate) fn handle_inlay_hints_resolve( let Some(data) = original_hint.data.take() else { return Ok(original_hint) }; let resolve_data: lsp_ext::InlayHintResolveData = serde_json::from_value(data)?; - let Some(hash) = resolve_data.hash.parse().ok() else { return Ok(original_hint) }; let file_id = FileId::from_raw(resolve_data.file_id); + if resolve_data.version != snap.file_version(file_id) { + tracing::warn!("Inlay hint resolve data is outdated"); + return Ok(original_hint); + } + let Some(hash) = resolve_data.hash.parse().ok() else { return Ok(original_hint) }; anyhow::ensure!(snap.file_exists(file_id), "Invalid LSP resolve data"); let line_index = snap.file_line_index(file_id)?; diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs index e189dbbbd67..2cf9b53f7c8 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs @@ -561,6 +561,7 @@ pub struct CodeAction { pub struct CodeActionData { pub code_action_params: lsp_types::CodeActionParams, pub id: String, + pub version: Option, } #[derive(Debug, Eq, PartialEq, Clone, Default, Deserialize, Serialize)] @@ -802,6 +803,7 @@ impl Request for OnTypeFormatting { pub struct CompletionResolveData { pub position: lsp_types::TextDocumentPositionParams, pub imports: Vec, + pub version: Option, } #[derive(Debug, Serialize, Deserialize)] @@ -809,6 +811,7 @@ pub struct InlayHintResolveData { pub file_id: u32, // This is a string instead of a u64 as javascript can't represent u64 fully pub hash: String, + pub version: Option, } #[derive(Debug, Serialize, Deserialize)] diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/from_proto.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/from_proto.rs index f42985a9161..b6b20296d80 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/from_proto.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/from_proto.rs @@ -9,10 +9,8 @@ use syntax::{TextRange, TextSize}; use vfs::AbsPathBuf; use crate::{ - from_json, global_state::GlobalStateSnapshot, line_index::{LineIndex, PositionEncoding}, - lsp::utils::invalid_params_error, lsp_ext, }; @@ -105,16 +103,13 @@ pub(crate) fn assist_kind(kind: lsp_types::CodeActionKind) -> Option pub(crate) fn annotation( snap: &GlobalStateSnapshot, - code_lens: lsp_types::CodeLens, + range: lsp_types::Range, + data: lsp_ext::CodeLensResolveData, ) -> anyhow::Result> { - let data = - code_lens.data.ok_or_else(|| invalid_params_error("code lens without data".to_owned()))?; - let resolve = from_json::("CodeLensResolveData", &data)?; - - match resolve.kind { + match data.kind { lsp_ext::CodeLensResolveDataKind::Impls(params) => { if snap.url_file_version(¶ms.text_document_position_params.text_document.uri) - != Some(resolve.version) + != Some(data.version) { return Ok(None); } @@ -123,19 +118,19 @@ pub(crate) fn annotation( let line_index = snap.file_line_index(file_id)?; Ok(Annotation { - range: text_range(&line_index, code_lens.range)?, + range: text_range(&line_index, range)?, kind: AnnotationKind::HasImpls { pos, data: None }, }) } lsp_ext::CodeLensResolveDataKind::References(params) => { - if snap.url_file_version(¶ms.text_document.uri) != Some(resolve.version) { + if snap.url_file_version(¶ms.text_document.uri) != Some(data.version) { return Ok(None); } let pos @ FilePosition { file_id, .. } = file_position(snap, params)?; let line_index = snap.file_line_index(file_id)?; Ok(Annotation { - range: text_range(&line_index, code_lens.range)?, + range: text_range(&line_index, range)?, kind: AnnotationKind::HasReferences { pos, data: None }, }) } diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs index d02f4612dc1..03daccc99c4 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs @@ -225,13 +225,14 @@ pub(crate) fn snippet_text_edit_vec( pub(crate) fn completion_items( config: &Config, line_index: &LineIndex, + version: Option, tdpp: lsp_types::TextDocumentPositionParams, items: Vec, ) -> Vec { let max_relevance = items.iter().map(|it| it.relevance.score()).max().unwrap_or_default(); let mut res = Vec::with_capacity(items.len()); for item in items { - completion_item(&mut res, config, line_index, &tdpp, max_relevance, item); + completion_item(&mut res, config, line_index, version, &tdpp, max_relevance, item); } if let Some(limit) = config.completion(None).limit { @@ -246,6 +247,7 @@ fn completion_item( acc: &mut Vec, config: &Config, line_index: &LineIndex, + version: Option, tdpp: &lsp_types::TextDocumentPositionParams, max_relevance: u32, item: CompletionItem, @@ -328,7 +330,7 @@ fn completion_item( }) .collect::>(); if !imports.is_empty() { - let data = lsp_ext::CompletionResolveData { position: tdpp.clone(), imports }; + let data = lsp_ext::CompletionResolveData { position: tdpp.clone(), imports, version }; lsp_item.data = Some(to_value(data).unwrap()); } } @@ -483,6 +485,7 @@ pub(crate) fn inlay_hint( to_value(lsp_ext::InlayHintResolveData { file_id: file_id.index(), hash: hash.to_string(), + version: snap.file_version(file_id), }) .unwrap(), ), @@ -1318,7 +1321,7 @@ pub(crate) fn code_action_kind(kind: AssistKind) -> lsp_types::CodeActionKind { pub(crate) fn code_action( snap: &GlobalStateSnapshot, assist: Assist, - resolve_data: Option<(usize, lsp_types::CodeActionParams)>, + resolve_data: Option<(usize, lsp_types::CodeActionParams, Option)>, ) -> Cancellable { let mut res = lsp_ext::CodeAction { title: assist.label.to_string(), @@ -1336,10 +1339,11 @@ pub(crate) fn code_action( match (assist.source_change, resolve_data) { (Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?), - (None, Some((index, code_action_params))) => { + (None, Some((index, code_action_params, version))) => { res.data = Some(lsp_ext::CodeActionData { id: format!("{}:{}:{index}", assist.id.0, assist.id.1.name()), code_action_params, + version, }); } (None, None) => { diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs index 95b3d3ecb3c..aee31623081 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/main_loop.rs @@ -901,6 +901,10 @@ impl GlobalState { use crate::handlers::request as handlers; use lsp_types::request as lsp_request; + const RETRY: bool = true; + const NO_RETRY: bool = false; + + #[rustfmt::skip] dispatcher // Request handlers that must run on the main thread // because they mutate GlobalState: @@ -926,62 +930,58 @@ impl GlobalState { // analysis on the main thread because that would block other // requests. Instead, we run these request handlers on higher priority // threads in the threadpool. - .on_latency_sensitive::(handlers::handle_completion) - .on_latency_sensitive::( - handlers::handle_completion_resolve, - ) - .on_latency_sensitive::( - handlers::handle_semantic_tokens_full, - ) - .on_latency_sensitive::( - handlers::handle_semantic_tokens_full_delta, - ) - .on_latency_sensitive::( - handlers::handle_semantic_tokens_range, - ) + // FIXME: Retrying can make the result of this stale? + .on_latency_sensitive::(handlers::handle_completion) + // FIXME: Retrying can make the result of this stale + .on_latency_sensitive::(handlers::handle_completion_resolve) + .on_latency_sensitive::(handlers::handle_semantic_tokens_full) + .on_latency_sensitive::(handlers::handle_semantic_tokens_full_delta) + .on_latency_sensitive::(handlers::handle_semantic_tokens_range) + // FIXME: Some of these NO_RETRY could be retries if the file they are interested didn't change. // All other request handlers - .on::(handlers::fetch_dependency_list) - .on::(handlers::handle_analyzer_status) - .on::(handlers::handle_syntax_tree) - .on::(handlers::handle_view_hir) - .on::(handlers::handle_view_mir) - .on::(handlers::handle_interpret_function) - .on::(handlers::handle_view_file_text) - .on::(handlers::handle_view_crate_graph) - .on::(handlers::handle_view_item_tree) - .on::(handlers::handle_discover_test) - .on::(handlers::handle_expand_macro) - .on::(handlers::handle_parent_module) - .on::(handlers::handle_runnables) - .on::(handlers::handle_related_tests) - .on::(handlers::handle_code_action) - .on::(handlers::handle_code_action_resolve) - .on::(handlers::handle_hover) - .on::(handlers::handle_open_docs) - .on::(handlers::handle_open_cargo_toml) - .on::(handlers::handle_move_item) - .on::(handlers::handle_workspace_symbol) - .on::(handlers::handle_document_symbol) - .on::(handlers::handle_goto_definition) - .on::(handlers::handle_goto_declaration) - .on::(handlers::handle_goto_implementation) - .on::(handlers::handle_goto_type_definition) - .on_no_retry::(handlers::handle_inlay_hints) - .on::(handlers::handle_inlay_hints_resolve) - .on::(handlers::handle_code_lens) - .on::(handlers::handle_code_lens_resolve) - .on::(handlers::handle_folding_range) - .on::(handlers::handle_signature_help) - .on::(handlers::handle_prepare_rename) - .on::(handlers::handle_rename) - .on::(handlers::handle_references) - .on::(handlers::handle_document_highlight) - .on::(handlers::handle_call_hierarchy_prepare) - .on::(handlers::handle_call_hierarchy_incoming) - .on::(handlers::handle_call_hierarchy_outgoing) - .on::(handlers::handle_will_rename_files) - .on::(handlers::handle_ssr) - .on::(handlers::handle_view_recursive_memory_layout) + .on::(handlers::handle_document_symbol) + .on::(handlers::handle_folding_range) + .on::(handlers::handle_signature_help) + .on::(handlers::handle_will_rename_files) + .on::(handlers::handle_goto_definition) + .on::(handlers::handle_goto_declaration) + .on::(handlers::handle_goto_implementation) + .on::(handlers::handle_goto_type_definition) + .on::(handlers::handle_inlay_hints) + .on::(handlers::handle_inlay_hints_resolve) + .on::(handlers::handle_code_lens) + .on::(handlers::handle_code_lens_resolve) + .on::(handlers::handle_prepare_rename) + .on::(handlers::handle_rename) + .on::(handlers::handle_references) + .on::(handlers::handle_document_highlight) + .on::(handlers::handle_call_hierarchy_prepare) + .on::(handlers::handle_call_hierarchy_incoming) + .on::(handlers::handle_call_hierarchy_outgoing) + // All other request handlers (lsp extension) + .on::(handlers::fetch_dependency_list) + .on::(handlers::handle_analyzer_status) + .on::(handlers::handle_view_file_text) + .on::(handlers::handle_view_crate_graph) + .on::(handlers::handle_view_item_tree) + .on::(handlers::handle_discover_test) + .on::(handlers::handle_workspace_symbol) + .on::(handlers::handle_ssr) + .on::(handlers::handle_view_recursive_memory_layout) + .on::(handlers::handle_syntax_tree) + .on::(handlers::handle_view_hir) + .on::(handlers::handle_view_mir) + .on::(handlers::handle_interpret_function) + .on::(handlers::handle_expand_macro) + .on::(handlers::handle_parent_module) + .on::(handlers::handle_runnables) + .on::(handlers::handle_related_tests) + .on::(handlers::handle_code_action) + .on::(handlers::handle_code_action_resolve) + .on::(handlers::handle_hover) + .on::(handlers::handle_open_docs) + .on::(handlers::handle_open_cargo_toml) + .on::(handlers::handle_move_item) .finish(); } diff --git a/src/tools/rust-analyzer/docs/dev/lsp-extensions.md b/src/tools/rust-analyzer/docs/dev/lsp-extensions.md index 7e12874c474..46c1ccb79bf 100644 --- a/src/tools/rust-analyzer/docs/dev/lsp-extensions.md +++ b/src/tools/rust-analyzer/docs/dev/lsp-extensions.md @@ -1,5 +1,5 @@