diff --git a/CHANGELOG.md b/CHANGELOG.md index d699978..424e988 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ ## 0.9.10 - Added indentation-style rules to the lint module with configurable indent size as specified in [example_lint_cfg.json](./example_files/example_lint_cfg.README) +- Configuration changes are now correctly tracked with the pull-model, and the + server should correctly update most settings without a restart on config + change. ## 0.9.9 - Fixed an issue that could cause analysis thread crash when an object was declared both diff --git a/clients.md b/clients.md index f8ca5cd..e845597 100644 --- a/clients.md +++ b/clients.md @@ -30,7 +30,11 @@ Once you have this basic support in place, the hard work begins: * Implement [extensions to the protocol](clients.md#extensions-to-the-language-server-protocol) * Client-side configuration. - You'll need to send the `workspace/didChangeConfiguration` notification when - configuration changes. + configuration changes. You can either send the entire configuration in the + notification, or send "null" if your client supports pull-style configuration updates + (note that you need to support dynamic registration of + "didChangeConfiguration" and support the "workspace/configuration" request on the client + for pull-style updates) - For the config options, see [config.rs](./src/config.rs#L99-L111) * Check for and install the DLS - Download the latest [binary](https://github.com/intel/dml-language-server/actions/workflows/rust.yml). @@ -90,6 +94,7 @@ From Server to client: * `client/registerCapability` * `client/unregisterCapability` * `textDocument/publishDiagnostics` +* `workspace/configuration` (if using pull-style configuration updates) ### Extensions to the Language Server Protocol @@ -165,6 +170,14 @@ From Server to Client, these notifcations have a specified data value: with the title "Analysing" when it starts on any long-term work, and a 'WorkDoneProgressEnd' with the corresponding id when it finishes all such work. +* `workspace/configuration` + The server sends a single ConfigurationItem with no scope and a + "simics-modeling.dls" section. The client response value should be the + an array containing one element which is an object with the full + configuration structure for the server. +* `client/registerCapability` + The server may try to register on 'DidChangeWatchedFiles' or 'DidChangeConfiguration', + the latter being required for pull-style configuration updates. ## Resources diff --git a/src/actions/analysis_storage.rs b/src/actions/analysis_storage.rs index 5c2b724..c320094 100644 --- a/src/actions/analysis_storage.rs +++ b/src/actions/analysis_storage.rs @@ -249,6 +249,10 @@ impl AnalysisStorage { self.isolated_analysis.contains_key(path) } + pub fn has_lint_analysis(&self, path: &CanonPath) -> bool { + self.lint_analysis.contains_key(path) + } + pub fn all_dependencies(&self, path: &CanonPath, context: Option<&CanonPath>) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index f1a27fc..7fddb36 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use crate::actions::InitActionContext; use crate::lsp_data::*; -use crate::server::ResponseError; +use crate::server::{Output, ResponseError}; #[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct Tooltip { @@ -16,8 +16,8 @@ pub struct Tooltip { } // Build a hover tooltip -pub fn tooltip( - ctx: &mut InitActionContext, +pub fn tooltip( + ctx: &mut InitActionContext, params: &TextDocumentPositionParams, ) -> Result { let hover_file_path = parse_file_path!(¶ms.text_document.uri, "hover")?; diff --git a/src/actions/mod.rs b/src/actions/mod.rs index f63aed4..5ebd8ad 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -3,7 +3,7 @@ //! Actions that the DLS can perform: responding to requests, watching files, //! etc. -use log::{debug, trace, error, warn}; +use log::{debug, info, trace, error, warn}; use thiserror::Error; use crossbeam::channel; use serde::Deserialize; @@ -31,7 +31,10 @@ use crate::file_management::{PathResolver, CanonPath}; use crate::lint::{LintCfg, maybe_parse_lint_cfg}; use crate::lsp_data; use crate::lsp_data::*; -use crate::server::{Output, ServerToHandle, error_message}; +use crate::server::{Output, ServerToHandle, error_message, + Request, RequestId, SentRequest}; +use crate::server::message::RawResponse; +use crate::server::dispatch::HandleResponseType; use crate::Span; use crate::span; use crate::span::{ZeroIndexed, FilePosition}; @@ -64,9 +67,9 @@ pub mod progress; pub mod work_pool; /// Persistent context shared across all requests and notifications. -pub enum ActionContext { +pub enum ActionContext { /// Context after server initialization. - Init(InitActionContext), + Init(InitActionContext), /// Context before initialization. Uninit(UninitActionContext), } @@ -81,20 +84,20 @@ impl From<()> for InitError { } } -impl ActionContext { +impl ActionContext { /// Construct a new, uninitialized context. pub fn new( vfs: Arc, config: Arc>, notify: channel::Sender, - ) -> ActionContext { + ) -> ActionContext { ActionContext::Uninit(UninitActionContext::new( Arc::new(Mutex::new(AnalysisStorage::init(notify))), vfs, config)) } /// Initialize this context, returns `Err(())` if it has already been initialized. - pub fn init( + pub fn init( &mut self, init_options: InitializationOptions, client_capabilities: lsp_data::ClientCapabilities, @@ -130,7 +133,7 @@ impl ActionContext { /// Returns an initialiased wrapped context, /// or `Err(())` if not initialised. - pub fn inited(&self) -> Result { + pub fn inited(&self) -> Result, InitError> { match *self { ActionContext::Uninit(_) => Err(().into()), ActionContext::Init(ref ctx) => Ok(ctx.clone()), @@ -218,7 +221,7 @@ pub struct AnalysisStateWaitDefinition { // (not concurrent), so make sure shared info is behind Arcs, and that no overly // large data structures are stored. #[derive(Clone)] -pub struct InitActionContext { +pub struct InitActionContext { pub analysis: Arc>, vfs: Arc, // Queues analysis jobs so that we don't over-use the CPU. @@ -246,6 +249,9 @@ pub struct InitActionContext { active_waits: Arc>>, + outstanding_requests: Arc>>>>, + pub config: Arc>, pub lint_config: Arc>, pub sent_warnings: Arc>>, @@ -301,7 +307,7 @@ impl ContextDefinition { pub type ActiveDeviceContexts = HashSet; -impl InitActionContext { +impl InitActionContext { fn new( analysis: Arc>, vfs: Arc, @@ -309,7 +315,7 @@ impl InitActionContext { client_capabilities: lsp_data::ClientCapabilities, pid: u32, _client_supports_cmd_run: bool, - ) -> InitActionContext { + ) -> InitActionContext { let lint_config = Arc::new(Mutex::new( config.lock().unwrap().lint_cfg_path.clone() .and_then(maybe_parse_lint_cfg) @@ -329,6 +335,7 @@ impl InitActionContext { has_notified_missing_builtins: false, //client_supports_cmd_run, active_waits: Arc::default(), + outstanding_requests: Arc::default(), shut_down: Arc::new(AtomicBool::new(false)), pid, workspace_roots: Arc::default(), @@ -339,12 +346,12 @@ impl InitActionContext { } } - fn add_direct_open(&mut self, path: PathBuf) { + fn add_direct_open(&self, path: PathBuf) { let canon_path: CanonPath = path.into(); self.direct_opens.lock().unwrap().insert(canon_path); } - fn remove_direct_open(&mut self, path: PathBuf) { + fn remove_direct_open(&self, path: PathBuf) { let canon_path: CanonPath = path.into(); if !self.direct_opens.lock().unwrap().remove(&canon_path) { debug!("Tried to remove a directly opened file ({:?}) \ @@ -352,13 +359,13 @@ impl InitActionContext { } } - fn init(&mut self, - _init_options: InitializationOptions, - out: O) { + fn init(&mut self, + _init_options: InitializationOptions, + out: O) { self.update_compilation_info(&out) } - pub fn update_workspaces(&mut self, + pub fn update_workspaces(&self, mut add: Vec, remove: Vec) { if let Ok(mut workspaces) = self.workspace_roots.lock() { @@ -368,7 +375,7 @@ impl InitActionContext { } } - fn update_linter_config(&mut self, _out: &O) { + fn update_linter_config(&self, _out: &O) { trace!("Updating linter config"); if let Ok(config) = self.config.lock() { *self.lint_config.lock().unwrap() = @@ -378,7 +385,7 @@ impl InitActionContext { } } - pub fn update_compilation_info(&mut self, out: &O) { + pub fn update_compilation_info(&self, out: &O) { trace!("Updating compile info"); if let Ok(config) = self.config.lock() { if let Some(compile_info) = &config.compile_info_path { @@ -424,20 +431,25 @@ impl InitActionContext { } } - pub fn report_errors(&mut self, output: &O) { + pub fn report_errors(&self, output: &O) { self.update_analysis(); let filter = Some(self.device_active_contexts.lock().unwrap().clone()); - let (isolated, device, lint) = + let (isolated, device, mut lint) = self.analysis.lock().unwrap().gather_errors(filter.as_ref()); let notifier = AnalysisDiagnosticsNotifier::new("indexing".to_string(), output.clone()); notifier.notify_begin_diagnostics(); + let config = self.config.lock().unwrap(); + if !config.linting_enabled { + // A slightly hacky way to not report linting errors + // when we have done linting but then turned off the setting + lint.clear(); + } let files: HashSet<&PathBuf> = isolated.keys() .chain(device.keys()) .chain(lint.keys()) .collect(); - let config = self.config.lock().unwrap(); let direct_opens = self.direct_opens.lock().unwrap(); for file in files { let mut sorted_errors: Vec = @@ -523,14 +535,12 @@ impl InitActionContext { Ok(new_compinfo) } - pub fn update_analysis(&mut self) { + pub fn update_analysis(&self) { self.analysis.lock().unwrap() .update_analysis(&self.construct_resolver()); } - pub fn trigger_device_analysis(&mut self, - file: &Path, - out: &O) { + pub fn trigger_device_analysis(&self, file: &Path, out: &O) { let canon_path: CanonPath = file.to_path_buf().into(); debug!("triggering devices dependant on {}", canon_path.as_str()); self.update_analysis(); @@ -561,14 +571,100 @@ impl InitActionContext { // Called when config might have changed, re-update include paths // and similar - pub fn maybe_changed_config(&mut self, out: &O) { + pub fn maybe_changed_config(&self, + old_config: Config, + out: &O) { trace!("Compilation info might have changed"); - self.update_compilation_info(out); - self.update_linter_config(out); + enum LintReissueRequirement { + None, + ReReport, + AnalyzeMissing, + AnalyzeAll, + } + impl LintReissueRequirement { + fn upgrade_to(self, to: LintReissueRequirement) -> + LintReissueRequirement { + match (self, to) { + (Self::None, o) => o, + (o, Self::None) => o, + (Self::AnalyzeAll, _) => Self::AnalyzeAll, + (_, Self::AnalyzeAll) => Self::AnalyzeAll, + (Self::AnalyzeMissing, _) => Self::AnalyzeMissing, + (_, Self::AnalyzeMissing) => Self::AnalyzeMissing, + _ => Self::ReReport, + } + } + } + let mut lint_reissue = LintReissueRequirement::None; + { + let config = self.config.lock().unwrap(); + if config.compile_info_path != old_config.compile_info_path { + self.update_compilation_info(out); + } + if config.linting_enabled != old_config.linting_enabled { + if config.linting_enabled { + lint_reissue = lint_reissue.upgrade_to( + LintReissueRequirement::AnalyzeMissing); + } else { + lint_reissue = lint_reissue.upgrade_to( + LintReissueRequirement::ReReport); + } + } + if config.suppress_imports != old_config.suppress_imports { + if config.suppress_imports { + lint_reissue = lint_reissue.upgrade_to( + LintReissueRequirement::ReReport); + } else { + lint_reissue = lint_reissue.upgrade_to( + LintReissueRequirement::AnalyzeMissing); + } + } + if config.lint_direct_only != old_config.lint_direct_only { + if config.lint_direct_only { + lint_reissue = lint_reissue.upgrade_to( + LintReissueRequirement::ReReport); + } else { + lint_reissue = lint_reissue.upgrade_to( + LintReissueRequirement::AnalyzeMissing); + } + } + if config.lint_cfg_path != old_config.lint_cfg_path { + self.update_linter_config(out); + lint_reissue = LintReissueRequirement::AnalyzeAll; + } + } + match lint_reissue { + LintReissueRequirement::None => (), + LintReissueRequirement::ReReport => + self.report_errors(out), + LintReissueRequirement::AnalyzeMissing => { + // Because lint_analyze re-locks self.analysis, we need + // to copy out the paths before iterating here + let paths: Vec<_> = { + let storage = self.analysis.lock().unwrap(); + storage.isolated_analysis.keys().filter( + |p|!storage.has_lint_analysis(p)).cloned().collect() + }; + for path in paths { + self.maybe_trigger_lint_analysis(path.as_path(), out); + } + self.report_errors(out); + }, + LintReissueRequirement::AnalyzeAll => { + let paths: Vec<_> = { + self.analysis.lock().unwrap() + .isolated_analysis.keys().cloned().collect() + }; + for path in paths { + self.maybe_trigger_lint_analysis(path.as_path(), out); + } + self.report_errors(out); + }, + } } // Call before adding new analysis - pub fn maybe_start_progress(&mut self, out: &O) { + pub fn maybe_start_progress(&self, out: &O) { let mut notifier = self.current_notifier.lock().unwrap(); @@ -580,7 +676,7 @@ impl InitActionContext { new_notifier.notify_begin_progress(); } } - pub fn maybe_end_progress(&mut self, out: &O) { + pub fn maybe_end_progress(&mut self, out: &O) { if !self.analysis_queue.has_work() { // Need the scope here to succesfully drop the guard lock before // going into maybe_warn_missing_builtins below @@ -598,7 +694,10 @@ impl InitActionContext { } } - fn maybe_warn_missing_builtins(&mut self, out: &O) { + // NOTE: Do not call this method from outside the main server loop, + // as notification/request handlers obtain _copies_ of the context, + // and not the context itself + fn maybe_warn_missing_builtins(&mut self, out: &O) { if !self.has_notified_missing_builtins && !self.analysis.lock().unwrap().has_client_file( &PathBuf::from("dml-builtins.dml")) { @@ -628,10 +727,10 @@ impl InitActionContext { toret } - pub fn isolated_analyze(&mut self, - client_path: &Path, - context: Option, - out: &O) { + pub fn isolated_analyze(&self, + client_path: &Path, + context: Option, + out: &O) { debug!("Wants isolated analysis of {:?}{}", client_path, context.as_ref().map(|s|format!(" under context {}", s.as_str())) @@ -657,7 +756,7 @@ impl InitActionContext { &self.vfs, context, path, client_path.to_path_buf(), token); } - fn device_analyze(&mut self, device: &CanonPath, out: &O) { + fn device_analyze(&self, device: &CanonPath, out: &O) { debug!("Wants device analysis of {:?}", device); self.maybe_start_progress(out); self.maybe_add_device_context(device); @@ -839,9 +938,7 @@ impl InitActionContext { } } - pub fn maybe_trigger_lint_analysis(&mut self, - file: &Path, - out: &O) { + pub fn maybe_trigger_lint_analysis(&self, file: &Path, out: &O) { if !self.config.lock().unwrap().linting_enabled { return; } @@ -860,11 +957,11 @@ impl InitActionContext { out); } - fn lint_analyze(&mut self, - file: &Path, - context: Option, - cfg: LintCfg, - out: &O) { + fn lint_analyze(&self, + file: &Path, + context: Option, + cfg: LintCfg, + out: &O) { debug!("Wants to lint {:?}", file); self.maybe_start_progress(out); let path = if let Some(p) = @@ -1117,6 +1214,33 @@ impl InitActionContext { true }); } + + pub fn send_request(&self, params: R::Params, out: &O) + where + R: SentRequest, + ::Params: std::fmt::Debug + { + let id = out.provide_id(); + info!("Sending a request {} with params {:?} and id {}", + ::METHOD, params, id); + let request = Request::::new(id.clone(), params); + self.outstanding_requests.lock().unwrap().insert( + id, Box::new(::handle_response)); + out.request(request); + } + + pub fn handle_request_response(&self, resp: RawResponse, out: &O) { + info!("Got a response to some request: {:?}", resp); + let maybe_req_fn = { self.outstanding_requests + .lock().unwrap().remove(&resp.id) }; + if let Some(request_fn) = maybe_req_fn { + request_fn(self, resp, out); + } else { + info!("Got response for request id {:?} but it was not tracked\ + (either timed out or was never sent)", + resp.id); + } + } } /// Some notifications come with sequence numbers, we check that these are in @@ -1177,7 +1301,7 @@ pub struct FileWatch { impl FileWatch { /// Construct a new `FileWatch`. - pub fn new(ctx: &InitActionContext) -> Option { + pub fn new(ctx: &InitActionContext) -> Option { match ctx.config.lock() { Ok(config) => { config.compile_info_path.as_ref().map( diff --git a/src/actions/notifications.rs b/src/actions/notifications.rs index 01b4384..7dcb4e8 100644 --- a/src/actions/notifications.rs +++ b/src/actions/notifications.rs @@ -5,7 +5,6 @@ use crate::actions::{FileWatch, InitActionContext, VersionOrdering, ContextDefinition}; use crate::file_management::CanonPath; -use crate::server::message::Request; use crate::span::{Span}; use crate::vfs::{Change, VfsSpan}; use crate::lsp_data::*; @@ -34,14 +33,30 @@ impl BlockingNotificationAction for Initialized { // Respond to the `initialized` notification. fn handle( _params: Self::Params, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { - // TODO: register any dynamic capabilities + // These are the requirements for the pull-variant of + // configuration update + // - Dynamically registered DidChangeConfiguration + // - ConfigurationRequest support + if ctx.client_capabilities.did_change_configuration_support() + == (true, true) + && ctx.client_capabilities.configuration_support() { + const CONFIG_ID: &str = "dls-config"; + let reg_params = RegistrationParams { + registrations: vec![Registration { + id: CONFIG_ID.to_owned(), + method: + ::METHOD.to_owned(), + register_options: None, + }], + }; + ctx.send_request::(reg_params, &out); + } // Register files we watch for changes based on config const WATCH_ID: &str = "dls-watch"; - let id = out.provide_id(); let reg_params = RegistrationParams { registrations: vec![Registration { id: WATCH_ID.to_owned(), @@ -51,9 +66,7 @@ impl BlockingNotificationAction for Initialized { |fw|fw.watchers_config()), }], }; - - let request = Request::::new(id, reg_params); - out.request(request); + ctx.send_request::(reg_params, &out); Ok(()) } } @@ -61,7 +74,7 @@ impl BlockingNotificationAction for Initialized { impl BlockingNotificationAction for DidOpenTextDocument { fn handle( params: Self::Params, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { debug!("on_open: {:?}", params.text_document.uri); @@ -80,7 +93,7 @@ impl BlockingNotificationAction for DidOpenTextDocument { impl BlockingNotificationAction for DidCloseTextDocument { fn handle( params: Self::Params, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { debug!("on_close: {:?}", params.text_document.uri); @@ -94,7 +107,7 @@ impl BlockingNotificationAction for DidCloseTextDocument { impl BlockingNotificationAction for DidChangeTextDocument { fn handle( params: Self::Params, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { debug!("on_change: {:?}, thread: {:?}", params, thread::current().id()); @@ -152,7 +165,7 @@ impl BlockingNotificationAction for DidChangeTextDocument { impl BlockingNotificationAction for Cancel { fn handle( _params: CancelParams, - _ctx: &mut InitActionContext, + _ctx: &mut InitActionContext, _out: O, ) -> Result<(), ResponseError> { // Nothing to do. @@ -163,10 +176,23 @@ impl BlockingNotificationAction for Cancel { impl BlockingNotificationAction for DidChangeConfiguration { fn handle( params: DidChangeConfigurationParams, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { debug!("config change: {:?}", params.settings); + // New style config update, send re-config request + if params.settings.is_null() || params.settings.as_object(). + map_or(false, |o|o.is_empty()) { + let config_params = lsp_types::ConfigurationParams { + items: vec![lsp_types::ConfigurationItem { + scope_uri: None, + section: Some("simics-modeling.dls".to_string()), + }], + }; + ctx.send_request::( + config_params, &out); + return Ok(()); + } use std::collections::HashMap; let mut dups = HashMap::new(); let mut unknowns = vec![]; @@ -188,9 +214,9 @@ impl BlockingNotificationAction for DidChangeConfiguration { return Err(().into()); } }; - + let old = ctx.config.lock().unwrap().clone(); ctx.config.lock().unwrap().update(new_config); - ctx.maybe_changed_config(&out); + ctx.maybe_changed_config(old, &out); Ok(()) } @@ -199,7 +225,7 @@ impl BlockingNotificationAction for DidChangeConfiguration { impl BlockingNotificationAction for DidSaveTextDocument { fn handle( params: DidSaveTextDocumentParams, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { let file_path = parse_file_path!(¶ms.text_document.uri, "on_save")?; @@ -217,12 +243,12 @@ impl BlockingNotificationAction for DidSaveTextDocument { impl BlockingNotificationAction for DidChangeWatchedFiles { fn handle( params: DidChangeWatchedFilesParams, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O, ) -> Result<(), ResponseError> { if let Some(file_watch) = FileWatch::new(ctx) { if params.changes.iter().any(|c| file_watch.is_relevant(c)) { - ctx.maybe_changed_config(&out); + ctx.update_compilation_info(&out); } } Ok(()) @@ -233,7 +259,7 @@ impl BlockingNotificationAction for DidChangeWorkspaceFolders { // Respond to the `initialized` notification. fn handle( params: DidChangeWorkspaceFoldersParams, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, _out: O, ) -> Result<(), ResponseError> { let added = params.event.added; @@ -286,7 +312,7 @@ impl LSPNotification for ChangeActiveContexts { impl BlockingNotificationAction for ChangeActiveContexts { fn handle( params: ChangeActiveContextsParams, - ctx: &mut InitActionContext, + ctx: &mut InitActionContext, out: O) -> Result<(), ResponseError> { debug!("ChangeActiveContexts: {:?}", params); let contexts: Vec = params.active_contexts diff --git a/src/actions/requests.rs b/src/actions/requests.rs index 79478e6..ff699cb 100644 --- a/src/actions/requests.rs +++ b/src/actions/requests.rs @@ -3,7 +3,7 @@ //! Requests that the DLS can respond to. use jsonrpc::error::{StandardError, standard_error}; -use log::{debug, error, trace, warn}; +use log::{debug, error, info, trace, warn}; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::HashSet; @@ -18,6 +18,7 @@ use crate::actions::notifications::ContextDefinitionKindParam; use crate::analysis::{ZeroSpan, ZeroFilePosition, SymbolRef}; use crate::analysis::reference::ReferenceKind; use crate::analysis::symbols::SimpleSymbol; +use crate::config::Config; pub use crate::lsp_data::request::{ ApplyWorkspaceEdit, @@ -34,8 +35,10 @@ pub use crate::lsp_data::request::{ HoverRequest, RangeFormatting, References, + RegisterCapability, Rename, ResolveCompletionItem as ResolveCompletion, + WorkspaceConfiguration, WorkspaceSymbolRequest, }; @@ -50,7 +53,7 @@ use crate::actions::analysis_storage::AnalysisLookupError; use crate::config::WarningFrequency; use crate::file_management::CanonPath; use crate::server; -use crate::server::{Ack, Output, Request, RequestAction, +use crate::server::{Ack, Output, Request, RequestAction, SentRequest, Response, ResponseError, ResponseWithMessage}; // Gives a slightly-better error message than the short-form one specified by @@ -84,11 +87,11 @@ fn warn_miss_lookup(error: AnalysisLookupError, file: Option<&str>) { } } -fn response_maybe_with_limitations( +fn response_maybe_with_limitations( path: &Path, response: R, limitations: I, - ctx: &InitActionContext) + ctx: &InitActionContext) -> ResponseWithMessage where I: IntoIterator, @@ -131,10 +134,12 @@ pub const TYPE_SEMANTIC_LIMITATION: DLSLimitation = DLSLimitation { }; // TODO: This function is getting bloated, refactor into several smaller ones -fn fp_to_symbol_refs(fp: &ZeroFilePosition, - ctx: &InitActionContext, - relevant_limitations: &mut HashSet) - -> Result, AnalysisLookupError> { +fn fp_to_symbol_refs + (fp: &ZeroFilePosition, + ctx: &InitActionContext, + relevant_limitations: &mut HashSet) + -> Result, AnalysisLookupError> +{ let analysis = ctx.analysis.lock().unwrap(); // This step-by-step approach could be folded into analysis_storage, // but I keep it as separate here so that we could, perhaps, @@ -218,9 +223,11 @@ fn fp_to_symbol_refs(fp: &ZeroFilePosition, Ok(definitions) } -fn handle_default_remapping(ctx: &InitActionContext, - symbols: Vec, - fp: &ZeroFilePosition) -> HashSet { +fn handle_default_remapping + (ctx: &InitActionContext, + symbols: Vec, + fp: &ZeroFilePosition) -> HashSet +{ let analysis = ctx.analysis.lock().unwrap(); let refr_opt = analysis.reference_at_pos(fp); // NOTE: Because the call to this is preceded by a symbol lookup, @@ -403,8 +410,8 @@ impl RequestAction for WorkspaceSymbolRequest { Ok(None) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { let analysis = ctx.analysis.lock().unwrap(); @@ -428,8 +435,8 @@ impl RequestAction for DocumentSymbolRequest { Ok(None) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { debug!("Handing doc symbol request {:?}", params); @@ -463,7 +470,7 @@ impl RequestAction for HoverRequest { range: None }) } - fn handle(mut ctx: InitActionContext, + fn handle(mut ctx: InitActionContext, params: Self::Params, ) -> Result { debug!("handling hover ({:?})", params); @@ -488,8 +495,8 @@ impl RequestAction for GotoImplementation { Ok(None.into()) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { debug!("Requesting implementations with params {:?}", params); @@ -552,8 +559,8 @@ impl RequestAction for GotoDeclaration { Ok(None.into()) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { debug!("Requesting declarations with params {:?}", params); @@ -610,8 +617,8 @@ impl RequestAction for GotoDefinition { Ok(None.into()) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { debug!("Requesting definitions with params {:?}", params); @@ -668,8 +675,8 @@ impl RequestAction for References { Ok(vec![].into()) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { debug!("Requesting references with params {:?}", params); @@ -721,8 +728,8 @@ impl RequestAction for Completion { Ok(vec![]) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: Acquire completions for location @@ -737,8 +744,8 @@ impl RequestAction for DocumentHighlightRequest { Ok(vec![]) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: Acquire highlighting info for file and span @@ -756,8 +763,8 @@ impl RequestAction for Rename { change_annotations: None })) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: Perform a rename @@ -800,8 +807,8 @@ impl RequestAction for ExecuteCommand { } /// Currently, no support for this - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: ExecuteCommandParams, ) -> Result { // TODO: handle specialized commands. or if no such commands, remove @@ -820,8 +827,8 @@ impl RequestAction for CodeActionRequest { Ok(vec![]) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: figure out if we want to use this @@ -840,8 +847,8 @@ impl RequestAction for Formatting { )) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: format document @@ -859,8 +866,8 @@ impl RequestAction for RangeFormatting { )) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: format range @@ -875,7 +882,7 @@ impl RequestAction for ResolveCompletion { Err(ResponseError::Empty) } - fn handle(_: InitActionContext, _params: Self::Params) -> Result { + fn handle(_: InitActionContext, _params: Self::Params) -> Result { // TODO: figure out if we want to use this Self::fallback_response() } @@ -888,8 +895,8 @@ impl RequestAction for CodeLensRequest { Err(ResponseError::Empty) } - fn handle( - _ctx: InitActionContext, + fn handle( + _ctx: InitActionContext, _params: Self::Params, ) -> Result { // TODO: figure out if we want to use this @@ -931,8 +938,8 @@ impl RequestAction for GetKnownContextsRequest { Err(ResponseError::Empty) } - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result { let for_these_paths: Vec = @@ -982,3 +989,57 @@ impl RequestAction for GetKnownContextsRequest { ) } } + +/// Server-to-client requests +impl SentRequest for RegisterCapability { + type Response = ::Result; + fn on_response + (_ctx: &InitActionContext, _response: Self::Response, _out: &O) { + info!("Successful registration on some capability"); + } +} + +impl SentRequest for WorkspaceConfiguration { + type Response = ::Result; + fn on_response + (ctx: &InitActionContext, response: Self::Response, out: &O) + { + info!("Acquired configuration updates {:?}", response); + + let dml_object = if response.len() == 1 { + &response[0] + } else { + error!("Received incorrectly formatted response to \ + 'workspace/configuration' from client (array length \ + should be 1)\ + : {:?}", + response); + return; + }; + use std::collections::HashMap; + let mut dups = HashMap::new(); + let mut unknowns = vec![]; + let mut deprecated = vec![]; + let settings = Config::try_deserialize( + dml_object, + &mut dups, + &mut unknowns, + &mut deprecated, + ); + crate::server::maybe_notify_unknown_configs(out, &unknowns); + crate::server::maybe_notify_deprecated_configs(out, &deprecated); + crate::server::maybe_notify_duplicated_configs(out, &dups); + + let new_config = match settings { + Ok(value) => value, + Err(err) => { + warn!("Received unactionable config: {:?} (error: {:?})", + &response, err); + return; + } + }; + let old = ctx.config.lock().unwrap().clone(); + ctx.config.lock().unwrap().update(new_config); + ctx.maybe_changed_config(old, out); + } +} diff --git a/src/cmd.rs b/src/cmd.rs index 353353a..1ea4cdd 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -156,7 +156,7 @@ fn def(file_name: &str, row: &str, col: &str) partial_result_token: None, }, }; - Request { id: next_id(), params, received: Instant::now(), _action: PhantomData } + Request { id: next_id(), params, received: Instant::now(), action: PhantomData } } fn workspace_symbol(query: &str) -> Request { @@ -169,7 +169,7 @@ fn workspace_symbol(query: &str) -> Request { work_done_token: None, }, }; - Request { id: next_id(), params, received: Instant::now(), _action: PhantomData } + Request { id: next_id(), params, received: Instant::now(), action: PhantomData } } fn document_symbol(file_name: &str) @@ -183,11 +183,11 @@ fn document_symbol(file_name: &str) work_done_token: None, }, }; - Request { id: next_id(), params, received: Instant::now(), _action: PhantomData } + Request { id: next_id(), params, received: Instant::now(), action: PhantomData } } pub fn shutdown() -> Request { - Request { id: next_id(), params: (), received: Instant::now(), _action: PhantomData } + Request { id: next_id(), params: (), received: Instant::now(), action: PhantomData } } pub fn exit() -> Notification { @@ -224,7 +224,7 @@ pub fn initialize(root_path: String) -> Request { work_done_token: None, }, }; - Request { id: next_id(), params, received: Instant::now(), _action: PhantomData } + Request { id: next_id(), params, received: Instant::now(), action: PhantomData } } fn open(file_name: &str) @@ -303,7 +303,7 @@ pub fn get_contexts(paths: Vec) -> Request, dups: &mut std::collections::HashMap>, unknowns: &mut Vec, deprecated: &mut Vec, @@ -249,9 +247,9 @@ impl Config { } } - if let serde_json::Value::Object(map) = val { - let seq = serde::de::value::MapDeserializer::new(map.iter().filter_map(|(k, v)| { - use heck::ToSnakeCase; + use heck::ToSnakeCase; + let seq = serde::de::value::MapDeserializer::new( + vals.iter().filter_map(|(k, v)| { let snake_case = k.to_snake_case(); let vec = dups.entry(snake_case.clone()).or_default(); vec.push(k.to_string()); @@ -266,19 +264,37 @@ impl Config { None } })); - match serde_ignored::deserialize(seq, |path| unknowns.push(path.to_string())) { - Ok(conf) => { - dups.retain(|_, v| v.len() > 1); - return Ok(conf); - } - _ => { - dups.retain(|_, v| v.len() > 1); - } + match serde_ignored::deserialize( + seq, |path| unknowns.push(path.to_string())) { + Ok(conf) => { + dups.retain(|_, v| v.len() > 1); + return Ok(conf); + } + _ => { + dups.retain(|_, v| v.len() > 1); } } Err(().into()) } + /// try to deserialize a Config from a json value, val is expected to be a + /// Value::Object, all first level keys of val are converted to snake_case, + /// duplicated and unknown keys are reported + pub fn try_deserialize( + val: &serde_json::value::Value, + dups: &mut std::collections::HashMap>, + unknowns: &mut Vec, + deprecated: &mut Vec, + ) -> Result { + if let serde_json::Value::Object(map) = val { + return Self::try_deserialize_vec( + &map.iter().map(|(k,v)|(k.clone(), v.clone())).collect(), + dups, unknowns, deprecated + ); + } + Err(().into()) + } + /// Join this configuration with the new config. pub fn update(&mut self, mut new: Config) { if let Some(new_retain) = new.analysis_retain_duration { diff --git a/src/lsp_data.rs b/src/lsp_data.rs index b144d80..e389719 100644 --- a/src/lsp_data.rs +++ b/src/lsp_data.rs @@ -316,7 +316,7 @@ impl InitializationOptions { #[serde(default)] pub struct ClientCapabilities { pub root: Option, - pub workspace_folders: bool, + pub capabilities: lsp_types::ClientCapabilities, } impl ClientCapabilities { @@ -327,14 +327,32 @@ impl ClientCapabilities { root: params.root_uri.as_ref().and_then( |uri|parse_file_path!(uri, "decode root").ok()), - workspace_folders: params.capabilities.workspace.as_ref().and_then( - |workspace|workspace.workspace_folders.and_then( - |folders|if folders { Some(folders)} else { None }) - ).is_some(), + capabilities: params.capabilities.clone(), } } } +// Fast-access to fields we care about +impl ClientCapabilities { + pub fn workspace_folder_support(&self) -> bool { + self.capabilities.workspace.as_ref() + .map_or(false, |w|w.workspace_folders.unwrap_or(false)) + } + // (supported, dynamic registration supported) + pub fn did_change_configuration_support(&self) -> (bool, bool) { + if let Some(dcc) = self.capabilities.workspace.as_ref() + .and_then(|w|w.did_change_configuration) { + (true, dcc.dynamic_registration.unwrap_or(false)) + } else { + (false, false) + } + } + pub fn configuration_support(&self) -> bool { + self.capabilities.workspace.as_ref() + .map_or(false, |w|w.configuration.unwrap_or(false)) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/src/server/dispatch.rs b/src/server/dispatch.rs index 29e0fe0..e3efacb 100644 --- a/src/server/dispatch.rs +++ b/src/server/dispatch.rs @@ -5,7 +5,7 @@ use std::thread; use std::time::Duration; use jsonrpc::error::StandardError::InternalError; -use log::debug; +use log::{info, error, debug}; use crate::actions::work_pool; use crate::actions::work_pool::WorkDescription; @@ -14,7 +14,7 @@ use crate::concurrency::{ConcurrentJob, JobToken}; use crate::lsp_data::LSPRequest; use crate::server; use crate::server::io::Output; -use crate::server::message::ResponseError; +use crate::server::message::{RawResponse, ResponseError}; use crate::server::{Request, Response}; use crate::actions::requests::*; @@ -48,7 +48,7 @@ macro_rules! define_dispatch_request_enum { )* impl DispatchRequest { - fn handle(self, ctx: InitActionContext, out: &O) { + fn handle(self, ctx: InitActionContext, out: &O) { match self { $( DispatchRequest::$request_type(req) => { @@ -110,14 +110,14 @@ define_dispatch_request_enum!( /// handle the requests sequentially, without blocking stdin. /// Requests dispatched this way are automatically timed out & avoid /// processing if have already timed out before starting. -pub(crate) struct Dispatcher { - sender: mpsc::Sender<(DispatchRequest, InitActionContext, JobToken)>, +pub(crate) struct Dispatcher { + sender: mpsc::Sender<(DispatchRequest, InitActionContext, JobToken)>, } -impl Dispatcher { +impl Dispatcher { /// Creates a new `Dispatcher` starting a new thread and channel. - pub(crate) fn new(out: O) -> Self { - let (sender, receiver) = mpsc::channel::<(DispatchRequest, InitActionContext, JobToken)>(); + pub(crate) fn new(out: O) -> Self { + let (sender, receiver) = mpsc::channel::<(DispatchRequest, InitActionContext, JobToken)>(); thread::Builder::new() .name("dispatch-worker".into()) @@ -136,7 +136,7 @@ impl Dispatcher { pub(crate) fn dispatch>( &mut self, request: R, - ctx: InitActionContext, + ctx: InitActionContext, ) { let (job, token) = ConcurrentJob::new(); ctx.add_job(job); @@ -161,8 +161,45 @@ pub trait RequestAction: LSPRequest { fn fallback_response() -> Result; /// Request processing logic. - fn handle( - ctx: InitActionContext, + fn handle( + ctx: InitActionContext, params: Self::Params, ) -> Result; } + +pub type HandleResponseType = fn (&InitActionContext, + RawResponse, + &O); +// Request sent from server to client +pub trait SentRequest: LSPRequest { + /// Expected deserializable response type + type Response: for <'a> serde::Deserialize<'a>; + + /// Max duration this request should finish within + fn timeout() -> Duration { + DEFAULT_REQUEST_TIMEOUT + } + + fn handle_response + (ctx: &InitActionContext, response: RawResponse, out: &O) + { + match response.result { + Ok(val) => { + match serde_json::from_value(val) { + Ok(response_value) => Self::on_response( + ctx, response_value, out), + Err(response_err) => + error!("Unexpected response value to request: {:?} \ + (wanted {})", + response_err, + std::any::type_name::()), + } + }, + Err(error) => { + info!("Received an error to request on client: {:?}", error); + } + } + } + fn on_response + (ctx: &InitActionContext, response: Self::Response, out: &O); +} diff --git a/src/server/message.rs b/src/server/message.rs index fa7a4de..054b43b 100644 --- a/src/server/message.rs +++ b/src/server/message.rs @@ -106,7 +106,7 @@ where R: Response + std::fmt::Debug + serde::Serialize { /// Blocks stdin whilst being handled. pub trait BlockingNotificationAction: LSPNotification { /// Handles this notification. - fn handle(_: Self::Params, _: &mut InitActionContext, _: O) + fn handle(_: Self::Params, _: &mut InitActionContext, _: O) -> Result<(), ResponseError>; } @@ -118,7 +118,7 @@ pub trait BlockingRequestAction: LSPRequest { fn handle( id: RequestId, params: Self::Params, - ctx: &mut ActionContext, + ctx: &mut ActionContext, out: O, ) -> Result; } @@ -175,6 +175,12 @@ impl From for RequestId { } } +impl From for RequestId { + fn from(value: String) -> Self { + RequestId::Str(value) + } +} + /// A request that gets JSON serialized in the language server protocol. pub struct Request { /// The unique request ID. @@ -184,13 +190,13 @@ pub struct Request { /// The extra action-specific parameters. pub params: A::Params, /// This request's handler action. - pub _action: PhantomData, + pub action: PhantomData, } impl Request { /// Creates a server `Request` structure with given `params`. pub fn new(id: RequestId, params: A::Params) -> Request { - Request { id, received: Instant::now(), params, _action: PhantomData } + Request { id, received: Instant::now(), params, action: PhantomData } } } @@ -253,7 +259,7 @@ where impl Request { pub fn blocking_dispatch( self, - ctx: &mut ActionContext, + ctx: &mut ActionContext, out: &O, ) -> Result { A::handle(self.id, self.params, ctx, out.clone()) @@ -261,7 +267,7 @@ impl Request { } impl Notification { - pub fn dispatch(self, ctx: &mut InitActionContext, out: O) + pub fn dispatch(self, ctx: &mut InitActionContext, out: O) -> Result<(), ResponseError> { A::handle(self.params, ctx, out)?; Ok(()) @@ -335,9 +341,9 @@ impl RawMessage { Value::String(ref s) => Some(RequestId::Str(s.to_string())), _ => None, }; - - match parsed_id { - Some(id) => Ok(Request { id, params, received: Instant::now(), _action: PhantomData }), + + match parsed_id { + Some(id) => Ok(Request { id, params, received: Instant::now(), action: PhantomData }), None => Err(rpc_error(InvalidRequest, "Failed to parse Id")), } } @@ -357,24 +363,15 @@ impl RawMessage { Ok(Notification { params, _action: PhantomData }) } - pub(crate) fn try_parse(msg: &str) - -> Result, jsonrpc::Error> { - // Parse the message. - let ls_command: serde_json::Value = - serde_json::from_str(msg) - .map_err(|e| rpc_error(ParseError, e))?; - + pub(crate) fn try_parse(ls_command: serde_json::Value) + -> Result { // Per JSON-RPC/LSP spec, Requests must have ID, whereas Notifications cannot. let id = ls_command .get("id") .map_or(Value::Null, |id| serde_json::from_value(id.to_owned()).unwrap()); - let method = match ls_command.get("method") { - Some(method) => method, - // No method means this is a response to one of our requests. - // FIXME: we should confirm these, but currently just ignore them. - None => return Ok(None), - }; + // Guaranteed by caller + let method = ls_command.get("method").unwrap(); let method = method.as_str().ok_or_else(|| rpc_error(InvalidRequest, "Method is not a string"))? @@ -394,7 +391,7 @@ impl RawMessage { format!("Unsupported parameter type: {v}"))), }; - Ok(Some(RawMessage { method, id, params })) + Ok(RawMessage { method, id, params }) } } @@ -420,6 +417,108 @@ impl Serialize for RawMessage { } } +#[derive(Debug, PartialEq, Deserialize)] +pub struct ErrorResponse { + code: i32, + message: String, + data: Option, +} + +pub type ResultOrError = Result; + +#[derive(Debug, PartialEq)] +pub struct RawResponse { + pub id: RequestId, + pub result: ResultOrError, +} + +impl RawResponse { + pub(crate) fn try_parse(mut ls_command: serde_json::Value) + -> Result { + // Get the ID (required in responses) + let id = ls_command.get("id") + .ok_or_else(||rpc_error(InvalidRequest, "No ID in response"))? + .to_owned(); + + let parsed_id = match id { + Value::Number(n) if n.is_u64() => + Some(RequestId::Num(n.as_u64().unwrap())), + Value::String(ref s) => Some(RequestId::Str(s.to_string())), + _ => None, + }; + + let parsed_id = match parsed_id { + Some(id) => id, + None => return Err(rpc_error(InvalidRequest, "Failed to parse Id")), + }; + + let result = ls_command.get_mut("result").map( + |r|r.take()); + let error = ls_command.get_mut("error").map( + |val|ErrorResponse::deserialize(val.take())); + let resultorerror = match (result, error) { + (Some(_), Some(_)) => + return Err(rpc_error(InvalidRequest, + "Both 'result' and 'error' \ + specified in response")), + (None, None) => + return Err(rpc_error(InvalidRequest, + "Neither 'result' and 'error' \ + specified in response")), + (Some(result), None) => + Ok(result), + (None, Some(error)) => + Err(error.map_err(|e|rpc_error(ParseError, e))?), + }; + + Ok(RawResponse { id: parsed_id, result: resultorerror }) + } +} + +pub enum RawMessageOrResponse { + Message(RawMessage), + Response(RawResponse), +} + +impl RawMessageOrResponse { + pub(crate) fn try_parse(msg: &str) -> + Result { + let ls_command: serde_json::Value = + serde_json::from_str(msg) + .map_err(|e| rpc_error(ParseError, e))?; + if ls_command.get("method").is_some() { + RawMessage::try_parse(ls_command) + .map(|raw|RawMessageOrResponse::Message(raw)) + } else { + RawResponse::try_parse(ls_command) + .map(|raw|RawMessageOrResponse::Response(raw)) + } + } + + pub fn is_message(&self) -> bool { + matches!(self, RawMessageOrResponse::Message(_)) + } + + pub fn is_response(&self) -> bool { + matches!(self, RawMessageOrResponse::Response(_)) + } + + #[allow(dead_code)] + pub(crate) fn as_message(&self) -> Option<&RawMessage> { + match self { + RawMessageOrResponse::Message(mess) => Some(mess), + _ => None, + } + } + #[allow(dead_code)] + pub(crate) fn as_response(&self) -> Option<&RawResponse> { + match self { + RawMessageOrResponse::Response(resp) => Some(resp), + _ => None, + } + } +} + #[cfg(test)] mod test { use super::*; @@ -459,7 +558,10 @@ mod test { // Missing parameters are represented internally as Null. params: serde_json::Value::Null, }; - assert_eq!(expected_msg, RawMessage::try_parse(&raw_json).unwrap().unwrap()); + + let raw = RawMessageOrResponse::try_parse(&raw_json).unwrap(); + let parsed = raw.as_message().unwrap(); + assert_eq!(&expected_msg, parsed); } #[test] @@ -477,7 +579,9 @@ mod test { // Missing parameters are represented internally as Null. params: serde_json::Value::Null, }; - assert_eq!(expected_msg, RawMessage::try_parse(&raw_json).unwrap().unwrap()); + let raw = RawMessageOrResponse::try_parse(&raw_json).unwrap(); + let parsed = raw.as_message().unwrap(); + assert_eq!(&expected_msg, parsed); } #[test] @@ -546,7 +650,62 @@ mod test { #[test] fn deserialize_message_empty_params() { let msg = r#"{"jsonrpc":"2.0","method":"initialized","params":{}}"#; - let parsed = RawMessage::try_parse(msg).unwrap().unwrap(); + let raw = RawMessageOrResponse::try_parse(msg).unwrap(); + let parsed = raw.as_message().unwrap(); parsed.parse_as_notification::().unwrap(); } + + #[test] + fn simple_request_response() { + let raw_json = json!({ + "jsonrpc": "2.0", + "id": "abc", + "result": { + "this" : 0, + "really" : "could", + "be" : ["anything"] + } + }) + .to_string(); + + let expected_resp = RawResponse { + id: RequestId::from("abc".to_string()), + result: ResultOrError::Ok(json!({ + "this" : 0, + "really" : "could", + "be" : ["anything"] + })) + }; + + let raw = RawMessageOrResponse::try_parse(&raw_json).unwrap(); + let parsed = raw.as_response().unwrap(); + assert_eq!(&expected_resp, parsed); + } + #[test] + fn simple_request_error() { + let raw_json = json!({ + "jsonrpc": "2.0", + "id": "abc", + "error": { + "code" : 0, + "message" : "oh no!", + } + }) + .to_string(); + + let expected_resp = RawResponse { + id: RequestId::from("abc".to_string()), + result: ResultOrError::Err( + ErrorResponse { + code: 0, + message: "oh no!".to_string(), + data: None, + } + ), + }; + + let raw = RawMessageOrResponse::try_parse(&raw_json).unwrap(); + let parsed = raw.as_response().unwrap(); + assert_eq!(&expected_resp, parsed); + } } diff --git a/src/server/mod.rs b/src/server/mod.rs index ce999ff..95ba735 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -14,10 +14,11 @@ use crate::lsp_data::{ ShowMessageParams, Workspace, }; use crate::server::dispatch::Dispatcher; -pub use crate::server::dispatch::{RequestAction, DEFAULT_REQUEST_TIMEOUT}; +pub use crate::server::dispatch::{RequestAction, SentRequest, + DEFAULT_REQUEST_TIMEOUT}; pub use crate::server::io::{MessageReader, Output}; use crate::server::io::{StdioMsgReader, StdioOutput}; -use crate::server::message::RawMessage; +use crate::server::message::{RawMessage, RawMessageOrResponse, RawResponse}; pub use crate::server::message::{ Ack, BlockingNotificationAction, BlockingRequestAction, NoResponse, Notification, Request, @@ -88,7 +89,7 @@ impl BlockingRequestAction for ShutdownRequest { fn handle( _id: RequestId, _params: Self::Params, - ctx: &mut ActionContext, + ctx: &mut ActionContext, _out: O, ) -> Result { if let Ok(ctx) = ctx.inited() { @@ -105,7 +106,7 @@ impl BlockingRequestAction for ShutdownRequest { } } -pub(crate) fn maybe_notify_unknown_configs(out: &O, unknowns: &[String]) { +pub(crate) fn maybe_notify_unknown_configs(_out: &O, unknowns: &[String]) { use std::fmt::Write; if unknowns.is_empty() { return; @@ -116,10 +117,11 @@ pub(crate) fn maybe_notify_unknown_configs(out: &O, unknowns: &[Strin write!(msg, "{}`{}` ", if first { ' ' } else { ',' }, key).unwrap(); first = false; } - out.notify(Notification::::new(ShowMessageParams { - typ: MessageType::WARNING, - message: msg, - })); + warn!("{}", msg); + // out.notify(Notification::::new(ShowMessageParams { + // typ: MessageType::WARNING, + // message: msg, + // })); } #[allow(dead_code)] @@ -192,7 +194,7 @@ impl BlockingRequestAction for InitializeRequest { fn handle( id: RequestId, mut params: Self::Params, - ctx: &mut ActionContext, + ctx: &mut ActionContext, out: O, ) -> Result { let mut dups = std::collections::HashMap::new(); @@ -276,8 +278,8 @@ pub struct LsService { output: O, server_send: channel::Sender, server_receive: channel::Receiver, - ctx: ActionContext, - dispatcher: Dispatcher, + ctx: ActionContext, + dispatcher: Dispatcher, } impl LsService { @@ -326,12 +328,15 @@ impl LsService { } }; trace!("Read a message `{}`", msg_string); - match RawMessage::try_parse(&msg_string) { - Ok(Some(rm)) => { + match RawMessageOrResponse::try_parse(&msg_string) { + Ok(RawMessageOrResponse::Message(rm)) => { debug!("Parsed a message: {}", rm.method); send.send(ServerToHandle::ClientMessage(rm)).ok(); }, - Ok(None) => (), + Ok(RawMessageOrResponse::Response(rr)) => { + debug!("Parsed a response: {}", rr.id); + send.send(ServerToHandle::ClientResponse(rr)).ok(); + }, Err(e) => { error!("parsing error, {:?}", e); output.custom_failure( @@ -365,6 +370,11 @@ impl LsService { ServerStateChange::Break { exit_code } => return exit_code, }, + ServerToHandle::ClientResponse(resp) => { + if let ActionContext::Init(ctx) = &mut self.ctx { + ctx.handle_request_response(resp, &self.output); + } + }, ServerToHandle::ExitCode(code) => return code, ServerToHandle::IsolatedAnalysisDone(path, context, requests) => { @@ -619,6 +629,7 @@ impl LsService { #[derive(PartialEq, Debug)] pub enum ServerToHandle { ClientMessage(RawMessage), + ClientResponse(RawResponse), ExitCode(i32), IsolatedAnalysisDone(CanonPath, Option, Vec), DeviceAnalysisDone(CanonPath), @@ -647,7 +658,7 @@ fn experimental_caps() -> Value { }).unwrap() } -fn server_caps(_ctx: &ActionContext) -> ServerCapabilities { +fn server_caps(_ctx: &ActionContext) -> ServerCapabilities { ServerCapabilities { call_hierarchy_provider: None, declaration_provider: Some(DeclarationCapability::Simple(true)), @@ -797,14 +808,12 @@ mod test { /// Some clients send empty object params for void params requests (see issue #1038). #[test] fn parse_shutdown_object_params() { - let raw = RawMessage::try_parse( + let raw = RawMessageOrResponse::try_parse( r#"{"jsonrpc": "2.0", "id": 2, "method": "shutdown", "params": {}}"#, - ) - .ok() - .and_then(|x| x) - .expect("raw parse failed"); + ).unwrap(); + let parsed = raw.as_message().unwrap(); let _request: Request = - raw.parse_as_request().expect("Boring validation is happening"); + parsed.parse_as_request().expect("Boring validation is happening"); } }