fix(lsp): avoid calling client while holding lock

cli/lsp/completions.rs

@@ -48,7 +48,7 @@ pub struct CompletionItemData {
 async fn check_auto_config_registry(
  url_str: &str,
  config: &ConfigSnapshot,
- client: Client,
+ client: &Client,
  module_registries: &ModuleRegistry,
 ) {
  // check to see if auto discovery is enabled
@@ -78,14 +78,12 @@ async fn check_auto_config_registry(
  // incompatible.
  // TODO(@kitsonk) clean up protocol when doing v2 of suggestions
  if suggestions {
- client
- .send_registry_state_notification(
- lsp_custom::RegistryStateNotificationParams {
- origin,
- suggestions,
- },
- )
- .await;
+ client.send_registry_state_notification(
+ lsp_custom::RegistryStateNotificationParams {
+ origin,
+ suggestions,
+ },
+ );
  }
  }
  }
@@ -139,7 +137,7 @@ pub async fn get_import_completions(
  specifier: &ModuleSpecifier,
  position: &lsp::Position,
  config: &ConfigSnapshot,
- client: Client,
+ client: &Client,
  module_registries: &ModuleRegistry,
  documents: &Documents,
  maybe_import_map: Option<Arc<ImportMap>>,

cli/lsp/config.rs

@@ -1,15 +1,14 @@
 // 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;
 use deno_core::error::AnyError;
 use deno_core::serde::Deserialize;
 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;
@@ -226,7 +225,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.
@@ -388,50 +387,41 @@ impl WorkspaceSettings {
 #[derive(Debug, Clone, Default)]
 pub struct ConfigSnapshot {
  pub client_capabilities: ClientCapabilities,
- pub enabled_paths: HashMap<String, Vec<String>>,
+ pub enabled_paths: HashMap<Url, Vec<Url>>,
  pub settings: Settings,
 }
 
 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()));
  }
  }
  }
- 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<ModuleSpecifier, (ModuleSpecifier, SpecifierSettings)>,
+ pub specifiers: BTreeMap<ModuleSpecifier, SpecifierSettings>,
  pub workspace: WorkspaceSettings,
 }
 
 #[derive(Debug)]
 pub struct Config {
  pub client_capabilities: ClientCapabilities,
- enabled_paths: HashMap<String, Vec<String>>,
+ enabled_paths: HashMap<Url, Vec<Url>>,
  pub root_uri: Option<ModuleSpecifier>,
  settings: Settings,
  pub workspace_folders: Option<Vec<(ModuleSpecifier, lsp::WorkspaceFolder)>>,
@@ -478,20 +468,20 @@ 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()));
  }
  }
  }
  self
  .settings
  .specifiers
  .get(specifier)
- .map(|(_, s)| s.enable)
+ .map(|settings| settings.enable)
  .unwrap_or_else(|| self.settings.workspace.enable)
  }
 
@@ -500,7 +490,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
  }
@@ -554,13 +544,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;
  }
  }
@@ -582,8 +574,6 @@ impl Config {
  workspace: ModuleSpecifier,
  enabled_paths: Vec<String>,
  ) -> 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 +582,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,38 +591,33 @@ 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
  }
 
- pub fn get_specifiers_with_client_uris(&self) -> Vec<SpecifierWithClientUri> {
- self
- .settings
- .specifiers
- .iter()
- .map(|(s, (u, _))| SpecifierWithClientUri {
- specifier: s.clone(),
- client_uri: u.clone(),
- })
- .collect()
+ pub fn get_specifiers(&self) -> Vec<ModuleSpecifier> {
+ 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
  }
 }
 
@@ -678,8 +663,8 @@ mod tests {
  assert!(!config.specifier_enabled(&specifier_b));
  let mut enabled_paths = HashMap::new();
  enabled_paths.insert(
- "file:https:///project/".to_string(),
- vec!["file:https:///project/worker/".to_string()],
+ Url::parse("file:https:///project/").unwrap(),
+ vec![Url::parse("file:https:///project/worker/").unwrap()],
  );
  config.enabled_paths = enabled_paths;
  assert!(config.specifier_enabled(&specifier_a));

cli/lsp/diagnostics.rs

@@ -88,6 +88,7 @@ impl DiagnosticsPublisher {
 
  self
  .client
+ .when_outside_lsp_lock()
  .publish_diagnostics(specifier, version_diagnostics.clone(), version)
  .await;
  }
@@ -1177,14 +1178,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(

cli/lsp/documents.rs

@@ -1184,12 +1184,8 @@ impl Documents {
  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()
  }