From 28a72d548801f81a96ff4bba750d8dc51a2b1567 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 11 May 2023 17:17:14 -0400 Subject: [PATCH] feat(lsp): ability to configure document pre-load limit (#19097) Adds a `deno.preloadLimit` option (ex. `"deno.preloadLimit": 2000`) which specifies how many file entries to traverse on the file system when the lsp loads or its configuration changes. Closes #18955 --- cli/lsp/client.rs | 20 --- cli/lsp/code_lens.rs | 2 +- cli/lsp/completions.rs | 2 +- cli/lsp/config.rs | 24 ++- cli/lsp/diagnostics.rs | 2 +- cli/lsp/documents.rs | 234 +++++++++++++++------------- cli/lsp/language_server.rs | 66 ++++---- cli/lsp/repl.rs | 1 + cli/lsp/tsc.rs | 2 +- cli/tests/integration/lsp_tests.rs | 43 +++++ cli/tests/integration/repl_tests.rs | 5 +- test_util/src/lsp.rs | 6 + 12 files changed, 239 insertions(+), 168 deletions(-) diff --git a/cli/lsp/client.rs b/cli/lsp/client.rs index e684dc09fc..d24d4c2a9e 100644 --- a/cli/lsp/client.rs +++ b/cli/lsp/client.rs @@ -26,13 +26,6 @@ pub enum TestingNotification { Progress(testing_lsp_custom::TestRunProgressParams), } -#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] -pub enum LspClientKind { - #[default] - CodeEditor, - Repl, -} - #[derive(Clone)] pub struct Client(Arc); @@ -51,10 +44,6 @@ impl Client { Self(Arc::new(ReplClient)) } - pub fn kind(&self) -> LspClientKind { - self.0.kind() - } - /// Gets additional methods that should only be called outside /// the LSP's lock to prevent deadlocking scenarios. pub fn when_outside_lsp_lock(&self) -> OutsideLockClient { @@ -160,7 +149,6 @@ impl OutsideLockClient { #[async_trait] trait ClientTrait: Send + Sync { - fn kind(&self) -> LspClientKind; async fn publish_diagnostics( &self, uri: lsp::Url, @@ -189,10 +177,6 @@ struct TowerClient(tower_lsp::Client); #[async_trait] impl ClientTrait for TowerClient { - fn kind(&self) -> LspClientKind { - LspClientKind::CodeEditor - } - async fn publish_diagnostics( &self, uri: lsp::Url, @@ -312,10 +296,6 @@ struct ReplClient; #[async_trait] impl ClientTrait for ReplClient { - fn kind(&self) -> LspClientKind { - LspClientKind::Repl - } - async fn publish_diagnostics( &self, _uri: lsp::Url, diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index 6327b7a9cf..fd7f350061 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -391,7 +391,7 @@ pub async fn collect( code_lenses.extend( collect_tsc( specifier, - &config.get_workspace_settings(), + config.workspace_settings(), line_index, navigation_tree, ) diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index d91fc29c28..070e3168a5 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -519,7 +519,7 @@ mod tests { source_fixtures: &[(&str, &str)], location: &Path, ) -> Documents { - let mut documents = Documents::new(location, Default::default()); + let mut documents = Documents::new(location); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index f4b2d8c09e..0a25e2b992 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -265,6 +265,10 @@ fn default_to_true() -> bool { true } +fn default_document_preload_limit() -> usize { + 1000 +} + fn empty_string_none<'de, D: serde::Deserializer<'de>>( d: D, ) -> Result, D::Error> { @@ -318,6 +322,10 @@ pub struct WorkspaceSettings { #[serde(default = "default_to_true")] pub lint: bool, + /// Limits the number of files that can be preloaded by the language server. + #[serde(default = "default_document_preload_limit")] + pub document_preload_limit: usize, + /// A flag that indicates if Dene should validate code against the unstable /// APIs for the workspace. #[serde(default)] @@ -354,6 +362,7 @@ impl Default for WorkspaceSettings { inlay_hints: Default::default(), internal_debug: false, lint: true, + document_preload_limit: default_document_preload_limit(), suggest: Default::default(), testing: Default::default(), tls_certificate: None, @@ -439,8 +448,8 @@ impl Config { } } - pub fn get_workspace_settings(&self) -> WorkspaceSettings { - self.settings.workspace.clone() + pub fn workspace_settings(&self) -> &WorkspaceSettings { + &self.settings.workspace } /// Set the workspace settings directly, which occurs during initialization @@ -714,7 +723,7 @@ mod tests { .set_workspace_settings(json!({})) .expect("could not update"); assert_eq!( - config.get_workspace_settings(), + config.workspace_settings().clone(), WorkspaceSettings { enable: false, enable_paths: Vec::new(), @@ -750,6 +759,7 @@ mod tests { }, internal_debug: false, lint: true, + document_preload_limit: 1_000, suggest: CompletionSettings { complete_function_calls: false, names: true, @@ -778,7 +788,7 @@ mod tests { .set_workspace_settings(json!({ "cache": "" })) .expect("could not update"); assert_eq!( - config.get_workspace_settings(), + config.workspace_settings().clone(), WorkspaceSettings::default() ); } @@ -790,7 +800,7 @@ mod tests { .set_workspace_settings(json!({ "import_map": "" })) .expect("could not update"); assert_eq!( - config.get_workspace_settings(), + config.workspace_settings().clone(), WorkspaceSettings::default() ); } @@ -802,7 +812,7 @@ mod tests { .set_workspace_settings(json!({ "tls_certificate": "" })) .expect("could not update"); assert_eq!( - config.get_workspace_settings(), + config.workspace_settings().clone(), WorkspaceSettings::default() ); } @@ -814,7 +824,7 @@ mod tests { .set_workspace_settings(json!({ "config": "" })) .expect("could not update"); assert_eq!( - config.get_workspace_settings(), + config.workspace_settings().clone(), WorkspaceSettings::default() ); } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index b650d8e558..7d13cfdb5f 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1096,7 +1096,7 @@ mod tests { location: &Path, maybe_import_map: Option<(&str, &str)>, ) -> StateSnapshot { - let mut documents = Documents::new(location, Default::default()); + let mut documents = Documents::new(location); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index b55d3ca206..6577d27692 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1,7 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use super::cache::calculate_fs_version; -use super::client::LspClientKind; use super::text::LineIndex; use super::tsc; use super::tsc::AssetDocument; @@ -793,6 +792,16 @@ fn get_document_path( } } +pub struct UpdateDocumentConfigOptions<'a> { + pub enabled_urls: Vec, + pub document_preload_limit: usize, + pub maybe_import_map: Option>, + pub maybe_config_file: Option<&'a ConfigFile>, + pub maybe_package_json: Option<&'a PackageJson>, + pub npm_registry_api: Arc, + pub npm_resolution: Arc, +} + /// Specify the documents to include on a `documents.documents(...)` call. #[derive(Debug, Clone, Copy)] pub enum DocumentsFilter { @@ -818,8 +827,6 @@ pub struct Documents { open_docs: HashMap, /// Documents stored on the file system. file_system_docs: Arc>, - /// Kind of the client that is using the documents. - lsp_client_kind: LspClientKind, /// Hash of the config used for resolution. When the hash changes we update /// dependencies. resolver_config_hash: u64, @@ -839,14 +846,13 @@ pub struct Documents { } impl Documents { - pub fn new(location: &Path, lsp_client_kind: LspClientKind) -> Self { + pub fn new(location: &Path) -> Self { Self { cache: HttpCache::new(location), dirty: true, dependents_map: Default::default(), open_docs: HashMap::default(), file_system_docs: Default::default(), - lsp_client_kind, resolver_config_hash: 0, imports: Default::default(), resolver: Default::default(), @@ -1161,15 +1167,7 @@ impl Documents { Ok(()) } - pub fn update_config( - &mut self, - enabled_urls: Vec, - maybe_import_map: Option>, - maybe_config_file: Option<&ConfigFile>, - maybe_package_json: Option<&PackageJson>, - npm_registry_api: Arc, - npm_resolution: Arc, - ) { + pub fn update_config(&mut self, options: UpdateDocumentConfigOptions) { fn calculate_resolver_config_hash( enabled_urls: &[Url], maybe_import_map: Option<&import_map::ImportMap>, @@ -1208,14 +1206,16 @@ impl Documents { hasher.finish() } - let maybe_package_json_deps = maybe_package_json.map(|package_json| { - package_json::get_local_package_json_version_reqs(package_json) - }); - let maybe_jsx_config = - maybe_config_file.and_then(|cf| cf.to_maybe_jsx_import_source_config()); + let maybe_package_json_deps = + options.maybe_package_json.map(|package_json| { + package_json::get_local_package_json_version_reqs(package_json) + }); + let maybe_jsx_config = options + .maybe_config_file + .and_then(|cf| cf.to_maybe_jsx_import_source_config()); let new_resolver_config_hash = calculate_resolver_config_hash( - &enabled_urls, - maybe_import_map.as_deref(), + &options.enabled_urls, + options.maybe_import_map.as_deref(), maybe_jsx_config.as_ref(), maybe_package_json_deps.as_ref(), ); @@ -1223,21 +1223,21 @@ impl Documents { Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps)); let deps_installer = Arc::new(PackageJsonDepsInstaller::new( deps_provider.clone(), - npm_registry_api.clone(), - npm_resolution.clone(), + options.npm_registry_api.clone(), + options.npm_resolution.clone(), )); self.resolver = Arc::new(CliGraphResolver::new( maybe_jsx_config, - maybe_import_map, + options.maybe_import_map, false, - npm_registry_api, - npm_resolution, + options.npm_registry_api, + options.npm_resolution, deps_provider, deps_installer, )); self.imports = Arc::new( if let Some(Ok(imports)) = - maybe_config_file.map(|cf| cf.to_maybe_imports()) + options.maybe_config_file.map(|cf| cf.to_maybe_imports()) { imports .into_iter() @@ -1257,14 +1257,21 @@ impl Documents { // only refresh the dependencies if the underlying configuration has changed if self.resolver_config_hash != new_resolver_config_hash { - self.refresh_dependencies(enabled_urls); + self.refresh_dependencies( + options.enabled_urls, + options.document_preload_limit, + ); self.resolver_config_hash = new_resolver_config_hash; } self.dirty = true; } - fn refresh_dependencies(&mut self, enabled_urls: Vec) { + fn refresh_dependencies( + &mut self, + enabled_urls: Vec, + document_preload_limit: usize, + ) { let resolver = self.resolver.as_graph_resolver(); for doc in self.open_docs.values_mut() { if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { @@ -1274,51 +1281,73 @@ impl Documents { // update the file system documents let mut fs_docs = self.file_system_docs.lock(); - match self.lsp_client_kind { - LspClientKind::CodeEditor => { - let mut not_found_docs = - fs_docs.docs.keys().cloned().collect::>(); - let open_docs = &mut self.open_docs; + if document_preload_limit > 0 { + let mut not_found_docs = + fs_docs.docs.keys().cloned().collect::>(); + let open_docs = &mut self.open_docs; - log::debug!("Preloading documents from enabled urls..."); - for specifier in PreloadDocumentFinder::from_enabled_urls(&enabled_urls) + log::debug!("Preloading documents from enabled urls..."); + let mut finder = PreloadDocumentFinder::from_enabled_urls_with_limit( + &enabled_urls, + document_preload_limit, + ); + for specifier in finder.by_ref() { + // mark this document as having been found + not_found_docs.remove(&specifier); + + if !open_docs.contains_key(&specifier) + && !fs_docs.docs.contains_key(&specifier) { - // mark this document as having been found - not_found_docs.remove(&specifier); - - if !open_docs.contains_key(&specifier) - && !fs_docs.docs.contains_key(&specifier) - { - fs_docs.refresh_document(&self.cache, resolver, &specifier); - } else { - // update the existing entry to have the new resolver - if let Some(doc) = fs_docs.docs.get_mut(&specifier) { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { - *doc = new_doc; - } + fs_docs.refresh_document(&self.cache, resolver, &specifier); + } else { + // update the existing entry to have the new resolver + if let Some(doc) = fs_docs.docs.get_mut(&specifier) { + if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + *doc = new_doc; } } } + } + if finder.hit_limit() { + lsp_warn!( + concat!( + "Hit the language server document preload limit of {} file system entries. ", + "You may want to use the \"deno.enablePaths\" configuration setting to only have Deno ", + "partially enable a workspace or increase the limit via \"deno.documentPreloadLimit\". ", + "In cases where Deno ends up using too much memory, you may want to lower the limit." + ), + document_preload_limit, + ); + + // since we hit the limit, just update everything to use the new resolver + for uri in not_found_docs { + if let Some(doc) = fs_docs.docs.get_mut(&uri) { + if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + *doc = new_doc; + } + } + } + } else { // clean up and remove any documents that weren't found for uri in not_found_docs { fs_docs.docs.remove(&uri); } } - LspClientKind::Repl => { - // This log statement is used in the tests to ensure preloading doesn't - // happen, which is not useful in the repl and could be very expensive - // if the repl is launched from a directory with a lot of descendants. - log::debug!("Skipping document preload for repl."); + } else { + // This log statement is used in the tests to ensure preloading doesn't + // happen, which is not useful in the repl and could be very expensive + // if the repl is launched from a directory with a lot of descendants. + log::debug!("Skipping document preload."); - // for the repl, just update to use the new resolver - for doc in fs_docs.docs.values_mut() { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { - *doc = new_doc; - } + // just update to use the new resolver + for doc in fs_docs.docs.values_mut() { + if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + *doc = new_doc; } } } + fs_docs.dirty = true; } @@ -1558,19 +1587,15 @@ enum PendingEntry { /// Iterator that finds documents that can be preloaded into /// the LSP on startup. struct PreloadDocumentFinder { - limit: u16, - entry_count: u16, + limit: usize, + entry_count: usize, pending_entries: VecDeque, } impl PreloadDocumentFinder { - pub fn from_enabled_urls(enabled_urls: &Vec) -> Self { - Self::from_enabled_urls_with_limit(enabled_urls, 1_000) - } - pub fn from_enabled_urls_with_limit( enabled_urls: &Vec, - limit: u16, + limit: usize, ) -> Self { fn is_allowed_root_dir(dir_path: &Path) -> bool { if dir_path.parent().is_none() { @@ -1605,6 +1630,10 @@ impl PreloadDocumentFinder { finder } + pub fn hit_limit(&self) -> bool { + self.entry_count >= self.limit + } + fn get_valid_specifier(path: &Path) -> Option { fn is_allowed_media_type(media_type: MediaType) -> bool { match media_type { @@ -1699,15 +1728,7 @@ impl Iterator for PreloadDocumentFinder { while let Some(entry) = entries.next() { self.entry_count += 1; - if self.entry_count >= self.limit { - lsp_warn!( - concat!( - "Hit the language server document preload limit of {} file system entries. ", - "You may want to use the \"deno.enablePaths\" configuration setting to only have Deno ", - "partially enable a workspace." - ), - self.limit, - ); + if self.hit_limit() { self.pending_entries.clear(); // stop searching return None; } @@ -1769,7 +1790,7 @@ mod tests { fn setup(temp_dir: &TempDir) -> (Documents, PathBuf) { let location = temp_dir.path().join("deps"); - let documents = Documents::new(&location, Default::default()); + let documents = Documents::new(&location); (documents, location) } @@ -1899,14 +1920,15 @@ console.log(b, "hello deno"); .append("test".to_string(), "./file2.ts".to_string()) .unwrap(); - documents.update_config( - vec![], - Some(Arc::new(import_map)), - None, - None, - npm_registry_api.clone(), - npm_resolution.clone(), - ); + documents.update_config(UpdateDocumentConfigOptions { + enabled_urls: vec![], + document_preload_limit: 1_000, + maybe_import_map: Some(Arc::new(import_map)), + maybe_config_file: None, + maybe_package_json: None, + npm_registry_api: npm_registry_api.clone(), + npm_resolution: npm_resolution.clone(), + }); // open the document let document = documents.open( @@ -1939,14 +1961,15 @@ console.log(b, "hello deno"); .append("test".to_string(), "./file3.ts".to_string()) .unwrap(); - documents.update_config( - vec![], - Some(Arc::new(import_map)), - None, - None, + documents.update_config(UpdateDocumentConfigOptions { + enabled_urls: vec![], + document_preload_limit: 1_000, + maybe_import_map: Some(Arc::new(import_map)), + maybe_config_file: None, + maybe_package_json: None, npm_registry_api, npm_resolution, - ); + }); // check the document's dependencies let document = documents.get(&file1_specifier).unwrap(); @@ -2001,12 +2024,15 @@ console.log(b, "hello deno"); temp_dir.create_dir_all("root3/"); temp_dir.write("root3/mod.ts", ""); // no, not provided - let mut urls = PreloadDocumentFinder::from_enabled_urls(&vec![ - temp_dir.uri().join("root1/").unwrap(), - temp_dir.uri().join("root2/file1.ts").unwrap(), - temp_dir.uri().join("root2/main.min.ts").unwrap(), - temp_dir.uri().join("root2/folder/").unwrap(), - ]) + let mut urls = PreloadDocumentFinder::from_enabled_urls_with_limit( + &vec![ + temp_dir.uri().join("root1/").unwrap(), + temp_dir.uri().join("root2/file1.ts").unwrap(), + temp_dir.uri().join("root2/main.min.ts").unwrap(), + temp_dir.uri().join("root2/folder/").unwrap(), + ], + 1_000, + ) .collect::>(); // Ideally we would test for order here, which should be BFS, but @@ -2048,18 +2074,18 @@ console.log(b, "hello deno"); #[test] pub fn test_pre_load_document_finder_disallowed_dirs() { if cfg!(windows) { - let paths = PreloadDocumentFinder::from_enabled_urls(&vec![Url::parse( - "file:///c:/", + let paths = PreloadDocumentFinder::from_enabled_urls_with_limit( + &vec![Url::parse("file:///c:/").unwrap()], + 1_000, ) - .unwrap()]) .collect::>(); assert_eq!(paths, vec![]); } else { - let paths = - PreloadDocumentFinder::from_enabled_urls(&vec![ - Url::parse("file:///").unwrap() - ]) - .collect::>(); + let paths = PreloadDocumentFinder::from_enabled_urls_with_limit( + &vec![Url::parse("file:///").unwrap()], + 1_000, + ) + .collect::>(); assert_eq!(paths, vec![]); } } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 1eb3944726..7fe986bfee 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -49,6 +49,7 @@ use super::documents::Document; use super::documents::Documents; use super::documents::DocumentsFilter; use super::documents::LanguageId; +use super::documents::UpdateDocumentConfigOptions; use super::logging::lsp_log; use super::logging::lsp_warn; use super::lsp_custom; @@ -285,7 +286,7 @@ impl LanguageServer { if let Some(testing_server) = &inner.maybe_testing_server { match params.map(serde_json::from_value) { Some(Ok(params)) => testing_server - .run_request(params, inner.config.get_workspace_settings()), + .run_request(params, inner.config.workspace_settings().clone()), Some(Err(err)) => Err(LspError::invalid_params(err.to_string())), None => Err(LspError::invalid_params("Missing parameters")), } @@ -489,7 +490,7 @@ impl Inner { let module_registries = ModuleRegistry::new(&module_registries_location, http_client.clone()); let location = dir.deps_folder_path(); - let documents = Documents::new(&location, client.kind()); + let documents = Documents::new(&location); let deps_http_cache = HttpCache::new(&location); let cache_metadata = cache::CacheMetadata::new(deps_http_cache.clone()); let performance = Arc::new(Performance::default()); @@ -602,9 +603,9 @@ impl Inner { } fn get_config_file(&self) -> Result, AnyError> { - let workspace_settings = self.config.get_workspace_settings(); - let maybe_config = workspace_settings.config; - if let Some(config_str) = &maybe_config { + let workspace_settings = self.config.workspace_settings(); + let maybe_config = &workspace_settings.config; + if let Some(config_str) = maybe_config { if !config_str.is_empty() { lsp_log!("Setting Deno configuration from: \"{}\"", config_str); let config_url = if let Ok(url) = Url::from_file_path(config_str) { @@ -744,8 +745,8 @@ impl Inner { pub fn update_cache(&mut self) -> Result<(), AnyError> { let mark = self.performance.mark("update_cache", None::<()>); self.performance.measure(mark); - let maybe_cache = self.config.get_workspace_settings().cache; - let maybe_cache_path = if let Some(cache_str) = &maybe_cache { + let maybe_cache = &self.config.workspace_settings().cache; + let maybe_cache_path = if let Some(cache_str) = maybe_cache { lsp_log!("Setting cache path from: \"{}\"", cache_str); let cache_url = if let Ok(url) = Url::from_file_path(cache_str) { Ok(url) @@ -785,7 +786,7 @@ impl Inner { .clone() .or_else(|| env::var("DENO_DIR").map(String::into).ok()); let dir = DenoDir::new(maybe_custom_root)?; - let workspace_settings = self.config.get_workspace_settings(); + let workspace_settings = self.config.workspace_settings(); let maybe_root_path = self .config .root_uri @@ -793,15 +794,17 @@ impl Inner { .and_then(|uri| specifier_to_file_path(uri).ok()); let root_cert_store = get_root_cert_store( maybe_root_path, - workspace_settings.certificate_stores, - workspace_settings.tls_certificate.map(CaData::File), + workspace_settings.certificate_stores.clone(), + workspace_settings.tls_certificate.clone().map(CaData::File), )?; let root_cert_store_provider = Arc::new(LspRootCertStoreProvider(root_cert_store)); let module_registries_location = dir.registries_folder_path(); self.http_client = Arc::new(HttpClient::new( Some(root_cert_store_provider), - workspace_settings.unsafely_ignore_certificate_errors, + workspace_settings + .unsafely_ignore_certificate_errors + .clone(), )); self.module_registries = ModuleRegistry::new( &module_registries_location, @@ -883,8 +886,9 @@ impl Inner { Ok( if let Some(import_map_str) = self .config - .get_workspace_settings() + .workspace_settings() .import_map + .clone() .and_then(|s| if s.is_empty() { None } else { Some(s) }) { lsp_log!( @@ -957,14 +961,14 @@ impl Inner { } pub fn update_debug_flag(&self) { - let internal_debug = self.config.get_workspace_settings().internal_debug; + 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("update_registries", None::<()>); self.recreate_http_client_and_dependents(self.maybe_cache_path.clone())?; - let workspace_settings = self.config.get_workspace_settings(); + 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); @@ -1037,7 +1041,7 @@ impl Inner { "useUnknownInCatchVariables": false, })); let config = &self.config; - let workspace_settings = config.get_workspace_settings(); + let workspace_settings = config.workspace_settings(); if workspace_settings.unstable { let unstable_libs = json!({ "lib": ["deno.ns", "deno.window", "deno.unstable"] @@ -1169,14 +1173,18 @@ impl Inner { } fn refresh_documents_config(&mut self) { - self.documents.update_config( - self.config.enabled_urls(), - self.maybe_import_map.clone(), - self.maybe_config_file.as_ref(), - self.maybe_package_json.as_ref(), - self.npm_api.clone(), - self.npm_resolution.clone(), - ); + self.documents.update_config(UpdateDocumentConfigOptions { + enabled_urls: self.config.enabled_urls(), + document_preload_limit: self + .config + .workspace_settings() + .document_preload_limit, + maybe_import_map: self.maybe_import_map.clone(), + maybe_config_file: self.maybe_config_file.as_ref(), + maybe_package_json: self.maybe_package_json.as_ref(), + npm_registry_api: self.npm_api.clone(), + npm_resolution: self.npm_resolution.clone(), + }); } async fn shutdown(&self) -> LspResult<()> { @@ -1871,7 +1879,7 @@ impl Inner { .normalize_url(¶ms.text_document.uri, LspUrlKind::File); if !self.is_diagnosable(&specifier) || !self.config.specifier_enabled(&specifier) - || !(self.config.get_workspace_settings().enabled_code_lens() + || !(self.config.workspace_settings().enabled_code_lens() || self.config.specifier_code_lens_test(&specifier)) { return Ok(None); @@ -2171,7 +2179,7 @@ impl Inner { ), include_automatic_optional_chain_completions: Some(true), include_completions_for_import_statements: Some( - self.config.get_workspace_settings().suggest.auto_imports, + self.config.workspace_settings().suggest.auto_imports, ), include_completions_for_module_exports: Some(true), include_completions_with_object_literal_method_snippets: Some( @@ -2205,7 +2213,7 @@ impl Inner { if let Some(completions) = maybe_completion_info { let results = completions.as_completion_response( line_index, - &self.config.get_workspace_settings().suggest, + &self.config.workspace_settings().suggest, &specifier, position, ); @@ -3315,7 +3323,7 @@ impl Inner { let specifier = self .url_map .normalize_url(¶ms.text_document.uri, LspUrlKind::File); - let workspace_settings = self.config.get_workspace_settings(); + let workspace_settings = self.config.workspace_settings(); if !self.is_diagnosable(&specifier) || !self.config.specifier_enabled(&specifier) || !workspace_settings.enabled_inlay_hints() @@ -3334,7 +3342,7 @@ impl Inner { let req = tsc::RequestMethod::ProvideInlayHints(( specifier, range, - (&workspace_settings).into(), + workspace_settings.into(), )); let maybe_inlay_hints: Option> = self .ts_server @@ -3388,7 +3396,7 @@ impl Inner { .collect::>(); documents_specifiers.sort(); let measures = self.performance.to_vec(); - let workspace_settings = self.config.get_workspace_settings(); + let workspace_settings = self.config.workspace_settings(); write!( contents, diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs index ada8b94041..ad0171629f 100644 --- a/cli/lsp/repl.rs +++ b/cli/lsp/repl.rs @@ -294,6 +294,7 @@ pub fn get_repl_workspace_settings() -> WorkspaceSettings { inlay_hints: Default::default(), internal_debug: false, lint: false, + document_preload_limit: 0, // don't pre-load any modules as it's expensive and not useful for the repl tls_certificate: None, unsafely_ignore_certificate_errors: None, unstable: false, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index c6035192c6..92407bec1a 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3525,7 +3525,7 @@ mod tests { fixtures: &[(&str, &str, i32, LanguageId)], location: &Path, ) -> StateSnapshot { - let mut documents = Documents::new(location, Default::default()); + let mut documents = Documents::new(location); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index aa69b0dafa..500a27ed2c 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -7418,6 +7418,49 @@ fn lsp_closed_file_find_references() { client.shutdown(); } +#[test] +fn lsp_closed_file_find_references_low_document_pre_load() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.create_dir_all("sub_dir"); + temp_dir.write("./other_file.ts", "export const b = 5;"); + temp_dir.write("./sub_dir/mod.ts", "export const a = 5;"); + temp_dir.write( + "./sub_dir/mod.test.ts", + "import { a } from './mod.ts'; console.log(a);", + ); + let temp_dir_url = temp_dir.uri(); + let mut client = context.new_lsp_command().build(); + client.initialize(|builder| { + builder.set_preload_limit(1); + }); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir_url.join("sub_dir/mod.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": r#"export const a = 5;"# + } + })); + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": temp_dir_url.join("sub_dir/mod.ts").unwrap(), + }, + "position": { "line": 0, "character": 13 }, + "context": { + "includeDeclaration": false + } + }), + ); + + // won't have results because the document won't be pre-loaded + assert_eq!(res, json!([])); + + client.shutdown(); +} + #[test] fn lsp_data_urls_with_jsx_compiler_option() { let context = TestContextBuilder::new().use_temp_cwd().build(); diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index 517fda1b73..e6fc7aa911 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -1014,9 +1014,6 @@ fn closed_file_pre_load_does_not_occur() { .new_command() .args_vec(["repl", "-A", "--log-level=debug"]) .with_pty(|console| { - assert_contains!( - console.all_output(), - "Skipping document preload for repl.", - ); + assert_contains!(console.all_output(), "Skipping document preload.",); }); } diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs index 3e9d0a80bb..a7061543ff 100644 --- a/test_util/src/lsp.rs +++ b/test_util/src/lsp.rs @@ -378,6 +378,12 @@ impl InitializeParamsBuilder { self } + pub fn set_preload_limit(&mut self, arg: usize) -> &mut Self { + let options = self.initialization_options_mut(); + options.insert("documentPreloadLimit".to_string(), arg.into()); + self + } + pub fn set_tls_certificate(&mut self, value: impl AsRef) -> &mut Self { let options = self.initialization_options_mut(); options.insert(