Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(lsp): cleanup workspace settings scopes #20937

Merged
merged 4 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 40 additions & 112 deletions cli/lsp/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ use async_trait::async_trait;
use deno_core::anyhow::anyhow;
use deno_core::anyhow::bail;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_core::serde_json::json;
use deno_core::unsync::spawn;
use tower_lsp::lsp_types as lsp;
use tower_lsp::lsp_types::ConfigurationItem;

use crate::lsp::repl::get_repl_workspace_settings;

use super::config::SpecifierSettings;
use super::config::WorkspaceSettings;
use super::config::SETTINGS_SECTION;
use super::lsp_custom;
Expand Down Expand Up @@ -125,46 +124,11 @@ impl OutsideLockClient {
self.0.register_capability(registrations).await
}

pub async fn specifier_configurations(
&self,
specifiers: Vec<LspClientUrl>,
) -> Result<Vec<Result<SpecifierSettings, AnyError>>, AnyError> {
self
.0
.specifier_configurations(
specifiers.into_iter().map(|s| s.into_url()).collect(),
)
.await
}

pub async fn specifier_configuration(
&self,
specifier: &LspClientUrl,
) -> Result<SpecifierSettings, AnyError> {
let values = self
.0
.specifier_configurations(vec![specifier.as_url().clone()])
.await?;
if let Some(value) = values.into_iter().next() {
value.map_err(|err| {
anyhow!(
"Error converting specifier settings ({}): {}",
specifier,
err
)
})
} else {
bail!(
"Expected the client to return a configuration item for specifier: {}",
specifier
);
}
}

pub async fn workspace_configuration(
&self,
) -> Result<WorkspaceSettings, AnyError> {
self.0.workspace_configuration().await
scopes: Vec<Option<lsp::Url>>,
) -> Result<Vec<WorkspaceSettings>, AnyError> {
self.0.workspace_configuration(scopes).await
}

