fix(lsp): respect null as invalid settings for workspace/didChangeConfiguration

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::<Vec<WorkspaceOption>>(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::<Vec<WorkspaceOption>>(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 {

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<serde_json::Value>) -> 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;
+    }
 }

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
+    }
 }