From 6125671efa33c96bb7c15d3327bb6a7aab0db431 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 28 Feb 2023 15:43:13 -0500 Subject: [PATCH 01/26] fix(lsp): add all documents to the lsp on initialize --- cli/lsp/documents.rs | 42 ++++++++++++++++++++++++++++++++++++++ cli/lsp/language_server.rs | 6 ++++++ 2 files changed, 48 insertions(+) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 0acfdbe1fe3c5a..46639cebe8a9d6 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -852,6 +852,48 @@ impl Documents { } } + pub fn open_root(&mut self, root_dir: &Path) { + let mut fs_docs = self.file_system_docs.lock(); + let resolver = self.resolver.as_graph_resolver(); + + let mut pending_dirs = VecDeque::new(); + pending_dirs.push_back(root_dir.to_path_buf()); + + while let Some(dir_path) = pending_dirs.pop_front() { + if let Ok(read_dir) = std::fs::read_dir(&dir_path) { + for entry in read_dir { + let entry = match entry { + Ok(entry) => entry, + Err(_) => continue, + }; + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(_) => continue, + }; + let new_path = dir_path.join(entry.file_name()); + if file_type.is_dir() { + pending_dirs.push_back(new_path); + } else if file_type.is_file() { + if let Some(ext) = new_path.extension() { + let ext = ext.to_string_lossy().to_lowercase(); + // todo: use media type here? + if matches!(ext.as_str(), "ts" | "js" | "mts" | "tsx") { + if let Ok(specifier) = ModuleSpecifier::from_file_path(new_path) + { + if !self.open_docs.contains_key(&specifier) + && !fs_docs.docs.contains_key(&specifier) + { + fs_docs.refresh_document(&self.cache, resolver, &specifier); + } + } + } + } + } + } + } + } + } + /// "Open" a document from the perspective of the editor, meaning that /// requests for information from the document will come from the in-memory /// representation received from the language server client, versus reading diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index ee91e045ba9c8e..1b0d4637ac32b0 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -995,6 +995,12 @@ impl Inner { self.assets.intitialize(self.snapshot()).await; + if let Some(root_uri) = &self.config.root_uri { + if let Ok(dir_path) = root_uri.to_file_path() { + self.documents.open_root(&dir_path); + } + } + self.performance.measure(mark); Ok(InitializeResult { capabilities, From f08a5bf6ecb6001005a19920cfe53c67fdf9012e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 28 Feb 2023 16:04:59 -0500 Subject: [PATCH 02/26] Fix --- cli/lsp/documents.rs | 5 +++++ cli/lsp/tsc.rs | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 46639cebe8a9d6..a4e1e0e3d381f0 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -853,6 +853,11 @@ impl Documents { } pub fn open_root(&mut self, root_dir: &Path) { + // todo(THIS PR): this is wrong. We should probably do this when + // updating the configuration and it should be based on the enabled + // paths rather than pulling in everything. Additionally, we should + // skip searching certain directories by default like node_modules + // and .git let mut fs_docs = self.file_system_docs.lock(); let resolver = self.resolver.as_graph_resolver(); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 2f66e2d2db6db0..9050b765a6a9f8 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2767,16 +2767,16 @@ fn op_respond(state: &mut OpState, args: Response) -> bool { fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); let documents = &state.state_snapshot.documents; - let open_docs = documents.documents(true, true); + let all_docs = documents.documents(false, true); - let mut result = Vec::with_capacity(open_docs.len() + 1); + let mut result = Vec::with_capacity(all_docs.len() + 1); if documents.has_injected_types_node_package() { // ensure this is first so it resolves the node types first result.push("asset:///node_types.d.ts".to_string()); } - result.extend(open_docs.into_iter().map(|d| d.specifier().to_string())); + result.extend(all_docs.into_iter().map(|d| d.specifier().to_string())); result } From ef6475cb1b09081a682c76783f928124d461bedd Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 1 Mar 2023 11:10:53 -0500 Subject: [PATCH 03/26] Take into account enabled_paths --- cli/lsp/config.rs | 85 +++++++++++++++--- cli/lsp/documents.rs | 172 ++++++++++++++++++++++--------------- cli/lsp/language_server.rs | 43 ++++------ 3 files changed, 191 insertions(+), 109 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 3da9f7a0722c4c..e89db5c6be5a82 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -10,6 +10,7 @@ use deno_core::serde::Serialize; use deno_core::serde_json; use deno_core::serde_json::Value; use deno_core::ModuleSpecifier; +use lsp::Url; use std::collections::BTreeMap; use std::collections::HashMap; use std::sync::Arc; @@ -388,7 +389,7 @@ impl WorkspaceSettings { #[derive(Debug, Clone, Default)] pub struct ConfigSnapshot { pub client_capabilities: ClientCapabilities, - pub enabled_paths: HashMap>, + pub enabled_paths: HashMap>, pub settings: Settings, } @@ -396,12 +397,12 @@ impl ConfigSnapshot { /// Determine if the provided specifier is enabled or not. pub fn specifier_enabled(&self, specifier: &ModuleSpecifier) -> bool { if !self.enabled_paths.is_empty() { - let specifier_str = specifier.to_string(); + let specifier_str = specifier.as_str(); for (workspace, enabled_paths) in self.enabled_paths.iter() { - if specifier_str.starts_with(workspace) { + if specifier_str.starts_with(workspace.as_str()) { return enabled_paths .iter() - .any(|path| specifier_str.starts_with(path)); + .any(|path| specifier_str.starts_with(path.as_str())); } } } @@ -431,7 +432,7 @@ pub struct Settings { #[derive(Debug)] pub struct Config { pub client_capabilities: ClientCapabilities, - enabled_paths: HashMap>, + enabled_paths: HashMap>, pub root_uri: Option, settings: Settings, pub workspace_folders: Option>, @@ -478,12 +479,12 @@ impl Config { pub fn specifier_enabled(&self, specifier: &ModuleSpecifier) -> bool { if !self.enabled_paths.is_empty() { - let specifier_str = specifier.to_string(); + let specifier_str = specifier.as_str(); for (workspace, enabled_paths) in self.enabled_paths.iter() { - if specifier_str.starts_with(workspace) { + if specifier_str.starts_with(workspace.as_str()) { return enabled_paths .iter() - .any(|path| specifier_str.starts_with(path)); + .any(|path| specifier_str.starts_with(path.as_str())); } } } @@ -495,6 +496,24 @@ impl Config { .unwrap_or_else(|| self.settings.workspace.enable) } + pub fn root_dirs(&self) -> Vec { + let mut dirs: Vec = Vec::new(); + for (workspace, enabled_paths) in &self.enabled_paths { + if enabled_paths.is_empty() { + dirs.extend(enabled_paths.iter().cloned()); + } else { + dirs.push(workspace.clone()); + } + } + if dirs.is_empty() { + if let Some(root_dir) = &self.root_uri { + dirs.push(root_dir.clone()) + } + } + sort_and_remove_child_dirs(&mut dirs); + dirs + } + pub fn specifier_code_lens_test(&self, specifier: &ModuleSpecifier) -> bool { let value = self .settings @@ -583,7 +602,6 @@ impl Config { enabled_paths: Vec, ) -> bool { let workspace = ensure_directory_specifier(workspace); - let key = workspace.to_string(); let mut touched = false; if !enabled_paths.is_empty() { if let Ok(workspace_path) = specifier_to_file_path(&workspace) { @@ -592,7 +610,7 @@ impl Config { let fs_path = workspace_path.join(path); match ModuleSpecifier::from_file_path(fs_path) { Ok(path_uri) => { - paths.push(path_uri.to_string()); + paths.push(path_uri); } Err(_) => { lsp_log!("Unable to resolve a file path for `deno.enablePath` from \"{}\" for workspace \"{}\".", path, workspace); @@ -601,12 +619,12 @@ impl Config { } if !paths.is_empty() { touched = true; - self.enabled_paths.insert(key, paths); + self.enabled_paths.insert(workspace.clone(), paths); } } } else { touched = true; - self.enabled_paths.remove(&key); + self.enabled_paths.remove(&workspace); } touched } @@ -636,6 +654,20 @@ impl Config { } } +fn sort_and_remove_child_dirs(dirs: &mut Vec) { + if dirs.is_empty() { + return; + } + + dirs.sort(); + for i in (0..dirs.len() - 1).rev() { + let prev = &dirs[i + 1]; + if prev.as_str().starts_with(dirs[i].as_str()) { + dirs.remove(i + 1); + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -678,8 +710,8 @@ mod tests { assert!(!config.specifier_enabled(&specifier_b)); let mut enabled_paths = HashMap::new(); enabled_paths.insert( - "file:///project/".to_string(), - vec!["file:///project/worker/".to_string()], + Url::parse("file:///project/").unwrap(), + vec![Url::parse("file:///project/worker/").unwrap()], ); config.enabled_paths = enabled_paths; assert!(config.specifier_enabled(&specifier_a)); @@ -800,4 +832,29 @@ mod tests { WorkspaceSettings::default() ); } + + #[test] + fn test_sort_and_remove_child_dirs() { + fn run_test(dirs: Vec<&str>, expected_output: Vec<&str>) { + let mut dirs = dirs + .into_iter() + .map(|dir| Url::parse(dir).unwrap()) + .collect(); + sort_and_remove_child_dirs(&mut dirs); + let dirs: Vec<_> = dirs.iter().map(|dir| dir.as_str()).collect(); + assert_eq!(dirs, expected_output); + } + + run_test( + vec![ + "file:///test/asdf/test/asdf/", + "file:///test/asdf/", + "file:///test/asdf/", + "file:///testing/456/893/", + "file:///testing/456/893/test/", + ], + vec!["file:///test/asdf/", "file:///testing/456/893/"], + ); + run_test(vec![], vec![]); + } } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index a4e1e0e3d381f0..7952d2774f2fad 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -43,6 +43,7 @@ use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::PackageJson; use deno_runtime::permissions::PermissionsContainer; use indexmap::IndexMap; +use lsp::Url; use once_cell::sync::Lazy; use std::collections::BTreeMap; use std::collections::HashMap; @@ -775,18 +776,6 @@ impl FileSystemDocuments { self.docs.insert(specifier.clone(), doc.clone()); Some(doc) } - - pub fn refresh_dependencies( - &mut self, - resolver: &dyn deno_graph::source::Resolver, - ) { - for doc in self.docs.values_mut() { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { - *doc = new_doc; - } - } - self.dirty = true; - } } fn get_document_path( @@ -852,53 +841,6 @@ impl Documents { } } - pub fn open_root(&mut self, root_dir: &Path) { - // todo(THIS PR): this is wrong. We should probably do this when - // updating the configuration and it should be based on the enabled - // paths rather than pulling in everything. Additionally, we should - // skip searching certain directories by default like node_modules - // and .git - let mut fs_docs = self.file_system_docs.lock(); - let resolver = self.resolver.as_graph_resolver(); - - let mut pending_dirs = VecDeque::new(); - pending_dirs.push_back(root_dir.to_path_buf()); - - while let Some(dir_path) = pending_dirs.pop_front() { - if let Ok(read_dir) = std::fs::read_dir(&dir_path) { - for entry in read_dir { - let entry = match entry { - Ok(entry) => entry, - Err(_) => continue, - }; - let file_type = match entry.file_type() { - Ok(file_type) => file_type, - Err(_) => continue, - }; - let new_path = dir_path.join(entry.file_name()); - if file_type.is_dir() { - pending_dirs.push_back(new_path); - } else if file_type.is_file() { - if let Some(ext) = new_path.extension() { - let ext = ext.to_string_lossy().to_lowercase(); - // todo: use media type here? - if matches!(ext.as_str(), "ts" | "js" | "mts" | "tsx") { - if let Ok(specifier) = ModuleSpecifier::from_file_path(new_path) - { - if !self.open_docs.contains_key(&specifier) - && !fs_docs.docs.contains_key(&specifier) - { - fs_docs.refresh_document(&self.cache, resolver, &specifier); - } - } - } - } - } - } - } - } - } - /// "Open" a document from the perspective of the editor, meaning that /// requests for information from the document will come from the in-memory /// representation received from the language server client, versus reading @@ -1209,6 +1151,7 @@ impl Documents { pub fn update_config( &mut self, + root_dirs: Vec, maybe_import_map: Option>, maybe_config_file: Option<&ConfigFile>, maybe_package_json: Option<&PackageJson>, @@ -1216,21 +1159,24 @@ impl Documents { npm_resolution: NpmResolution, ) { fn calculate_resolver_config_hash( + root_dirs: &[Url], maybe_import_map: Option<&import_map::ImportMap>, maybe_jsx_config: Option<&JsxImportSourceConfig>, maybe_package_json_deps: Option<&BTreeMap>, ) -> u64 { let mut hasher = FastInsecureHasher::default(); + hasher.write_hashable(&{ + // ensure these are sorted + let mut root_dirs = root_dirs.to_vec(); + root_dirs.sort(); + root_dirs + }); if let Some(import_map) = maybe_import_map { hasher.write_str(&import_map.to_json()); hasher.write_str(import_map.base_url().as_str()); } - if let Some(jsx_config) = maybe_jsx_config { - hasher.write_hashable(&jsx_config); - } - if let Some(deps) = maybe_package_json_deps { - hasher.write_hashable(&deps); - } + hasher.write_hashable(&maybe_jsx_config); + hasher.write_hashable(&maybe_package_json_deps); hasher.finish() } @@ -1246,6 +1192,7 @@ impl Documents { let maybe_jsx_config = maybe_config_file.and_then(|cf| cf.to_maybe_jsx_import_source_config()); let new_resolver_config_hash = calculate_resolver_config_hash( + &root_dirs, maybe_import_map.as_deref(), maybe_jsx_config.as_ref(), maybe_package_json_deps.as_ref(), @@ -1295,21 +1242,108 @@ impl Documents { // only refresh the dependencies if the underlying configuration has changed if self.resolver_config_hash != new_resolver_config_hash { - self.refresh_dependencies(); + self.refresh_dependencies(root_dirs); self.resolver_config_hash = new_resolver_config_hash; } self.dirty = true; } - fn refresh_dependencies(&mut self) { + fn refresh_dependencies(&mut self, root_dirs: Vec) { 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) { *doc = new_doc; } } - self.file_system_docs.lock().refresh_dependencies(resolver); + + // update the file system document + let mut fs_docs = self.file_system_docs.lock(); + let mut not_found_docs = + fs_docs.docs.keys().cloned().collect::>(); + + let mut pending_dirs = VecDeque::new(); + for dir in root_dirs { + if let Ok(path) = dir.to_file_path() { + pending_dirs.push_back(path.to_path_buf()); + } + } + + while let Some(dir_path) = pending_dirs.pop_front() { + if let Some(dir_name) = dir_path.file_name() { + let dir_name = dir_name.to_string_lossy().to_lowercase(); + if matches!(dir_name.as_str(), "node_modules" | ".git") { + continue; + } + } + let read_dir = match std::fs::read_dir(&dir_path) { + Ok(entry) => entry, + Err(_) => continue, + }; + + for entry in read_dir { + let entry = match entry { + Ok(entry) => entry, + Err(_) => continue, + }; + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(_) => continue, + }; + let new_path = dir_path.join(entry.file_name()); + if file_type.is_dir() { + pending_dirs.push_back(new_path); + } else if file_type.is_file() { + let media_type: MediaType = (&new_path).into(); + let is_diagnosable = match media_type { + MediaType::JavaScript + | MediaType::Jsx + | MediaType::Mjs + | MediaType::Cjs + | MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Tsx => true, + MediaType::Json + | MediaType::Wasm + | MediaType::SourceMap + | MediaType::TsBuildInfo + | MediaType::Unknown => false, + }; + if !is_diagnosable { + continue; + } + let specifier = match ModuleSpecifier::from_file_path(new_path) { + Ok(specifier) => specifier, + Err(_) => continue, + }; + + if !self.open_docs.contains_key(&specifier) + && !fs_docs.docs.contains_key(&specifier) + { + fs_docs.refresh_document(&self.cache, resolver, &specifier); + } else { + not_found_docs.remove(&specifier); + // 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; + } + } + } + } + } + } + + // clean up and remove any documents that weren't found + for uri in not_found_docs { + fs_docs.docs.remove(&uri); + } + + fs_docs.dirty = true; } /// Iterate through the documents, building a map where the key is a unique @@ -1681,6 +1715,7 @@ console.log(b, "hello deno"); .unwrap(); documents.update_config( + vec![], Some(Arc::new(import_map)), None, None, @@ -1720,6 +1755,7 @@ console.log(b, "hello deno"); .unwrap(); documents.update_config( + vec![], Some(Arc::new(import_map)), None, None, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 1b0d4637ac32b0..5a4663b6ce4bdd 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -985,22 +985,10 @@ impl Inner { if let Err(err) = self.update_registries().await { self.client.show_message(MessageType::WARNING, err).await; } - self.documents.update_config( - self.maybe_import_map.clone(), - self.maybe_config_file.as_ref(), - self.maybe_package_json.as_ref(), - self.npm_resolver.api().clone(), - self.npm_resolver.resolution().clone(), - ); + self.refresh_config(); self.assets.intitialize(self.snapshot()).await; - if let Some(root_uri) = &self.config.root_uri { - if let Ok(dir_path) = root_uri.to_file_path() { - self.documents.open_root(&dir_path); - } - } - self.performance.measure(mark); Ok(InitializeResult { capabilities, @@ -1009,6 +997,17 @@ impl Inner { }) } + fn refresh_config(&mut self) { + self.documents.update_config( + self.config.root_dirs(), + self.maybe_import_map.clone(), + self.maybe_config_file.as_ref(), + self.maybe_package_json.as_ref(), + self.npm_resolver.api().clone(), + self.npm_resolver.resolution().clone(), + ); + } + async fn initialized(&mut self, _: InitializedParams) { if self .config @@ -1039,6 +1038,7 @@ impl Inner { } } self.config.update_enabled_paths(self.client.clone()).await; + self.refresh_config(); if self.config.client_capabilities.testing_api { let test_server = testing::TestServer::new( @@ -1184,13 +1184,7 @@ impl Inner { self.client.show_message(MessageType::WARNING, err).await; } - self.documents.update_config( - self.maybe_import_map.clone(), - self.maybe_config_file.as_ref(), - self.maybe_package_json.as_ref(), - self.npm_resolver.api().clone(), - self.npm_resolver.resolution().clone(), - ); + self.refresh_config(); self.send_diagnostics_update(); self.send_testing_update(); @@ -1242,13 +1236,7 @@ impl Inner { } } if touched { - self.documents.update_config( - self.maybe_import_map.clone(), - self.maybe_config_file.as_ref(), - self.maybe_package_json.as_ref(), - self.npm_resolver.api().clone(), - self.npm_resolver.resolution().clone(), - ); + self.refresh_config(); self.refresh_npm_specifiers().await; self.diagnostics_server.invalidate_all(); self.restart_ts_server().await; @@ -2826,6 +2814,7 @@ impl tower_lsp::LanguageServer for LanguageServer { } let mut ls = language_server.0.write().await; if ls.config.update_enabled_paths(client).await { + ls.refresh_config(); ls.diagnostics_server.invalidate_all(); // this will be called in the inner did_change_configuration, but the // problem then becomes, if there was a change, the snapshot used From 17b1f0bb0e634456c89380fca69cb256175e9708 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 1 Mar 2023 11:17:15 -0500 Subject: [PATCH 04/26] Another one. --- cli/lsp/language_server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 5a4663b6ce4bdd..375cced2322b73 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -2873,6 +2873,7 @@ impl tower_lsp::LanguageServer for LanguageServer { tokio::spawn(async move { let mut ls = language_server.0.write().await; if ls.config.update_enabled_paths(client).await { + ls.refresh_config(); ls.diagnostics_server.invalidate_all(); ls.send_diagnostics_update(); } From 67439dd8340d35fe37681de06e580884f65071d2 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 3 Mar 2023 20:50:21 -0500 Subject: [PATCH 05/26] More work. --- cli/lsp/config.rs | 61 ++++----- cli/lsp/diagnostics.rs | 13 +- cli/lsp/language_server.rs | 246 ++++++++++++++++++++----------------- cli/lsp/urls.rs | 21 ++-- 4 files changed, 175 insertions(+), 166 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index e89db5c6be5a82..72588c4da83b74 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -1,6 +1,5 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use super::client::Client; use super::logging::lsp_log; use crate::util::path::ensure_directory_specifier; use crate::util::path::specifier_to_file_path; @@ -227,7 +226,7 @@ impl Default for ImportCompletionSettings { /// Deno language server specific settings that can be applied uniquely to a /// specifier. -#[derive(Debug, Default, Clone, Deserialize)] +#[derive(Debug, Default, Clone, Deserialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub struct SpecifierSettings { /// A flag that indicates if Deno is enabled for this specifier or not. @@ -406,26 +405,17 @@ impl ConfigSnapshot { } } } - if let Some((_, SpecifierSettings { enable, .. })) = - self.settings.specifiers.get(specifier) - { - *enable + if let Some(settings) = self.settings.specifiers.get(specifier) { + settings.enable } else { self.settings.workspace.enable } } } -#[derive(Debug, Clone)] -pub struct SpecifierWithClientUri { - pub specifier: ModuleSpecifier, - pub client_uri: ModuleSpecifier, -} - #[derive(Debug, Default, Clone)] pub struct Settings { - pub specifiers: - BTreeMap, + pub specifiers: BTreeMap, pub workspace: WorkspaceSettings, } @@ -492,7 +482,7 @@ impl Config { .settings .specifiers .get(specifier) - .map(|(_, s)| s.enable) + .map(|settings| settings.enable) .unwrap_or_else(|| self.settings.workspace.enable) } @@ -519,7 +509,7 @@ impl Config { .settings .specifiers .get(specifier) - .map(|(_, s)| s.code_lens.test) + .map(|settings| settings.code_lens.test) .unwrap_or_else(|| self.settings.workspace.code_lens.test); value } @@ -573,13 +563,15 @@ impl Config { /// Given the configured workspaces or root URI and the their settings, /// update and resolve any paths that should be enabled - pub async fn update_enabled_paths(&mut self, client: Client) -> bool { + pub fn update_enabled_paths(&mut self) -> bool { if let Some(workspace_folders) = self.workspace_folders.clone() { let mut touched = false; - for (workspace, folder) in workspace_folders { - if let Ok(settings) = client.specifier_configuration(&folder.uri).await - { - if self.update_enabled_paths_entry(workspace, settings.enable_paths) { + for (workspace, _) in workspace_folders { + if let Some(settings) = self.settings.specifiers.get(&workspace) { + if self.update_enabled_paths_entry( + workspace, + settings.enable_paths.clone(), + ) { touched = true; } } @@ -629,28 +621,23 @@ impl Config { touched } - pub fn get_specifiers_with_client_uris(&self) -> Vec { - self - .settings - .specifiers - .iter() - .map(|(s, (u, _))| SpecifierWithClientUri { - specifier: s.clone(), - client_uri: u.clone(), - }) - .collect() + pub fn get_specifiers(&self) -> Vec { + self.settings.specifiers.keys().cloned().collect() } pub fn set_specifier_settings( &mut self, specifier: ModuleSpecifier, - client_uri: ModuleSpecifier, settings: SpecifierSettings, - ) { - self - .settings - .specifiers - .insert(specifier, (client_uri, settings)); + ) -> bool { + if let Some(existing) = self.settings.specifiers.get(&specifier) { + if *existing == settings { + return false; + } + } + + self.settings.specifiers.insert(specifier, settings); + true } } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index f938ba6f450967..3e59fefc224d96 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1179,14 +1179,11 @@ let c: number = "a"; let mut disabled_config = mock_config(); disabled_config.settings.specifiers.insert( specifier.clone(), - ( - specifier.clone(), - SpecifierSettings { - enable: false, - enable_paths: Vec::new(), - code_lens: Default::default(), - }, - ), + SpecifierSettings { + enable: false, + enable_paths: Vec::new(), + code_lens: Default::default(), + }, ); let diagnostics = generate_lint_diagnostics( diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 375cced2322b73..703243a904bdcc 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -316,6 +316,89 @@ impl LanguageServer { None => Err(LspError::invalid_params("Missing parameters")), } } + + pub async fn refresh_specifiers_from_client(&self) { + let (client, specifiers) = + { + let ls = self.0.read().await; + let specifiers = + if ls.config.client_capabilities.workspace_configuration { + let root_capacity = match &ls.config.workspace_folders { + Some(folder) => folder.len(), + None => 1, + }; + let config_specifiers = ls.config.get_specifiers(); + let mut specifiers = + HashMap::with_capacity(root_capacity + config_specifiers.len()); + match &ls.config.workspace_folders { + Some(entry) => { + for (specifier, folder) in entry { + specifiers.insert(specifier.clone(), folder.uri.clone()); + } + } + None => { + if let Some(root_uri) = &ls.config.root_uri { + specifiers.insert( + root_uri.clone(), + ls.url_map.normalize_specifier(root_uri).unwrap(), + ); + } + } + } + specifiers.extend(ls.config.get_specifiers().iter().map(|s| { + (s.clone(), ls.url_map.normalize_specifier(s).unwrap()) + })); + + Some(specifiers.into_iter().collect::>()) + } else { + None + }; + + (ls.client.clone(), specifiers) + }; + + let mut touched = false; + if let Some(specifiers) = specifiers { + let configs_result = client + .specifier_configurations( + specifiers + .iter() + .map(|(_, client_uri)| client_uri.clone()) + .collect(), + ) + .await; + + let mut ls = self.0.write().await; + if let Ok(configs) = configs_result { + for (i, value) in configs.into_iter().enumerate() { + match value { + Ok(specifier_settings) => { + let internal_uri = specifiers[i].1.clone(); + if ls + .config + .set_specifier_settings(internal_uri, specifier_settings) + { + touched = true; + } + } + Err(err) => { + error!("{}", err); + } + } + } + } + + if ls.config.update_enabled_paths() { + touched = true; + } + + if touched { + ls.refresh_documents_config(); + ls.diagnostics_server.invalidate_all(); + ls.send_diagnostics_update(); + } + } + } } fn create_lsp_npm_resolver( @@ -986,7 +1069,7 @@ impl Inner { self.client.show_message(MessageType::WARNING, err).await; } - self.refresh_config(); + // self.refresh_documents_config(); // todo(THIS PR): REMOVE self.assets.intitialize(self.snapshot()).await; self.performance.measure(mark); @@ -997,7 +1080,7 @@ impl Inner { }) } - fn refresh_config(&mut self) { + fn refresh_documents_config(&mut self) { self.documents.update_config( self.config.root_dirs(), self.maybe_import_map.clone(), @@ -1037,8 +1120,6 @@ impl Inner { warn!("Client errored on capabilities.\n{:#}", err); } } - self.config.update_enabled_paths(self.client.clone()).await; - self.refresh_config(); if self.config.client_capabilities.testing_api { let test_server = testing::TestServer::new( @@ -1048,8 +1129,6 @@ impl Inner { ); self.maybe_testing_server = Some(test_server); } - - lsp_log!("Server ready."); } async fn shutdown(&self) -> LspResult<()> { @@ -1184,7 +1263,7 @@ impl Inner { self.client.show_message(MessageType::WARNING, err).await; } - self.refresh_config(); + self.refresh_documents_config(); self.send_diagnostics_update(); self.send_testing_update(); @@ -1236,7 +1315,7 @@ impl Inner { } } if touched { - self.refresh_config(); + self.refresh_documents_config(); self.refresh_npm_specifiers().await; self.diagnostics_server.invalidate_all(); self.restart_ts_server().await; @@ -1246,13 +1325,10 @@ impl Inner { self.performance.measure(mark); } - async fn did_change_workspace_folders( + fn did_change_workspace_folders( &mut self, params: DidChangeWorkspaceFoldersParams, ) { - let mark = self - .performance - .mark("did_change_workspace_folders", Some(¶ms)); let mut workspace_folders = params .event .added @@ -1271,7 +1347,6 @@ impl Inner { } self.config.workspace_folders = Some(workspace_folders); - self.performance.measure(mark); } async fn document_symbol( @@ -2677,7 +2752,11 @@ impl tower_lsp::LanguageServer for LanguageServer { } async fn initialized(&self, params: InitializedParams) { - self.0.write().await.initialized(params).await + self.0.write().await.initialized(params).await; + + self.refresh_specifiers_from_client().await; + + lsp_log!("Server ready."); } async fn shutdown(&self) -> LspResult<()> { @@ -2692,11 +2771,11 @@ impl tower_lsp::LanguageServer for LanguageServer { return; } - let (client, uri, specifier, had_specifier_settings) = { + let (client, client_uri, specifier, had_specifier_settings) = { let mut inner = self.0.write().await; let client = inner.client.clone(); - let uri = params.text_document.uri.clone(); - let specifier = inner.url_map.normalize_url(&uri); + let client_uri = params.text_document.uri.clone(); + let specifier = inner.url_map.normalize_url(&client_uri); let document = inner.did_open(&specifier, params).await; let has_specifier_settings = inner.config.has_specifier_settings(&specifier); @@ -2710,39 +2789,34 @@ impl tower_lsp::LanguageServer for LanguageServer { inner.send_testing_update(); } } - (client, uri, specifier, has_specifier_settings) + (client, client_uri, specifier, has_specifier_settings) }; // retrieve the specifier settings outside the lock if - // they haven't been asked for yet on its own time + // they haven't been asked for yet if !had_specifier_settings { - let language_server = self.clone(); - tokio::spawn(async move { - let response = client.specifier_configuration(&uri).await; - let mut inner = language_server.0.write().await; - match response { - Ok(specifier_settings) => { - // now update the config and send a diagnostics update - inner.config.set_specifier_settings( - specifier.clone(), - uri, - specifier_settings, - ); - } - Err(err) => { - error!("{}", err); - } + let response = client.specifier_configuration(&client_uri).await; + let mut ls = self.0.write().await; + match response { + Ok(specifier_settings) => { + ls.config + .set_specifier_settings(specifier.clone(), specifier_settings); + ls.config.update_enabled_paths(); } - if inner - .documents - .get(&specifier) - .map(|d| d.is_diagnosable()) - .unwrap_or(false) - { - inner.send_diagnostics_update(); - inner.send_testing_update(); + Err(err) => { + error!("{}", err); } - }); + } + + if ls + .documents + .get(&specifier) + .map(|d| d.is_diagnosable()) + .unwrap_or(false) + { + ls.refresh_documents_config(); + ls.send_diagnostics_update(); + } } } @@ -2763,67 +2837,18 @@ impl tower_lsp::LanguageServer for LanguageServer { &self, params: DidChangeConfigurationParams, ) { - let (has_workspace_capability, client, specifiers, mark) = { - let inner = self.0.write().await; - let mark = inner - .performance - .mark("did_change_configuration", Some(¶ms)); - - let specifiers = - if inner.config.client_capabilities.workspace_configuration { - Some(inner.config.get_specifiers_with_client_uris()) - } else { - None - }; + let (mark, has_workspace_capability, client) = { + let inner = self.0.read().await; ( + inner + .performance + .mark("did_change_configuration", Some(¶ms)), inner.config.client_capabilities.workspace_configuration, inner.client.clone(), - specifiers, - mark, ) }; - // start retrieving all the specifiers' settings outside the lock on its own - // time - if let Some(specifiers) = specifiers { - let language_server = self.clone(); - let client = client.clone(); - tokio::spawn(async move { - if let Ok(configs) = client - .specifier_configurations( - specifiers.iter().map(|s| s.client_uri.clone()).collect(), - ) - .await - { - let mut inner = language_server.0.write().await; - for (i, value) in configs.into_iter().enumerate() { - match value { - Ok(specifier_settings) => { - let entry = specifiers[i].clone(); - inner.config.set_specifier_settings( - entry.specifier, - entry.client_uri, - specifier_settings, - ); - } - Err(err) => { - error!("{}", err); - } - } - } - } - let mut ls = language_server.0.write().await; - if ls.config.update_enabled_paths(client).await { - ls.refresh_config(); - ls.diagnostics_server.invalidate_all(); - // this will be called in the inner did_change_configuration, but the - // problem then becomes, if there was a change, the snapshot used - // will be an out of date one, so we will call it again here if the - // workspace folders have been touched - ls.send_diagnostics_update(); - } - }); - } + self.refresh_specifiers_from_client().await; // Get the configuration from the client outside of the lock // in order to prevent potential deadlocking scenarios where @@ -2864,20 +2889,17 @@ impl tower_lsp::LanguageServer for LanguageServer { &self, params: DidChangeWorkspaceFoldersParams, ) { - let client = { - let mut inner = self.0.write().await; - inner.did_change_workspace_folders(params).await; - inner.client.clone() + let mark = { + let mut ls = self.0.write().await; + let mark = ls + .performance + .mark("did_change_workspace_folders", Some(¶ms)); + ls.did_change_workspace_folders(params); + mark }; - let language_server = self.clone(); - tokio::spawn(async move { - let mut ls = language_server.0.write().await; - if ls.config.update_enabled_paths(client).await { - ls.refresh_config(); - ls.diagnostics_server.invalidate_all(); - ls.send_diagnostics_update(); - } - }); + + self.refresh_specifiers_from_client().await; + self.0.read().await.performance.measure(mark); } async fn document_symbol( diff --git a/cli/lsp/urls.rs b/cli/lsp/urls.rs index 4fba0c9ff3a373..2168dfed413e52 100644 --- a/cli/lsp/urls.rs +++ b/cli/lsp/urls.rs @@ -81,7 +81,7 @@ impl LspUrlMapInner { } /// A bi-directional map of URLs sent to the LSP client and internal module -/// specifiers. We need to map internal specifiers into `deno:` schema URLs +/// specifiers. We need to map internal specifiers into `deno:` schema URLs /// to allow the Deno language server to manage these as virtual documents. #[derive(Debug, Default, Clone)] pub struct LspUrlMap(Arc>); @@ -143,15 +143,18 @@ impl LspUrlMap { /// where the client encodes a file URL differently than Rust does by default /// causing issues with string matching of URLs. pub fn normalize_url(&self, url: &Url) -> ModuleSpecifier { - if let Some(specifier) = self.0.lock().get_specifier(url).cloned() { - return specifier; - } - if url.scheme() == "file" { - if let Ok(path) = url.to_file_path() { - return Url::from_file_path(path).unwrap(); - } + let mut inner = self.0.lock(); + if let Some(specifier) = inner.get_specifier(url).cloned() { + specifier + } else { + let specifier = if let Ok(path) = url.to_file_path() { + Url::from_file_path(path).unwrap() + } else { + url.clone() + }; + inner.put(specifier.clone(), url.clone()); + specifier } - url.clone() } } From 459ce44c86dde32a178c7d8365393c53dd7227fa Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 15 Mar 2023 11:12:52 -0400 Subject: [PATCH 06/26] Fix inverted logic. --- cli/lsp/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 3d4cca356fda23..211a16b8c373fd 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -488,7 +488,7 @@ impl Config { pub fn root_dirs(&self) -> Vec { let mut dirs: Vec = Vec::new(); for (workspace, enabled_paths) in &self.enabled_paths { - if enabled_paths.is_empty() { + if !enabled_paths.is_empty() { dirs.extend(enabled_paths.iter().cloned()); } else { dirs.push(workspace.clone()); From 72bfa94eb3bcf9764bb91866059e7b73a806dbfc Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 15 Mar 2023 11:27:32 -0400 Subject: [PATCH 07/26] Compile --- cli/lsp/documents.rs | 2 +- cli/lsp/language_server.rs | 1 + cli/lsp/tsc.rs | 19 ++++++------------- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 3d4f5bcf906fcf..913259ea2e5a6f 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1174,7 +1174,7 @@ impl Documents { hasher.write_hashable(&{ // ensure these are sorted let mut root_dirs = root_dirs.to_vec(); - root_dirs.sort(); + root_dirs.sort_unstable(); root_dirs }); if let Some(import_map) = maybe_import_map { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index fc87001af35b1f..bcf31933020f9c 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1133,6 +1133,7 @@ impl Inner { fn refresh_documents_config(&mut self) { self.documents.update_config( + self.config.root_dirs(), self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), self.maybe_package_json.as_ref(), diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 1ca966aff72ac0..34da9c758b84fb 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2749,10 +2749,8 @@ fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); let documents = &state.state_snapshot.documents; let all_docs = documents.documents(false, true); - let mut result = Vec::new(); let mut seen = HashSet::new(); - - let mut result = Vec::with_capacity(all_docs.len() + 1); + let mut result = Vec::new(); if documents.has_injected_types_node_package() { // ensure this is first so it resolves the node types first @@ -2768,23 +2766,18 @@ fn op_script_names(state: &mut OpState) -> Vec { } } - // finally include the documents and all their dependencies + // finally, include the documents and all their dependencies for doc in &all_docs { let specifier = doc.specifier(); - if seen.insert(specifier.as_str()) { + if seen.insert(specifier.as_str()) && documents.exists(specifier) { + // only include dependencies we know to exist otherwise typescript will error result.push(specifier.to_string()); } - } - // and then all their dependencies (do this after to avoid exists calls) - for doc in &open_docs { for dep in doc.dependencies().values() { if let Some(specifier) = dep.get_type().or_else(|| dep.get_code()) { - if seen.insert(specifier.as_str()) { - // only include dependencies we know to exist otherwise typescript will error - if documents.exists(specifier) { - result.push(specifier.to_string()); - } + if seen.insert(specifier.as_str()) && documents.exists(specifier) { + result.push(specifier.to_string()); } } } From cdc4af00ee91e0e4ec7ff00be29ee6420a6e844b Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 15 Mar 2023 11:53:54 -0400 Subject: [PATCH 08/26] Ignore minified files --- cli/lsp/documents.rs | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 913259ea2e5a6f..a5f255d8e56f0c 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1254,6 +1254,27 @@ impl Documents { } fn refresh_dependencies(&mut self, root_dirs: Vec) { + fn is_auto_discoverable_dir(dir_path: &Path) -> bool { + if let Some(dir_name) = dir_path.file_name() { + let dir_name = dir_name.to_string_lossy().to_lowercase(); + matches!(dir_name.as_str(), "node_modules" | ".git") + } else { + false + } + } + + fn is_auto_discoverable_file(file_path: &Path) -> bool { + // Don't auto-discover minified files as they are likely to be very large + // and likely not to have dependencies on code outside them that would + // be useful in the LSP + if let Some(file_name) = file_path.file_name() { + let file_name = file_name.to_string_lossy().to_lowercase(); + !file_name.as_str().contains(".min.") + } else { + false + } + } + 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) { @@ -1261,7 +1282,7 @@ impl Documents { } } - // update the file system document + // update the file system documents let mut fs_docs = self.file_system_docs.lock(); let mut not_found_docs = fs_docs.docs.keys().cloned().collect::>(); @@ -1274,11 +1295,8 @@ impl Documents { } while let Some(dir_path) = pending_dirs.pop_front() { - if let Some(dir_name) = dir_path.file_name() { - let dir_name = dir_name.to_string_lossy().to_lowercase(); - if matches!(dir_name.as_str(), "node_modules" | ".git") { - continue; - } + if !is_auto_discoverable_dir(&dir_path) { + continue; } let read_dir = match std::fs::read_dir(&dir_path) { Ok(entry) => entry, @@ -1317,10 +1335,10 @@ impl Documents { | MediaType::TsBuildInfo | MediaType::Unknown => false, }; - if !is_diagnosable { + if !is_diagnosable || !is_auto_discoverable_file(&new_path) { continue; } - let specifier = match ModuleSpecifier::from_file_path(new_path) { + let specifier = match ModuleSpecifier::from_file_path(&new_path) { Ok(specifier) => specifier, Err(_) => continue, }; From 868871812454d7f6b209d6304e8ba1e484350b0d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 11:55:49 -0400 Subject: [PATCH 09/26] Handle files in enabled paths --- cli/lsp/config.rs | 25 ++++++++++++++----------- cli/lsp/documents.rs | 28 +++++++++++++++------------- cli/lsp/language_server.rs | 2 +- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 211a16b8c373fd..96441db194063c 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -485,22 +485,24 @@ impl Config { .unwrap_or_else(|| self.settings.workspace.enable) } - pub fn root_dirs(&self) -> Vec { - let mut dirs: Vec = Vec::new(); + /// Gets the root urls that may contain directories or specific + /// file paths. + pub fn root_urls(&self) -> Vec { + let mut urls: Vec = Vec::new(); for (workspace, enabled_paths) in &self.enabled_paths { if !enabled_paths.is_empty() { - dirs.extend(enabled_paths.iter().cloned()); + urls.extend(enabled_paths.iter().cloned()); } else { - dirs.push(workspace.clone()); + urls.push(workspace.clone()); } } - if dirs.is_empty() { + if urls.is_empty() { if let Some(root_dir) = &self.root_uri { - dirs.push(root_dir.clone()) + urls.push(root_dir.clone()) } } - sort_and_remove_child_dirs(&mut dirs); - dirs + sort_and_remove_non_leaf_urls(&mut urls); + urls } pub fn specifier_code_lens_test(&self, specifier: &ModuleSpecifier) -> bool { @@ -639,7 +641,8 @@ impl Config { } } -fn sort_and_remove_child_dirs(dirs: &mut Vec) { +/// Removes any URLs that are a descendant of another URL in the collection. +fn sort_and_remove_non_leaf_urls(dirs: &mut Vec) { if dirs.is_empty() { return; } @@ -819,13 +822,13 @@ mod tests { } #[test] - fn test_sort_and_remove_child_dirs() { + fn test_sort_and_remove_non_leaf_urls() { fn run_test(dirs: Vec<&str>, expected_output: Vec<&str>) { let mut dirs = dirs .into_iter() .map(|dir| Url::parse(dir).unwrap()) .collect(); - sort_and_remove_child_dirs(&mut dirs); + sort_and_remove_non_leaf_urls(&mut dirs); let dirs: Vec<_> = dirs.iter().map(|dir| dir.as_str()).collect(); assert_eq!(dirs, expected_output); } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 45d25413c512d5..84954a32ace97f 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1156,7 +1156,7 @@ impl Documents { pub fn update_config( &mut self, - root_dirs: Vec, + root_urls: Vec, maybe_import_map: Option>, maybe_config_file: Option<&ConfigFile>, maybe_package_json: Option<&PackageJson>, @@ -1164,17 +1164,17 @@ impl Documents { npm_resolution: NpmResolution, ) { fn calculate_resolver_config_hash( - root_dirs: &[Url], + root_urls: &[Url], maybe_import_map: Option<&import_map::ImportMap>, maybe_jsx_config: Option<&JsxImportSourceConfig>, maybe_package_json_deps: Option<&PackageJsonDeps>, ) -> u64 { let mut hasher = FastInsecureHasher::default(); hasher.write_hashable(&{ - // ensure these are sorted - let mut root_dirs = root_dirs.to_vec(); - root_dirs.sort_unstable(); - root_dirs + // ensure these are sorted (they should be, but this is a safeguard) + let mut root_urls = root_urls.to_vec(); + root_urls.sort_unstable(); + root_urls }); if let Some(import_map) = maybe_import_map { hasher.write_str(&import_map.to_json()); @@ -1191,7 +1191,7 @@ impl Documents { let maybe_jsx_config = maybe_config_file.and_then(|cf| cf.to_maybe_jsx_import_source_config()); let new_resolver_config_hash = calculate_resolver_config_hash( - &root_dirs, + &root_urls, maybe_import_map.as_deref(), maybe_jsx_config.as_ref(), maybe_package_json_deps.as_ref(), @@ -1245,14 +1245,14 @@ impl Documents { // only refresh the dependencies if the underlying configuration has changed if self.resolver_config_hash != new_resolver_config_hash { - self.refresh_dependencies(root_dirs); + self.refresh_dependencies(root_urls); self.resolver_config_hash = new_resolver_config_hash; } self.dirty = true; } - fn refresh_dependencies(&mut self, root_dirs: Vec) { + fn refresh_dependencies(&mut self, root_urls: Vec) { fn is_auto_discoverable_dir(dir_path: &Path) -> bool { if let Some(dir_name) = dir_path.file_name() { let dir_name = dir_name.to_string_lossy().to_lowercase(); @@ -1287,9 +1287,11 @@ impl Documents { fs_docs.docs.keys().cloned().collect::>(); let mut pending_dirs = VecDeque::new(); - for dir in root_dirs { - if let Ok(path) = dir.to_file_path() { - pending_dirs.push_back(path.to_path_buf()); + for root_url in root_urls { + if let Ok(path) = root_url.to_file_path() { + if path.is_dir() { + pending_dirs.push_back(path.to_path_buf()); + } } } @@ -1315,7 +1317,7 @@ impl Documents { if file_type.is_dir() { pending_dirs.push_back(new_path); } else if file_type.is_file() { - let media_type: MediaType = (&new_path).into(); + let media_type = MediaType::from_path(&new_path); let is_diagnosable = match media_type { MediaType::JavaScript | MediaType::Jsx diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index e530bd503fb9c6..6407696049c3dd 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1137,7 +1137,7 @@ impl Inner { fn refresh_documents_config(&mut self) { self.documents.update_config( - self.config.root_dirs(), + self.config.root_urls(), self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), self.maybe_package_json.as_ref(), From 8b0ab709ccc616e03b4b062378f1ca4a7d5e908d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 12:07:47 -0400 Subject: [PATCH 10/26] Consolidate code --- cli/lsp/tsc.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 1e8193d3912d65..15e8a2282bda3b 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2778,19 +2778,18 @@ fn op_script_names(state: &mut OpState) -> Vec { } } - // finally, include the documents and all their dependencies + // finally include the documents and all their dependencies for doc in &all_docs { - let specifier = doc.specifier(); - if seen.insert(specifier.as_str()) && documents.exists(specifier) { - // only include dependencies we know to exist otherwise typescript will error - result.push(specifier.to_string()); - } - - for dep in doc.dependencies().values() { - if let Some(specifier) = dep.get_type().or_else(|| dep.get_code()) { - if seen.insert(specifier.as_str()) && documents.exists(specifier) { - result.push(specifier.to_string()); - } + let specifiers = std::iter::once(doc.specifier()).chain( + doc + .dependencies() + .values() + .filter_map(|dep| dep.get_type().or_else(|| dep.get_code())), + ); + for specifier in specifiers { + if seen.insert(specifier.as_str()) && documents.exists(specifier) { + // only include dependencies we know to exist otherwise typescript will error + result.push(specifier.to_string()); } } } From 4c9e1025517f7032c8f078099b8cb36b1e4b17eb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 12:24:21 -0400 Subject: [PATCH 11/26] refactor(lsp): remove boolean parameters on `documents.documents(...)` --- cli/lsp/completions.rs | 5 ++- cli/lsp/diagnostics.rs | 12 ++++-- cli/lsp/documents.rs | 76 +++++++++++++++++++++----------------- cli/lsp/language_server.rs | 7 +++- cli/lsp/testing/server.rs | 6 ++- cli/lsp/tsc.rs | 3 +- 6 files changed, 66 insertions(+), 43 deletions(-) diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index ccb945a0b5c299..cb8bd446da2a02 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -3,6 +3,7 @@ use super::client::Client; use super::config::ConfigSnapshot; use super::documents::Documents; +use super::documents::DocumentsFilter; use super::lsp_custom; use super::registries::ModuleRegistry; use super::tsc; @@ -278,7 +279,7 @@ fn get_import_map_completions( if let Ok(resolved) = import_map.resolve(&key, specifier) { let resolved = resolved.to_string(); let workspace_items: Vec = documents - .documents(false, true) + .documents(DocumentsFilter::AllDiagnosable) .into_iter() .filter_map(|d| { let specifier_str = d.specifier().to_string(); @@ -460,7 +461,7 @@ fn get_workspace_completions( documents: &Documents, ) -> Vec { let workspace_specifiers = documents - .documents(false, true) + .documents(DocumentsFilter::AllDiagnosable) .into_iter() .map(|d| d.specifier().clone()) .collect(); diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index a9274801a9d7aa..82cdb46ded67b7 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -6,6 +6,7 @@ use super::client::Client; use super::config::ConfigSnapshot; use super::documents; use super::documents::Document; +use super::documents::DocumentsFilter; use super::language_server; use super::language_server::StateSnapshot; use super::performance::Performance; @@ -454,7 +455,9 @@ async fn generate_lint_diagnostics( lint_options: &LintOptions, token: CancellationToken, ) -> DiagnosticVec { - let documents = snapshot.documents.documents(true, true); + let documents = snapshot + .documents + .documents(DocumentsFilter::OpenAndDiagnosable); let workspace_settings = config.settings.workspace.clone(); let lint_rules = get_configured_rules(lint_options.rules.clone()); let mut diagnostics_vec = Vec::new(); @@ -530,7 +533,7 @@ async fn generate_ts_diagnostics( let mut diagnostics_vec = Vec::new(); let specifiers = snapshot .documents - .documents(true, true) + .documents(DocumentsFilter::OpenAndDiagnosable) .into_iter() .map(|d| d.specifier().clone()); let (enabled_specifiers, disabled_specifiers) = specifiers @@ -1025,7 +1028,10 @@ async fn generate_deno_diagnostics( ) -> DiagnosticVec { let mut diagnostics_vec = Vec::new(); - for document in snapshot.documents.documents(true, true) { + for document in snapshot + .documents + .documents(DocumentsFilter::OpenAndDiagnosable) + { if token.is_cancelled() { break; } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 20ca755d93181e..6679e4fc1993a9 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -800,6 +800,17 @@ fn get_document_path( } } +/// Specify the documents to include on a `documents.documents(...)` call. +#[derive(Debug, Clone, Copy)] +pub enum DocumentsFilter { + /// Includes all the documents (diagnosable & non-diagnosable, open & file system). + All, + /// Includes all the diagnosable documents (open & file system). + AllDiagnosable, + /// Includes only the diagnosable documents that are open. + OpenAndDiagnosable, +} + #[derive(Debug, Clone, Default)] pub struct Documents { /// The DENO_DIR that the documents looks for non-file based modules. @@ -1011,47 +1022,44 @@ impl Documents { } } - /// Return a vector of documents that are contained in the document store, - /// where `open_only` flag would provide only those documents currently open - /// in the editor and `diagnosable_only` would provide only those documents - /// that the language server can provide diagnostics for. - pub fn documents( - &self, - open_only: bool, - diagnosable_only: bool, - ) -> Vec { - if open_only { - self + /// Return a collection of documents that are contained in the document store + /// based on the provided filter. + pub fn documents(&self, filter: DocumentsFilter) -> Vec { + match filter { + DocumentsFilter::OpenAndDiagnosable => self .open_docs .values() .filter_map(|doc| { - if !diagnosable_only || doc.is_diagnosable() { + if doc.is_diagnosable() { Some(doc.clone()) } else { None } }) - .collect() - } else { - // it is technically possible for a Document to end up in both the open - // and closed documents so we need to ensure we don't return duplicates - let mut seen_documents = HashSet::new(); - let file_system_docs = self.file_system_docs.lock(); - self - .open_docs - .values() - .chain(file_system_docs.docs.values()) - .filter_map(|doc| { - // this prefers the open documents - if seen_documents.insert(doc.specifier().clone()) - && (!diagnosable_only || doc.is_diagnosable()) - { - Some(doc.clone()) - } else { - None - } - }) - .collect() + .collect(), + DocumentsFilter::AllDiagnosable | DocumentsFilter::All => { + let diagnosable_only = + matches!(filter, DocumentsFilter::AllDiagnosable); + // it is technically possible for a Document to end up in both the open + // and closed documents so we need to ensure we don't return duplicates + let mut seen_documents = HashSet::new(); + let file_system_docs = self.file_system_docs.lock(); + self + .open_docs + .values() + .chain(file_system_docs.docs.values()) + .filter_map(|doc| { + // this prefers the open documents + if seen_documents.insert(doc.specifier().clone()) + && (!diagnosable_only || doc.is_diagnosable()) + { + Some(doc.clone()) + } else { + None + } + }) + .collect() + } } } @@ -1592,7 +1600,7 @@ console.log(b, "hello deno"); // At this point the document will be in both documents and the shared file system documents. // Now make sure that the original documents doesn't return both copies - assert_eq!(documents.documents(false, false).len(), 1); + assert_eq!(documents.documents(DocumentsFilter::All).len(), 1); } #[test] diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 72beb04bb1a4e4..a00634fe60db4e 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -44,6 +44,7 @@ use super::documents::to_lsp_range; use super::documents::AssetOrDocument; use super::documents::Document; use super::documents::Documents; +use super::documents::DocumentsFilter; use super::documents::LanguageId; use super::logging::lsp_log; use super::lsp_custom; @@ -3223,7 +3224,9 @@ impl Inner { )?; cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone()); - let open_docs = self.documents.documents(true, true); + let open_docs = self + .documents + .documents(DocumentsFilter::OpenAndDiagnosable); Ok(Some(PrepareCacheResult { cli_options, open_docs, @@ -3341,7 +3344,7 @@ impl Inner { let mut contents = String::new(); let mut documents_specifiers = self .documents - .documents(false, false) + .documents(DocumentsFilter::All) .into_iter() .map(|d| d.specifier().clone()) .collect::>(); diff --git a/cli/lsp/testing/server.rs b/cli/lsp/testing/server.rs index 61db4316a28148..638ab5b55271b5 100644 --- a/cli/lsp/testing/server.rs +++ b/cli/lsp/testing/server.rs @@ -8,6 +8,7 @@ use super::lsp_custom; use crate::lsp::client::Client; use crate::lsp::client::TestingNotification; use crate::lsp::config; +use crate::lsp::documents::DocumentsFilter; use crate::lsp::language_server::StateSnapshot; use crate::lsp::performance::Performance; @@ -92,7 +93,10 @@ impl TestServer { // eliminating any we go over when iterating over the document let mut keys: HashSet = tests.keys().cloned().collect(); - for document in snapshot.documents.documents(false, true) { + for document in snapshot + .documents + .documents(DocumentsFilter::AllDiagnosable) + { let specifier = document.specifier(); keys.remove(specifier); let script_version = document.script_version(); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 3c70e40296c69f..75d1b8fe891aa1 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3,6 +3,7 @@ use super::code_lens; use super::config; use super::documents::AssetOrDocument; +use super::documents::DocumentsFilter; use super::language_server; use super::language_server::StateSnapshot; use super::performance::Performance; @@ -2760,7 +2761,7 @@ fn op_respond(state: &mut OpState, args: Response) -> bool { fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); let documents = &state.state_snapshot.documents; - let open_docs = documents.documents(true, true); + let open_docs = documents.documents(DocumentsFilter::OpenAndDiagnosable); let mut result = Vec::new(); let mut seen = HashSet::new(); From d9c5e91ee24f418bef7f36bd6ddf14fdffe92f48 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 12:26:31 -0400 Subject: [PATCH 12/26] Remove word "And" --- cli/lsp/diagnostics.rs | 6 +++--- cli/lsp/documents.rs | 4 ++-- cli/lsp/language_server.rs | 4 +--- cli/lsp/tsc.rs | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 82cdb46ded67b7..8ba8ce074a9a49 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -457,7 +457,7 @@ async fn generate_lint_diagnostics( ) -> DiagnosticVec { let documents = snapshot .documents - .documents(DocumentsFilter::OpenAndDiagnosable); + .documents(DocumentsFilter::OpenDiagnosable); let workspace_settings = config.settings.workspace.clone(); let lint_rules = get_configured_rules(lint_options.rules.clone()); let mut diagnostics_vec = Vec::new(); @@ -533,7 +533,7 @@ async fn generate_ts_diagnostics( let mut diagnostics_vec = Vec::new(); let specifiers = snapshot .documents - .documents(DocumentsFilter::OpenAndDiagnosable) + .documents(DocumentsFilter::OpenDiagnosable) .into_iter() .map(|d| d.specifier().clone()); let (enabled_specifiers, disabled_specifiers) = specifiers @@ -1030,7 +1030,7 @@ async fn generate_deno_diagnostics( for document in snapshot .documents - .documents(DocumentsFilter::OpenAndDiagnosable) + .documents(DocumentsFilter::OpenDiagnosable) { if token.is_cancelled() { break; diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 6679e4fc1993a9..d8a94e5388e72b 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -808,7 +808,7 @@ pub enum DocumentsFilter { /// Includes all the diagnosable documents (open & file system). AllDiagnosable, /// Includes only the diagnosable documents that are open. - OpenAndDiagnosable, + OpenDiagnosable, } #[derive(Debug, Clone, Default)] @@ -1026,7 +1026,7 @@ impl Documents { /// based on the provided filter. pub fn documents(&self, filter: DocumentsFilter) -> Vec { match filter { - DocumentsFilter::OpenAndDiagnosable => self + DocumentsFilter::OpenDiagnosable => self .open_docs .values() .filter_map(|doc| { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index a00634fe60db4e..372a1489d240a4 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3224,9 +3224,7 @@ impl Inner { )?; cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone()); - let open_docs = self - .documents - .documents(DocumentsFilter::OpenAndDiagnosable); + let open_docs = self.documents.documents(DocumentsFilter::OpenDiagnosable); Ok(Some(PrepareCacheResult { cli_options, open_docs, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 75d1b8fe891aa1..6ef9b1dc37f81a 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2761,7 +2761,7 @@ fn op_respond(state: &mut OpState, args: Response) -> bool { fn op_script_names(state: &mut OpState) -> Vec { let state = state.borrow_mut::(); let documents = &state.state_snapshot.documents; - let open_docs = documents.documents(DocumentsFilter::OpenAndDiagnosable); + let open_docs = documents.documents(DocumentsFilter::OpenDiagnosable); let mut result = Vec::new(); let mut seen = HashSet::new(); From 08ef6af27863fa555256619e999d574ca58d2566 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 13:18:14 -0400 Subject: [PATCH 13/26] Working. --- cli/lsp/config.rs | 8 +++- cli/lsp/documents.rs | 94 +++++++++++++++++++++----------------- cli/lsp/language_server.rs | 2 +- 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 96441db194063c..a95636e94e33ee 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -487,8 +487,14 @@ impl Config { /// Gets the root urls that may contain directories or specific /// file paths. - pub fn root_urls(&self) -> Vec { + pub fn enabled_root_urls(&self) -> Vec { let mut urls: Vec = Vec::new(); + + if !self.settings.workspace.enable && self.enabled_paths.is_empty() { + // do not return any urls when disabled + return urls; + } + for (workspace, enabled_paths) in &self.enabled_paths { if !enabled_paths.is_empty() { urls.extend(enabled_paths.iter().cloned()); diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index d9df9282454d72..aaece5ab142c8b 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1264,7 +1264,7 @@ impl Documents { fn is_auto_discoverable_dir(dir_path: &Path) -> bool { if let Some(dir_name) = dir_path.file_name() { let dir_name = dir_name.to_string_lossy().to_lowercase(); - matches!(dir_name.as_str(), "node_modules" | ".git") + !matches!(dir_name.as_str(), "node_modules" | ".git") } else { false } @@ -1282,6 +1282,27 @@ impl Documents { } } + fn is_diagnosable(media_type: MediaType) -> bool { + match media_type { + MediaType::JavaScript + | MediaType::Jsx + | MediaType::Mjs + | MediaType::Cjs + | MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Tsx => true, + MediaType::Json + | MediaType::Wasm + | MediaType::SourceMap + | MediaType::TsBuildInfo + | MediaType::Unknown => false, + } + } + 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) { @@ -1293,12 +1314,42 @@ impl Documents { let mut fs_docs = self.file_system_docs.lock(); let mut not_found_docs = fs_docs.docs.keys().cloned().collect::>(); + let open_docs = &mut self.open_docs; + + let mut handle_file_path = |file_path: &Path| { + let media_type = MediaType::from_path(&file_path); + if !is_diagnosable(media_type) || !is_auto_discoverable_file(&file_path) { + return; + } + let specifier = match ModuleSpecifier::from_file_path(&file_path) { + Ok(specifier) => specifier, + Err(_) => return, + }; + + // 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; + } + } + } + }; let mut pending_dirs = VecDeque::new(); for root_url in root_urls { if let Ok(path) = root_url.to_file_path() { if path.is_dir() { pending_dirs.push_back(path.to_path_buf()); + } else { + handle_file_path(&path); } } } @@ -1325,46 +1376,7 @@ impl Documents { if file_type.is_dir() { pending_dirs.push_back(new_path); } else if file_type.is_file() { - let media_type = MediaType::from_path(&new_path); - let is_diagnosable = match media_type { - MediaType::JavaScript - | MediaType::Jsx - | MediaType::Mjs - | MediaType::Cjs - | MediaType::TypeScript - | MediaType::Mts - | MediaType::Cts - | MediaType::Dts - | MediaType::Dmts - | MediaType::Dcts - | MediaType::Tsx => true, - MediaType::Json - | MediaType::Wasm - | MediaType::SourceMap - | MediaType::TsBuildInfo - | MediaType::Unknown => false, - }; - if !is_diagnosable || !is_auto_discoverable_file(&new_path) { - continue; - } - let specifier = match ModuleSpecifier::from_file_path(&new_path) { - Ok(specifier) => specifier, - Err(_) => continue, - }; - - if !self.open_docs.contains_key(&specifier) - && !fs_docs.docs.contains_key(&specifier) - { - fs_docs.refresh_document(&self.cache, resolver, &specifier); - } else { - not_found_docs.remove(&specifier); - // 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; - } - } - } + handle_file_path(&new_path); } } } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 39c9743eedebc6..d6256c91a06357 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1138,7 +1138,7 @@ impl Inner { fn refresh_documents_config(&mut self) { self.documents.update_config( - self.config.root_urls(), + self.config.enabled_root_urls(), self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), self.maybe_package_json.as_ref(), From 2dc17b008fc2e1d53168f861dd389b20a3709f31 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 14:29:36 -0400 Subject: [PATCH 14/26] Failing test because our textDocument/references implementation is broken. --- cli/lsp/config.rs | 3 +- cli/tests/integration/lsp_tests.rs | 53 ++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index a95636e94e33ee..c929f0cc116a85 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -485,8 +485,7 @@ impl Config { .unwrap_or_else(|| self.settings.workspace.enable) } - /// Gets the root urls that may contain directories or specific - /// file paths. + /// Gets the root directories or file paths based on the workspace config. pub fn enabled_root_urls(&self) -> Vec { let mut urls: Vec = Vec::new(); diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 11800c2f764591..ae76d529696564 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -7024,3 +7024,56 @@ Deno.test({ client.shutdown(); } + +#[test] +fn lsp_closed_file_find_references() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.write("./mod.ts", "export const a = 5;"); + temp_dir.write( + "./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_default(); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir_url.join("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("mod.ts").unwrap(), + }, + "position": { "line": 0, "character": 13 }, + "context": { + "includeDeclaration": false + } + }), + ); + + assert_eq!( + res, + json!([{ + "uri": temp_dir_url.join("mod.test.ts").unwrap(), + "range": { + "start": { "line": 0, "character": 9 }, + "end": { "line": 0, "character": 10 } + } + }, { + "uri": temp_dir_url.join("mod.test.ts").unwrap(), + "range": { + "start": { "line": 0, "character": 42 }, + "end": { "line": 0, "character": 43 } + } + }]) + ); + + client.shutdown(); +} From bab99d7b1391dae31a7363de9aabb1919bd00707 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 15:00:46 -0400 Subject: [PATCH 15/26] fix(lsp): `textDocument/references` should respect `includeDeclaration` --- cli/lsp/code_lens.rs | 11 ++-- cli/lsp/language_server.rs | 17 +++--- cli/lsp/tsc.rs | 36 +++++++++--- cli/tests/integration/lsp_tests.rs | 92 ++++++++++++++++++++++++++++++ cli/tsc/99_main_compiler.js | 4 +- cli/tsc/compiler.d.ts | 6 +- 6 files changed, 142 insertions(+), 24 deletions(-) diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index 1253fe5aa15baa..c3d73bac3de7e9 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -300,25 +300,26 @@ async fn resolve_references_code_lens( let asset_or_document = language_server.get_asset_or_document(&data.specifier)?; let line_index = asset_or_document.line_index(); - let req = tsc::RequestMethod::GetReferences(( + let req = tsc::RequestMethod::FindReferences(( data.specifier.clone(), line_index.offset_tsc(code_lens.range.start)?, )); let snapshot = language_server.snapshot(); - let maybe_references: Option> = + let maybe_referenced_symbols: Option> = language_server.ts_server.request(snapshot, req).await?; - if let Some(references) = maybe_references { + if let Some(symbols) = maybe_referenced_symbols { let mut locations = Vec::new(); - for reference in references { + for reference in symbols.iter().flat_map(|s| &s.references) { if reference.is_definition { continue; } let reference_specifier = - resolve_url(&reference.document_span.file_name)?; + resolve_url(&reference.entry.document_span.file_name)?; let asset_or_doc = language_server.get_asset_or_document(&reference_specifier)?; locations.push( reference + .entry .to_location(asset_or_doc.line_index(), &language_server.url_map), ); } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 72beb04bb1a4e4..9bf338eb4a3a98 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1947,11 +1947,11 @@ impl Inner { let mark = self.performance.mark("references", Some(¶ms)); let asset_or_doc = self.get_asset_or_document(&specifier)?; let line_index = asset_or_doc.line_index(); - let req = tsc::RequestMethod::GetReferences(( + let req = tsc::RequestMethod::FindReferences(( specifier.clone(), line_index.offset_tsc(params.text_document_position.position)?, )); - let maybe_references: Option> = self + let maybe_referenced_symbols: Option> = self .ts_server .request(self.snapshot(), req) .await @@ -1960,14 +1960,14 @@ impl Inner { LspError::internal_error() })?; - if let Some(references) = maybe_references { + if let Some(symbols) = maybe_referenced_symbols { let mut results = Vec::new(); - for reference in references { + for reference in symbols.iter().flat_map(|s| &s.references) { if !params.context.include_declaration && reference.is_definition { continue; } let reference_specifier = - resolve_url(&reference.document_span.file_name).unwrap(); + resolve_url(&reference.entry.document_span.file_name).unwrap(); let reference_line_index = if reference_specifier == specifier { line_index.clone() } else { @@ -1975,8 +1975,11 @@ impl Inner { self.get_asset_or_document(&reference_specifier)?; asset_or_doc.line_index() }; - results - .push(reference.to_location(reference_line_index, &self.url_map)); + results.push( + reference + .entry + .to_location(reference_line_index, &self.url_map), + ); } self.performance.measure(mark); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 3c70e40296c69f..657d2dd197d43e 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -148,7 +148,8 @@ impl TsServer { if self.0.send((req, snapshot, tx, token)).is_err() { return Err(anyhow!("failed to send request to tsc thread")); } - rx.await?.map(|v| serde_json::from_value::(v).unwrap()) + let value = rx.await??; + Ok(serde_json::from_value::(value)?) } } @@ -1687,10 +1688,31 @@ pub struct CombinedCodeActions { #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct ReferenceEntry { - // is_write_access: bool, +pub struct ReferencedSymbol { + pub definition: ReferencedSymbolDefinitionInfo, + pub references: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReferencedSymbolDefinitionInfo { + #[serde(flatten)] + pub definition_info: DefinitionInfo, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReferencedSymbolEntry { #[serde(default)] pub is_definition: bool, + #[serde(flatten)] + pub entry: ReferenceEntry, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReferenceEntry { + // is_write_access: bool, // is_in_string: Option, #[serde(flatten)] pub document_span: DocumentSpan, @@ -3177,8 +3199,8 @@ pub enum RequestMethod { GetOutliningSpans(ModuleSpecifier), /// Return quick info at position (hover information). GetQuickInfo((ModuleSpecifier, u32)), - /// Get document references for a specific position. - GetReferences((ModuleSpecifier, u32)), + /// Finds the document references for a specific position. + FindReferences((ModuleSpecifier, u32)), /// Get signature help items for a specific position. GetSignatureHelpItems((ModuleSpecifier, u32, SignatureHelpItemsOptions)), /// Get a selection range for a specific position. @@ -3348,9 +3370,9 @@ impl RequestMethod { "specifier": state.denormalize_specifier(specifier), "position": position, }), - RequestMethod::GetReferences((specifier, position)) => json!({ + RequestMethod::FindReferences((specifier, position)) => json!({ "id": id, - "method": "getReferences", + "method": "findReferences", "specifier": state.denormalize_specifier(specifier), "position": position, }), diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 11800c2f764591..13586281c3a012 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -3091,6 +3091,98 @@ fn lsp_nav_tree_updates() { client.shutdown(); } +#[test] +fn lsp_find_references() { + let mut client = LspClientBuilder::new().build(); + client.initialize_default(); + client.did_open(json!({ + "textDocument": { + "uri": "file:///a/mod.ts", + "languageId": "typescript", + "version": 1, + "text": r#"export const a = 5;"# + } + })); + client.did_open(json!({ + "textDocument": { + "uri": "file:///a/mod.test.ts", + "languageId": "typescript", + "version": 1, + "text": r#"import { a } from './mod.ts'; console.log(a);"# + } + })); + + // test without including the declaration + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": "file:///a/mod.ts", + }, + "position": { "line": 0, "character": 13 }, + "context": { + "includeDeclaration": false + } + }), + ); + + assert_eq!( + res, + json!([{ + "uri": "file:///a/mod.test.ts", + "range": { + "start": { "line": 0, "character": 9 }, + "end": { "line": 0, "character": 10 } + } + }, { + "uri": "file:///a/mod.test.ts", + "range": { + "start": { "line": 0, "character": 42 }, + "end": { "line": 0, "character": 43 } + } + }]) + ); + + // test with including the declaration + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": "file:///a/mod.ts", + }, + "position": { "line": 0, "character": 13 }, + "context": { + "includeDeclaration": true + } + }), + ); + + assert_eq!( + res, + json!([{ + "uri": "file:///a/mod.ts", + "range": { + "start": { "line": 0, "character": 13 }, + "end": { "line": 0, "character": 14 } + } + }, { + "uri": "file:///a/mod.test.ts", + "range": { + "start": { "line": 0, "character": 9 }, + "end": { "line": 0, "character": 10 } + } + }, { + "uri": "file:///a/mod.test.ts", + "range": { + "start": { "line": 0, "character": 42 }, + "end": { "line": 0, "character": 43 } + } + }]) + ); + + client.shutdown(); +} + #[test] fn lsp_signature_help() { let mut client = LspClientBuilder::new().build(); diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index a00b946e22d72a..b8189278c05401 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -1121,10 +1121,10 @@ delete Object.prototype.__proto__; ), ); } - case "getReferences": { + case "findReferences": { return respond( id, - languageService.getReferencesAtPosition( + languageService.findReferences( request.specifier, request.position, ), diff --git a/cli/tsc/compiler.d.ts b/cli/tsc/compiler.d.ts index a1ee4579716545..b59f6dca8163fa 100644 --- a/cli/tsc/compiler.d.ts +++ b/cli/tsc/compiler.d.ts @@ -75,7 +75,7 @@ declare global { | GetNavigationTree | GetOutliningSpans | GetQuickInfoRequest - | GetReferencesRequest + | FindReferencesRequest | GetSignatureHelpItemsRequest | GetSmartSelectionRange | GetSupportedCodeFixes @@ -212,8 +212,8 @@ declare global { position: number; } - interface GetReferencesRequest extends BaseLanguageServerRequest { - method: "getReferences"; + interface FindReferencesRequest extends BaseLanguageServerRequest { + method: "findReferences"; specifier: string; position: number; } From b7dee9912a0ed197169640a57bfdbb9d6ded0881 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 15:09:35 -0400 Subject: [PATCH 16/26] Small refactor. --- cli/lsp/code_lens.rs | 14 ++++++++------ cli/lsp/language_server.rs | 18 +++++++----------- cli/lsp/tsc.rs | 30 ++++++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index c3d73bac3de7e9..4ebdb1342747ef 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -300,13 +300,15 @@ async fn resolve_references_code_lens( let asset_or_document = language_server.get_asset_or_document(&data.specifier)?; let line_index = asset_or_document.line_index(); - let req = tsc::RequestMethod::FindReferences(( - data.specifier.clone(), - line_index.offset_tsc(code_lens.range.start)?, - )); let snapshot = language_server.snapshot(); - let maybe_referenced_symbols: Option> = - language_server.ts_server.request(snapshot, req).await?; + let maybe_referenced_symbols = language_server + .ts_server + .find_references( + snapshot, + &data.specifier, + line_index.offset_tsc(code_lens.range.start)?, + ) + .await?; if let Some(symbols) = maybe_referenced_symbols { let mut locations = Vec::new(); for reference in symbols.iter().flat_map(|s| &s.references) { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 9bf338eb4a3a98..aceb626c0c4db2 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1947,18 +1947,14 @@ impl Inner { let mark = self.performance.mark("references", Some(¶ms)); let asset_or_doc = self.get_asset_or_document(&specifier)?; let line_index = asset_or_doc.line_index(); - let req = tsc::RequestMethod::FindReferences(( - specifier.clone(), - line_index.offset_tsc(params.text_document_position.position)?, - )); - let maybe_referenced_symbols: Option> = self + let maybe_referenced_symbols = self .ts_server - .request(self.snapshot(), req) - .await - .map_err(|err| { - error!("Unable to get references from TypeScript: {}", err); - LspError::internal_error() - })?; + .find_references( + self.snapshot(), + &specifier, + line_index.offset_tsc(params.text_document_position.position)?, + ) + .await?; if let Some(symbols) = maybe_referenced_symbols { let mut results = Vec::new(); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 657d2dd197d43e..abefbbf6da14c0 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -151,6 +151,26 @@ impl TsServer { let value = rx.await??; Ok(serde_json::from_value::(value)?) } + + // todo(dsherret): refactor the rest of the request methods to have + // methods to call on this struct, then make `RequestMethod` and + // friends internal + + pub async fn find_references( + &self, + snapshot: Arc, + specifier: &ModuleSpecifier, + position: u32, + ) -> Result>, LspError> { + let req = RequestMethod::FindReferences { + specifier: specifier.clone(), + position, + }; + self.request(snapshot, req).await.map_err(|err| { + log::error!("Unable to get references from TypeScript: {}", err); + LspError::internal_error() + }) + } } #[derive(Debug, Clone)] @@ -3200,7 +3220,10 @@ pub enum RequestMethod { /// Return quick info at position (hover information). GetQuickInfo((ModuleSpecifier, u32)), /// Finds the document references for a specific position. - FindReferences((ModuleSpecifier, u32)), + FindReferences { + specifier: ModuleSpecifier, + position: u32, + }, /// Get signature help items for a specific position. GetSignatureHelpItems((ModuleSpecifier, u32, SignatureHelpItemsOptions)), /// Get a selection range for a specific position. @@ -3370,7 +3393,10 @@ impl RequestMethod { "specifier": state.denormalize_specifier(specifier), "position": position, }), - RequestMethod::FindReferences((specifier, position)) => json!({ + RequestMethod::FindReferences { + specifier, + position, + } => json!({ "id": id, "method": "findReferences", "specifier": state.denormalize_specifier(specifier), From a8cb32933ed85b166df8b4412c709235ab127871 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 29 Mar 2023 15:26:16 -0400 Subject: [PATCH 17/26] Fix codelens test --- cli/tests/integration/lsp_tests.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 13586281c3a012..27df9742f2bb48 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -2428,18 +2428,12 @@ fn lsp_code_lens() { "end": { "line": 0, "character": 7 } }, "command": { - "title": "2 references", + "title": "1 reference", "command": "deno.showReferences", "arguments": [ "file:///a/file.ts", { "line": 0, "character": 6 }, [{ - "uri": "file:///a/file.ts", - "range": { - "start": { "line": 0, "character": 6 }, - "end": { "line": 0, "character": 7 } - } - }, { "uri": "file:///a/file.ts", "range": { "start": { "line": 12, "character": 14 }, From b7d75411bb7aa5dcc6805c36e36c768411036b3f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 10:41:24 -0400 Subject: [PATCH 18/26] Add test and improve code --- cli/lsp/code_lens.rs | 99 +++++++++++----------- cli/tests/integration/lsp_tests.rs | 128 ++++++++++++++++++++++++++--- 2 files changed, 166 insertions(+), 61 deletions(-) diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index 4ebdb1342747ef..650e5e2416c9f3 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -297,19 +297,14 @@ async fn resolve_references_code_lens( data: CodeLensData, language_server: &language_server::Inner, ) -> Result { - let asset_or_document = - language_server.get_asset_or_document(&data.specifier)?; - let line_index = asset_or_document.line_index(); - let snapshot = language_server.snapshot(); - let maybe_referenced_symbols = language_server - .ts_server - .find_references( - snapshot, - &data.specifier, - line_index.offset_tsc(code_lens.range.start)?, - ) - .await?; - if let Some(symbols) = maybe_referenced_symbols { + fn get_locations( + maybe_referenced_symbols: Option>, + language_server: &language_server::Inner, + ) -> Result, AnyError> { + let symbols = match maybe_referenced_symbols { + Some(symbols) => symbols, + None => return Ok(Vec::new()), + }; let mut locations = Vec::new(); for reference in symbols.iter().flat_map(|s| &s.references) { if reference.is_definition { @@ -325,45 +320,49 @@ async fn resolve_references_code_lens( .to_location(asset_or_doc.line_index(), &language_server.url_map), ); } - let command = if !locations.is_empty() { - let title = if locations.len() > 1 { - format!("{} references", locations.len()) - } else { - "1 reference".to_string() - }; - lsp::Command { - title, - command: "deno.showReferences".to_string(), - arguments: Some(vec![ - json!(data.specifier), - json!(code_lens.range.start), - json!(locations), - ]), - } - } else { - lsp::Command { - title: "0 references".to_string(), - command: "".to_string(), - arguments: None, - } - }; - Ok(lsp::CodeLens { - range: code_lens.range, - command: Some(command), - data: None, - }) + Ok(locations) + } + + let asset_or_document = + language_server.get_asset_or_document(&data.specifier)?; + let line_index = asset_or_document.line_index(); + let snapshot = language_server.snapshot(); + let maybe_referenced_symbols = language_server + .ts_server + .find_references( + snapshot, + &data.specifier, + line_index.offset_tsc(code_lens.range.start)?, + ) + .await?; + let locations = get_locations(maybe_referenced_symbols, language_server)?; + let title = if locations.len() == 1 { + "1 reference".to_string() } else { - let command = lsp::Command { - title: "0 references".to_string(), - command: "".to_string(), + format!("{} references", locations.len()) + }; + let command = if locations.is_empty() { + lsp::Command { + title, + command: String::new(), arguments: None, - }; - Ok(lsp::CodeLens { - range: code_lens.range, - command: Some(command), - data: None, - }) - } + } + } else { + lsp::Command { + title, + command: "deno.showReferences".to_string(), + arguments: Some(vec![ + json!(data.specifier), + json!(code_lens.range.start), + json!(locations), + ]), + } + }; + Ok(lsp::CodeLens { + range: code_lens.range, + command: Some(command), + data: None, + }) } pub async fn resolve_code_lens( diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 27df9742f2bb48..d1ede3da1dcdb8 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -2367,16 +2367,32 @@ fn lsp_semantic_tokens() { fn lsp_code_lens() { let mut client = LspClientBuilder::new().build(); client.initialize_default(); - client.did_open( - json!({ - "textDocument": { - "uri": "file:///a/file.ts", - "languageId": "typescript", - "version": 1, - "text": "class A {\n a = \"a\";\n\n b() {\n console.log(this.a);\n }\n\n c() {\n this.a = \"c\";\n }\n}\n\nconst a = new A();\na.b();\n" - } - }), - ); + client.did_open(json!({ + "textDocument": { + "uri": "file:///a/file.ts", + "languageId": "typescript", + "version": 1, + "text": concat!( + "class A {\n", + " a = \"a\";\n", + "\n", + " b() {\n", + " console.log(this.a);\n", + " }\n", + "\n", + " c() {\n", + " this.a = \"c\";\n", + " }\n", + "}\n", + "\n", + "const a = new A();\n", + "a.b();\n", + "const b = 2;\n", + "const c = 3;\n", + "c; c;", + ), + } + })); let res = client.write_request( "textDocument/codeLens", json!({ @@ -2444,6 +2460,80 @@ fn lsp_code_lens() { } }) ); + + // 0 references + let res = client.write_request( + "codeLens/resolve", + json!({ + "range": { + "start": { "line": 14, "character": 6 }, + "end": { "line": 14, "character": 7 } + }, + "data": { + "specifier": "file:///a/file.ts", + "source": "references" + } + }), + ); + assert_eq!( + res, + json!({ + "range": { + "start": { "line": 14, "character": 6 }, + "end": { "line": 14, "character": 7 } + }, + "command": { + "title": "0 references", + "command": "", + } + }) + ); + + // 2 references + let res = client.write_request( + "codeLens/resolve", + json!({ + "range": { + "start": { "line": 15, "character": 6 }, + "end": { "line": 15, "character": 7 } + }, + "data": { + "specifier": "file:///a/file.ts", + "source": "references" + } + }), + ); + assert_eq!( + res, + json!({ + "range": { + "start": { "line": 15, "character": 6 }, + "end": { "line": 15, "character": 7 } + }, + "command": { + "title": "2 references", + "command": "deno.showReferences", + "arguments": [ + "file:///a/file.ts", + { "line": 15, "character": 6 }, + [{ + "uri": "file:///a/file.ts", + "range": { + "start": { "line": 16, "character": 0 }, + "end": { "line": 16, "character": 1 } + } + },{ + "uri": "file:///a/file.ts", + "range": { + "start": { "line": 16, "character": 3 }, + "end": { "line": 16, "character": 4 } + } + }] + ] + } + }) + ); + client.shutdown(); } @@ -3094,7 +3184,7 @@ fn lsp_find_references() { "uri": "file:///a/mod.ts", "languageId": "typescript", "version": 1, - "text": r#"export const a = 5;"# + "text": r#"export const a = 1;\nexport const b = 2;"# } })); client.did_open(json!({ @@ -3174,6 +3264,22 @@ fn lsp_find_references() { }]) ); + // test 0 references + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": "file:///a/mod.ts", + }, + "position": { "line": 1, "character": 13 }, + "context": { + "includeDeclaration": false + } + }), + ); + + assert_eq!(res, json!([])); + client.shutdown(); } From 8dec2d3c78a7976a965e06d9bc6b44822bca3862 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 11:08:47 -0400 Subject: [PATCH 19/26] Oops... Fix test --- cli/tests/integration/lsp_tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index d1ede3da1dcdb8..35c115d56449e9 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -3184,7 +3184,7 @@ fn lsp_find_references() { "uri": "file:///a/mod.ts", "languageId": "typescript", "version": 1, - "text": r#"export const a = 1;\nexport const b = 2;"# + "text": r#"export const a = 1;\nconst b = 2;"# } })); client.did_open(json!({ @@ -3271,14 +3271,14 @@ fn lsp_find_references() { "textDocument": { "uri": "file:///a/mod.ts", }, - "position": { "line": 1, "character": 13 }, + "position": { "line": 1, "character": 6 }, "context": { "includeDeclaration": false } }), ); - assert_eq!(res, json!([])); + assert_eq!(res, json!(null)); // seems it always returns null for this, which is ok client.shutdown(); } From 5a55a7e493c5c25d1e68dadea148a4a42c7c55ca Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 11:35:42 -0400 Subject: [PATCH 20/26] Reduce flakiness of package_json_uncached_no_error --- cli/tests/integration/repl_tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index 27a9b716c510cb..73c8918f9d9b85 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -905,7 +905,7 @@ fn package_json_uncached_no_error() { ); test_context.new_command().with_pty(|mut console| { console.write_line("console.log(123 + 456);"); - console.expect("579"); + console.expect_all(&["579", "undefined"]); assert_not_contains!( console.all_output(), "Could not set npm package requirements", @@ -914,7 +914,7 @@ fn package_json_uncached_no_error() { // should support getting the package now though console .write_line("import { getValue, setValue } from '@denotest/esm-basic';"); - console.expect("undefined"); + console.expect_all(&["undefined", "Initialize"]); console.write_line("setValue(12 + 30);"); console.expect("undefined"); console.write_line("getValue()"); From 7b96f00626579a4ad3c12273ac17818216b77b9d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 12:19:22 -0400 Subject: [PATCH 21/26] Add enabled_root_urls tests --- cli/lsp/config.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index c929f0cc116a85..418ffdc483bdae 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -850,4 +850,44 @@ mod tests { ); run_test(vec![], vec![]); } + + #[test] + fn config_enabled_root_urls() { + let mut config = Config::new(); + let root_dir = Url::parse("file:///example/").unwrap(); + config.root_uri = Some(root_dir.clone()); + config.settings.workspace.enable = false; + config.settings.workspace.enable_paths = Vec::new(); + assert_eq!(config.enabled_root_urls(), vec![]); + + config.settings.workspace.enable = true; + assert_eq!(config.enabled_root_urls(), vec![root_dir]); + + config.settings.workspace.enable = false; + let root_dir1 = Url::parse("file:///root1/").unwrap(); + let root_dir2 = Url::parse("file:///root2/").unwrap(); + let root_dir3 = Url::parse("file:///root3/").unwrap(); + config.enabled_paths = HashMap::from([ + ( + root_dir1.clone(), + vec![ + root_dir1.join("sub_dir").unwrap(), + root_dir1.join("sub_dir/other").unwrap(), + root_dir1.join("test.ts").unwrap(), + ], + ), + (root_dir2.clone(), vec![root_dir2.join("other.ts").unwrap()]), + (root_dir3.clone(), vec![]), + ]); + + assert_eq!( + config.enabled_root_urls(), + vec![ + root_dir1.join("sub_dir").unwrap(), + root_dir1.join("test.ts").unwrap(), + root_dir2.join("other.ts").unwrap(), + root_dir3 + ] + ); + } } From 697ace63caf192a475f7107e5ec2aea451d88cfb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 13:36:47 -0400 Subject: [PATCH 22/26] Refactor document finder to be more easily unit testable --- cli/lsp/documents.rs | 290 +++++++++++++++++++++++++++++-------------- 1 file changed, 200 insertions(+), 90 deletions(-) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index d1965b42c12484..0c27893a798dd6 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -49,6 +49,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; use std::fs; +use std::fs::ReadDir; use std::ops::Range; use std::path::Path; use std::path::PathBuf; @@ -1238,48 +1239,6 @@ impl Documents { } fn refresh_dependencies(&mut self, root_urls: Vec) { - fn is_auto_discoverable_dir(dir_path: &Path) -> bool { - if let Some(dir_name) = dir_path.file_name() { - let dir_name = dir_name.to_string_lossy().to_lowercase(); - !matches!(dir_name.as_str(), "node_modules" | ".git") - } else { - false - } - } - - fn is_auto_discoverable_file(file_path: &Path) -> bool { - // Don't auto-discover minified files as they are likely to be very large - // and likely not to have dependencies on code outside them that would - // be useful in the LSP - if let Some(file_name) = file_path.file_name() { - let file_name = file_name.to_string_lossy().to_lowercase(); - !file_name.as_str().contains(".min.") - } else { - false - } - } - - fn is_diagnosable(media_type: MediaType) -> bool { - match media_type { - MediaType::JavaScript - | MediaType::Jsx - | MediaType::Mjs - | MediaType::Cjs - | MediaType::TypeScript - | MediaType::Mts - | MediaType::Cts - | MediaType::Dts - | MediaType::Dmts - | MediaType::Dcts - | MediaType::Tsx => true, - MediaType::Json - | MediaType::Wasm - | MediaType::SourceMap - | MediaType::TsBuildInfo - | MediaType::Unknown => false, - } - } - 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) { @@ -1293,16 +1252,7 @@ impl Documents { fs_docs.docs.keys().cloned().collect::>(); let open_docs = &mut self.open_docs; - let mut handle_file_path = |file_path: &Path| { - let media_type = MediaType::from_path(&file_path); - if !is_diagnosable(media_type) || !is_auto_discoverable_file(&file_path) { - return; - } - let specifier = match ModuleSpecifier::from_file_path(&file_path) { - Ok(specifier) => specifier, - Err(_) => return, - }; - + for specifier in PreloadDocumentFinder::from_root_urls(&root_urls) { // mark this document as having been found not_found_docs.remove(&specifier); @@ -1318,44 +1268,6 @@ impl Documents { } } } - }; - - let mut pending_dirs = VecDeque::new(); - for root_url in root_urls { - if let Ok(path) = root_url.to_file_path() { - if path.is_dir() { - pending_dirs.push_back(path.to_path_buf()); - } else { - handle_file_path(&path); - } - } - } - - while let Some(dir_path) = pending_dirs.pop_front() { - if !is_auto_discoverable_dir(&dir_path) { - continue; - } - let read_dir = match std::fs::read_dir(&dir_path) { - Ok(entry) => entry, - Err(_) => continue, - }; - - for entry in read_dir { - let entry = match entry { - Ok(entry) => entry, - Err(_) => continue, - }; - let file_type = match entry.file_type() { - Ok(file_type) => file_type, - Err(_) => continue, - }; - let new_path = dir_path.join(entry.file_name()); - if file_type.is_dir() { - pending_dirs.push_back(new_path); - } else if file_type.is_file() { - handle_file_path(&new_path); - } - } } // clean up and remove any documents that weren't found @@ -1595,12 +1507,150 @@ fn analyze_module( } } +/// Iterator that finds documents that can be preloaded into +/// the LSP on startup. +struct PreloadDocumentFinder { + pending_dirs: Vec, + pending_files: Vec, + current_entries: Option, +} + +impl PreloadDocumentFinder { + pub fn from_root_urls(root_urls: &Vec) -> Self { + let mut finder = PreloadDocumentFinder { + pending_dirs: Default::default(), + pending_files: Default::default(), + current_entries: Default::default(), + }; + for root_url in root_urls { + if let Ok(path) = root_url.to_file_path() { + if path.is_dir() { + finder.pending_dirs.push(path); + } else { + finder.pending_files.push(path); + } + } + } + finder + } + + fn read_next_file_entry(&mut self) -> Option { + fn is_discoverable_dir(dir_path: &Path) -> bool { + if let Some(dir_name) = dir_path.file_name() { + let dir_name = dir_name.to_string_lossy().to_lowercase(); + !matches!(dir_name.as_str(), "node_modules" | ".git") + } else { + false + } + } + + if let Some(mut entries) = self.current_entries.take() { + while let Some(entry) = entries.next() { + if let Ok(entry) = entry { + let path = entry.path(); + if let Ok(file_type) = entry.file_type() { + if file_type.is_dir() && is_discoverable_dir(&path) { + self.pending_dirs.push(path); + } else if file_type.is_file() { + if let Some(specifier) = Self::get_valid_specifier(&path) { + // restore the next entries for next time + self.current_entries = Some(entries); + return Some(specifier); + } + } + } + } + } + } + + None + } + + fn get_valid_specifier(path: &Path) -> Option { + fn is_discoverable_file(file_path: &Path) -> bool { + // Don't auto-discover minified files as they are likely to be very large + // and likely not to have dependencies on code outside them that would + // be useful in the LSP + if let Some(file_name) = file_path.file_name() { + let file_name = file_name.to_string_lossy().to_lowercase(); + !file_name.as_str().contains(".min.") + } else { + false + } + } + + fn is_discoverable_media_type(media_type: MediaType) -> bool { + match media_type { + MediaType::JavaScript + | MediaType::Jsx + | MediaType::Mjs + | MediaType::Cjs + | MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Tsx => true, + MediaType::Json // ignore because json never depends on other files + | MediaType::Wasm + | MediaType::SourceMap + | MediaType::TsBuildInfo + | MediaType::Unknown => false, + } + } + + let media_type = MediaType::from_path(path); + if is_discoverable_media_type(media_type) && is_discoverable_file(path) { + if let Ok(specifier) = ModuleSpecifier::from_file_path(path) { + return Some(specifier); + } + } + None + } + + fn queue_next_file_entries(&mut self) { + debug_assert!(self.current_entries.is_none()); + while let Some(dir_path) = self.pending_dirs.pop() { + if let Ok(entries) = fs::read_dir(&dir_path) { + self.current_entries = Some(entries); + break; + } + } + } +} + +impl Iterator for PreloadDocumentFinder { + type Item = ModuleSpecifier; + + fn next(&mut self) -> Option { + // drain the pending files + while let Some(path) = self.pending_files.pop() { + if let Some(specifier) = Self::get_valid_specifier(&path) { + return Some(specifier); + } + } + + // then go through the current entries and directories + while !self.pending_dirs.is_empty() || self.current_entries.is_some() { + match self.read_next_file_entry() { + Some(entry) => return Some(entry), + None => { + self.queue_next_file_entries(); + } + } + } + None + } +} + #[cfg(test)] mod tests { use crate::npm::NpmResolution; use super::*; use import_map::ImportMap; + use pretty_assertions::assert_eq; use test_util::TempDir; fn setup(temp_dir: &TempDir) -> (Documents, PathBuf) { @@ -1795,4 +1845,64 @@ console.log(b, "hello deno"); ); } } + + #[test] + pub fn test_pre_load_document_finder() { + let temp_dir = TempDir::new(); + temp_dir.create_dir_all("root1/node_modules/"); + temp_dir.write("root1/node_modules/mod.ts", ""); // no, node_modules + + temp_dir.create_dir_all("root1/sub_dir"); + temp_dir.create_dir_all("root1/.git"); + temp_dir.create_dir_all("root1/file.ts"); // no, directory + temp_dir.write("root1/mod1.ts", ""); // yes + temp_dir.write("root1/mod2.js", ""); // yes + temp_dir.write("root1/mod3.tsx", ""); // yes + temp_dir.write("root1/mod4.d.ts", ""); // yes + temp_dir.write("root1/mod5.jsx", ""); // yes + temp_dir.write("root1/mod6.mjs", ""); // yes + temp_dir.write("root1/mod7.mts", ""); // yes + temp_dir.write("root1/mod8.d.mts", ""); // yes + temp_dir.write("root1/other.json", ""); // no, json + temp_dir.write("root1/other.txt", ""); // no, text file + temp_dir.write("root1/other.wasm", ""); // no, don't load wasm + temp_dir.write("root1/sub_dir/mod.ts", ""); // yes + temp_dir.write("root1/sub_dir/data.min.ts", ""); // no, minified file + temp_dir.write("root1/.git/main.ts", ""); // no, .git folder + + temp_dir.create_dir_all("root2/folder"); + temp_dir.write("root2/file1.ts", ""); // yes, provided + temp_dir.write("root2/file2.ts", ""); // no, not provided + temp_dir.write("root2/folder/main.ts", ""); // yes, provided + + temp_dir.create_dir_all("root3/"); + temp_dir.write("root3/mod.ts", ""); // no, not provided + + let mut urls = PreloadDocumentFinder::from_root_urls(&vec![ + temp_dir.uri().join("root1/").unwrap(), + temp_dir.uri().join("root2/file1.ts").unwrap(), + temp_dir.uri().join("root2/folder/").unwrap(), + ]) + .collect::>(); + + // order doesn't matter + urls.sort(); + + assert_eq!( + urls, + vec![ + temp_dir.uri().join("root1/mod1.ts").unwrap(), + temp_dir.uri().join("root1/mod2.js").unwrap(), + temp_dir.uri().join("root1/mod3.tsx").unwrap(), + temp_dir.uri().join("root1/mod4.d.ts").unwrap(), + temp_dir.uri().join("root1/mod5.jsx").unwrap(), + temp_dir.uri().join("root1/mod6.mjs").unwrap(), + temp_dir.uri().join("root1/mod7.mts").unwrap(), + temp_dir.uri().join("root1/mod8.d.mts").unwrap(), + temp_dir.uri().join("root1/sub_dir/mod.ts").unwrap(), + temp_dir.uri().join("root2/file1.ts").unwrap(), + temp_dir.uri().join("root2/folder/main.ts").unwrap(), + ] + ); + } } From 22164029e3dd8896fa982f514070a86f346c472a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 14:17:31 -0400 Subject: [PATCH 23/26] Another test. --- cli/tests/integration/lsp_tests.rs | 64 +++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 397296ab997645..ba6c8f67322d5e 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -929,11 +929,11 @@ fn lsp_inlay_hints_not_enabled() { #[test] fn lsp_workspace_enable_paths() { fn run_test(use_trailing_slash: bool) { - let context = TestContextBuilder::new().build(); - // we aren't actually writing anything to the tempdir in this test, but we - // just need a legitimate file path on the host system so that logic that - // tries to convert to and from the fs paths works on all env + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); + temp_dir.create_dir_all("worker"); + temp_dir.write("worker/shared.ts", "export const a = 1"); + temp_dir.write("worker/other.ts", "import { a } from './shared.ts';\na;"); let root_specifier = temp_dir.uri(); @@ -985,7 +985,11 @@ fn lsp_workspace_enable_paths() { "uri": root_specifier.join("./worker/file.ts").unwrap(), "languageId": "typescript", "version": 1, - "text": "console.log(Date.now());\n" + "text": concat!( + "console.log(Date.now());\n", + "import { a } from './shared.ts';\n", + "a;\n", + ), } })); @@ -1072,6 +1076,56 @@ fn lsp_workspace_enable_paths() { }) ); + // check that the file system documents were auto-discovered + // via the enabled paths + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": root_specifier.join("./worker/file.ts").unwrap(), + }, + "position": { "line": 2, "character": 0 }, + "context": { + "includeDeclaration": true + } + }), + ); + + assert_eq!( + res, + json!([{ + "uri": root_specifier.join("./worker/file.ts").unwrap(), + "range": { + "start": { "line": 1, "character": 9 }, + "end": { "line": 1, "character": 10 } + } + }, { + "uri": root_specifier.join("./worker/file.ts").unwrap(), + "range": { + "start": { "line": 2, "character": 0 }, + "end": { "line": 2, "character": 1 } + } + }, { + "uri": root_specifier.join("./worker/shared.ts").unwrap(), + "range": { + "start": { "line": 0, "character": 13 }, + "end": { "line": 0, "character": 14 } + } + }, { + "uri": root_specifier.join("./worker/other.ts").unwrap(), + "range": { + "start": { "line": 0, "character": 9 }, + "end": { "line": 0, "character": 10 } + } + }, { + "uri": root_specifier.join("./worker/other.ts").unwrap(), + "range": { + "start": { "line": 1, "character": 0 }, + "end": { "line": 1, "character": 1 } + } + }]) + ); + client.shutdown(); } From 4d8a9b099d5c76863100f5397c6c55905f7d56a9 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 15:14:58 -0400 Subject: [PATCH 24/26] Fix tests --- cli/cache/http_cache.rs | 3 +- cli/tests/integration/repl_tests.rs | 92 +++++++++++++++++-------- cli/tests/integration/run_tests.rs | 103 ++++++++++++++-------------- cli/tests/integration/test_tests.rs | 5 +- test_util/src/builders.rs | 7 +- test_util/src/lib.rs | 10 +-- 6 files changed, 126 insertions(+), 94 deletions(-) diff --git a/cli/cache/http_cache.rs b/cli/cache/http_cache.rs index 2e784765e2e570..b10c597561f97e 100644 --- a/cli/cache/http_cache.rs +++ b/cli/cache/http_cache.rs @@ -11,7 +11,6 @@ use deno_core::serde::Deserialize; use deno_core::serde::Serialize; use deno_core::serde_json; use deno_core::url::Url; -use log::error; use std::fs; use std::fs::File; use std::io; @@ -42,7 +41,7 @@ fn base_url_to_filename(url: &Url) -> Option { } "data" | "blob" => (), scheme => { - error!("Don't know how to create cache name for scheme: {}", scheme); + log::debug!("Don't know how to create cache name for scheme: {}", scheme); return None; } }; diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index 73c8918f9d9b85..c3e7c42d65860d 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -131,24 +131,43 @@ fn pty_complete_expression() { #[test] fn pty_complete_imports() { - util::with_pty(&["repl", "-A"], |mut console| { - // single quotes - console.write_line_raw("import './run/001_hel\t'"); - console.expect("Hello World"); - // double quotes - console.write_line_raw("import { output } from \"./run/045_out\t\""); - console.expect("\"./run/045_output.ts\""); - console.write_line_raw("output('testing output');"); - console.expect("testing output"); - }); + let context = TestContextBuilder::default().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.create_dir_all("subdir"); + temp_dir.write("./subdir/my_file.ts", ""); + temp_dir.create_dir_all("run"); + temp_dir.write("./run/hello.ts", "console.log('Hello World');"); + temp_dir.write( + "./run/output.ts", + r#"export function output(text: string) { + console.log(text); +} +"#, + ); + context + .new_command() + .args_vec(["repl", "-A"]) + .with_pty(|mut console| { + // single quotes + console.write_line_raw("import './run/hel\t'"); + console.expect("Hello World"); + // double quotes + console.write_line_raw("import { output } from \"./run/out\t\""); + console.expect("\"./run/output.ts\""); + console.write_line_raw("output('testing output');"); + console.expect("testing output"); + }); // ensure when the directory changes that the suggestions come from the cwd - util::with_pty(&["repl", "-A"], |mut console| { - console.write_line("Deno.chdir('./subdir');"); - console.expect("undefined"); - console.write_line_raw("import '../run/001_hel\t'"); - console.expect("Hello World"); - }); + context + .new_command() + .args_vec(["repl", "-A"]) + .with_pty(|mut console| { + console.write_line("Deno.chdir('./subdir');"); + console.expect("undefined"); + console.write_line_raw("import '../run/he\t'"); + console.expect("Hello World"); + }); } #[test] @@ -283,10 +302,15 @@ fn let_redeclaration() { #[test] fn repl_cwd() { - util::with_pty(&["repl", "-A"], |mut console| { - console.write_line("Deno.cwd()"); - console.expect("testdata"); - }); + let context = TestContextBuilder::default().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + context + .new_command() + .args_vec(["repl", "-A"]) + .with_pty(|mut console| { + console.write_line("Deno.cwd()"); + console.expect(temp_dir.path().file_name().unwrap().to_str().unwrap()); + }); } #[test] @@ -384,18 +408,30 @@ fn multiline() { #[test] fn import() { - util::with_pty(&["repl", "-A"], |mut console| { - console.write_line("import('./subdir/auto_print_hello.ts')"); - console.expect("hello!"); - }); + let context = TestContextBuilder::default() + .use_copy_temp_dir("./subdir") + .build(); + context + .new_command() + .args_vec(["repl", "-A"]) + .with_pty(|mut console| { + console.write_line("import('./subdir/auto_print_hello.ts')"); + console.expect("hello!"); + }); } #[test] fn import_declarations() { - util::with_pty(&["repl", "-A"], |mut console| { - console.write_line("import './subdir/auto_print_hello.ts'"); - console.expect("hello!"); - }); + let context = TestContextBuilder::default() + .use_copy_temp_dir("./subdir") + .build(); + context + .new_command() + .args_vec(["repl", "-A"]) + .with_pty(|mut console| { + console.write_line("import './subdir/auto_print_hello.ts'"); + console.expect("hello!"); + }); } #[test] diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 003bc59fc50f76..04385b83cbc956 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -14,6 +14,7 @@ use trust_dns_client::serialize::txt::Lexer; use trust_dns_client::serialize::txt::Parser; use util::assert_contains; use util::env_vars_for_npm_tests_no_sync_download; +use util::TestContext; use util::TestContextBuilder; itest!(stdout_write_all { @@ -571,9 +572,10 @@ itest!(_089_run_allow_list { #[test] fn _090_run_permissions_request() { - util::with_pty( - &["run", "--quiet", "run/090_run_permissions_request.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/090_run_permissions_request.ts"]) + .with_pty(|mut console| { console.expect(concat!( "┌ ⚠️ Deno requests run access to \"ls\".\r\n", "├ Requested by `Deno.permissions.query()` API.\r\n", @@ -592,15 +594,15 @@ fn _090_run_permissions_request() { console.expect("Denied run access to \"cat\"."); console.expect("granted"); console.expect("denied"); - }, - ); + }); } #[test] fn _090_run_permissions_request_sync() { - util::with_pty( - &["run", "--quiet", "run/090_run_permissions_request_sync.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/090_run_permissions_request_sync.ts"]) + .with_pty(|mut console| { console.expect(concat!( "┌ ⚠️ Deno requests run access to \"ls\".\r\n", "├ Requested by `Deno.permissions.query()` API.\r\n", @@ -619,15 +621,15 @@ fn _090_run_permissions_request_sync() { console.expect("Denied run access to \"cat\"."); console.expect("granted"); console.expect("denied"); - }, - ); + }); } #[test] fn permissions_prompt_allow_all() { - util::with_pty( - &["run", "--quiet", "run/permissions_prompt_allow_all.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/permissions_prompt_allow_all.ts"]) + .with_pty(|mut console| { // "run" permissions console.expect(concat!( "┌ ⚠️ Deno requests run access to \"FOO\".\r\n", @@ -697,9 +699,10 @@ fn permissions_prompt_allow_all() { #[test] fn permissions_prompt_allow_all_2() { - util::with_pty( - &["run", "--quiet", "run/permissions_prompt_allow_all_2.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/permissions_prompt_allow_all_2.ts"]) + .with_pty(|mut console| { // "env" permissions console.expect(concat!( "┌ ⚠️ Deno requests env access to \"FOO\".\r\n", @@ -728,15 +731,15 @@ fn permissions_prompt_allow_all_2() { )); console.write_line_raw("A"); console.expect("✅ Granted all read access."); - }, - ); + }); } #[test] fn permissions_prompt_allow_all_lowercase_a() { - util::with_pty( - &["run", "--quiet", "run/permissions_prompt_allow_all.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/permissions_prompt_allow_all.ts"]) + .with_pty(|mut console| { // "run" permissions console.expect(concat!( "┌ ⚠️ Deno requests run access to \"FOO\".\r\n", @@ -746,8 +749,7 @@ fn permissions_prompt_allow_all_lowercase_a() { )); console.write_line_raw("a"); console.expect("Unrecognized option."); - }, - ); + }); } itest!(_091_use_define_for_class_fields { @@ -2141,6 +2143,7 @@ fn dont_cache_on_check_fail() { mod permissions { use test_util as util; + use util::TestContext; // TODO(bartlomieju): remove --unstable once Deno.Command is stabilized #[test] @@ -2503,9 +2506,10 @@ mod permissions { #[test] fn _061_permissions_request() { - util::with_pty( - &["run", "--quiet", "run/061_permissions_request.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/061_permissions_request.ts"]) + .with_pty(|mut console| { console.expect(concat!( "┌ ⚠️ Deno requests read access to \"foo\".\r\n", "├ Requested by `Deno.permissions.query()` API.\r\n", @@ -2523,15 +2527,15 @@ mod permissions { console.expect("granted"); console.expect("prompt"); console.expect("denied"); - }, - ); + }); } #[test] fn _061_permissions_request_sync() { - util::with_pty( - &["run", "--quiet", "run/061_permissions_request_sync.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/061_permissions_request_sync.ts"]) + .with_pty(|mut console| { console.expect(concat!( "┌ ⚠️ Deno requests read access to \"foo\".\r\n", "├ Requested by `Deno.permissions.query()` API.\r\n", @@ -2549,15 +2553,15 @@ mod permissions { console.expect("granted"); console.expect("prompt"); console.expect("denied"); - }, - ); + }); } #[test] fn _062_permissions_request_global() { - util::with_pty( - &["run", "--quiet", "run/062_permissions_request_global.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/062_permissions_request_global.ts"]) + .with_pty(|mut console| { console.expect(concat!( "┌ ⚠️ Deno requests read access.\r\n", "├ Requested by `Deno.permissions.query()` API.\r\n", @@ -2571,19 +2575,15 @@ mod permissions { .expect("PermissionStatus { state: \"granted\", onchange: null }"); console .expect("PermissionStatus { state: \"granted\", onchange: null }"); - }, - ); + }); } #[test] fn _062_permissions_request_global_sync() { - util::with_pty( - &[ - "run", - "--quiet", - "run/062_permissions_request_global_sync.ts", - ], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "run/062_permissions_request_global_sync.ts"]) + .with_pty(|mut console| { console.expect(concat!( "┌ ⚠️ Deno requests read access.\r\n", "├ Requested by `Deno.permissions.query()` API.\r\n", @@ -2597,8 +2597,7 @@ mod permissions { .expect("PermissionStatus { state: \"granted\", onchange: null }"); console .expect("PermissionStatus { state: \"granted\", onchange: null }"); - }, - ); + }); } itest!(_063_permissions_revoke { @@ -2623,9 +2622,10 @@ mod permissions { #[test] fn _066_prompt() { - util::with_pty( - &["run", "--quiet", "--unstable", "run/066_prompt.ts"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["run", "--quiet", "--unstable", "run/066_prompt.ts"]) + .with_pty(|mut console| { console.expect("What is your name? [Jane Doe] "); console.write_line_raw("John Doe"); console.expect("Your name is John Doe."); @@ -2658,8 +2658,7 @@ mod permissions { console.expect("What is EOF? "); console.write_line(""); console.expect("Your answer is null"); - }, - ); + }); } itest!(dynamic_import_permissions_remote_remote { diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index 3a7f37db8bc6c0..8e2404530165fa 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -406,10 +406,9 @@ fn file_protocol() { .unwrap() .to_string(); - let context = TestContext::default(); - context + TestContext::default() .new_command() - .args_vec(vec!["test".to_string(), file_url]) + .args_vec(["test", file_url.as_str()]) .run() .assert_matches_file("test/file_protocol.out"); } diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs index 4997dac2ce0dbb..0a0f2244fd91a1 100644 --- a/test_util/src/builders.rs +++ b/test_util/src/builders.rs @@ -228,8 +228,11 @@ impl TestCommandBuilder { self } - pub fn args_vec(&mut self, args: Vec) -> &mut Self { - self.args_vec = args; + pub fn args_vec, I: IntoIterator>( + &mut self, + args: I, + ) -> &mut Self { + self.args_vec = args.into_iter().map(|a| a.as_ref().to_string()).collect(); self } diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index c844e594f1cc92..5c6bc97f7ab9ba 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -2093,8 +2093,7 @@ impl<'a> CheckOutputIntegrationTest<'a> { command_builder.args(self.args); } if !self.args_vec.is_empty() { - command_builder - .args_vec(self.args_vec.iter().map(|a| a.to_string()).collect()); + command_builder.args_vec(self.args_vec.clone()); } if let Some(input) = &self.input { command_builder.stdin(input); @@ -2167,11 +2166,8 @@ pub fn pattern_match(pattern: &str, s: &str, wildcard: &str) -> bool { } pub fn with_pty(deno_args: &[&str], action: impl FnMut(Pty)) { - let context = TestContextBuilder::default().build(); - context - .new_command() - .args_vec(deno_args.iter().map(ToString::to_string).collect()) - .with_pty(action); + let context = TestContextBuilder::default().use_temp_cwd().build(); + context.new_command().args_vec(deno_args).with_pty(action); } pub struct WrkOutput { From 055f94da1bf0ca89d6a8ee8b729af55d80e3ef0e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 15:43:59 -0400 Subject: [PATCH 25/26] Fix two failing tests. --- cli/tests/integration/run_tests.rs | 47 +++++++++++++++-------------- cli/tests/integration/task_tests.rs | 11 ++++--- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 04385b83cbc956..dc726a287aacfd 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2714,28 +2714,31 @@ itest!(byte_order_mark { #[test] fn issue9750() { - util::with_pty(&["run", "--prompt", "run/issue9750.js"], |mut console| { - console.expect("Enter 'yy':"); - console.write_line_raw("yy"); - console.expect(concat!( - "┌ ⚠️ Deno requests env access.\r\n", - "├ Requested by `Deno.permissions.query()` API.\r\n", - "├ Run again with --allow-env to bypass this prompt.\r\n", - "└ Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all env permissions)", - )); - console.write_line_raw("n"); - console.expect("Denied env access."); - console.expect(concat!( - "┌ ⚠️ Deno requests env access to \"SECRET\".\r\n", - "├ Run again with --allow-env to bypass this prompt.\r\n", - "└ Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all env permissions)", - )); - console.write_line_raw("n"); - console.expect_all(&[ - "Denied env access to \"SECRET\".", - "PermissionDenied: Requires env access to \"SECRET\", run again with the --allow-env flag", - ]); - }); + TestContext::default() + .new_command() + .args_vec(&["run", "--prompt", "run/issue9750.js"]) + .with_pty(|mut console| { + console.expect("Enter 'yy':"); + console.write_line_raw("yy"); + console.expect(concat!( + "┌ ⚠️ Deno requests env access.\r\n", + "├ Requested by `Deno.permissions.query()` API.\r\n", + "├ Run again with --allow-env to bypass this prompt.\r\n", + "└ Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all env permissions)", + )); + console.write_line_raw("n"); + console.expect("Denied env access."); + console.expect(concat!( + "┌ ⚠️ Deno requests env access to \"SECRET\".\r\n", + "├ Run again with --allow-env to bypass this prompt.\r\n", + "└ Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all env permissions)", + )); + console.write_line_raw("n"); + console.expect_all(&[ + "Denied env access to \"SECRET\".", + "PermissionDenied: Requires env access to \"SECRET\", run again with the --allow-env flag", + ]); + }); } // Regression test for https://github.com/denoland/deno/issues/11451. diff --git a/cli/tests/integration/task_tests.rs b/cli/tests/integration/task_tests.rs index f090deff5d1f51..45b091b08305cc 100644 --- a/cli/tests/integration/task_tests.rs +++ b/cli/tests/integration/task_tests.rs @@ -4,6 +4,7 @@ // These tests are intended to only test integration. use test_util::env_vars_for_npm_tests; +use test_util::TestContext; itest!(task_no_args { args: "task -q --config task/deno_json/deno.json", @@ -53,12 +54,12 @@ itest!(task_non_existent { #[test] fn task_emoji() { // this bug only appears when using a pty/tty - test_util::with_pty( - &["task", "--config", "task/deno_json/deno.json", "echo_emoji"], - |mut console| { + TestContext::default() + .new_command() + .args_vec(["task", "--config", "task/deno_json/deno.json", "echo_emoji"]) + .with_pty(|mut console| { console.expect("Task echo_emoji echo 🔥\r\n🔥"); - }, - ); + }); } itest!(task_boolean_logic { From 6cc5a9793ac3bd486e9d8777df69ef298e5fd161 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 30 Mar 2023 15:54:24 -0400 Subject: [PATCH 26/26] Update lsp tests to use a temp cwd --- cli/tests/integration/lsp_tests.rs | 268 +++++++++++++++++++---------- cli/tests/integration/run_tests.rs | 2 +- 2 files changed, 182 insertions(+), 88 deletions(-) diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index ba6c8f67322d5e..15f0ea4900f5f2 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -11,21 +11,21 @@ use std::fs; use std::process::Stdio; use test_util::deno_cmd_with_deno_dir; use test_util::env_vars_for_npm_tests; -use test_util::lsp::LspClientBuilder; use test_util::testdata_path; use test_util::TestContextBuilder; use tower_lsp::lsp_types as lsp; #[test] fn lsp_startup_shutdown() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.shutdown(); } #[test] fn lsp_init_tsconfig() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -58,7 +58,7 @@ fn lsp_init_tsconfig() { #[test] fn lsp_tsconfig_types() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -98,7 +98,8 @@ fn lsp_tsconfig_types() { #[test] fn lsp_tsconfig_bad_config_path() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder .set_config("bad_tsconfig.json") @@ -123,7 +124,7 @@ fn lsp_tsconfig_bad_config_path() { #[test] fn lsp_triple_slash_types() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); let a_dts = "// deno-lint-ignore-file no-var\ndeclare var a: string;"; temp_dir.write("a.d.ts", a_dts); @@ -146,7 +147,7 @@ fn lsp_triple_slash_types() { #[test] fn lsp_import_map() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); let import_map = r#"{ "imports": { @@ -205,7 +206,7 @@ fn lsp_import_map() { #[test] fn lsp_import_map_data_url() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.set_import_map("data:application/json;utf8,{\"imports\": { \"example\": \"https://deno.land/x/example/mod.ts\" }}"); @@ -230,7 +231,7 @@ fn lsp_import_map_data_url() { #[test] fn lsp_import_map_config_file() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( "deno.import_map.jsonc", @@ -297,7 +298,7 @@ fn lsp_import_map_config_file() { #[test] fn lsp_import_map_embedded_in_config_file() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( "deno.embedded_import_map.jsonc", @@ -358,7 +359,7 @@ fn lsp_import_map_embedded_in_config_file() { #[test] fn lsp_deno_task() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( "deno.jsonc", @@ -393,7 +394,7 @@ fn lsp_deno_task() { #[test] fn lsp_import_assertions() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.set_import_map("data:application/json;utf8,{\"imports\": { \"example\": \"https://deno.land/x/example/mod.ts\" }}"); @@ -509,7 +510,7 @@ fn lsp_import_assertions() { #[test] fn lsp_import_map_import_completions() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( "import-map.json", @@ -649,7 +650,8 @@ fn lsp_import_map_import_completions() { #[test] fn lsp_hover() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -690,7 +692,8 @@ fn lsp_hover() { #[test] fn lsp_hover_asset() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -747,7 +750,7 @@ fn lsp_hover_asset() { #[test] fn lsp_hover_disabled() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.set_deno_enable(false); @@ -779,7 +782,7 @@ fn lsp_hover_disabled() { #[test] fn lsp_inlay_hints() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.enable_inlay_hints(); @@ -882,7 +885,8 @@ fn lsp_inlay_hints() { #[test] fn lsp_inlay_hints_not_enabled() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -1135,7 +1139,8 @@ fn lsp_workspace_enable_paths() { #[test] fn lsp_hover_unstable_disabled() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -1174,7 +1179,8 @@ fn lsp_hover_unstable_disabled() { #[test] fn lsp_hover_unstable_enabled() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.set_unstable(true); }); @@ -1217,7 +1223,8 @@ fn lsp_hover_unstable_enabled() { #[test] fn lsp_hover_change_mbc() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -1282,7 +1289,7 @@ fn lsp_hover_change_mbc() { #[test] fn lsp_hover_closed_document() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write("a.ts", r#"export const a = "a";"#); temp_dir.write("b.ts", r#"export * from "./a.ts";"#); @@ -1374,7 +1381,10 @@ fn lsp_hover_closed_document() { #[test] fn lsp_hover_dependency() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ @@ -1520,7 +1530,8 @@ fn lsp_hover_dependency() { // unable to resolve dependencies when there was an invalid syntax in the module #[test] fn lsp_hover_deps_preserved_when_invalid_parse() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -1611,7 +1622,10 @@ fn lsp_hover_deps_preserved_when_invalid_parse() { #[test] fn lsp_hover_typescript_types() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( @@ -1664,7 +1678,8 @@ fn lsp_hover_typescript_types() { #[test] fn lsp_hover_jsdoc_symbol_link() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -1714,7 +1729,8 @@ fn lsp_hover_jsdoc_symbol_link() { #[test] fn lsp_goto_type_definition() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -1756,7 +1772,8 @@ fn lsp_goto_type_definition() { #[test] fn lsp_call_hierarchy() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -1885,7 +1902,8 @@ fn lsp_call_hierarchy() { #[test] fn lsp_large_doc_changes() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); let large_file_text = fs::read_to_string(testdata_path().join("lsp").join("large_file.txt")) @@ -1986,7 +2004,8 @@ fn lsp_large_doc_changes() { #[test] fn lsp_document_symbol() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -2185,7 +2204,8 @@ fn lsp_document_symbol() { #[test] fn lsp_folding_range() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -2231,7 +2251,8 @@ fn lsp_folding_range() { #[test] fn lsp_rename() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -2283,7 +2304,8 @@ fn lsp_rename() { #[test] fn lsp_selection_range() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -2360,7 +2382,8 @@ fn lsp_selection_range() { #[test] fn lsp_semantic_tokens() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -2419,7 +2442,8 @@ fn lsp_semantic_tokens() { #[test] fn lsp_code_lens() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -2593,7 +2617,8 @@ fn lsp_code_lens() { #[test] fn lsp_code_lens_impl() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -2739,7 +2764,8 @@ fn lsp_code_lens_impl() { #[test] fn lsp_code_lens_test() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.disable_testing_api().set_code_lens(None); }); @@ -2994,7 +3020,8 @@ fn lsp_code_lens_test() { #[test] fn lsp_code_lens_test_disabled() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.disable_testing_api().set_code_lens(Some(json!({ "implementations": true, @@ -3034,7 +3061,8 @@ fn lsp_code_lens_test_disabled() { #[test] fn lsp_code_lens_non_doc_nav_tree() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -3091,7 +3119,8 @@ fn lsp_code_lens_non_doc_nav_tree() { #[test] fn lsp_nav_tree_updates() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -3231,7 +3260,8 @@ fn lsp_nav_tree_updates() { #[test] fn lsp_find_references() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -3339,7 +3369,8 @@ fn lsp_find_references() { #[test] fn lsp_signature_help() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -3459,7 +3490,8 @@ fn lsp_signature_help() { #[test] fn lsp_code_actions() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -3646,7 +3678,8 @@ fn lsp_code_actions() { #[test] fn lsp_code_actions_deno_cache() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); let diagnostics = client.did_open(json!({ "textDocument": { @@ -3735,7 +3768,8 @@ fn lsp_code_actions_deno_cache() { #[test] fn lsp_code_actions_deno_cache_npm() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); let diagnostics = client.did_open(json!({ "textDocument": { @@ -3819,7 +3853,8 @@ fn lsp_code_actions_deno_cache_npm() { #[test] fn lsp_code_actions_imports() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -4059,7 +4094,8 @@ export class DuckConfig { #[test] fn lsp_code_actions_refactor() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -4268,7 +4304,8 @@ fn lsp_code_actions_refactor() { #[test] fn lsp_code_actions_refactor_no_disabled_support() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.with_capabilities(|c| { let doc = c.text_document.as_mut().unwrap(); @@ -4337,7 +4374,8 @@ fn lsp_code_actions_refactor_no_disabled_support() { #[test] fn lsp_code_actions_deadlock() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); let large_file_text = fs::read_to_string(testdata_path().join("lsp").join("large_file.txt")) @@ -4460,7 +4498,8 @@ fn lsp_code_actions_deadlock() { #[test] fn lsp_completions() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -4518,7 +4557,8 @@ fn lsp_completions() { #[test] fn lsp_completions_private_fields() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -4542,7 +4582,8 @@ fn lsp_completions_private_fields() { #[test] fn lsp_completions_optional() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -4624,7 +4665,8 @@ fn lsp_completions_optional() { #[test] fn lsp_completions_auto_import() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -4708,7 +4750,8 @@ fn lsp_completions_auto_import() { #[test] fn lsp_completions_snippet() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( json!({ @@ -4802,7 +4845,8 @@ fn lsp_completions_snippet() { #[test] fn lsp_completions_no_snippet() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.with_capabilities(|c| { let doc = c.text_document.as_mut().unwrap(); @@ -4856,7 +4900,10 @@ fn lsp_completions_no_snippet() { #[test] fn lsp_completions_npm() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( @@ -4992,7 +5039,10 @@ fn lsp_completions_npm() { #[test] fn lsp_npm_specifier_unopened_file() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); @@ -5071,7 +5121,10 @@ fn lsp_npm_specifier_unopened_file() { #[test] fn lsp_completions_node_specifier() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); let diagnostics = client.did_open(json!({ @@ -5292,7 +5345,10 @@ fn lsp_completions_node_specifier() { #[test] fn lsp_completions_registry() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.add_test_server_suggestions(); @@ -5355,7 +5411,10 @@ fn lsp_completions_registry() { #[test] fn lsp_completions_registry_empty() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder.add_test_server_suggestions(); @@ -5415,7 +5474,10 @@ fn lsp_completions_registry_empty() { #[test] fn lsp_auto_discover_registry() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ @@ -5448,7 +5510,10 @@ fn lsp_auto_discover_registry() { #[test] fn lsp_cache_location() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let temp_dir = context.temp_dir(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { @@ -5537,7 +5602,10 @@ fn lsp_cache_location() { /// and cache files. #[test] fn lsp_tls_cert() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize(|builder| { builder @@ -5624,7 +5692,10 @@ fn lsp_tls_cert() { #[test] fn lsp_diagnostics_warn_redirect() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( @@ -5700,7 +5771,10 @@ fn lsp_diagnostics_warn_redirect() { #[test] fn lsp_redirect_quick_fix() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open( @@ -5784,7 +5858,8 @@ fn lsp_redirect_quick_fix() { #[test] fn lsp_diagnostics_deprecated() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); let diagnostics = client.did_open(json!({ "textDocument": { @@ -5830,7 +5905,8 @@ fn lsp_diagnostics_deprecated() { #[test] fn lsp_diagnostics_deno_types() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); let diagnostics = client .did_open(json!({ @@ -5857,7 +5933,8 @@ fn lsp_diagnostics_deno_types() { #[test] fn lsp_diagnostics_refresh_dependents() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -5945,7 +6022,8 @@ struct PerformanceAverages { #[test] fn lsp_performance() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -5997,7 +6075,8 @@ fn lsp_performance() { #[test] fn lsp_format_no_changes() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6026,7 +6105,8 @@ fn lsp_format_no_changes() { #[test] fn lsp_format_error() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6054,7 +6134,8 @@ fn lsp_format_error() { #[test] fn lsp_format_mbc() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6109,7 +6190,7 @@ fn lsp_format_mbc() { #[test] fn lsp_format_exclude_with_config() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -6162,7 +6243,7 @@ fn lsp_format_exclude_with_config() { #[test] fn lsp_format_exclude_default_config() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -6215,7 +6296,8 @@ fn lsp_format_exclude_default_config() { #[test] fn lsp_format_json() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6270,7 +6352,8 @@ fn lsp_format_json() { #[test] fn lsp_json_no_diagnostics() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6307,7 +6390,8 @@ fn lsp_json_no_diagnostics() { #[test] fn lsp_format_markdown() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6354,7 +6438,7 @@ fn lsp_format_markdown() { #[test] fn lsp_format_with_config() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( "deno.fmt.jsonc", @@ -6461,7 +6545,8 @@ fn lsp_format_with_config() { #[test] fn lsp_markdown_no_diagnostics() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6498,7 +6583,10 @@ fn lsp_markdown_no_diagnostics() { #[test] fn lsp_configuration_did_change() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ @@ -6591,7 +6679,8 @@ fn lsp_configuration_did_change() { #[test] fn lsp_workspace_symbol() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6685,7 +6774,8 @@ fn lsp_workspace_symbol() { #[test] fn lsp_code_actions_ignore_lint() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6808,7 +6898,8 @@ fn lsp_code_actions_ignore_lint() { /// This test exercises updating an existing deno-lint-ignore-file comment. #[test] fn lsp_code_actions_update_ignore_lint() { - let mut client = LspClientBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ "textDocument": { @@ -6933,7 +7024,7 @@ console.log(snake_case); #[test] fn lsp_lint_with_config() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -6974,7 +7065,7 @@ fn lsp_lint_with_config() { #[test] fn lsp_lint_exclude_with_config() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); temp_dir.write( @@ -7015,7 +7106,10 @@ fn lsp_lint_exclude_with_config() { #[test] fn lsp_jsx_import_source_pragma() { - let context = TestContextBuilder::new().use_http_server().build(); + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); client.did_open(json!({ @@ -7112,7 +7206,7 @@ struct TestRunResponseParams { #[test] fn lsp_testing_api() { - let context = TestContextBuilder::new().build(); + let context = TestContextBuilder::new().use_temp_cwd().build(); let temp_dir = context.temp_dir(); let contents = r#" diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index dc726a287aacfd..b661e135ae559f 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2716,7 +2716,7 @@ itest!(byte_order_mark { fn issue9750() { TestContext::default() .new_command() - .args_vec(&["run", "--prompt", "run/issue9750.js"]) + .args_vec(["run", "--prompt", "run/issue9750.js"]) .with_pty(|mut console| { console.expect("Enter 'yy':"); console.write_line_raw("yy");