From 3b9eeba02c65326023df077a6997fd13b22d1f3a Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 11 Jun 2024 21:06:43 +0100 Subject: [PATCH] fix(lsp): don't sort workspace files (#24180) --- cli/lsp/config.rs | 4 ++-- cli/lsp/documents.rs | 5 +++-- cli/lsp/language_server.rs | 28 +++++++++++++++++----------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 75e0cd6bc3..75433ad8a6 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -35,10 +35,10 @@ use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::package::PackageNv; use deno_semver::Version; use import_map::ImportMap; +use indexmap::IndexSet; use lsp::Url; use lsp_types::ClientCapabilities; use std::collections::BTreeMap; -use std::collections::BTreeSet; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; @@ -1675,7 +1675,7 @@ impl ConfigTree { pub async fn refresh( &mut self, settings: &Settings, - workspace_files: &BTreeSet, + workspace_files: &IndexSet, file_fetcher: &Arc, ) { lsp_log!("Refreshing configuration tree..."); diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index a0e99d9bef..6d7c2ca7ef 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -33,6 +33,7 @@ use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use indexmap::IndexMap; +use indexmap::IndexSet; use std::borrow::Cow; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -1261,7 +1262,7 @@ impl Documents { config: &Config, resolver: &Arc, cache: &LspCache, - workspace_files: &BTreeSet, + workspace_files: &IndexSet, ) { self.config = Arc::new(config.clone()); self.cache = Arc::new(cache.clone()); @@ -1691,7 +1692,7 @@ console.log(b, "hello deno"); [&file1_specifier, &file2_specifier, &file3_specifier] .into_iter() .cloned() - .collect::>(); + .collect::>(); // set the initial import map and point to file 2 { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 657913cfb0..d70b418c02 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -20,6 +20,7 @@ use indexmap::IndexSet; use log::error; use serde::Deserialize; use serde_json::from_value; +use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::HashSet; @@ -210,7 +211,7 @@ pub struct Inner { pub ts_server: Arc, /// A map of specifiers and URLs used to translate over the LSP. pub url_map: urls::LspUrlMap, - workspace_files: BTreeSet, + workspace_files: IndexSet, /// Set to `self.config.settings.enable_settings_hash()` after /// refreshing `self.workspace_files`. workspace_files_hash: u64, @@ -801,12 +802,12 @@ impl Inner { }) } - fn walk_workspace(config: &Config) -> (BTreeSet, bool) { + fn walk_workspace(config: &Config) -> (IndexSet, bool) { if !config.workspace_capable() { log::debug!("Skipped workspace walk due to client incapability."); return (Default::default(), false); } - let mut workspace_files = BTreeSet::default(); + let mut workspace_files = IndexSet::default(); let entry_limit = 1000; let mut pending = VecDeque::new(); let mut entry_count = 0; @@ -844,6 +845,9 @@ impl Inner { } } while let Some((parent_path, read_dir)) = pending.pop_front() { + // Sort entries from each dir for consistency across operating systems. + let mut dir_files = BTreeSet::new(); + let mut dir_subdirs = BTreeMap::new(); for entry in read_dir { let Ok(entry) = entry else { continue; @@ -883,7 +887,7 @@ impl Inner { continue; } if let Ok(read_dir) = std::fs::read_dir(&path) { - pending.push_back((path, read_dir)); + dir_subdirs.insert(specifier, (path, read_dir)); } } else if file_type.is_file() || file_type.is_symlink() @@ -918,9 +922,11 @@ impl Inner { } } } - workspace_files.insert(specifier); + dir_files.insert(specifier); } } + workspace_files.extend(dir_files); + pending.extend(dir_subdirs.into_values()); } (workspace_files, false) } @@ -3709,7 +3715,7 @@ mod tests { 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/folder"); temp_dir.create_dir_all("root1/target"); temp_dir.create_dir_all("root1/node_modules"); temp_dir.create_dir_all("root1/.git"); @@ -3727,8 +3733,8 @@ mod tests { temp_dir.write("root1/other.txt", ""); // no, text file temp_dir.write("root1/other.wasm", ""); // no, don't load wasm temp_dir.write("root1/Cargo.toml", ""); // no - 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/folder/mod.ts", ""); // yes + temp_dir.write("root1/folder/data.min.ts", ""); // no, minified file temp_dir.write("root1/.git/main.ts", ""); // no, .git folder temp_dir.write("root1/node_modules/main.ts", ""); // no, because it's in a node_modules folder temp_dir.write("root1/target/main.ts", ""); // no, because there is a Cargo.toml in the root directory @@ -3814,6 +3820,7 @@ mod tests { assert_eq!( json!(workspace_files), json!([ + temp_dir.uri().join("root4_parent/deno.json").unwrap(), temp_dir.uri().join("root1/mod0.ts").unwrap(), temp_dir.uri().join("root1/mod1.js").unwrap(), temp_dir.uri().join("root1/mod2.tsx").unwrap(), @@ -3824,12 +3831,11 @@ mod tests { temp_dir.uri().join("root1/mod7.d.mts").unwrap(), temp_dir.uri().join("root1/mod8.json").unwrap(), temp_dir.uri().join("root1/mod9.jsonc").unwrap(), - temp_dir.uri().join("root1/sub_dir/mod.ts").unwrap(), temp_dir.uri().join("root2/file1.ts").unwrap(), + temp_dir.uri().join("root4_parent/root4/main.ts").unwrap(), + temp_dir.uri().join("root1/folder/mod.ts").unwrap(), temp_dir.uri().join("root2/folder/main.ts").unwrap(), temp_dir.uri().join("root2/root2.1/main.ts").unwrap(), - temp_dir.uri().join("root4_parent/deno.json").unwrap(), - temp_dir.uri().join("root4_parent/root4/main.ts").unwrap(), ]) ); }