Repository: denoland/deno
Pull request: fix(lsp): don't sort workspace files

This stops nested `deno.json`s from being added to the tree before their parents. This would break workspaces for subdirs with names preceding `deno.json`, but it's not observable yet.

Changed files:
filename: cli/lsp/config.rs
patch: @@ -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<ModuleSpecifier>,
+    workspace_files: &IndexSet<ModuleSpecifier>,
     file_fetcher: &Arc<FileFetcher>,
   ) {
     lsp_log!("Refreshing configuration tree...");

filename: cli/lsp/documents.rs
patch: @@ -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<LspResolver>,
     cache: &LspCache,
-    workspace_files: &BTreeSet<ModuleSpecifier>,
+    workspace_files: &IndexSet<ModuleSpecifier>,
   ) {
     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::<BTreeSet<_>>();
+        .collect::<IndexSet<_>>();
 
     // set the initial import map and point to file 2
     {

filename: cli/lsp/language_server.rs
patch: @@ -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<TsServer>,
   /// A map of specifiers and URLs used to translate over the LSP.
   pub url_map: urls::LspUrlMap,
-  workspace_files: BTreeSet<ModuleSpecifier>,
+  workspace_files: IndexSet<ModuleSpecifier>,
   /// 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<ModuleSpecifier>, bool) {
+  fn walk_workspace(config: &Config) -> (IndexSet<ModuleSpecifier>, 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(),
       ])
     );
   }