From 1fce59281c38c83c937c01ff92f8656da44ccada Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 3 May 2024 20:52:58 +0100 Subject: [PATCH] refactor(lsp): cleanup cache and module registry update (#23620) --- cli/file_fetcher.rs | 8 ++ cli/lsp/jsr.rs | 7 + cli/lsp/language_server.rs | 230 ++++++++++++++------------------- cli/lsp/npm.rs | 6 + cli/lsp/registries.rs | 38 +++--- tests/integration/lsp_tests.rs | 2 +- 6 files changed, 138 insertions(+), 153 deletions(-) diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index ee226ca349..01e1e31583 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -116,6 +116,10 @@ impl MemoryFiles { pub fn insert(&self, specifier: ModuleSpecifier, file: File) -> Option { self.0.lock().insert(specifier, file) } + + pub fn clear(&self) { + self.0.lock().clear(); + } } /// Fetch a source file from the local file system. @@ -616,6 +620,10 @@ impl FileFetcher { pub fn insert_memory_files(&self, file: File) -> Option { self.memory_files.insert(file.specifier.clone(), file) } + + pub fn clear_memory_files(&self) { + self.memory_files.clear(); + } } #[derive(Debug, Eq, PartialEq)] diff --git a/cli/lsp/jsr.rs b/cli/lsp/jsr.rs index ca0dae9589..75906f8ab4 100644 --- a/cli/lsp/jsr.rs +++ b/cli/lsp/jsr.rs @@ -39,6 +39,13 @@ impl CliJsrSearchApi { pub fn get_resolver(&self) -> &JsrFetchResolver { &self.resolver } + + pub fn clear_cache(&self) { + self.file_fetcher.clear_memory_files(); + self.search_cache.clear(); + self.versions_cache.clear(); + self.exports_cache.clear(); + } } #[async_trait::async_trait] diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 07d3d8cb78..ff0fa296e3 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -174,27 +174,28 @@ pub struct Inner { /// Cached versions of "fixed" assets that can either be inlined in Rust or /// are part of the TypeScript snapshot and have to be fetched out. assets: Assets, + /// This may be a copy of `self.global_cache`, or a vendor dir if one is + /// configured. + cache: Arc, /// A representation of metadata associated with specifiers in the DENO_DIR - /// which is used by the language server + /// or vendor dir which is used by the language server. cache_metadata: cache::CacheMetadata, /// The LSP client that this LSP server is connected to. pub client: Client, /// Configuration information. pub config: Config, - deps_http_cache: Arc, diagnostics_state: Arc, diagnostics_server: diagnostics::DiagnosticsServer, /// The collection of documents that the server is currently handling, either /// on disk or "open" within the client. pub documents: Documents, + global_cache: Arc, initial_cwd: PathBuf, jsr_search_api: CliJsrSearchApi, http_client: Arc, task_queue: LanguageServerTaskQueue, /// Handles module registries, which allow discovery of modules - module_registries: ModuleRegistry, - /// The path to the module registries cache - module_registries_location: PathBuf, + module_registry: ModuleRegistry, /// An optional path to the DENO_DIR which has been specified in the client /// options. maybe_global_cache_path: Option, @@ -441,26 +442,23 @@ impl LanguageServer { impl Inner { fn new(client: Client) -> Self { let dir = DenoDir::new(None).expect("could not access DENO_DIR"); - let module_registries_location = dir.registries_folder_path(); let http_client = Arc::new(HttpClient::new(None, None)); - let module_registries = ModuleRegistry::new( - module_registries_location.clone(), - http_client.clone(), - ); + let module_registry = + ModuleRegistry::new(dir.registries_folder_path(), http_client.clone()); let jsr_search_api = - CliJsrSearchApi::new(module_registries.file_fetcher.clone()); + CliJsrSearchApi::new(module_registry.file_fetcher.clone()); let npm_search_api = - CliNpmSearchApi::new(module_registries.file_fetcher.clone()); - let deps_http_cache = Arc::new(GlobalHttpCache::new( + CliNpmSearchApi::new(module_registry.file_fetcher.clone()); + let global_cache = Arc::new(GlobalHttpCache::new( dir.deps_folder_path(), crate::cache::RealDenoCacheEnv, )); - let documents = Documents::new(deps_http_cache.clone()); - let cache_metadata = cache::CacheMetadata::new(deps_http_cache.clone()); + let cache = global_cache.clone(); + let documents = Documents::new(cache.clone()); + let cache_metadata = cache::CacheMetadata::new(cache.clone()); let performance = Arc::new(Performance::default()); let config = Config::default(); - let ts_server = - Arc::new(TsServer::new(performance.clone(), deps_http_cache.clone())); + let ts_server = Arc::new(TsServer::new(performance.clone(), cache.clone())); let diagnostics_state = Arc::new(DiagnosticsState::default()); let diagnostics_server = DiagnosticsServer::new( client.clone(), @@ -475,13 +473,14 @@ impl Inner { Self { assets, + cache, cache_metadata, client, config, - deps_http_cache, diagnostics_state, diagnostics_server, documents, + global_cache, http_client, initial_cwd: initial_cwd.clone(), jsr_search_api, @@ -489,8 +488,7 @@ impl Inner { project_version: 0, task_queue: Default::default(), maybe_testing_server: None, - module_registries, - module_registries_location, + module_registry, npm_search_api, performance, resolver: Default::default(), @@ -598,51 +596,38 @@ impl Inner { }) } - pub fn update_cache(&mut self) -> Result<(), AnyError> { - let mark = self.performance.mark("lsp.update_cache"); - self.performance.measure(mark); + pub async fn update_global_cache(&mut self) { + let mark = self.performance.mark("lsp.update_global_cache"); let maybe_cache = &self.config.workspace_settings().cache; - let maybe_global_cache_path = if let Some(cache_str) = maybe_cache { - lsp_log!("Setting global cache path from: \"{}\"", cache_str); + self.maybe_global_cache_path = if let Some(cache_str) = maybe_cache { let cache_url = if let Ok(url) = Url::from_file_path(cache_str) { Ok(url) } else if let Some(root_uri) = self.config.root_uri() { - let root_path = specifier_to_file_path(root_uri)?; - let cache_path = root_path.join(cache_str); - Url::from_file_path(cache_path).map_err(|_| { - anyhow!("Bad file path for import path: {:?}", cache_str) - }) + root_uri.join(cache_str).map_err(|e| e.into()) } else { Err(anyhow!( - "The path to the cache path (\"{}\") is not resolvable.", - cache_str + "The configured cache path \"{cache_str}\" is not resolvable outside of a workspace.", )) - }?; - let cache_path = specifier_to_file_path(&cache_url)?; - lsp_log!( - " Resolved global cache path: \"{}\"", - cache_path.to_string_lossy() - ); - Some(cache_path) + }; + cache_url + .and_then(|s| specifier_to_file_path(&s)) + .inspect(|p| { + lsp_log!("Resolved global cache path: \"{}\"", p.to_string_lossy()); + }) + .inspect_err(|err| { + lsp_warn!("Failed to resolve custom cache path: {err}"); + }) + .ok() } else { None }; - if self.maybe_global_cache_path != maybe_global_cache_path { - self.set_new_global_cache_path(maybe_global_cache_path)?; - } - Ok(()) - } - - fn recreate_http_client_and_dependents(&mut self) -> Result<(), AnyError> { - self.set_new_global_cache_path(self.maybe_global_cache_path.clone()) - } - - /// Recreates the http client and all dependent structs. - fn set_new_global_cache_path( - &mut self, - new_cache_path: Option, - ) -> Result<(), AnyError> { - let dir = DenoDir::new(new_cache_path.clone())?; + let deno_dir = match DenoDir::new(self.maybe_global_cache_path.clone()) { + Ok(d) => d, + Err(err) => { + lsp_warn!("Couldn't access DENO_DIR: {err}"); + return; + } + }; let workspace_settings = self.config.workspace_settings(); let maybe_root_path = self .config @@ -652,7 +637,9 @@ impl Inner { maybe_root_path, workspace_settings.certificate_stores.clone(), workspace_settings.tls_certificate.clone().map(CaData::File), - )?; + ) + .inspect_err(|err| lsp_warn!("Failed to load root cert store: {err}")) + .unwrap_or_else(|_| RootCertStore::empty()); let root_cert_store_provider = Arc::new(LspRootCertStoreProvider(root_cert_store)); self.http_client = Arc::new(HttpClient::new( @@ -661,72 +648,53 @@ impl Inner { .unsafely_ignore_certificate_errors .clone(), )); - self.module_registries_location = dir.registries_folder_path(); - self.module_registries = ModuleRegistry::new( - self.module_registries_location.clone(), + self.module_registry = ModuleRegistry::new( + deno_dir.registries_folder_path(), self.http_client.clone(), ); + let workspace_settings = self.config.workspace_settings(); + for (registry, enabled) in workspace_settings.suggest.imports.hosts.iter() { + if *enabled { + lsp_log!("Enabling import suggestions for: {}", registry); + self.module_registry.enable(registry).await; + } else { + self.module_registry.disable(registry); + } + } self.jsr_search_api = - CliJsrSearchApi::new(self.module_registries.file_fetcher.clone()); + CliJsrSearchApi::new(self.module_registry.file_fetcher.clone()); self.npm_search_api = - CliNpmSearchApi::new(self.module_registries.file_fetcher.clone()); - // update the cache path - let global_cache = Arc::new(GlobalHttpCache::new( - dir.deps_folder_path(), + CliNpmSearchApi::new(self.module_registry.file_fetcher.clone()); + self.global_cache = Arc::new(GlobalHttpCache::new( + deno_dir.deps_folder_path(), crate::cache::RealDenoCacheEnv, )); + self.performance.measure(mark); + } + + pub fn update_cache(&mut self) { + let mark = self.performance.mark("lsp.update_cache"); let maybe_local_cache = self.config.tree.root_vendor_dir().map(|local_path| { Arc::new(LocalLspHttpCache::new( local_path.clone(), - global_cache.clone(), + self.global_cache.clone(), )) }); - let cache: Arc = maybe_local_cache + self.url_map.set_cache(maybe_local_cache.clone()); + self.cache = maybe_local_cache .clone() .map(|c| c as Arc) - .unwrap_or(global_cache); - self.deps_http_cache = cache.clone(); - self.documents.set_cache(cache.clone()); - self.cache_metadata.set_cache(cache); - self.url_map.set_cache(maybe_local_cache); - self.maybe_global_cache_path = new_cache_path; - Ok(()) - } - - fn create_file_fetcher(&self, cache_setting: CacheSetting) -> FileFetcher { - let mut file_fetcher = FileFetcher::new( - self.deps_http_cache.clone(), - cache_setting, - true, - self.http_client.clone(), - Default::default(), - None, - ); - file_fetcher.set_download_log_level(super::logging::lsp_log_level()); - file_fetcher + .unwrap_or(self.global_cache.clone()); + self.documents.set_cache(self.cache.clone()); + self.cache_metadata.set_cache(self.cache.clone()); + self.performance.measure(mark); } pub fn update_debug_flag(&self) { let internal_debug = self.config.workspace_settings().internal_debug; super::logging::set_lsp_debug_flag(internal_debug) } - - async fn update_registries(&mut self) -> Result<(), AnyError> { - let mark = self.performance.mark("lsp.update_registries"); - self.recreate_http_client_and_dependents()?; - let workspace_settings = self.config.workspace_settings(); - for (registry, enabled) in workspace_settings.suggest.imports.hosts.iter() { - if *enabled { - lsp_log!("Enabling import suggestions for: {}", registry); - self.module_registries.enable(registry).await?; - } else { - self.module_registries.disable(registry)?; - } - } - self.performance.measure(mark); - Ok(()) - } } // lspower::LanguageServer methods. This file's LanguageServer delegates to us. @@ -822,12 +790,10 @@ impl Inner { }; self.update_debug_flag(); + self.update_global_cache().await; self.refresh_workspace_files(); self.refresh_config_tree().await; - if let Err(err) = self.update_cache() { - lsp_warn!("Error updating cache: {:#}", err); - self.client.show_message(MessageType::WARNING, err); - } + self.update_cache(); if capabilities.code_action_provider.is_some() { let fixable_diagnostics = self @@ -836,13 +802,6 @@ impl Inner { .await?; self.ts_fixable_diagnostics = fixable_diagnostics; } - - // Check to see if we need to setup any module registries - if let Err(err) = self.update_registries().await { - lsp_warn!("Error updating registries: {:#}", err); - self.client.show_message(MessageType::WARNING, err); - } - self.assets.initialize(self.snapshot()).await; self.performance.measure(mark); @@ -983,7 +942,15 @@ impl Inner { } async fn refresh_config_tree(&mut self) { - let file_fetcher = self.create_file_fetcher(CacheSetting::RespectHeaders); + let mut file_fetcher = FileFetcher::new( + self.global_cache.clone(), + CacheSetting::RespectHeaders, + true, + self.http_client.clone(), + Default::default(), + None, + ); + file_fetcher.set_download_log_level(super::logging::lsp_log_level()); self .config .tree @@ -1159,16 +1126,10 @@ impl Inner { }; self.update_debug_flag(); + self.update_global_cache().await; self.refresh_workspace_files(); self.refresh_config_tree().await; - if let Err(err) = self.update_cache() { - lsp_warn!("Error updating cache: {:#}", err); - self.client.show_message(MessageType::WARNING, err); - } - if let Err(err) = self.update_registries().await { - lsp_warn!("Error updating registries: {:#}", err); - self.client.show_message(MessageType::WARNING, err); - } + self.update_cache(); self.refresh_documents_config().await; self.diagnostics_server.invalidate_all(); self.send_diagnostics_update(); @@ -1469,12 +1430,12 @@ impl Inner { ), (true, true, _) => unreachable!("{}", json!(params)), }; - let value = - if let Some(docs) = self.module_registries.get_hover(&dep).await { - format!("{value}\n\n---\n\n{docs}") - } else { - value - }; + let value = if let Some(docs) = self.module_registry.get_hover(&dep).await + { + format!("{value}\n\n---\n\n{docs}") + } else { + value + }; Some(Hover { contents: HoverContents::Markup(MarkupContent { kind: MarkupKind::Markdown, @@ -2146,7 +2107,7 @@ impl Inner { ¶ms.text_document_position.position, &self.config, &self.client, - &self.module_registries, + &self.module_registry, &self.jsr_search_api, &self.npm_search_api, &self.documents, @@ -2277,7 +2238,7 @@ impl Inner { } } else if let Some(url) = data.documentation { CompletionItem { - documentation: self.module_registries.get_documentation(&url).await, + documentation: self.module_registry.get_documentation(&url).await, data: None, ..params } @@ -3470,16 +3431,15 @@ impl Inner { } async fn reload_import_registries(&mut self) -> LspResult> { - remove_dir_all_if_exists(&self.module_registries_location) + remove_dir_all_if_exists(&self.module_registry.location) .await .map_err(|err| { error!("Unable to remove registries cache: {:#}", err); LspError::internal_error() })?; - self.update_registries().await.map_err(|err| { - error!("Unable to update registries: {:#}", err); - LspError::internal_error() - })?; + self.module_registry.clear_cache(); + self.jsr_search_api.clear_cache(); + self.npm_search_api.clear_cache(); Ok(Some(json!(true))) } diff --git a/cli/lsp/npm.rs b/cli/lsp/npm.rs index 830aaed952..6cd6882b49 100644 --- a/cli/lsp/npm.rs +++ b/cli/lsp/npm.rs @@ -34,6 +34,12 @@ impl CliNpmSearchApi { versions_cache: Default::default(), } } + + pub fn clear_cache(&self) { + self.file_fetcher.clear_memory_files(); + self.search_cache.clear(); + self.versions_cache.clear(); + } } #[async_trait::async_trait] diff --git a/cli/lsp/registries.rs b/cli/lsp/registries.rs index a145322b50..a17cd1228f 100644 --- a/cli/lsp/registries.rs +++ b/cli/lsp/registries.rs @@ -416,6 +416,7 @@ enum VariableItems { #[derive(Debug, Clone)] pub struct ModuleRegistry { origins: HashMap>, + pub location: PathBuf, pub file_fetcher: FileFetcher, http_cache: Arc, } @@ -424,7 +425,7 @@ impl ModuleRegistry { pub fn new(location: PathBuf, http_client: Arc) -> Self { // the http cache should always be the global one for registry completions let http_cache = Arc::new(GlobalHttpCache::new( - location, + location.clone(), crate::cache::RealDenoCacheEnv, )); let mut file_fetcher = FileFetcher::new( @@ -439,6 +440,7 @@ impl ModuleRegistry { Self { origins: HashMap::new(), + location, file_fetcher, http_cache, } @@ -488,10 +490,12 @@ impl ModuleRegistry { } /// Disable a registry, removing its configuration, if any, from memory. - pub fn disable(&mut self, origin: &str) -> Result<(), AnyError> { - let origin = base_url(&Url::parse(origin)?); + pub fn disable(&mut self, origin: &str) { + let Ok(origin_url) = Url::parse(origin) else { + return; + }; + let origin = base_url(&origin_url); self.origins.remove(&origin); - Ok(()) } /// Check to see if the given origin has a registry configuration. @@ -536,13 +540,17 @@ impl ModuleRegistry { /// Enable a registry by attempting to retrieve its configuration and /// validating it. - pub async fn enable(&mut self, origin: &str) -> Result<(), AnyError> { - let origin_url = Url::parse(origin)?; + pub async fn enable(&mut self, origin: &str) { + let Ok(origin_url) = Url::parse(origin) else { + return; + }; let origin = base_url(&origin_url); #[allow(clippy::map_entry)] // we can't use entry().or_insert_with() because we can't use async closures if !self.origins.contains_key(&origin) { - let specifier = origin_url.join(CONFIG_PATH)?; + let Ok(specifier) = origin_url.join(CONFIG_PATH) else { + return; + }; match self.fetch_config(&specifier).await { Ok(configs) => { self.origins.insert(origin, configs); @@ -557,8 +565,6 @@ impl ModuleRegistry { } } } - - Ok(()) } #[cfg(test)] @@ -1092,6 +1098,10 @@ impl ModuleRegistry { .ok()?; Some(items) } + + pub fn clear_cache(&self) { + self.file_fetcher.clear_memory_files(); + } } #[cfg(test)] @@ -1256,10 +1266,7 @@ mod tests { let location = temp_dir.path().join("registries").to_path_buf(); let mut module_registry = ModuleRegistry::new(location, Arc::new(HttpClient::new(None, None))); - module_registry - .enable("http://localhost:4545/") - .await - .expect("could not enable"); + module_registry.enable("http://localhost:4545/").await; let range = lsp::Range { start: lsp::Position { line: 0, @@ -1317,10 +1324,7 @@ mod tests { let location = temp_dir.path().join("registries").to_path_buf(); let mut module_registry = ModuleRegistry::new(location, Arc::new(HttpClient::new(None, None))); - module_registry - .enable("http://localhost:4545/") - .await - .expect("could not enable"); + module_registry.enable("http://localhost:4545/").await; let range = lsp::Range { start: lsp::Position { line: 0, diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index d9f2f10cc7..faec27e1c5 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -9041,7 +9041,7 @@ fn lsp_performance() { "lsp.update_diagnostics_deps", "lsp.update_diagnostics_lint", "lsp.update_diagnostics_ts", - "lsp.update_registries", + "lsp.update_global_cache", "tsc.host.$getAssets", "tsc.host.$getDiagnostics", "tsc.host.$getSupportedCodeFixes",