refactor(lsp): cleanup document preload (#20520)

cli/lsp/config.rs

@@ -4,7 +4,6 @@ use super::logging::lsp_log;
 use crate::args::ConfigFile;
 use crate::lsp::logging::lsp_warn;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
-use crate::util::path::specifier_from_file_path;
 use crate::util::path::specifier_to_file_path;
 use deno_core::error::AnyError;
 use deno_core::parking_lot::Mutex;
@@ -645,77 +644,44 @@ impl Config {
     true
   }
 
-  /// Gets the directories or specifically enabled file paths based on the
+  pub fn get_enabled_paths(&self) -> Vec<PathBuf> {
-  /// workspace config.
+    let mut paths = vec![];
-  ///
-  /// WARNING: This may incorrectly have some directory urls as being
-  /// represented as file urls.
-  pub fn enabled_urls(&self) -> Vec<Url> {
-    let mut urls = vec![];
     for (workspace_uri, _) in &self.workspace_folders {
       let Ok(workspace_path) = specifier_to_file_path(workspace_uri) else {
         lsp_log!("Unable to convert uri \"{}\" to path.", workspace_uri);
         continue;
       };
       let specifier_settings = self.settings.specifiers.get(workspace_uri);
-      let enable = specifier_settings
-        .and_then(|s| s.enable)
-        .or(self.settings.workspace.enable)
-        .unwrap_or(self.has_config_file());
-      let disable_paths = specifier_settings
-        .map(|s| &s.disable_paths)
-        .unwrap_or(&self.settings.workspace.disable_paths);
-      let resolved_disable_paths = disable_paths
-        .iter()
-        .map(|p| workspace_path.join(p))
-        .collect::<Vec<_>>();
       let enable_paths = specifier_settings
         .and_then(|s| s.enable_paths.as_ref())
         .or(self.settings.workspace.enable_paths.as_ref());
       if let Some(enable_paths) = enable_paths {
         for path in enable_paths {
-          let path = workspace_path.join(path);
+          paths.push(workspace_path.join(path));
-          let Ok(path_uri) = specifier_from_file_path(&path) else {
-            lsp_log!("Unable to convert path \"{}\" to uri.", path.display());
-            continue;
-          };
-          if !resolved_disable_paths.iter().any(|p| path.starts_with(p)) {
-            urls.push(path_uri);
-          }
         }
-      } else if enable
+      } else {
-        && !resolved_disable_paths
+        paths.push(workspace_path);
-          .iter()
-          .any(|p| workspace_path.starts_with(p))
-      {
-        urls.push(workspace_uri.clone());
       }
     }
-
+    paths.sort();
-    // sort for determinism
+    paths.dedup();
-    urls.sort();
+    paths
-    urls.dedup();
-    urls
   }
 
-  pub fn disabled_urls(&self) -> Vec<Url> {
+  pub fn get_disabled_paths(&self) -> Vec<PathBuf> {
+    let mut paths = vec![];
+    if let Some(cf) = self.maybe_config_file() {
+      if let Some(files) = cf.to_files_config().ok().flatten() {
+        for path in files.exclude {
+          paths.push(path);
+        }
+      }
+    }
     let root_enable = self
       .settings
       .workspace
       .enable
       .unwrap_or(self.has_config_file());
-    let mut urls = vec![];
-    if let Some(cf) = self.maybe_config_file() {
-      if let Some(files) = cf.to_files_config().ok().flatten() {
-        for path in files.exclude {
-          let Ok(path_uri) = specifier_from_file_path(&path) else {
-            lsp_log!("Unable to convert path \"{}\" to uri.", path.display());
-            continue;
-          };
-          urls.push(path_uri);
-        }
-      }
-    }
     for (workspace_uri, _) in &self.workspace_folders {
       let Ok(workspace_path) = specifier_to_file_path(workspace_uri) else {
         lsp_log!("Unable to convert uri \"{}\" to path.", workspace_uri);
@@ -730,20 +696,15 @@ impl Config {
           .map(|s| &s.disable_paths)
           .unwrap_or(&self.settings.workspace.disable_paths);
         for path in disable_paths {
-          let path = workspace_path.join(path);
+          paths.push(workspace_path.join(path));
-          let Ok(path_uri) = specifier_from_file_path(&path) else {
-            lsp_log!("Unable to convert path \"{}\" to uri.", path.display());
-            continue;
-          };
-          urls.push(path_uri);
         }
       } else {
-        urls.push(workspace_uri.clone());
+        paths.push(workspace_path);
       }
     }
-    urls.sort();
+    paths.sort();
-    urls.dedup();
+    paths.dedup();
-    urls
+    paths
   }
 
   pub fn specifier_code_lens_test(&self, specifier: &ModuleSpecifier) -> bool {
@@ -1129,45 +1090,33 @@ mod tests {
   }
 
   #[test]
-  fn config_enabled_urls() {
+  fn config_get_enabled_paths() {
-    let root_dir = resolve_url("file:///example/").unwrap();
+    let mut config = Config::new();
-    let mut config = Config::new_with_root(root_dir.clone());
-    config.settings.workspace.enable = Some(false);
-    config.settings.workspace.enable_paths = None;
-    assert_eq!(config.enabled_urls(), vec![]);
-
-    config.settings.workspace.enable = Some(true);
-    assert_eq!(config.enabled_urls(), vec![root_dir]);
-
-    config.settings.workspace.enable = Some(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.workspace_folders = vec![
       (
-        root_dir1.clone(),
+        Url::parse("file:///root1/").unwrap(),
         lsp::WorkspaceFolder {
-          uri: root_dir1.clone(),
+          uri: Url::parse("file:///root1/").unwrap(),
           name: "1".to_string(),
         },
       ),
       (
-        root_dir2.clone(),
+        Url::parse("file:///root2/").unwrap(),
         lsp::WorkspaceFolder {
-          uri: root_dir2.clone(),
+          uri: Url::parse("file:///root2/").unwrap(),
           name: "2".to_string(),
         },
       ),
       (
-        root_dir3.clone(),
+        Url::parse("file:///root3/").unwrap(),
         lsp::WorkspaceFolder {
-          uri: root_dir3.clone(),
+          uri: Url::parse("file:///root3/").unwrap(),
           name: "3".to_string(),
         },
       ),
     ];
     config.set_specifier_settings(
-      root_dir1.clone(),
+      Url::parse("file:///root1/").unwrap(),
       SpecifierSettings {
         enable_paths: Some(vec![
           "sub_dir".to_string(),
@@ -1178,14 +1127,14 @@ mod tests {
       },
     );
     config.set_specifier_settings(
-      root_dir2.clone(),
+      Url::parse("file:///root2/").unwrap(),
       SpecifierSettings {
         enable_paths: Some(vec!["other.ts".to_string()]),
         ..Default::default()
       },
     );
     config.set_specifier_settings(
-      root_dir3.clone(),
+      Url::parse("file:///root3/").unwrap(),
       SpecifierSettings {
         enable: Some(true),
         ..Default::default()
@@ -1193,13 +1142,13 @@ mod tests {
     );
 
     assert_eq!(
-      config.enabled_urls(),
+      config.get_enabled_paths(),
       vec![
-        root_dir1.join("sub_dir").unwrap(),
+        PathBuf::from("/root1/sub_dir"),
-        root_dir1.join("sub_dir/other").unwrap(),
+        PathBuf::from("/root1/sub_dir/other"),
-        root_dir1.join("test.ts").unwrap(),
+        PathBuf::from("/root1/test.ts"),
-        root_dir2.join("other.ts").unwrap(),
+        PathBuf::from("/root2/other.ts"),
-        root_dir3
+        PathBuf::from("/root3/"),
       ]
     );
   }
@@ -1209,12 +1158,12 @@ mod tests {
     let root_uri = resolve_url("file:///root/").unwrap();
     let mut config = Config::new_with_root(root_uri.clone());
     config.settings.workspace.enable = None;
-    assert_eq!(config.enabled_urls(), vec![]);
+    assert!(!config.specifier_enabled(&root_uri));
 
     config.set_config_file(
       ConfigFile::new("{}", root_uri.join("deno.json").unwrap()).unwrap(),
     );
-    assert_eq!(config.enabled_urls(), vec![root_uri]);
+    assert!(config.specifier_enabled(&root_uri));
   }
 
   // Regression test for https://github.com/denoland/vscode_deno/issues/917.

cli/lsp/documents.rs

@@ -803,8 +803,8 @@ impl FileSystemDocuments {
 }
 
 pub struct UpdateDocumentConfigOptions<'a> {
-  pub enabled_urls: Vec<Url>,
+  pub enabled_paths: Vec<PathBuf>,
-  pub disabled_urls: Vec<Url>,
+  pub disabled_paths: Vec<PathBuf>,
   pub document_preload_limit: usize,
   pub maybe_import_map: Option<Arc<import_map::ImportMap>>,
   pub maybe_config_file: Option<&'a ConfigFile>,
@@ -1183,7 +1183,7 @@ impl Documents {
 
   pub fn update_config(&mut self, options: UpdateDocumentConfigOptions) {
     fn calculate_resolver_config_hash(
-      enabled_urls: &[Url],
+      enabled_paths: &[PathBuf],
       document_preload_limit: usize,
       maybe_import_map: Option<&import_map::ImportMap>,
       maybe_jsx_config: Option<&JsxImportSourceConfig>,
@@ -1194,9 +1194,9 @@ impl Documents {
       hasher.write_hashable(document_preload_limit);
       hasher.write_hashable(&{
         // ensure these are sorted so the hashing is deterministic
-        let mut enabled_urls = enabled_urls.to_vec();
+        let mut enabled_paths = enabled_paths.to_vec();
-        enabled_urls.sort_unstable();
+        enabled_paths.sort_unstable();
-        enabled_urls
+        enabled_paths
       });
       if let Some(import_map) = maybe_import_map {
         hasher.write_str(&import_map.to_json());
@@ -1233,7 +1233,7 @@ impl Documents {
       .maybe_config_file
       .and_then(|cf| cf.to_maybe_jsx_import_source_config().ok().flatten());
     let new_resolver_config_hash = calculate_resolver_config_hash(
-      &options.enabled_urls,
+      &options.enabled_paths,
       options.document_preload_limit,
       options.maybe_import_map.as_deref(),
       maybe_jsx_config.as_ref(),
@@ -1278,16 +1278,8 @@ impl Documents {
     // only refresh the dependencies if the underlying configuration has changed
     if self.resolver_config_hash != new_resolver_config_hash {
       self.refresh_dependencies(
-        options
+        options.enabled_paths,
-          .enabled_urls
+        options.disabled_paths,
-          .iter()
-          .filter_map(|url| specifier_to_file_path(url).ok())
-          .collect(),
-        options
-          .disabled_urls
-          .iter()
-          .filter_map(|url| specifier_to_file_path(url).ok())
-          .collect(),
         options.document_preload_limit,
       );
       self.resolver_config_hash = new_resolver_config_hash;
@@ -1710,14 +1702,18 @@ impl PreloadDocumentFinder {
     // initialize the finder with the initial paths
     let mut dirs = Vec::with_capacity(options.enabled_paths.len());
     for path in options.enabled_paths {
-      if path.is_dir() {
+      if !finder.disabled_paths.contains(&path)
-        if is_allowed_root_dir(&path) {
+        && !finder.disabled_globs.matches_path(&path)
-          dirs.push(path);
+      {
+        if path.is_dir() {
+          if is_allowed_root_dir(&path) {
+            dirs.push(path);
+          }
+        } else {
+          finder
+            .pending_entries
+            .push_back(PendingEntry::SpecifiedRootFile(path));
         }
-      } else {
-        finder
-          .pending_entries
-          .push_back(PendingEntry::SpecifiedRootFile(path));
       }
     }
     for dir in sort_and_remove_non_leaf_dirs(dirs) {
@@ -2028,8 +2024,8 @@ console.log(b, "hello deno");
         .unwrap();
 
       documents.update_config(UpdateDocumentConfigOptions {
-        enabled_urls: vec![],
+        enabled_paths: vec![],
-        disabled_urls: vec![],
+        disabled_paths: vec![],
         document_preload_limit: 1_000,
         maybe_import_map: Some(Arc::new(import_map)),
         maybe_config_file: None,
@@ -2070,8 +2066,8 @@ console.log(b, "hello deno");
         .unwrap();
 
       documents.update_config(UpdateDocumentConfigOptions {
-        enabled_urls: vec![],
+        enabled_paths: vec![],
-        disabled_urls: vec![],
+        disabled_paths: vec![],
         document_preload_limit: 1_000,
         maybe_import_map: Some(Arc::new(import_map)),
         maybe_config_file: None,

cli/lsp/language_server.rs

@@ -1353,8 +1353,8 @@ impl Inner {
 
   async fn refresh_documents_config(&mut self) {
     self.documents.update_config(UpdateDocumentConfigOptions {
-      enabled_urls: self.config.enabled_urls(),
+      enabled_paths: self.config.get_enabled_paths(),
-      disabled_urls: self.config.disabled_urls(),
+      disabled_paths: self.config.get_disabled_paths(),
       document_preload_limit: self
         .config
         .workspace_settings()

cli/util/path.rs

@@ -122,28 +122,6 @@ pub fn specifier_to_file_path(
   }
 }
 
-/// Attempts to convert a file path to a specifier. By default, uses the Url
-/// crate's `from_file_path()` method, but falls back to try and resolve
-/// unix-style paths on Windows.
-pub fn specifier_from_file_path(
-  path: &Path,
-) -> Result<ModuleSpecifier, AnyError> {
-  if cfg!(windows) {
-    match ModuleSpecifier::from_file_path(path) {
-      Ok(url) => Ok(url),
-      Err(()) => {
-        let mut url = ModuleSpecifier::parse("file:///").unwrap();
-        url.set_path(&path.to_string_lossy());
-        Ok(url)
-      }
-    }
-  } else {
-    ModuleSpecifier::from_file_path(path).map_err(|()| {
-      uri_error(format!("Invalid file path.\n  Path: {}", path.display()))
-    })
-  }
-}
-
 /// `from.make_relative(to)` but with fixes.
 pub fn relative_specifier(
   from: &ModuleSpecifier,