From 14a74600de6f1c206ffe4750f6cb6da59657db8e Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 29 May 2024 01:26:43 +0100 Subject: [PATCH] perf(lsp): lock out requests until init is complete (#23998) --- cli/lsp/diagnostics.rs | 5 +- cli/lsp/documents.rs | 15 +- cli/lsp/language_server.rs | 368 ++++++++++++++++++++++----------- cli/lsp/mod.rs | 10 +- cli/lsp/resolver.rs | 51 +---- cli/lsp/tsc.rs | 5 +- cli/util/sync.rs | 18 ++ tests/integration/lsp_tests.rs | 87 +++----- tests/util/server/src/lsp.rs | 1 + 9 files changed, 321 insertions(+), 239 deletions(-) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 5c982b0f3f..35da778dc8 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1629,9 +1629,8 @@ mod tests { .unwrap(); config.tree.inject_config_file(config_file).await; } - let resolver = LspResolver::default() - .with_new_config(&config, &cache, None) - .await; + let resolver = + Arc::new(LspResolver::from_config(&config, &cache, None).await); let mut documents = Documents::default(); documents.update_config(&config, &resolver, &cache, &Default::default()); for (specifier, source, version, language_id) in sources { diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index b30217bcb4..5a94daed7c 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1534,9 +1534,8 @@ mod tests { let temp_dir = TempDir::new(); let cache = LspCache::new(Some(temp_dir.uri())); let config = Config::default(); - let resolver = LspResolver::default() - .with_new_config(&config, &cache, None) - .await; + let resolver = + Arc::new(LspResolver::from_config(&config, &cache, None).await); let mut documents = Documents::default(); documents.update_config(&config, &resolver, &cache, &Default::default()); (documents, cache, temp_dir) @@ -1684,9 +1683,8 @@ console.log(b, "hello deno"); ) .await; - let resolver = LspResolver::default() - .with_new_config(&config, &cache, None) - .await; + let resolver = + Arc::new(LspResolver::from_config(&config, &cache, None).await); documents.update_config(&config, &resolver, &cache, &workspace_files); // open the document @@ -1729,9 +1727,8 @@ console.log(b, "hello deno"); ) .await; - let resolver = LspResolver::default() - .with_new_config(&config, &cache, None) - .await; + let resolver = + Arc::new(LspResolver::from_config(&config, &cache, None).await); documents.update_config(&config, &resolver, &cache, &workspace_files); // check the document's dependencies diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 621f45f96f..9abefb141b 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -31,7 +31,6 @@ use std::sync::Arc; use tokio::sync::mpsc::unbounded_channel; use tokio::sync::mpsc::UnboundedReceiver; use tokio::sync::mpsc::UnboundedSender; -use tokio_util::sync::CancellationToken; use tower_lsp::jsonrpc::Error as LspError; use tower_lsp::jsonrpc::Result as LspResult; use tower_lsp::lsp_types::request::*; @@ -105,6 +104,7 @@ use crate::tools::upgrade::upgrade_check_enabled; use crate::util::fs::remove_dir_all_if_exists; use crate::util::path::is_importable_ext; use crate::util::path::to_percent_decoded_str; +use crate::util::sync::AsyncFlag; use deno_runtime::fs_util::specifier_to_file_path; struct LspRootCertStoreProvider(RootCertStore); @@ -116,10 +116,17 @@ impl RootCertStoreProvider for LspRootCertStoreProvider { } #[derive(Debug, Clone)] -pub struct LanguageServer( - pub Arc>, - CancellationToken, -); +pub struct LanguageServer { + pub inner: Arc>, + /// This is used to block out standard request handling until the complete + /// user configuration has been fetched. This is done in the `initialized` + /// handler which normally may occur concurrently with those other requests. + /// TODO(nayeemrmn): This wouldn't be necessary if LSP allowed + /// `workspace/configuration` requests in the `initialize` handler. See: + /// https://github.com/Microsoft/language-server-protocol/issues/567#issuecomment-2085131917 + init_flag: AsyncFlag, + shutdown_flag: AsyncFlag, +} /// Snapshot of the state used by TSC. #[derive(Clone, Debug, Default)] @@ -210,11 +217,12 @@ pub struct Inner { } impl LanguageServer { - pub fn new(client: Client, token: CancellationToken) -> Self { - Self( - Arc::new(tokio::sync::RwLock::new(Inner::new(client))), - token, - ) + pub fn new(client: Client, shutdown_flag: AsyncFlag) -> Self { + Self { + inner: Arc::new(tokio::sync::RwLock::new(Inner::new(client))), + init_flag: Default::default(), + shutdown_flag, + } } /// Similar to `deno cache` on the command line, where modules will be cached @@ -225,6 +233,9 @@ impl LanguageServer { referrer: ModuleSpecifier, force_global_cache: bool, ) -> LspResult> { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } async fn create_graph_for_caching( cli_options: CliOptions, roots: Vec, @@ -272,13 +283,13 @@ impl LanguageServer { // prepare the cache inside the lock let maybe_prepare_cache_result = { - let mut inner = self.0.write().await; // ensure dropped + let mut inner = self.inner.write().await; match inner.prepare_cache(specifiers, referrer, force_global_cache) { Ok(maybe_cache_result) => maybe_cache_result, Err(err) => { lsp_warn!("Error preparing caching: {:#}", err); self - .0 + .inner .read() .await .client @@ -298,7 +309,7 @@ impl LanguageServer { if let Err(err) = handle.await.unwrap() { lsp_warn!("Error caching: {:#}", err); self - .0 + .inner .read() .await .client @@ -306,7 +317,7 @@ impl LanguageServer { } // now get the lock back to update with the new information - let mut inner = self.0.write().await; + let mut inner = self.inner.write().await; inner.resolver.did_cache(); inner.refresh_npm_specifiers().await; inner.diagnostics_server.invalidate_all(); @@ -327,9 +338,12 @@ impl LanguageServer { pub async fn latest_diagnostic_batch_index_request( &self, ) -> LspResult> { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } Ok( self - .0 + .inner .read() .await .diagnostics_server @@ -339,18 +353,27 @@ impl LanguageServer { } pub async fn performance_request(&self) -> LspResult> { - Ok(Some(self.0.read().await.get_performance())) + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + Ok(Some(self.inner.read().await.get_performance())) } pub async fn task_definitions(&self) -> LspResult> { - self.0.read().await.task_definitions() + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.task_definitions() } pub async fn test_run_request( &self, params: Option, ) -> LspResult> { - let inner = self.0.read().await; + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + let inner = self.inner.read().await; if let Some(testing_server) = &inner.maybe_testing_server { match params.map(serde_json::from_value) { Some(Ok(params)) => { @@ -370,7 +393,11 @@ impl LanguageServer { &self, params: Option, ) -> LspResult> { - if let Some(testing_server) = &self.0.read().await.maybe_testing_server { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + if let Some(testing_server) = &self.inner.read().await.maybe_testing_server + { match params.map(serde_json::from_value) { Some(Ok(params)) => testing_server.run_cancel_request(params), Some(Err(err)) => Err(LspError::invalid_params(err.to_string())), @@ -385,10 +412,13 @@ impl LanguageServer { &self, params: Option, ) -> LspResult> { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } match params.map(serde_json::from_value) { Some(Ok(params)) => Ok(Some( serde_json::to_value( - self.0.read().await.virtual_text_document(params)?, + self.inner.read().await.virtual_text_document(params)?, ) .map_err(|err| { error!( @@ -405,11 +435,11 @@ impl LanguageServer { pub async fn refresh_configuration(&self) { let (client, folders, capable) = { - let ls = self.0.read().await; + let inner = self.inner.read().await; ( - ls.client.clone(), - ls.config.workspace_folders.clone(), - ls.config.client_capabilities.workspace_configuration, + inner.client.clone(), + inner.config.workspace_folders.clone(), + inner.config.client_capabilities.workspace_configuration, ) }; if capable { @@ -433,8 +463,10 @@ impl LanguageServer { for (folder_uri, _) in &folders { folder_settings.push((folder_uri.clone(), configs.next().unwrap())); } - let mut ls = self.0.write().await; - ls.config.set_workspace_settings(unscoped, folder_settings); + let mut inner = self.inner.write().await; + inner + .config + .set_workspace_settings(unscoped, folder_settings); } } } @@ -751,10 +783,6 @@ impl Inner { }; self.update_debug_flag(); - self.update_global_cache().await; - self.refresh_workspace_files(); - self.refresh_config_tree().await; - self.update_cache(); if capabilities.code_action_provider.is_some() { let fixable_diagnostics = self @@ -947,10 +975,14 @@ impl Inner { } async fn refresh_resolver(&mut self) { - self.resolver = self - .resolver - .with_new_config(&self.config, &self.cache, Some(&self.http_client)) - .await; + self.resolver = Arc::new( + LspResolver::from_config( + &self.config, + &self.cache, + Some(&self.http_client), + ) + .await, + ); } async fn refresh_documents_config(&mut self) { @@ -968,10 +1000,6 @@ impl Inner { self.project_changed([], true); } - fn shutdown(&self) -> LspResult<()> { - Ok(()) - } - async fn did_open(&mut self, params: DidOpenTextDocumentParams) { let mark = self.performance.mark_with_args("lsp.did_open", ¶ms); if params.text_document.uri.scheme() == "deno" { @@ -1149,7 +1177,17 @@ impl Inner { self.workspace_files_hash = 0; self.refresh_workspace_files(); self.refresh_config_tree().await; + self.update_cache(); self.refresh_resolver().await; + self.refresh_documents_config().await; + self.project_changed( + changes.iter().map(|(s, _)| (s, ChangeKind::Modified)), + false, + ); + self.ts_server.cleanup_semantic_cache(self.snapshot()).await; + self.diagnostics_server.invalidate_all(); + self.send_diagnostics_update(); + self.send_testing_update(); deno_config_changes.extend(changes.iter().filter_map(|(s, e)| { self.config.tree.watched_file_type(s).and_then(|t| { let configuration_type = match t.1 { @@ -1178,15 +1216,6 @@ impl Inner { }, ); } - self.refresh_documents_config().await; - self.diagnostics_server.invalidate_all(); - self.project_changed( - changes.iter().map(|(s, _)| (s, ChangeKind::Modified)), - false, - ); - self.ts_server.cleanup_semantic_cache(self.snapshot()).await; - self.send_diagnostics_update(); - self.send_testing_update(); } self.performance.measure(mark); } @@ -1254,9 +1283,8 @@ impl Inner { } let document = file_referrer.and_then(|r| self.documents.get_or_load(&specifier, &r)); - let document = match document { - Some(doc) if doc.is_open() => doc, - _ => return Ok(None), + let Some(document) = document else { + return Ok(None); }; // Detect vendored paths. Vendor file URLs will normalize to their remote // counterparts, but for formatting we want to favour the file URL. @@ -2812,6 +2840,9 @@ impl tower_lsp::LanguageServer for LanguageServer { &self, params: ExecuteCommandParams, ) -> LspResult> { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } if params.command == "deno.cache" { #[derive(Default, Deserialize)] #[serde(rename_all = "camelCase")] @@ -2828,7 +2859,7 @@ impl tower_lsp::LanguageServer for LanguageServer { .cache(specifiers, referrer, options.force_global_cache) .await } else if params.command == "deno.reloadImportRegistries" { - self.0.write().await.reload_import_registries().await + self.inner.write().await.reload_import_registries().await } else { Ok(None) } @@ -2838,14 +2869,25 @@ impl tower_lsp::LanguageServer for LanguageServer { &self, params: InitializeParams, ) -> LspResult { - self.0.write().await.initialize(params).await + self.inner.write().await.initialize(params).await } async fn initialized(&self, _: InitializedParams) { + self.refresh_configuration().await; let mut registrations = Vec::with_capacity(2); let (client, http_client) = { - let mut ls = self.0.write().await; - if ls + let mut inner = self.inner.write().await; + init_log_file(inner.config.log_file()); + inner.update_debug_flag(); + inner.update_global_cache().await; + inner.refresh_workspace_files(); + inner.refresh_config_tree().await; + inner.update_cache(); + inner.refresh_resolver().await; + inner.refresh_documents_config().await; + inner.task_queue.start(self.clone()); + self.init_flag.raise(); + if inner .config .client_capabilities .workspace_did_change_watched_files @@ -2867,7 +2909,7 @@ impl tower_lsp::LanguageServer for LanguageServer { register_options: Some(serde_json::to_value(options).unwrap()), }); } - if ls.config.client_capabilities.workspace_will_rename_files { + if inner.config.client_capabilities.workspace_will_rename_files { let options = FileOperationRegistrationOptions { filters: vec![FileOperationFilter { scheme: Some("file".to_string()), @@ -2885,17 +2927,17 @@ impl tower_lsp::LanguageServer for LanguageServer { }); } - if ls.config.client_capabilities.testing_api { + if inner.config.client_capabilities.testing_api { let test_server = testing::TestServer::new( - ls.client.clone(), - ls.performance.clone(), - ls.config.root_uri().cloned(), + inner.client.clone(), + inner.performance.clone(), + inner.config.root_uri().cloned(), ); - ls.maybe_testing_server = Some(test_server); + inner.maybe_testing_server = Some(test_server); } let mut config_events = vec![]; - for (scope_uri, config_data) in ls.config.tree.data_by_scope().iter() { + for (scope_uri, config_data) in inner.config.tree.data_by_scope().iter() { if let Some(config_file) = &config_data.config_file { config_events.push(lsp_custom::DenoConfigurationChangeEvent { scope_uri: scope_uri.clone(), @@ -2914,14 +2956,16 @@ impl tower_lsp::LanguageServer for LanguageServer { } } if !config_events.is_empty() { - ls.client.send_did_change_deno_configuration_notification( - lsp_custom::DidChangeDenoConfigurationNotificationParams { - changes: config_events, - }, - ); + inner + .client + .send_did_change_deno_configuration_notification( + lsp_custom::DidChangeDenoConfigurationNotificationParams { + changes: config_events, + }, + ); } - (ls.client.clone(), ls.http_client.clone()) + (inner.client.clone(), inner.http_client.clone()) }; for registration in registrations { @@ -2934,18 +2978,6 @@ impl tower_lsp::LanguageServer for LanguageServer { } } - self.refresh_configuration().await; - - { - let mut ls = self.0.write().await; - init_log_file(ls.config.log_file()); - ls.refresh_resolver().await; - ls.refresh_documents_config().await; - ls.diagnostics_server.invalidate_all(); - ls.send_diagnostics_update(); - ls.task_queue.start(self.clone()); - }; - if upgrade_check_enabled() { // spawn to avoid lsp send/sync requirement, but also just // to ensure this initialized method returns quickly @@ -2972,38 +3004,53 @@ impl tower_lsp::LanguageServer for LanguageServer { } async fn shutdown(&self) -> LspResult<()> { - self.1.cancel(); - self.0.write().await.shutdown() + self.shutdown_flag.raise(); + Ok(()) } async fn did_open(&self, params: DidOpenTextDocumentParams) { - self.0.write().await.did_open(params).await; + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.write().await.did_open(params).await; } async fn did_change(&self, params: DidChangeTextDocumentParams) { - self.0.write().await.did_change(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.write().await.did_change(params).await } async fn did_save(&self, params: DidSaveTextDocumentParams) { - self.0.write().await.did_save(params); + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.write().await.did_save(params); } async fn did_close(&self, params: DidCloseTextDocumentParams) { - self.0.write().await.did_close(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.write().await.did_close(params).await } async fn did_change_configuration( &self, params: DidChangeConfigurationParams, ) { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } let mark = { - let inner = self.0.read().await; + let inner = self.inner.read().await; inner .performance .mark_with_args("lsp.did_change_configuration", ¶ms) }; self.refresh_configuration().await; - let mut inner = self.0.write().await; + let mut inner = self.inner.write().await; if !inner.config.client_capabilities.workspace_configuration { let config = params.settings.as_object().map(|settings| { let deno = @@ -3035,15 +3082,26 @@ impl tower_lsp::LanguageServer for LanguageServer { &self, params: DidChangeWatchedFilesParams, ) { - self.0.write().await.did_change_watched_files(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self + .inner + .write() + .await + .did_change_watched_files(params) + .await } async fn did_change_workspace_folders( &self, params: DidChangeWorkspaceFoldersParams, ) { + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } let mark = { - let mut inner = self.0.write().await; + let mut inner = self.inner.write().await; let mark = inner .performance .mark_with_args("lsp.did_change_workspace_folders", ¶ms); @@ -3070,7 +3128,7 @@ impl tower_lsp::LanguageServer for LanguageServer { mark }; self.refresh_configuration().await; - let mut inner = self.0.write().await; + let mut inner = self.inner.write().await; inner.refresh_workspace_files(); inner.refresh_config_tree().await; inner.refresh_resolver().await; @@ -3085,176 +3143,254 @@ impl tower_lsp::LanguageServer for LanguageServer { &self, params: DocumentSymbolParams, ) -> LspResult> { - self.0.read().await.document_symbol(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.document_symbol(params).await } async fn formatting( &self, params: DocumentFormattingParams, ) -> LspResult>> { - self.0.read().await.formatting(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.formatting(params).await } async fn hover(&self, params: HoverParams) -> LspResult> { - self.0.read().await.hover(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.hover(params).await } async fn inlay_hint( &self, params: InlayHintParams, ) -> LspResult>> { - self.0.read().await.inlay_hint(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.inlay_hint(params).await } async fn code_action( &self, params: CodeActionParams, ) -> LspResult> { - self.0.read().await.code_action(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.code_action(params).await } async fn code_action_resolve( &self, params: CodeAction, ) -> LspResult { - self.0.read().await.code_action_resolve(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.code_action_resolve(params).await } async fn code_lens( &self, params: CodeLensParams, ) -> LspResult>> { - self.0.read().await.code_lens(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.code_lens(params).await } async fn code_lens_resolve(&self, params: CodeLens) -> LspResult { - self.0.read().await.code_lens_resolve(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.code_lens_resolve(params).await } async fn document_highlight( &self, params: DocumentHighlightParams, ) -> LspResult>> { - self.0.read().await.document_highlight(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.document_highlight(params).await } async fn references( &self, params: ReferenceParams, ) -> LspResult>> { - self.0.read().await.references(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.references(params).await } async fn goto_definition( &self, params: GotoDefinitionParams, ) -> LspResult> { - self.0.read().await.goto_definition(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.goto_definition(params).await } async fn goto_type_definition( &self, params: GotoTypeDefinitionParams, ) -> LspResult> { - self.0.read().await.goto_type_definition(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.goto_type_definition(params).await } async fn completion( &self, params: CompletionParams, ) -> LspResult> { - self.0.read().await.completion(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.completion(params).await } async fn completion_resolve( &self, params: CompletionItem, ) -> LspResult { - self.0.read().await.completion_resolve(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.completion_resolve(params).await } async fn goto_implementation( &self, params: GotoImplementationParams, ) -> LspResult> { - self.0.read().await.goto_implementation(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.goto_implementation(params).await } async fn folding_range( &self, params: FoldingRangeParams, ) -> LspResult>> { - self.0.read().await.folding_range(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.folding_range(params).await } async fn incoming_calls( &self, params: CallHierarchyIncomingCallsParams, ) -> LspResult>> { - self.0.read().await.incoming_calls(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.incoming_calls(params).await } async fn outgoing_calls( &self, params: CallHierarchyOutgoingCallsParams, ) -> LspResult>> { - self.0.read().await.outgoing_calls(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.outgoing_calls(params).await } async fn prepare_call_hierarchy( &self, params: CallHierarchyPrepareParams, ) -> LspResult>> { - self.0.read().await.prepare_call_hierarchy(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.prepare_call_hierarchy(params).await } async fn rename( &self, params: RenameParams, ) -> LspResult> { - self.0.read().await.rename(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.rename(params).await } async fn selection_range( &self, params: SelectionRangeParams, ) -> LspResult>> { - self.0.read().await.selection_range(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.selection_range(params).await } async fn semantic_tokens_full( &self, params: SemanticTokensParams, ) -> LspResult> { - self.0.read().await.semantic_tokens_full(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.semantic_tokens_full(params).await } async fn semantic_tokens_range( &self, params: SemanticTokensRangeParams, ) -> LspResult> { - self.0.read().await.semantic_tokens_range(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.semantic_tokens_range(params).await } async fn signature_help( &self, params: SignatureHelpParams, ) -> LspResult> { - self.0.read().await.signature_help(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.signature_help(params).await } async fn will_rename_files( &self, params: RenameFilesParams, ) -> LspResult> { - self.0.read().await.will_rename_files(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.will_rename_files(params).await } async fn symbol( &self, params: WorkspaceSymbolParams, ) -> LspResult>> { - self.0.read().await.symbol(params).await + if !self.init_flag.is_raised() { + self.init_flag.wait_raised().await; + } + self.inner.read().await.symbol(params).await } } diff --git a/cli/lsp/mod.rs b/cli/lsp/mod.rs index 07d829d919..79aa4d8f07 100644 --- a/cli/lsp/mod.rs +++ b/cli/lsp/mod.rs @@ -2,11 +2,11 @@ use deno_core::error::AnyError; use deno_core::unsync::spawn; -use tokio_util::sync::CancellationToken; use tower_lsp::LspService; use tower_lsp::Server; use crate::lsp::language_server::LanguageServer; +use crate::util::sync::AsyncFlag; pub use repl::ReplCompletionItem; pub use repl::ReplLanguageServer; @@ -44,11 +44,11 @@ pub async fn start() -> Result<(), AnyError> { let stdin = tokio::io::stdin(); let stdout = tokio::io::stdout(); - let token = CancellationToken::new(); + let shutdown_flag = AsyncFlag::default(); let builder = LspService::build(|client| { language_server::LanguageServer::new( client::Client::from_tower(client), - token.clone(), + shutdown_flag.clone(), ) }) .custom_method( @@ -80,7 +80,7 @@ pub async fn start() -> Result<(), AnyError> { let (service, socket) = builder.finish(); - // TODO(nayeemrmn): This cancellation token is a workaround for + // TODO(nayeemrmn): This shutdown flag is a workaround for // https://github.com/denoland/deno/issues/20700. Remove when // https://github.com/ebkalderon/tower-lsp/issues/399 is fixed. // Force end the server 8 seconds after receiving a shutdown request. @@ -88,7 +88,7 @@ pub async fn start() -> Result<(), AnyError> { biased; _ = Server::new(stdin, stdout, socket).serve(service) => {} _ = spawn(async move { - token.cancelled().await; + shutdown_flag.wait_raised().await; tokio::time::sleep(std::time::Duration::from_secs(8)).await; }) => {} } diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 0567bba868..5c6708c79b 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -3,7 +3,6 @@ use crate::args::create_default_npmrc; use crate::args::package_json; use crate::args::CacheSetting; -use crate::cache::FastInsecureHasher; use crate::graph_util::CliJsrUrlProvider; use crate::http_util::HttpClient; use crate::jsr::JsrCacheResolver; @@ -60,7 +59,6 @@ pub struct LspResolver { jsr_resolver: Option>, npm_resolver: Option>, node_resolver: Option>, - npm_config_hash: LspNpmConfigHash, redirect_resolver: Option>, graph_imports: Arc>, config: Arc, @@ -73,7 +71,6 @@ impl Default for LspResolver { jsr_resolver: None, npm_resolver: None, node_resolver: None, - npm_config_hash: LspNpmConfigHash(0), redirect_resolver: None, graph_imports: Default::default(), config: Default::default(), @@ -82,26 +79,17 @@ impl Default for LspResolver { } impl LspResolver { - pub async fn with_new_config( - &self, + pub async fn from_config( config: &Config, cache: &LspCache, http_client: Option<&Arc>, - ) -> Arc { - let npm_config_hash = LspNpmConfigHash::new(config, cache); + ) -> Self { let config_data = config.tree.root_data(); let mut npm_resolver = None; let mut node_resolver = None; - if npm_config_hash != self.npm_config_hash { - if let (Some(http_client), Some(config_data)) = (http_client, config_data) - { - npm_resolver = - create_npm_resolver(config_data, cache, http_client).await; - node_resolver = create_node_resolver(npm_resolver.as_ref()); - } - } else { - npm_resolver = self.npm_resolver.clone(); - node_resolver = self.node_resolver.clone(); + if let (Some(http_client), Some(config_data)) = (http_client, config_data) { + npm_resolver = create_npm_resolver(config_data, cache, http_client).await; + node_resolver = create_node_resolver(npm_resolver.as_ref()); } let graph_resolver = create_graph_resolver( config_data, @@ -136,16 +124,15 @@ impl LspResolver { ) }) .unwrap_or_default(); - Arc::new(Self { + Self { graph_resolver, jsr_resolver, npm_resolver, node_resolver, - npm_config_hash, redirect_resolver, graph_imports, config: Arc::new(config.clone()), - }) + } } pub fn snapshot(&self) -> Arc { @@ -162,7 +149,6 @@ impl LspResolver { jsr_resolver: self.jsr_resolver.clone(), npm_resolver, node_resolver, - npm_config_hash: self.npm_config_hash.clone(), redirect_resolver: self.redirect_resolver.clone(), graph_imports: self.graph_imports.clone(), config: self.config.clone(), @@ -336,7 +322,7 @@ async fn create_npm_resolver( let options = if config_data.byonm { CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions { fs: Arc::new(deno_fs::RealFs), - root_node_modules_dir: node_modules_dir, + root_node_modules_dir: node_modules_dir.clone(), }) } else { CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions { @@ -422,27 +408,6 @@ fn create_graph_resolver( })) } -#[derive(Debug, Clone, PartialEq, Eq)] -struct LspNpmConfigHash(u64); - -impl LspNpmConfigHash { - pub fn new(config: &Config, cache: &LspCache) -> Self { - let config_data = config.tree.root_data(); - let scope = config_data.map(|d| &d.scope); - let node_modules_dir = - config_data.and_then(|d| d.node_modules_dir.as_ref()); - let lockfile = config_data.and_then(|d| d.lockfile.as_ref()); - let mut hasher = FastInsecureHasher::new(); - hasher.write_hashable(scope); - hasher.write_hashable(node_modules_dir); - if let Some(lockfile) = lockfile { - hasher.write_hashable(&*lockfile.lock()); - } - hasher.write_hashable(cache.deno_dir().npm_folder_path()); - Self(hasher.finish()) - } -} - #[derive(Debug, Eq, PartialEq)] struct RedirectEntry { headers: Arc>, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 9215be0742..ebd3fc973d 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -5184,9 +5184,8 @@ mod tests { .unwrap(), ) .await; - let resolver = LspResolver::default() - .with_new_config(&config, &cache, None) - .await; + let resolver = + Arc::new(LspResolver::from_config(&config, &cache, None).await); let mut documents = Documents::default(); documents.update_config(&config, &resolver, &cache, &Default::default()); for (specifier, source, version, language_id) in sources { diff --git a/cli/util/sync.rs b/cli/util/sync.rs index dddb5991ce..3986c883e3 100644 --- a/cli/util/sync.rs +++ b/cli/util/sync.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use deno_core::futures::task::AtomicWaker; use deno_core::futures::Future; use deno_core::parking_lot::Mutex; +use tokio_util::sync::CancellationToken; /// Simplifies the use of an atomic boolean as a flag. #[derive(Debug, Default)] @@ -160,6 +161,23 @@ impl<'a> Future for TaskQueuePermitAcquireFuture<'a> { } } +#[derive(Debug, Default, Clone)] +pub struct AsyncFlag(CancellationToken); + +impl AsyncFlag { + pub fn raise(&self) { + self.0.cancel(); + } + + pub fn is_raised(&self) -> bool { + self.0.is_cancelled() + } + + pub fn wait_raised(&self) -> impl std::future::Future + '_ { + self.0.cancelled() + } +} + #[cfg(test)] mod test { use deno_core::futures; diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 33943d83ea..09882a666d 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -933,27 +933,30 @@ fn lsp_workspace_enable_paths_no_workspace_configuration() { fn lsp_did_change_deno_configuration_notification() { let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); + temp_dir.write("deno.json", json!({}).to_string()); + temp_dir.write("package.json", json!({}).to_string()); let mut client = context.new_lsp_command().build(); client.initialize_default(); - temp_dir.write("deno.json", json!({}).to_string()); - client.did_change_watched_files(json!({ - "changes": [{ - "uri": temp_dir.uri().join("deno.json").unwrap(), - "type": 1, - }], - })); let res = client .read_notification_with_method::("deno/didChangeDenoConfiguration"); assert_eq!( res, Some(json!({ - "changes": [{ - "scopeUri": temp_dir.uri(), - "fileUri": temp_dir.uri().join("deno.json").unwrap(), - "type": "added", - "configurationType": "denoJson" - }], + "changes": [ + { + "scopeUri": temp_dir.uri(), + "fileUri": temp_dir.uri().join("deno.json").unwrap(), + "type": "added", + "configurationType": "denoJson" + }, + { + "scopeUri": temp_dir.uri(), + "fileUri": temp_dir.uri().join("package.json").unwrap(), + "type": "added", + "configurationType": "packageJson" + }, + ], })) ); @@ -1002,27 +1005,6 @@ fn lsp_did_change_deno_configuration_notification() { })) ); - temp_dir.write("package.json", json!({}).to_string()); - client.did_change_watched_files(json!({ - "changes": [{ - "uri": temp_dir.uri().join("package.json").unwrap(), - "type": 1, - }], - })); - let res = client - .read_notification_with_method::("deno/didChangeDenoConfiguration"); - assert_eq!( - res, - Some(json!({ - "changes": [{ - "scopeUri": temp_dir.uri(), - "fileUri": temp_dir.uri().join("package.json").unwrap(), - "type": "added", - "configurationType": "packageJson" - }], - })) - ); - temp_dir.write("package.json", json!({ "type": "module" }).to_string()); client.did_change_watched_files(json!({ "changes": [{ @@ -9691,26 +9673,17 @@ fn lsp_format_exclude_default_config() { #[test] fn lsp_format_json() { let context = TestContextBuilder::new().use_temp_cwd().build(); - let temp_dir_path = context.temp_dir().path(); - // Also test out using a non-json file extension here. - // What should matter is the language identifier. - let lock_file_path = temp_dir_path.join("file.lock"); + let temp_dir = context.temp_dir(); + let json_file = + source_file(temp_dir.path().join("file.json"), "{\"key\":\"value\"}"); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.did_open(json!({ - "textDocument": { - "uri": lock_file_path.uri_file(), - "languageId": "json", - "version": 1, - "text": "{\"key\":\"value\"}" - } - })); let res = client.write_request( "textDocument/formatting", json!({ "textDocument": { - "uri": lock_file_path.uri_file(), + "uri": json_file.uri(), }, "options": { "tabSize": 2, @@ -9751,7 +9724,7 @@ fn lsp_json_no_diagnostics() { let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.did_open(json!({ + client.did_open_raw(json!({ "textDocument": { "uri": "file:///a/file.json", "languageId": "json", @@ -9798,7 +9771,7 @@ fn lsp_json_import_with_query_string() { ); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.did_open(json!({ + client.did_open_raw(json!({ "textDocument": { "uri": temp_dir.uri().join("data.json").unwrap(), "languageId": "json", @@ -9821,23 +9794,17 @@ fn lsp_json_import_with_query_string() { #[test] fn lsp_format_markdown() { let context = TestContextBuilder::new().use_temp_cwd().build(); - let markdown_file = context.temp_dir().path().join("file.md"); + let temp_dir = context.temp_dir(); + let markdown_file = + source_file(temp_dir.path().join("file.md"), "# Hello World"); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.did_open(json!({ - "textDocument": { - "uri": markdown_file.uri_file(), - "languageId": "markdown", - "version": 1, - "text": "# Hello World" - } - })); let res = client.write_request( "textDocument/formatting", json!({ "textDocument": { - "uri": markdown_file.uri_file() + "uri": markdown_file.uri() }, "options": { "tabSize": 2, @@ -9980,7 +9947,7 @@ fn lsp_markdown_no_diagnostics() { let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.did_open(json!({ + client.did_open_raw(json!({ "textDocument": { "uri": "file:///a/file.md", "languageId": "markdown", diff --git a/tests/util/server/src/lsp.rs b/tests/util/server/src/lsp.rs index 7c48bae23e..f210356105 100644 --- a/tests/util/server/src/lsp.rs +++ b/tests/util/server/src/lsp.rs @@ -1292,6 +1292,7 @@ impl SourceFile { "js" => "javascript", "ts" | "d.ts" => "typescript", "json" => "json", + "md" => "markdown", other => panic!("unsupported file extension: {other}"), }; Self {