pub async fn publish_diagnostics(
Expand Down Expand Up @@ -201,13 +165,10 @@ trait ClientTrait: Send + Sync {
&self,
params: lsp_custom::DidChangeDenoConfigurationNotificationParams,
);
async fn specifier_configurations(
&self,
uris: Vec<lsp::Url>,
) -> Result<Vec<Result<SpecifierSettings, AnyError>>, AnyError>;
async fn workspace_configuration(
&self,
) -> Result<WorkspaceSettings, AnyError>;
scopes: Vec<Option<lsp::Url>>,
) -> Result<Vec<WorkspaceSettings>, AnyError>;
async fn show_message(&self, message_type: lsp::MessageType, text: String);
async fn register_capability(
&self,
Expand Down Expand Up @@ -288,67 +249,50 @@ impl ClientTrait for TowerClient {
.await
}

async fn specifier_configurations(
async fn workspace_configuration(
&self,
uris: Vec<lsp::Url>,
) -> Result<Vec<Result<SpecifierSettings, AnyError>>, AnyError> {
scopes: Vec<Option<lsp::Url>>,
) -> Result<Vec<WorkspaceSettings>, AnyError> {
let config_response = self
.0
.configuration(
uris
.into_iter()
.map(|uri| ConfigurationItem {
scope_uri: Some(uri),
section: Some(SETTINGS_SECTION.to_string()),
scopes
.iter()
.flat_map(|scope_uri| {
vec![
ConfigurationItem {
scope_uri: scope_uri.clone(),
section: Some(SETTINGS_SECTION.to_string()),
},
ConfigurationItem {
scope_uri: scope_uri.clone(),
section: Some("javascript".to_string()),
},
ConfigurationItem {
scope_uri: scope_uri.clone(),
section: Some("typescript".to_string()),
},
]
})
.collect(),
)
.await?;

Ok(
config_response
.into_iter()
.map(|value| {
serde_json::from_value::<SpecifierSettings>(value).map_err(|err| {
anyhow!("Error converting specifier settings: {}", err)
})
})
.collect(),
)
}

async fn workspace_configuration(
&self,
) -> Result<WorkspaceSettings, AnyError> {
let config_response = self
.0
.configuration(vec![
ConfigurationItem {
scope_uri: None,
section: Some(SETTINGS_SECTION.to_string()),
},
ConfigurationItem {
scope_uri: None,
section: Some("javascript".to_string()),
},
ConfigurationItem {
scope_uri: None,
section: Some("typescript".to_string()),
},
])
.await;
match config_response {
Ok(configs) => {
let mut configs = configs.into_iter();
let deno = serde_json::to_value(configs.next()).unwrap();
let javascript = serde_json::to_value(configs.next()).unwrap();
let typescript = serde_json::to_value(configs.next()).unwrap();
Ok(WorkspaceSettings::from_raw_settings(
deno, javascript, typescript,
))
let mut result = Vec::with_capacity(scopes.len());
for _ in 0..scopes.len() {
let deno = json!(configs.next());
let javascript = json!(configs.next());
let typescript = json!(configs.next());
result.push(WorkspaceSettings::from_raw_settings(
deno, javascript, typescript,
));
}
Ok(result)
}
Err(err) => {
bail!("Error getting workspace configuration: {}", err)
bail!("Error getting workspace configurations: {}", err)
}
}
}
Expand Down Expand Up @@ -406,27 +350,11 @@ impl ClientTrait for ReplClient {
) {
}

async fn specifier_configurations(
&self,
uris: Vec<lsp::Url>,
) -> Result<Vec<Result<SpecifierSettings, AnyError>>, AnyError> {
// all specifiers are enabled for the REPL
let settings = uris
.into_iter()
.map(|_| {
Ok(SpecifierSettings {
enable: Some(true),
..Default::default()
})
})
.collect();
Ok(settings)
}

async fn workspace_configuration(
&self,
) -> Result<WorkspaceSettings, AnyError> {
Ok(get_repl_workspace_settings())
scopes: Vec<Option<lsp::Url>>,
) -> Result<Vec<WorkspaceSettings>, AnyError> {
Ok(vec![get_repl_workspace_settings(); scopes.len()])
}

async fn show_message(
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/code_lens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ fn collect_test(
config: &Config,
) -> Result<Vec<lsp::CodeLens>, AnyError> {
if config.specifier_enabled_for_test(specifier)
&& config.specifier_code_lens_test(specifier)
&& config.enabled_code_lens_test_for_specifier(specifier)
{
if let Some(parsed_source) = parsed_source {
let mut collector =
Expand Down
24 changes: 17 additions & 7 deletions cli/lsp/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use super::client::Client;
use super::config::ConfigSnapshot;
use super::config::WorkspaceSettings;
use super::documents::file_like_to_file_specifier;
use super::documents::Documents;
use super::documents::DocumentsFilter;
Expand Down Expand Up @@ -52,12 +53,12 @@ pub struct CompletionItemData {
/// a notification to the client.
async fn check_auto_config_registry(
url_str: &str,
config: &ConfigSnapshot,
workspace_settings: &WorkspaceSettings,
client: &Client,
module_registries: &ModuleRegistry,
) {
// check to see if auto discovery is enabled
if config.settings.workspace.suggest.imports.auto_discover {
if workspace_settings.suggest.imports.auto_discover {
if let Ok(specifier) = resolve_url(url_str) {
let scheme = specifier.scheme();
let path = &specifier[Position::BeforePath..];
Expand All @@ -67,11 +68,14 @@ async fn check_auto_config_registry(
{
// check to see if this origin is already explicitly set
let in_config =
config.settings.workspace.suggest.imports.hosts.iter().any(
|(h, _)| {
workspace_settings
.suggest
.imports
.hosts
.iter()
.any(|(h, _)| {
resolve_url(h).map(|u| u.origin()) == Ok(specifier.origin())
},
);
});
// if it isn't in the configuration, we will check to see if it supports
// suggestions and send a notification to the client.
if !in_config {
Expand Down Expand Up @@ -176,7 +180,13 @@ pub async fn get_import_completions(
}))
} else if !text.is_empty() {
// completion of modules from a module registry or cache
check_auto_config_registry(&text, config, client, module_registries).await;
check_auto_config_registry(
&text,
config.workspace_settings_for_specifier(specifier),
client,
module_registries,
)
.await;
let offset = if position.character > range.start.character {
(position.character - range.start.character) as usize
} else {
Expand Down
Loading