diff --git a/crates/oxc_language_server/src/backend.rs b/crates/oxc_language_server/src/backend.rs index 361f802732a06..c7ae3fe55cc0f 100644 --- a/crates/oxc_language_server/src/backend.rs +++ b/crates/oxc_language_server/src/backend.rs @@ -3,6 +3,7 @@ use std::{str::FromStr, sync::Arc}; use futures::future::join_all; use log::{debug, info, warn}; use rustc_hash::FxBuildHasher; +use serde_json::Value; use tokio::sync::{OnceCell, RwLock, SetError}; use tower_lsp_server::{ Client, LanguageServer, @@ -280,22 +281,26 @@ impl LanguageServer for Backend { let mut removing_registrations = vec![]; let mut adding_registrations = vec![]; - // new valid configuration is passed - let options = serde_json::from_value::>(params.settings.clone()) - .ok() - .or_else(|| { - // fallback to old configuration - // for all workers (default only one) - let options = workers - .iter() - .map(|worker| WorkspaceOption { - workspace_uri: worker.get_root_uri().clone(), - options: params.settings.clone(), - }) - .collect(); - - Some(options) - }); + // when null, request configuration from client; otherwise, parse as per-workspace options or use as global configuration + let options = if params.settings == Value::Null { + None + } else { + serde_json::from_value::>(params.settings.clone()).ok().or_else( + || { + // fallback to old configuration + // for all workers (default only one) + let options = workers + .iter() + .map(|worker| WorkspaceOption { + workspace_uri: worker.get_root_uri().clone(), + options: params.settings.clone(), + }) + .collect(); + + Some(options) + }, + ) + }; // the client passed valid options. let resolved_options = if let Some(options) = options { diff --git a/crates/oxc_language_server/src/tests.rs b/crates/oxc_language_server/src/tests.rs index 3580104d6324a..715d60599121c 100644 --- a/crates/oxc_language_server/src/tests.rs +++ b/crates/oxc_language_server/src/tests.rs @@ -51,10 +51,37 @@ impl Tool for FakeTool { fn handle_configuration_change( &self, - _root_uri: &Uri, + root_uri: &Uri, _old_options_json: &serde_json::Value, - _new_options_json: serde_json::Value, + new_options_json: serde_json::Value, ) -> ToolRestartChanges { + if new_options_json.as_u64() == Some(1) { + return ToolRestartChanges { + tool: Some(FakeToolBuilder.build_boxed(root_uri, new_options_json)), + diagnostic_reports: None, + watch_patterns: None, + }; + } + if new_options_json.as_u64() == Some(2) { + return ToolRestartChanges { + tool: None, + diagnostic_reports: None, + watch_patterns: Some(vec!["**/new_watcher.config".to_string()]), + }; + } + if new_options_json.as_u64() == Some(3) { + return ToolRestartChanges { + tool: None, + diagnostic_reports: Some(vec![( + root_uri.to_string(), + vec![Diagnostic { + message: "Fake diagnostic".to_string(), + ..Default::default() + }], + )]), + watch_patterns: None, + }; + } ToolRestartChanges { tool: None, diagnostic_reports: None, watch_patterns: None } } @@ -342,6 +369,12 @@ fn did_change_watched_files(uri: &str) -> Request { .finish() } +fn did_change_configuration(new_config: Option) -> Request { + Request::build("workspace/didChangeConfiguration") + .params(json!(DidChangeConfigurationParams { settings: new_config.unwrap_or_default() })) + .finish() +} + #[cfg(test)] mod test_suite { use serde_json::json; @@ -357,9 +390,9 @@ mod test_suite { backend::Backend, tests::{ FAKE_COMMAND, FakeToolBuilder, TestServer, WORKSPACE, acknowledge_registrations, - acknowledge_unregistrations, did_change_watched_files, execute_command_request, - initialize_request, initialized_notification, response_to_configuration, - shutdown_request, workspace_folders_changed, + acknowledge_unregistrations, did_change_configuration, did_change_watched_files, + execute_command_request, initialize_request, initialized_notification, + response_to_configuration, shutdown_request, workspace_folders_changed, }, }; @@ -749,4 +782,73 @@ mod test_suite { server.shutdown_with_watchers(3).await; } + + #[tokio::test] + async fn test_did_change_configuration_no_changes() { + let mut server = TestServer::new_initialized( + |client| Backend::new(client, server_info(), vec![Box::new(FakeToolBuilder)]), + initialize_request(false, true, false), + ) + .await; + acknowledge_registrations(&mut server).await; + + // Simulate a configuration change that does not affect the tool + let config_change_notification = did_change_configuration(None); + server.send_request(config_change_notification).await; + + // When `null` is sent and the client does not support workspace configuration requests, + // no configuration changes occur, so no diagnostics or registrations are expected. + server.shutdown_with_watchers(3).await; + } + + #[tokio::test] + async fn test_did_change_configuration_config_passed_new_watchers() { + let mut server = TestServer::new_initialized( + |client| Backend::new(client, server_info(), vec![Box::new(FakeToolBuilder)]), + initialize_request(false, true, false), + ) + .await; + acknowledge_registrations(&mut server).await; + + // Simulate a configuration change that affects watcher patterns + let config_change_notification = did_change_configuration(Some(json!([ + { + "workspaceUri": WORKSPACE, + "options": 2 + } + ]))); + server.send_request(config_change_notification).await; + + // Old watcher unregistration expected + acknowledge_unregistrations(&mut server).await; + // New watcher registration expected + acknowledge_registrations(&mut server).await; + + server.shutdown_with_watchers(3).await; + } + + #[tokio::test] + async fn test_did_change_configuration_config_requested_new_watchers() { + let mut server = TestServer::new_initialized( + |client| Backend::new(client, server_info(), vec![Box::new(FakeToolBuilder)]), + initialize_request(true, true, false), + ) + .await; + response_to_configuration(&mut server, vec![json!(null)]).await; + + acknowledge_registrations(&mut server).await; + + // Simulate a configuration change that affects watcher patterns + let config_change_notification = did_change_configuration(None); + server.send_request(config_change_notification).await; + + // requesting workspace configuration + response_to_configuration(&mut server, vec![json!(2)]).await; + // Old watcher unregistration expected + acknowledge_unregistrations(&mut server).await; + // New watcher registration expected + acknowledge_registrations(&mut server).await; + + server.shutdown_with_watchers(3).await; + } } diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs index 147fe8092c7ff..918c9b1f6151c 100644 --- a/crates/oxc_language_server/src/worker.rs +++ b/crates/oxc_language_server/src/worker.rs @@ -494,4 +494,41 @@ mod tests { // TODO: add test for tool replacement } + + #[tokio::test] + async fn test_did_change_configuration() { + let worker = WorkspaceWorker::new(Uri::from_str("file:///root/").unwrap()); + worker + .start_worker(serde_json::json!({"some_option": true}), &[Box::new(FakeToolBuilder)]) + .await; + + let (diagnostics, registrations, unregistrations) = + worker.did_change_configuration(serde_json::json!({"some_option": false})).await; + + // Since FakeToolBuilder does not change anything based on configuration, no diagnostics or registrations are expected + assert!(diagnostics.is_none()); + assert_eq!(registrations.len(), 0); // No new registrations expected + assert_eq!(unregistrations.len(), 0); // No unregistrations expected + + let (diagnostics, registrations, unregistrations) = + worker.did_change_configuration(serde_json::json!(2)).await; + + // Since FakeToolBuilder changes watcher patterns based on configuration, registrations are expected + assert!(diagnostics.is_none()); + assert_eq!(unregistrations.len(), 1); // One unregistration expected + assert_eq!(unregistrations[0].id, "watcher-FakeTool-file:///root/"); + assert_eq!(registrations.len(), 1); // One new registration expected + assert_eq!(registrations[0].id, "watcher-FakeTool-file:///root/"); + + let (diagnostics, registrations, unregistrations) = + worker.did_change_configuration(serde_json::json!(3)).await; + + // Since FakeToolBuilder changes diagnostics based on configuration, diagnostics are expected + assert!(diagnostics.is_some()); + assert_eq!(diagnostics.unwrap().len(), 1); // One diagnostic report expected + assert_eq!(registrations.len(), 0); // No new registrations expected + assert_eq!(unregistrations.len(), 0); // No unregistrations expected + + // TODO: add test for tool replacement + } }