summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNayeem Rahman <nayeemrmn99@gmail.com>2023-09-02 16:36:04 +0100
committerGitHub <noreply@github.com>2023-09-02 16:36:04 +0100
commit12f6ad32c2108efb1492df6192a1f8cdd5eafa76 (patch)
tree8afd5f0a4890f62eba658850e294663540e949c4
parent83426be6eead06c680ae527468aeaf8723543ff2 (diff)
fix(lsp): properly handle disabled configuration requests (#20358)
Fixes #19802. Properly respect when clients do not have the `workspace/configuration` capability, a.k.a. when an editor cannot provide scoped settings on request from the LSP. - Fix one spot where we weren't checking for the capability before sending this request. - For `enablePaths`, fall back to the settings passed in the initialization options in more cases. - Respect the `workspace/configuration` capability in the test harness client. See: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_configuration.
-rw-r--r--cli/lsp/config.rs13
-rw-r--r--cli/lsp/language_server.rs15
-rw-r--r--cli/tests/integration/lsp_tests.rs81
-rw-r--r--test_util/src/lsp.rs18
4 files changed, 111 insertions, 16 deletions
diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
index b70af6519..2d77abaee 100644
--- a/cli/lsp/config.rs
+++ b/cli/lsp/config.rs
@@ -718,13 +718,12 @@ impl Config {
if let Some(workspace_folders) = self.workspace_folders.clone() {
let mut touched = false;
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;
- }
+ let enabled_paths = match self.settings.specifiers.get(&workspace) {
+ Some(settings) => settings.enable_paths.clone(),
+ None => self.settings.workspace.enable_paths.clone(),
+ };
+ if self.update_enabled_paths_entry(workspace, enabled_paths) {
+ touched = true;
}
}
touched
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index dc85eb1cc..d4e5b3da9 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -1275,6 +1275,7 @@ impl Inner {
error!("Cannot set workspace settings: {}", err);
LspError::internal_error()
})?;
+ self.config.update_enabled_paths();
}
self.config.workspace_folders = params.workspace_folders.map(|folders| {
folders
@@ -1471,6 +1472,7 @@ impl Inner {
if let Err(err) = self.config.set_workspace_settings(value) {
error!("failed to update settings: {}", err);
}
+ self.config.update_enabled_paths();
}
self.update_debug_flag();
@@ -3125,7 +3127,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
return;
}
- let (client, client_uri, specifier, had_specifier_settings) = {
+ let (client, client_uri, specifier, should_get_specifier_settings) = {
let mut inner = self.0.write().await;
let client = inner.client.clone();
let client_uri = LspClientUrl::new(params.text_document.uri.clone());
@@ -3133,24 +3135,25 @@ impl tower_lsp::LanguageServer for LanguageServer {
.url_map
.normalize_url(client_uri.as_url(), LspUrlKind::File);
let document = inner.did_open(&specifier, params).await;
- let has_specifier_settings =
- inner.config.has_specifier_settings(&specifier);
+ let should_get_specifier_settings =
+ !inner.config.has_specifier_settings(&specifier)
+ && inner.config.client_capabilities.workspace_configuration;
if document.is_diagnosable() {
inner.refresh_npm_specifiers().await;
let specifiers = inner.documents.dependents(&specifier);
inner.diagnostics_server.invalidate(&specifiers);
// don't send diagnostics yet if we don't have the specifier settings
- if has_specifier_settings {
+ if !should_get_specifier_settings {
inner.send_diagnostics_update();
inner.send_testing_update();
}
}
- (client, client_uri, specifier, has_specifier_settings)
+ (client, client_uri, specifier, should_get_specifier_settings)
};
// retrieve the specifier settings outside the lock if
// they haven't been asked for yet
- if !had_specifier_settings {
+ if should_get_specifier_settings {
let response = client
.when_outside_lsp_lock()
.specifier_configuration(&client_uri)
diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs
index 218b1db40..19b968602 100644
--- a/cli/tests/integration/lsp_tests.rs
+++ b/cli/tests/integration/lsp_tests.rs
@@ -643,6 +643,87 @@ fn lsp_import_map_node_specifiers() {
client.shutdown();
}
+// Regression test for https://github.com/denoland/deno/issues/19802.
+// Disable the `workspace/configuration` capability. Ensure the LSP falls back
+// to using `enablePaths` from the `InitializationOptions`.
+#[test]
+fn lsp_workspace_enable_paths_no_workspace_configuration() {
+ let context = TestContextBuilder::new().use_temp_cwd().build();
+ let temp_dir = context.temp_dir();
+ temp_dir.write("main_disabled.ts", "Date.now()");
+ temp_dir.write("main_enabled.ts", "Date.now()");
+
+ let mut client = context.new_lsp_command().build();
+ client.initialize(|builder| {
+ builder.with_capabilities(|capabilities| {
+ capabilities.workspace.as_mut().unwrap().configuration = Some(false);
+ });
+ builder.set_workspace_folders(vec![lsp::WorkspaceFolder {
+ uri: temp_dir.uri(),
+ name: "project".to_string(),
+ }]);
+ builder.set_root_uri(temp_dir.uri());
+ builder.set_enable_paths(vec!["./main_enabled.ts".to_string()]);
+ });
+
+ client.did_open(json!({
+ "textDocument": {
+ "uri": temp_dir.uri().join("main_disabled.ts").unwrap(),
+ "languageId": "typescript",
+ "version": 1,
+ "text": temp_dir.read_to_string("main_disabled.ts"),
+ }
+ }));
+
+ client.did_open(json!({
+ "textDocument": {
+ "uri": temp_dir.uri().join("main_enabled.ts").unwrap(),
+ "languageId": "typescript",
+ "version": 1,
+ "text": temp_dir.read_to_string("main_enabled.ts"),
+ }
+ }));
+
+ let res = client.write_request(
+ "textDocument/hover",
+ json!({
+ "textDocument": {
+ "uri": temp_dir.uri().join("main_disabled.ts").unwrap(),
+ },
+ "position": { "line": 0, "character": 5 }
+ }),
+ );
+ assert_eq!(res, json!(null));
+
+ let res = client.write_request(
+ "textDocument/hover",
+ json!({
+ "textDocument": {
+ "uri": temp_dir.uri().join("main_enabled.ts").unwrap(),
+ },
+ "position": { "line": 0, "character": 5 }
+ }),
+ );
+ assert_eq!(
+ res,
+ json!({
+ "contents": [
+ {
+ "language": "typescript",
+ "value": "(method) DateConstructor.now(): number",
+ },
+ "Returns the number of milliseconds elapsed since midnight, January 1, 1970 Universal Coordinated Time (UTC)."
+ ],
+ "range": {
+ "start": { "line": 0, "character": 5, },
+ "end": { "line": 0, "character": 8, }
+ }
+ })
+ );
+
+ client.shutdown();
+}
+
#[test]
fn lsp_deno_task() {
let context = TestContextBuilder::new().use_temp_cwd().build();
diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs
index 8a1f24b32..5369d6361 100644
--- a/test_util/src/lsp.rs
+++ b/test_util/src/lsp.rs
@@ -591,6 +591,7 @@ impl LspClientBuilder {
writer,
deno_dir,
stderr_lines_rx,
+ supports_workspace_configuration: false,
})
}
}
@@ -604,6 +605,7 @@ pub struct LspClient {
deno_dir: TempDir,
context: TestContext,
stderr_lines_rx: Option<mpsc::Receiver<String>>,
+ supports_workspace_configuration: bool,
}
impl Drop for LspClient {
@@ -689,9 +691,17 @@ impl LspClient {
let mut builder = InitializeParamsBuilder::new();
builder.set_root_uri(self.context.temp_dir().uri());
do_build(&mut builder);
- self.write_request("initialize", builder.build());
+ let params: InitializeParams = builder.build();
+ self.supports_workspace_configuration = match &params.capabilities.workspace
+ {
+ Some(workspace) => workspace.configuration == Some(true),
+ _ => false,
+ };
+ self.write_request("initialize", params);
self.write_notification("initialized", json!({}));
- self.handle_configuration_request(config);
+ if self.supports_workspace_configuration {
+ self.handle_configuration_request(config);
+ }
}
pub fn did_open(&mut self, params: Value) -> CollectedDiagnostics {
@@ -712,7 +722,9 @@ impl LspClient {
config: Value,
) -> CollectedDiagnostics {
self.did_open_raw(params);
- self.handle_configuration_request(config);
+ if self.supports_workspace_configuration {
+ self.handle_configuration_request(config);
+ }
self.read_diagnostics()
}