From 8fe20b19d4702fc6d6933c31abddc8539d2581f0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 6 Apr 2021 14:16:35 +0300 Subject: More robust status notifications --- crates/rust-analyzer/src/config.rs | 4 +- crates/rust-analyzer/src/global_state.rs | 23 ++-- crates/rust-analyzer/src/lsp_ext.rs | 30 +++--- crates/rust-analyzer/src/main_loop.rs | 24 +++-- crates/rust-analyzer/src/op_queue.rs | 16 ++- crates/rust-analyzer/src/reload.rs | 119 ++++++++++++--------- .../rust-analyzer/tests/rust-analyzer/support.rs | 11 +- docs/dev/lsp-extensions.md | 30 ++++-- editors/code/src/client.ts | 2 +- editors/code/src/ctx.ts | 47 ++++---- editors/code/src/lsp_ext.ts | 9 +- 11 files changed, 165 insertions(+), 150 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index cda272fd4..e012b4452 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -445,8 +445,8 @@ impl Config { pub fn hover_actions(&self) -> bool { self.experimental("hoverActions") } - pub fn status_notification(&self) -> bool { - self.experimental("statusNotification") + pub fn server_status_notification(&self) -> bool { + self.experimental("serverStatusNotification") } pub fn publish_diagnostics(&self) -> bool { diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 44a656e62..adeb7a97e 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -23,6 +23,7 @@ use crate::{ document::DocumentData, from_proto, line_index::{LineEndings, LineIndex}, + lsp_ext, main_loop::Task, op_queue::OpQueue, reload::SourceRootConfig, @@ -32,20 +33,6 @@ use crate::{ Result, }; -#[derive(Eq, PartialEq, Copy, Clone)] -pub(crate) enum Status { - Loading, - Ready { partial: bool }, - Invalid, - NeedsReload, -} - -impl Default for Status { - fn default() -> Self { - Status::Loading - } -} - // Enforces drop order pub(crate) struct Handle { pub(crate) handle: H, @@ -73,7 +60,7 @@ pub(crate) struct GlobalState { pub(crate) mem_docs: FxHashMap, pub(crate) semantic_tokens_cache: Arc>>, pub(crate) shutdown_requested: bool, - pub(crate) status: Status, + pub(crate) last_reported_status: Option, pub(crate) source_root_config: SourceRootConfig, pub(crate) proc_macro_client: Option, @@ -83,6 +70,7 @@ pub(crate) struct GlobalState { pub(crate) vfs: Arc)>>, pub(crate) vfs_config_version: u32, + pub(crate) vfs_progress_config_version: u32, pub(crate) vfs_progress_n_total: usize, pub(crate) vfs_progress_n_done: usize, @@ -141,7 +129,7 @@ impl GlobalState { mem_docs: FxHashMap::default(), semantic_tokens_cache: Arc::new(Default::default()), shutdown_requested: false, - status: Status::default(), + last_reported_status: None, source_root_config: SourceRootConfig::default(), proc_macro_client: None, @@ -151,14 +139,15 @@ impl GlobalState { vfs: Arc::new(RwLock::new((vfs::Vfs::default(), FxHashMap::default()))), vfs_config_version: 0, + vfs_progress_config_version: 0, vfs_progress_n_total: 0, vfs_progress_n_done: 0, workspaces: Arc::new(Vec::new()), fetch_workspaces_queue: OpQueue::default(), workspace_build_data: None, - fetch_build_data_queue: OpQueue::default(), + fetch_build_data_queue: OpQueue::default(), latest_requests: Default::default(), } } diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 0e1fec209..81a6f22f1 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -241,26 +241,26 @@ pub struct SsrParams { pub selections: Vec, } -pub enum StatusNotification {} +pub enum ServerStatusNotification {} -#[derive(Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub enum Status { - Loading, - ReadyPartial, - Ready, - NeedsReload, - Invalid, +impl Notification for ServerStatusNotification { + type Params = ServerStatusParams; + const METHOD: &'static str = "experimental/serverStatus"; } -#[derive(Deserialize, Serialize)] -pub struct StatusParams { - pub status: Status, +#[derive(Deserialize, Serialize, PartialEq, Eq, Clone)] +pub struct ServerStatusParams { + pub health: Health, + pub quiescent: bool, + pub message: Option, } -impl Notification for StatusNotification { - type Params = StatusParams; - const METHOD: &'static str = "rust-analyzer/status"; +#[derive(Serialize, Deserialize, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub enum Health { + Ok, + Warning, + Error, } pub enum CodeActionRequest {} diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 4a4705e1f..a5655116b 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -21,7 +21,7 @@ use crate::{ dispatch::{NotificationDispatcher, RequestDispatcher}, document::DocumentData, from_proto, - global_state::{file_id_to_url, url_to_file_id, GlobalState, Status}, + global_state::{file_id_to_url, url_to_file_id, GlobalState}, handlers, lsp_ext, lsp_utils::{apply_document_changes, is_canceled, notification_is, Progress}, reload::{BuildDataProgress, ProjectWorkspaceProgress}, @@ -189,7 +189,7 @@ impl GlobalState { log::info!("task queue len: {}", task_queue_len); } - let mut new_status = self.status; + let was_quiescent = self.is_quiescent(); match event { Event::Lsp(msg) => match msg { lsp_server::Message::Request(req) => self.on_request(loop_start, req)?, @@ -314,9 +314,12 @@ impl GlobalState { } } vfs::loader::Message::Progress { n_total, n_done, config_version } => { + always!(config_version <= self.vfs_config_version); + + self.vfs_progress_config_version = config_version; self.vfs_progress_n_total = n_total; self.vfs_progress_n_done = n_done; - always!(config_version <= self.vfs_config_version); + let state = if n_done == 0 { Progress::Begin } else if n_done < n_total { @@ -406,18 +409,14 @@ impl GlobalState { } let state_changed = self.process_changes(); - let prev_status = self.status; - if prev_status != new_status { - self.transition(new_status); - } - let is_ready = matches!(self.status, Status::Ready { .. }); - if prev_status == Status::Loading && is_ready { + + if self.is_quiescent() && !was_quiescent { for flycheck in &self.flycheck { flycheck.update(); } } - if is_ready && (state_changed || prev_status == Status::Loading) { + if self.is_quiescent() && (!was_quiescent || state_changed) { self.update_file_notifications_on_threadpool(); // Refresh semantic tokens if the client supports it. @@ -451,6 +450,8 @@ impl GlobalState { } self.fetch_build_data_if_needed(); + self.report_new_status_if_needed(); + let loop_duration = loop_start.elapsed(); if loop_duration > Duration::from_millis(100) { log::warn!("overly long loop turn: {:?}", loop_duration); @@ -477,7 +478,8 @@ impl GlobalState { return Ok(()); } - if self.status == Status::Loading && req.method != "shutdown" { + // Avoid flashing a bunch of unresolved references during initial load. + if self.workspaces.is_empty() && !self.is_quiescent() { self.respond(lsp_server::Response::new_err( req.id, // FIXME: i32 should impl From (from() guarantees lossless conversion) diff --git a/crates/rust-analyzer/src/op_queue.rs b/crates/rust-analyzer/src/op_queue.rs index f71b718bc..1d612a933 100644 --- a/crates/rust-analyzer/src/op_queue.rs +++ b/crates/rust-analyzer/src/op_queue.rs @@ -2,27 +2,27 @@ //! at a time. pub(crate) struct OpQueue { - op_scheduled: Option, + op_requested: Option, op_in_progress: bool, last_op_result: Output, } impl Default for OpQueue { fn default() -> Self { - Self { op_scheduled: None, op_in_progress: false, last_op_result: Default::default() } + Self { op_requested: None, op_in_progress: false, last_op_result: Default::default() } } } impl OpQueue { pub(crate) fn request_op(&mut self, data: Args) { - self.op_scheduled = Some(data); + self.op_requested = Some(data); } pub(crate) fn should_start_op(&mut self) -> Option { if self.op_in_progress { return None; } - self.op_in_progress = self.op_scheduled.is_some(); - self.op_scheduled.take() + self.op_in_progress = self.op_requested.is_some(); + self.op_requested.take() } pub(crate) fn op_completed(&mut self, result: Output) { assert!(self.op_in_progress); @@ -34,4 +34,10 @@ impl OpQueue { pub(crate) fn last_op_result(&self) -> &Output { &self.last_op_result } + pub(crate) fn op_in_progress(&self) -> bool { + self.op_in_progress + } + pub(crate) fn op_requested(&self) -> bool { + self.op_requested.is_some() + } } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 5ff60c22a..301c7003b 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -9,11 +9,10 @@ use vfs::{file_set::FileSetConfig, AbsPath, AbsPathBuf, ChangeKind}; use crate::{ config::{Config, FilesWatcher, LinkedProject}, - global_state::{GlobalState, Status}, + global_state::GlobalState, lsp_ext, main_loop::Task, }; -use lsp_ext::StatusParams; #[derive(Debug)] pub(crate) enum ProjectWorkspaceProgress { @@ -30,6 +29,13 @@ pub(crate) enum BuildDataProgress { } impl GlobalState { + pub(crate) fn is_quiescent(&self) -> bool { + !(self.fetch_workspaces_queue.op_in_progress() + || self.fetch_build_data_queue.op_in_progress() + || self.vfs_progress_config_version < self.vfs_config_version + || self.vfs_progress_n_done < self.vfs_progress_n_total) + } + pub(crate) fn update_configuration(&mut self, config: Config) { let _p = profile::span("GlobalState::update_configuration"); let old_config = mem::replace(&mut self.config, Arc::new(config)); @@ -46,17 +52,13 @@ impl GlobalState { if !changes.iter().any(|(path, kind)| is_interesting(path, *kind)) { return; } - match self.status { - Status::Loading | Status::NeedsReload => return, - Status::Ready { .. } | Status::Invalid => (), - } log::info!( - "Reloading workspace because of the following changes: {}", + "Requesting workspace reload because of the following changes: {}", itertools::join( changes .iter() .filter(|(path, kind)| is_interesting(path, *kind)) - .map(|(path, kind)| format!("{}/{:?}", path.display(), kind)), + .map(|(path, kind)| format!("{}: {:?}", path.display(), kind)), ", " ) ); @@ -97,19 +99,31 @@ impl GlobalState { false } } - pub(crate) fn transition(&mut self, new_status: Status) { - self.status = new_status; - if self.config.status_notification() { - let lsp_status = match new_status { - Status::Loading => lsp_ext::Status::Loading, - Status::Ready { partial: true } => lsp_ext::Status::ReadyPartial, - Status::Ready { partial: false } => lsp_ext::Status::Ready, - Status::Invalid => lsp_ext::Status::Invalid, - Status::NeedsReload => lsp_ext::Status::NeedsReload, - }; - self.send_notification::(StatusParams { - status: lsp_status, - }); + pub(crate) fn report_new_status_if_needed(&mut self) { + if !self.config.server_status_notification() { + return; + } + + let mut status = lsp_ext::ServerStatusParams { + health: lsp_ext::Health::Ok, + quiescent: self.is_quiescent(), + message: None, + }; + if !self.config.cargo_autoreload() + && self.is_quiescent() + && self.fetch_workspaces_queue.op_requested() + { + status.health = lsp_ext::Health::Warning; + status.message = Some("Workspace reload required".to_string()) + } + if let Some(error) = self.loading_error() { + status.health = lsp_ext::Health::Error; + status.message = Some(format!("Workspace reload failed: {}", error)) + } + + if self.last_reported_status.as_ref() != Some(&status) { + self.last_reported_status = Some(status.clone()); + self.send_notification::(status); } } @@ -201,45 +215,28 @@ impl GlobalState { pub(crate) fn switch_workspaces(&mut self) { let _p = profile::span("GlobalState::switch_workspaces"); - let workspaces = self.fetch_workspaces_queue.last_op_result(); - log::info!("will switch workspaces: {:?}", workspaces); + log::info!("will switch workspaces"); + + if let Some(error_message) = self.loading_error() { + log::error!("failed to switch workspaces: {}", error_message); + self.show_message(lsp_types::MessageType::Error, error_message); + if !self.workspaces.is_empty() { + return; + } + } - let mut error_message = None; - let workspaces = workspaces + let workspaces = self + .fetch_workspaces_queue + .last_op_result() .iter() - .filter_map(|res| match res { - Ok(it) => Some(it.clone()), - Err(err) => { - log::error!("failed to load workspace: {:#}", err); - let message = error_message.get_or_insert_with(String::new); - stdx::format_to!( - message, - "rust-analyzer failed to load workspace: {:#}\n", - err - ); - None - } - }) + .filter_map(|res| res.as_ref().ok().cloned()) .collect::>(); let workspace_build_data = match self.fetch_build_data_queue.last_op_result() { Some(Ok(it)) => Some(it.clone()), - None => None, - Some(Err(err)) => { - log::error!("failed to fetch build data: {:#}", err); - let message = error_message.get_or_insert_with(String::new); - stdx::format_to!(message, "rust-analyzer failed to fetch build data: {:#}\n", err); - None - } + None | Some(Err(_)) => None, }; - if let Some(error_message) = error_message { - self.show_message(lsp_types::MessageType::Error, error_message); - if !self.workspaces.is_empty() { - return; - } - } - if *self.workspaces == workspaces && self.workspace_build_data == workspace_build_data { return; } @@ -346,6 +343,24 @@ impl GlobalState { log::info!("did switch workspaces"); } + fn loading_error(&self) -> Option { + let mut message = None; + + for ws in self.fetch_workspaces_queue.last_op_result() { + if let Err(err) = ws { + let message = message.get_or_insert_with(String::new); + stdx::format_to!(message, "rust-analyzer failed to load workspace: {:#}\n", err); + } + } + + if let Some(Err(err)) = self.fetch_build_data_queue.last_op_result() { + let message = message.get_or_insert_with(String::new); + stdx::format_to!(message, "rust-analyzer failed to fetch build data: {:#}\n", err); + } + + message + } + fn reload_flycheck(&mut self) { let _p = profile::span("GlobalState::reload_flycheck"); let config = match self.config.flycheck() { diff --git a/crates/rust-analyzer/tests/rust-analyzer/support.rs b/crates/rust-analyzer/tests/rust-analyzer/support.rs index 95bf26f01..8d68f1b7d 100644 --- a/crates/rust-analyzer/tests/rust-analyzer/support.rs +++ b/crates/rust-analyzer/tests/rust-analyzer/support.rs @@ -103,7 +103,7 @@ impl<'a> Project<'a> { ..Default::default() }), experimental: Some(json!({ - "statusNotification": true, + "serverStatusNotification": true, })), ..Default::default() }, @@ -213,13 +213,12 @@ impl Server { } pub(crate) fn wait_until_workspace_is_loaded(self) -> Server { self.wait_for_message_cond(1, &|msg: &Message| match msg { - Message::Notification(n) if n.method == "rust-analyzer/status" => { + Message::Notification(n) if n.method == "experimental/serverStatus" => { let status = n .clone() - .extract::("rust-analyzer/status") - .unwrap() - .status; - matches!(status, lsp_ext::Status::Ready) + .extract::("experimental/serverStatus") + .unwrap(); + status.quiescent } _ => false, }) diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index 73be59a82..989771ac6 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -1,5 +1,5 @@