From 014d8d51c0bfb10a8f47544a94d5a2b6cea7b578 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Thu, 20 May 2021 19:56:48 +1000 Subject: [PATCH] fix(lsp): re-enable the per resource configuration without a deadlock (#10625) Fixes #10603 --- cli/bench/lsp.rs | 145 +++++++-- cli/lsp/config.rs | 289 +++++++++++++++--- cli/lsp/diagnostics.rs | 6 +- cli/lsp/language_server.rs | 206 ++++++------- cli/tests/integration_tests_lsp.rs | 26 ++ cli/tests/lsp/initialize_params.json | 4 + cli/tests/lsp/initialize_params_disabled.json | 4 + cli/tests/lsp/initialize_params_registry.json | 4 + cli/tests/lsp/initialize_params_unstable.json | 4 + 9 files changed, 499 insertions(+), 189 deletions(-) diff --git a/cli/bench/lsp.rs b/cli/bench/lsp.rs index cabcef1dbf..a227e5a996 100644 --- a/cli/bench/lsp.rs +++ b/cli/bench/lsp.rs @@ -5,6 +5,8 @@ use deno_core::serde::Deserialize; use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::serde_json::Value; +use deno_core::url::Url; +use lspower::lsp; use std::collections::HashMap; use std::path::Path; use std::time::Duration; @@ -62,16 +64,15 @@ fn bench_big_file_edits(deno_exe: &Path) -> Result { }), )?; - // TODO(@kitsonk) work around https://github.com/denoland/deno/issues/10603 - // let (id, method, _): (u64, String, Option) = client.read_request()?; - // assert_eq!(method, "workspace/configuration"); + let (id, method, _): (u64, String, Option) = client.read_request()?; + assert_eq!(method, "workspace/configuration"); - // client.write_response( - // id, - // json!({ - // "enable": true - // }), - // )?; + client.write_response( + id, + json!({ + "enable": true + }), + )?; let (method, _): (String, Option) = client.read_notification()?; assert_eq!(method, "textDocument/publishDiagnostics"); @@ -122,13 +123,108 @@ fn bench_big_file_edits(deno_exe: &Path) -> Result { Ok(client.duration()) } +fn bench_find_replace(deno_exe: &Path) -> Result { + let mut client = LspClient::new(deno_exe)?; + + let params: Value = serde_json::from_slice(FIXTURE_INIT_JSON)?; + let (_, maybe_err) = + client.write_request::<_, _, Value>("initialize", params)?; + assert!(maybe_err.is_none()); + client.write_notification("initialized", json!({}))?; + + for i in 0..10 { + client.write_notification( + "textDocument/didOpen", + json!({ + "textDocument": { + "uri": format!("file:///a/file_{}.ts", i), + "languageId": "typescript", + "version": 1, + "text": "console.log(\"000\");\n" + } + }), + )?; + } + + for _ in 0..10 { + let (id, method, _) = client.read_request::()?; + assert_eq!(method, "workspace/configuration"); + client.write_response(id, json!({ "enable": true }))?; + } + + for _ in 0..3 { + let (method, _): (String, Option) = client.read_notification()?; + assert_eq!(method, "textDocument/publishDiagnostics"); + } + + for i in 0..10 { + let file_name = format!("file:///a/file_{}.ts", i); + client.write_notification( + "textDocument/didChange", + lsp::DidChangeTextDocumentParams { + text_document: lsp::VersionedTextDocumentIdentifier { + uri: Url::parse(&file_name).unwrap(), + version: 2, + }, + content_changes: vec![lsp::TextDocumentContentChangeEvent { + range: Some(lsp::Range { + start: lsp::Position { + line: 0, + character: 13, + }, + end: lsp::Position { + line: 0, + character: 16, + }, + }), + range_length: None, + text: "111".to_string(), + }], + }, + )?; + } + + for i in 0..10 { + let file_name = format!("file:///a/file_{}.ts", i); + let (maybe_res, maybe_err) = client.write_request::<_, _, Value>( + "textDocument/formatting", + lsp::DocumentFormattingParams { + text_document: lsp::TextDocumentIdentifier { + uri: Url::parse(&file_name).unwrap(), + }, + options: lsp::FormattingOptions { + tab_size: 2, + insert_spaces: true, + ..Default::default() + }, + work_done_progress_params: Default::default(), + }, + )?; + assert!(maybe_err.is_none()); + assert!(maybe_res.is_some()); + } + + for _ in 0..3 { + let (method, _): (String, Option) = client.read_notification()?; + assert_eq!(method, "textDocument/publishDiagnostics"); + } + + let (_, response_error): (Option, Option) = + client.write_request("shutdown", json!(null))?; + assert!(response_error.is_none()); + + client.write_notification("exit", json!(null))?; + + Ok(client.duration()) +} + /// A test that starts up the LSP, opens a single line document, and exits. fn bench_startup_shutdown(deno_exe: &Path) -> Result { let mut client = LspClient::new(deno_exe)?; let params: Value = serde_json::from_slice(FIXTURE_INIT_JSON)?; - let (_, response_error): (Option, Option) = - client.write_request("initialize", params)?; + let (_, response_error) = + client.write_request::<_, _, Value>("initialize", params)?; assert!(response_error.is_none()); client.write_notification("initialized", json!({}))?; @@ -145,16 +241,15 @@ fn bench_startup_shutdown(deno_exe: &Path) -> Result { }), )?; - // TODO(@kitsonk) work around https://github.com/denoland/deno/issues/10603 - // let (id, method, _): (u64, String, Option) = client.read_request()?; - // assert_eq!(method, "workspace/configuration"); + let (id, method, _) = client.read_request::()?; + assert_eq!(method, "workspace/configuration"); - // client.write_response( - // id, - // json!({ - // "enable": true - // }), - // )?; + client.write_response( + id, + json!({ + "enable": true + }), + )?; let (method, _): (String, Option) = client.read_notification()?; assert_eq!(method, "textDocument/publishDiagnostics"); @@ -199,6 +294,16 @@ pub(crate) fn benchmarks( println!(" ({} runs, mean: {}ms)", times.len(), mean); exec_times.insert("big_file_edits".to_string(), mean); + println!(" - Find/Replace"); + let mut times = Vec::new(); + for _ in 0..10 { + times.push(bench_find_replace(deno_exe)?); + } + let mean = + (times.iter().sum::() / times.len() as u32).as_millis() as u64; + println!(" ({} runs, mean: {}ms)", times.len(), mean); + exec_times.insert("find_replace".to_string(), mean); + println!("<- End benchmarking lsp"); Ok(exec_times) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 441b2cc99d..3e1b9fe85b 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -1,14 +1,22 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. +use crate::tokio_util::create_basic_runtime; + +use deno_core::error::anyhow; +use deno_core::error::AnyError; use deno_core::serde::Deserialize; use deno_core::serde_json; use deno_core::serde_json::Value; use deno_core::url::Url; use deno_core::ModuleSpecifier; -use lspower::jsonrpc::Error as LSPError; -use lspower::jsonrpc::Result as LSPResult; +use log::error; use lspower::lsp; +use std::collections::BTreeMap; use std::collections::HashMap; +use std::sync::Arc; +use std::sync::RwLock; +use std::thread; +use tokio::sync::mpsc; pub const SETTINGS_SECTION: &str = "deno"; @@ -139,25 +147,201 @@ impl WorkspaceSettings { } } +#[derive(Debug, Clone, Default)] +pub struct ConfigSnapshot { + pub client_capabilities: ClientCapabilities, + pub root_uri: Option, + pub settings: Settings, +} + +impl ConfigSnapshot { + pub fn specifier_enabled(&self, specifier: &ModuleSpecifier) -> bool { + if let Some(settings) = self.settings.specifiers.get(specifier) { + settings.1.enable + } else { + self.settings.workspace.enable + } + } +} + +enum ConfigRequest { + Specifier(ModuleSpecifier, ModuleSpecifier), + Workspace, +} + #[derive(Debug, Default, Clone)] +pub struct Settings { + pub specifiers: + BTreeMap, + pub workspace: WorkspaceSettings, +} + +#[derive(Debug)] pub struct Config { pub client_capabilities: ClientCapabilities, pub root_uri: Option, - pub specifier_settings: HashMap, - pub workspace_settings: WorkspaceSettings, + settings: Arc>, + tx: mpsc::Sender, } impl Config { - #[allow(unused)] - pub fn contains(&self, specifier: &ModuleSpecifier) -> bool { - self.specifier_settings.contains_key(specifier) + pub fn new(client: lspower::Client) -> Self { + let (tx, mut rx) = mpsc::channel::(100); + let settings = Arc::new(RwLock::new(Settings::default())); + let settings_ref = settings.clone(); + + let _join_handle = thread::spawn(move || { + let runtime = create_basic_runtime(); + + runtime.block_on(async { + loop { + match rx.recv().await { + None => break, + Some(ConfigRequest::Workspace) => { + let mut items = vec![lsp::ConfigurationItem { + scope_uri: None, + section: Some(SETTINGS_SECTION.to_string()), + }]; + let (specifier_uri_map, mut specifier_items): ( + Vec<(ModuleSpecifier, ModuleSpecifier)>, + Vec, + ) = { + let settings = settings_ref.read().unwrap(); + ( + settings + .specifiers + .iter() + .map(|(s, (u, _))| (s.clone(), u.clone())) + .collect(), + settings + .specifiers + .iter() + .map(|(_, (uri, _))| lsp::ConfigurationItem { + scope_uri: Some(uri.clone()), + section: Some(SETTINGS_SECTION.to_string()), + }) + .collect(), + ) + }; + items.append(&mut specifier_items); + if let Ok(configs) = client.configuration(items).await { + let mut settings = settings_ref.write().unwrap(); + for (i, value) in configs.into_iter().enumerate() { + match i { + 0 => { + match serde_json::from_value::(value) { + Ok(workspace_settings) => { + settings.workspace = workspace_settings; + } + Err(err) => { + error!( + "Error converting workspace settings: {}", + err + ); + } + } + } + _ => { + match serde_json::from_value::(value) { + Ok(specifier_settings) => { + let (specifier, uri) = + specifier_uri_map[i - 1].clone(); + settings + .specifiers + .insert(specifier, (uri, specifier_settings)); + } + Err(err) => { + error!( + "Error converting specifier settings: {}", + err + ); + } + } + } + } + } + } + } + Some(ConfigRequest::Specifier(specifier, uri)) => { + if settings_ref + .read() + .unwrap() + .specifiers + .contains_key(&specifier) + { + continue; + } + if let Ok(value) = client + .configuration(vec![lsp::ConfigurationItem { + scope_uri: Some(uri.clone()), + section: Some(SETTINGS_SECTION.to_string()), + }]) + .await + { + match serde_json::from_value::( + value[0].clone(), + ) { + Ok(specifier_settings) => { + settings_ref + .write() + .unwrap() + .specifiers + .insert(specifier, (uri, specifier_settings)); + } + Err(err) => { + error!("Error converting specifier settings: {}", err); + } + } + } else { + error!( + "Error retrieving settings for specifier: {}", + specifier + ); + } + } + } + } + }) + }); + + Self { + client_capabilities: ClientCapabilities::default(), + root_uri: None, + settings, + tx, + } + } + + pub fn get_workspace_settings(&self) -> WorkspaceSettings { + self.settings.read().unwrap().workspace.clone() + } + + /// Set the workspace settings directly, which occurs during initialization + /// and when the client does not support workspace configuration requests + pub fn set_workspace_settings(&self, value: Value) -> Result<(), AnyError> { + let workspace_settings = serde_json::from_value(value)?; + self.settings.write().unwrap().workspace = workspace_settings; + Ok(()) + } + + pub fn snapshot(&self) -> Result { + Ok(ConfigSnapshot { + client_capabilities: self.client_capabilities.clone(), + root_uri: self.root_uri.clone(), + settings: self + .settings + .try_read() + .map_err(|_| anyhow!("Error reading settings."))? + .clone(), + }) } pub fn specifier_enabled(&self, specifier: &ModuleSpecifier) -> bool { - if let Some(settings) = self.specifier_settings.get(specifier) { - settings.enable + let settings = self.settings.read().unwrap(); + if let Some(specifier_settings) = settings.specifiers.get(specifier) { + specifier_settings.1.enable } else { - self.workspace_settings.enable + settings.workspace.enable } } @@ -192,23 +376,24 @@ impl Config { } } - pub fn update_specifier( - &mut self, - specifier: ModuleSpecifier, - value: Value, - ) -> LSPResult<()> { - let settings: SpecifierSettings = serde_json::from_value(value) - .map_err(|err| LSPError::invalid_params(err.to_string()))?; - self.specifier_settings.insert(specifier, settings); - Ok(()) + pub async fn update_specifier_settings( + &self, + specifier: &ModuleSpecifier, + uri: &ModuleSpecifier, + ) -> Result<(), AnyError> { + self + .tx + .send(ConfigRequest::Specifier(specifier.clone(), uri.clone())) + .await + .map_err(|_| anyhow!("Error sending config update task.")) } - pub fn update_workspace(&mut self, value: Value) -> LSPResult<()> { - let settings: WorkspaceSettings = serde_json::from_value(value) - .map_err(|err| LSPError::invalid_params(err.to_string()))?; - self.workspace_settings = settings; - self.specifier_settings = HashMap::new(); - Ok(()) + pub async fn update_workspace_settings(&self) -> Result<(), AnyError> { + self + .tx + .send(ConfigRequest::Workspace) + .await + .map_err(|_| anyhow!("Error sending config update task.")) } } @@ -218,41 +403,45 @@ mod tests { use deno_core::resolve_url; use deno_core::serde_json::json; - #[test] - fn test_config_contains() { - let mut config = Config::default(); - let specifier = resolve_url("https://deno.land/x/a.ts").unwrap(); - assert!(!config.contains(&specifier)); - config - .update_specifier( - specifier.clone(), - json!({ - "enable": true - }), - ) - .expect("could not update specifier"); - assert!(config.contains(&specifier)); + #[derive(Debug, Default)] + struct MockLanguageServer; + + #[lspower::async_trait] + impl lspower::LanguageServer for MockLanguageServer { + async fn initialize( + &self, + _params: lspower::lsp::InitializeParams, + ) -> lspower::jsonrpc::Result { + Ok(lspower::lsp::InitializeResult { + capabilities: lspower::lsp::ServerCapabilities::default(), + server_info: None, + }) + } + + async fn shutdown(&self) -> lspower::jsonrpc::Result<()> { + Ok(()) + } + } + + fn setup() -> Config { + let mut maybe_client: Option = None; + let (_service, _) = lspower::LspService::new(|client| { + maybe_client = Some(client); + MockLanguageServer::default() + }); + Config::new(maybe_client.unwrap()) } #[test] fn test_config_specifier_enabled() { - let mut config = Config::default(); + let config = setup(); let specifier = resolve_url("file:///a.ts").unwrap(); assert!(!config.specifier_enabled(&specifier)); config - .update_workspace(json!({ + .set_workspace_settings(json!({ "enable": true })) .expect("could not update"); assert!(config.specifier_enabled(&specifier)); - config - .update_specifier( - specifier.clone(), - json!({ - "enable": false - }), - ) - .expect("could not update"); - assert!(!config.specifier_enabled(&specifier)); } } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 33705dbff0..ea781abdef 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -170,7 +170,7 @@ impl DiagnosticsServer { dirty = false; debounce_timer.as_mut().reset(Instant::now() + NEVER); - let snapshot = language_server.lock().await.snapshot(); + let snapshot = language_server.lock().await.snapshot().unwrap(); update_diagnostics( &client, collection.clone(), @@ -314,7 +314,7 @@ async fn generate_lint_diagnostics( collection: Arc>, ) -> Result { let documents = snapshot.documents.clone(); - let workspace_settings = snapshot.config.workspace_settings.clone(); + let workspace_settings = snapshot.config.settings.workspace.clone(); tokio::task::spawn(async move { let mut diagnostics_vec = Vec::new(); if workspace_settings.lint { @@ -487,7 +487,7 @@ async fn publish_diagnostics( if let Some(changes) = collection.take_changes() { for specifier in changes { let mut diagnostics: Vec = - if snapshot.config.workspace_settings.lint { + if snapshot.config.settings.workspace.lint { collection .get(&specifier, DiagnosticSource::DenoLint) .cloned() diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index f9bb52cbeb..d86ccefd52 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -38,6 +38,7 @@ use super::analysis::ResolvedDependency; use super::capabilities; use super::completions; use super::config::Config; +use super::config::ConfigSnapshot; use super::config::SETTINGS_SECTION; use super::diagnostics; use super::diagnostics::DiagnosticSource; @@ -77,7 +78,7 @@ pub struct LanguageServer(Arc>); #[derive(Debug, Clone, Default)] pub struct StateSnapshot { pub assets: Assets, - pub config: Config, + pub config: ConfigSnapshot, pub documents: DocumentCache, pub module_registries: registries::ModuleRegistry, pub performance: Performance, @@ -141,11 +142,12 @@ impl Inner { let ts_server = Arc::new(TsServer::new()); let performance = Performance::default(); let diagnostics_server = diagnostics::DiagnosticsServer::new(); + let config = Config::new(client.clone()); Self { assets: Default::default(), client, - config: Default::default(), + config, diagnostics_server, documents: Default::default(), maybe_config_uri: Default::default(), @@ -282,7 +284,7 @@ impl Inner { let navigation_tree: tsc::NavigationTree = self .ts_server .request( - self.snapshot(), + self.snapshot()?, tsc::RequestMethod::GetNavigationTree(specifier.clone()), ) .await?; @@ -294,16 +296,19 @@ impl Inner { } } - pub(crate) fn snapshot(&self) -> StateSnapshot { - StateSnapshot { + pub(crate) fn snapshot(&self) -> LspResult { + Ok(StateSnapshot { assets: self.assets.clone(), - config: self.config.clone(), + config: self.config.snapshot().map_err(|err| { + error!("{}", err); + LspError::internal_error() + })?, documents: self.documents.clone(), module_registries: self.module_registries.clone(), performance: self.performance.clone(), sources: self.sources.clone(), url_map: self.url_map.clone(), - } + }) } pub async fn update_import_map(&mut self) -> Result<(), AnyError> { @@ -311,7 +316,7 @@ impl Inner { let (maybe_import_map, maybe_root_uri) = { let config = &self.config; ( - config.workspace_settings.import_map.clone(), + config.get_workspace_settings().import_map, config.root_uri.clone(), ) }; @@ -357,10 +362,11 @@ impl Inner { } pub fn update_debug_flag(&self) -> bool { + let internal_debug = self.config.get_workspace_settings().internal_debug; logger::LSP_DEBUG_FLAG .compare_exchange( - !self.config.workspace_settings.internal_debug, - self.config.workspace_settings.internal_debug, + !internal_debug, + internal_debug, Ordering::Acquire, Ordering::Relaxed, ) @@ -369,8 +375,13 @@ impl Inner { async fn update_registries(&mut self) -> Result<(), AnyError> { let mark = self.performance.mark("update_registries", None::<()>); - for (registry, enabled) in - self.config.workspace_settings.suggest.imports.hosts.iter() + for (registry, enabled) in self + .config + .get_workspace_settings() + .suggest + .imports + .hosts + .iter() { if *enabled { info!("Enabling auto complete registry for: {}", registry); @@ -401,16 +412,14 @@ impl Inner { })); let (maybe_config, maybe_root_uri) = { let config = &self.config; - if config.workspace_settings.unstable { + let workspace_settings = config.get_workspace_settings(); + if workspace_settings.unstable { let unstable_libs = json!({ "lib": ["deno.ns", "deno.window", "deno.unstable"] }); tsconfig.merge(&unstable_libs); } - ( - config.workspace_settings.config.clone(), - config.root_uri.clone(), - ) + (workspace_settings.config, config.root_uri.clone()) }; if let Some(config_str) = &maybe_config { info!("Updating TypeScript configuration from: \"{}\"", config_str); @@ -443,7 +452,7 @@ impl Inner { } let _ok: bool = self .ts_server - .request(self.snapshot(), tsc::RequestMethod::Configure(tsconfig)) + .request(self.snapshot()?, tsc::RequestMethod::Configure(tsconfig)) .await?; self.performance.measure(mark); Ok(()) @@ -464,7 +473,7 @@ impl Inner { return Ok(maybe_asset.clone()); } else { let maybe_asset = - tsc::get_asset(&specifier, &self.ts_server, self.snapshot()).await?; + tsc::get_asset(&specifier, &self.ts_server, self.snapshot()?).await?; self.assets.insert(specifier.clone(), maybe_asset.clone()); Ok(maybe_asset) } @@ -507,7 +516,10 @@ impl Inner { let config = &mut self.config; config.root_uri = params.root_uri; if let Some(value) = params.initialization_options { - config.update_workspace(value)?; + config.set_workspace_settings(value).map_err(|err| { + error!("Cannot set workspace settings: {}", err); + LspError::internal_error() + })?; } config.update_capabilities(¶ms.capabilities); } @@ -520,7 +532,7 @@ impl Inner { if capabilities.code_action_provider.is_some() { let fixable_diagnostics: Vec = self .ts_server - .request(self.snapshot(), tsc::RequestMethod::GetSupportedCodeFixes) + .request(self.snapshot()?, tsc::RequestMethod::GetSupportedCodeFixes) .await .map_err(|err| { error!("Unable to get fixable diagnostics: {}", err); @@ -592,27 +604,13 @@ impl Inner { let mark = self.performance.mark("did_open", Some(¶ms)); let specifier = self.url_map.normalize_url(¶ms.text_document.uri); - // we only query the individual resource file if the client supports it - // TODO(@kitsonk) workaround https://github.com/denoland/deno/issues/10603 - // if self.config.client_capabilities.workspace_configuration - // && !self.config.contains(&specifier) - // { - // if let Ok(value) = self - // .client - // .configuration(vec![ConfigurationItem { - // scope_uri: Some(params.text_document.uri.clone()), - // section: Some(SETTINGS_SECTION.to_string()), - // }]) - // .await - // { - // if let Err(err) = self - // .config - // .update_specifier(specifier.clone(), value[0].clone()) - // { - // warn!("Error updating specifier configuration: {}", err); - // } - // } - // } + if let Err(err) = self + .config + .update_specifier_settings(&specifier, ¶ms.text_document.uri) + .await + { + error!("Error updating specifier settings: {}", err); + } if params.text_document.uri.scheme() == "deno" { // we can ignore virtual text documents opening, as they don't need to @@ -679,32 +677,8 @@ impl Inner { .mark("did_change_configuration", Some(¶ms)); if self.config.client_capabilities.workspace_configuration { - let specifiers: Vec = - self.config.specifier_settings.keys().cloned().collect(); - let mut snapshot = self.snapshot(); - let mut config_items = specifiers - .iter() - .map(|s| ConfigurationItem { - scope_uri: Some(snapshot.url_map.normalize_specifier(s).unwrap()), - section: Some(SETTINGS_SECTION.to_string()), - }) - .collect(); - let mut items = vec![ConfigurationItem { - scope_uri: None, - section: Some(SETTINGS_SECTION.to_string()), - }]; - items.append(&mut config_items); - if let Ok(configs) = self.client.configuration(items).await { - for (i, value) in configs.into_iter().enumerate() { - if let Err(err) = match i { - 0 => self.config.update_workspace(value), - _ => self - .config - .update_specifier(specifiers[i - 1].clone(), value), - } { - error!("failed to update settings: {}", err); - } - } + if let Err(err) = self.config.update_workspace_settings().await { + error!("Error updating workspace settings: {}", err); } } else if let Some(config) = params .settings @@ -713,7 +687,7 @@ impl Inner { .flatten() .cloned() { - if let Err(err) = self.config.update_workspace(config) { + if let Err(err) = self.config.set_workspace_settings(config) { error!("failed to update settings: {}", err); } } @@ -804,7 +778,7 @@ impl Inner { let req = tsc::RequestMethod::GetNavigationTree(specifier); let navigation_tree: tsc::NavigationTree = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -900,7 +874,7 @@ impl Inner { )); let maybe_quick_info: Option = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Unable to get quick info: {}", err); @@ -977,7 +951,7 @@ impl Inner { codes, )); let actions: Vec = - match self.ts_server.request(self.snapshot(), req).await { + match self.ts_server.request(self.snapshot()?, req).await { Ok(items) => items, Err(err) => { // sometimes tsc reports errors when retrieving code actions @@ -1040,7 +1014,7 @@ impl Inner { )); let combined_code_actions: tsc::CombinedCodeActions = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Unable to get combined fix from TypeScript: {}", err); @@ -1074,7 +1048,7 @@ impl Inner { ) -> LspResult>> { let specifier = self.url_map.normalize_url(¶ms.text_document.uri); if !self.config.specifier_enabled(&specifier) - || !self.config.workspace_settings.enabled_code_lens() + || !self.config.get_workspace_settings().enabled_code_lens() { return Ok(None); } @@ -1093,9 +1067,10 @@ impl Inner { let cl = Rc::new(RefCell::new(Vec::new())); navigation_tree.walk(&|i, mp| { let mut code_lenses = cl.borrow_mut(); + let workspace_settings = self.config.get_workspace_settings(); // TSC Implementations Code Lens - if self.config.workspace_settings.code_lens.implementations { + if workspace_settings.code_lens.implementations { let source = CodeLensSource::Implementations; match i.kind { tsc::ScriptElementKind::InterfaceElement => { @@ -1119,7 +1094,7 @@ impl Inner { } // TSC References Code Lens - if self.config.workspace_settings.code_lens.references { + if workspace_settings.code_lens.references { let source = CodeLensSource::References; if let Some(parent) = &mp { if parent.kind == tsc::ScriptElementKind::EnumElement { @@ -1128,12 +1103,7 @@ impl Inner { } match i.kind { tsc::ScriptElementKind::FunctionElement => { - if self - .config - .workspace_settings - .code_lens - .references_all_functions - { + if workspace_settings.code_lens.references_all_functions { code_lenses.push(i.to_code_lens( &line_index, &specifier, @@ -1214,12 +1184,14 @@ impl Inner { line_index.offset_tsc(params.range.start)?, )); let maybe_implementations: Option> = - self.ts_server.request(self.snapshot(), req).await.map_err( - |err| { + self + .ts_server + .request(self.snapshot()?, req) + .await + .map_err(|err| { error!("Error processing TypeScript request: {}", err); LspError::internal_error() - }, - )?; + })?; if let Some(implementations) = maybe_implementations { let mut locations = Vec::new(); for implementation in implementations { @@ -1292,13 +1264,14 @@ impl Inner { code_lens_data.specifier.clone(), line_index.offset_tsc(params.range.start)?, )); - let maybe_references: Option> = - self.ts_server.request(self.snapshot(), req).await.map_err( - |err| { - error!("Error processing TypeScript request: {}", err); - LspError::internal_error() - }, - )?; + let maybe_references: Option> = self + .ts_server + .request(self.snapshot()?, req) + .await + .map_err(|err| { + error!("Error processing TypeScript request: {}", err); + LspError::internal_error() + })?; if let Some(references) = maybe_references { let mut locations = Vec::new(); for reference in references { @@ -1408,7 +1381,7 @@ impl Inner { )); let maybe_document_highlights: Option> = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Unable to get document highlights from TypeScript: {}", err); @@ -1455,7 +1428,7 @@ impl Inner { )); let maybe_references: Option> = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Unable to get references from TypeScript: {}", err); @@ -1510,7 +1483,7 @@ impl Inner { )); let maybe_definition: Option = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Unable to get definition from TypeScript: {}", err); @@ -1545,7 +1518,7 @@ impl Inner { let response = if let Some(response) = completions::get_import_completions( &specifier, ¶ms.text_document_position.position, - &self.snapshot(), + &self.snapshot()?, ) .await { @@ -1580,7 +1553,7 @@ impl Inner { )); let maybe_completion_info: Option = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Unable to get completion info from TypeScript: {}", err); @@ -1590,7 +1563,7 @@ impl Inner { if let Some(completions) = maybe_completion_info { let results = completions.as_completion_response( &line_index, - &self.config.workspace_settings.suggest, + &self.config.get_workspace_settings().suggest, &specifier, position, ); @@ -1618,13 +1591,14 @@ impl Inner { })?; if let Some(data) = data.tsc { let req = tsc::RequestMethod::GetCompletionDetails(data.into()); - let maybe_completion_info: Option = - self.ts_server.request(self.snapshot(), req).await.map_err( - |err| { - error!("Unable to get completion info from TypeScript: {}", err); - LspError::internal_error() - }, - )?; + let maybe_completion_info: Option = self + .ts_server + .request(self.snapshot()?, req) + .await + .map_err(|err| { + error!("Unable to get completion info from TypeScript: {}", err); + LspError::internal_error() + })?; if let Some(completion_info) = maybe_completion_info { completion_info.as_completion_item(¶ms) } else { @@ -1670,7 +1644,7 @@ impl Inner { )); let maybe_implementations: Option> = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -1716,7 +1690,7 @@ impl Inner { let req = tsc::RequestMethod::GetOutliningSpans(specifier.clone()); let outlining_spans: Vec = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -1776,7 +1750,7 @@ impl Inner { )); let incoming_calls: Vec = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -1830,7 +1804,7 @@ impl Inner { )); let outgoing_calls: Vec = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -1890,7 +1864,7 @@ impl Inner { let maybe_one_or_many: Option> = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -1970,7 +1944,7 @@ impl Inner { let maybe_locations: Option> = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -2057,7 +2031,7 @@ impl Inner { let selection_range: tsc::SelectionRange = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -2099,7 +2073,7 @@ impl Inner { )); let semantic_classification: tsc::Classifications = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -2147,7 +2121,7 @@ impl Inner { )); let semantic_classification: tsc::Classifications = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver {}", err); @@ -2204,7 +2178,7 @@ impl Inner { )); let maybe_signature_help_items: Option = self .ts_server - .request(self.snapshot(), req) + .request(self.snapshot()?, req) .await .map_err(|err| { error!("Failed to request to tsserver: {}", err); diff --git a/cli/tests/integration_tests_lsp.rs b/cli/tests/integration_tests_lsp.rs index 15abfe6781..a6d4f2d297 100644 --- a/cli/tests/integration_tests_lsp.rs +++ b/cli/tests/integration_tests_lsp.rs @@ -38,6 +38,13 @@ where client .write_notification("textDocument/didOpen", params) .unwrap(); + + let (id, method, _) = client.read_request::().unwrap(); + assert_eq!(method, "workspace/configuration"); + client + .write_response(id, json!({ "enable": true })) + .unwrap(); + let (method, _) = client.read_notification::().unwrap(); assert_eq!(method, "textDocument/publishDiagnostics"); let (method, _) = client.read_notification::().unwrap(); @@ -209,6 +216,13 @@ fn lsp_hover_disabled() { }), ) .unwrap(); + + let (id, method, _) = client.read_request::().unwrap(); + assert_eq!(method, "workspace/configuration"); + client + .write_response(id, json!({ "enable": false })) + .unwrap(); + let (maybe_res, maybe_err) = client .write_request( "textDocument/hover", @@ -453,6 +467,12 @@ fn lsp_hover_closed_document() { }), ) .unwrap(); + let (id, method, _) = client.read_request::().unwrap(); + assert_eq!(method, "workspace/configuration"); + client + .write_response(id, json!({ "enable": true })) + .unwrap(); + client .write_notification( "textDocument/didOpen", @@ -466,6 +486,12 @@ fn lsp_hover_closed_document() { }), ) .unwrap(); + let (id, method, _) = client.read_request::().unwrap(); + assert_eq!(method, "workspace/configuration"); + client + .write_response(id, json!({ "enable": true })) + .unwrap(); + let (method, _) = client.read_notification::().unwrap(); assert_eq!(method, "textDocument/publishDiagnostics"); let (method, _) = client.read_notification::().unwrap(); diff --git a/cli/tests/lsp/initialize_params.json b/cli/tests/lsp/initialize_params.json index b00e5720dd..98ec53aa6f 100644 --- a/cli/tests/lsp/initialize_params.json +++ b/cli/tests/lsp/initialize_params.json @@ -51,6 +51,10 @@ "willSaveWaitUntil": true, "didSave": true } + }, + "workspace": { + "configuration": true, + "workspaceFolders": true } } } diff --git a/cli/tests/lsp/initialize_params_disabled.json b/cli/tests/lsp/initialize_params_disabled.json index ea12dfac58..d5e59770ad 100644 --- a/cli/tests/lsp/initialize_params_disabled.json +++ b/cli/tests/lsp/initialize_params_disabled.json @@ -51,6 +51,10 @@ "willSaveWaitUntil": true, "didSave": true } + }, + "workspace": { + "configuration": true, + "workspaceFolders": true } } } diff --git a/cli/tests/lsp/initialize_params_registry.json b/cli/tests/lsp/initialize_params_registry.json index 38777b3d48..0a19c50ec8 100644 --- a/cli/tests/lsp/initialize_params_registry.json +++ b/cli/tests/lsp/initialize_params_registry.json @@ -53,6 +53,10 @@ "willSaveWaitUntil": true, "didSave": true } + }, + "workspace": { + "configuration": true, + "workspaceFolders": true } } } diff --git a/cli/tests/lsp/initialize_params_unstable.json b/cli/tests/lsp/initialize_params_unstable.json index fd8ccdd83a..153e3aef15 100644 --- a/cli/tests/lsp/initialize_params_unstable.json +++ b/cli/tests/lsp/initialize_params_unstable.json @@ -51,6 +51,10 @@ "willSaveWaitUntil": true, "didSave": true } + }, + "workspace": { + "configuration": true, + "workspaceFolders": true } } }