From b7e8bca52e71dc40d6a2ffd00a8bb37a8d4648ed Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Tue, 23 Dec 2025 19:57:18 +0000 Subject: [PATCH] fix(lsp): do not sent de-registrations for watched files on shutdown request (#17318) TLDR: it is the client's fault to respect server registrations after shutdown Some client (like helix editor) do not send an acknowledgment to the deregistration, because it already sent the shutdown request. (Some) notification are somehow possible. [The LSP Specs](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#shutdown) are unclear about this behavior. So to support fully one more editor, we expect that the deregistration of the file watchers are automatically handled by the client. Why should he still watch for them, when can not report to the server :) closes https://github.com/oxc-project/oxc/issues/17190 --- crates/oxc_language_server/src/backend.rs | 16 +++----- crates/oxc_language_server/src/tests.rs | 48 ++++------------------- 2 files changed, 13 insertions(+), 51 deletions(-) diff --git a/crates/oxc_language_server/src/backend.rs b/crates/oxc_language_server/src/backend.rs index 739020736bdcb..f71d0da39a616 100644 --- a/crates/oxc_language_server/src/backend.rs +++ b/crates/oxc_language_server/src/backend.rs @@ -247,17 +247,18 @@ impl LanguageServer for Backend { } } - /// This method clears all diagnostics and the in-memory file system if dynamic formatting is enabled. + /// This method clears all diagnostics and the in-memory file system. /// /// See: async fn shutdown(&self) -> Result<()> { let mut clearing_diagnostics = Vec::new(); - let mut removed_registrations = Vec::new(); for worker in &*self.workspace_workers.read().await { - let (uris, unregistrations) = worker.shutdown().await; + // shutdown each worker and collect the URIs to clear diagnostics. + // unregistering file watchers is not necessary, because the client will do it automatically on shutdown. + // some clients (`helix`) do not expect any requests after shutdown is sent. + let (uris, _) = worker.shutdown().await; clearing_diagnostics.extend(uris); - removed_registrations.extend(unregistrations); } // only clear diagnostics when we are using push diagnostics @@ -266,13 +267,6 @@ impl LanguageServer for Backend { { self.clear_diagnostics(clearing_diagnostics).await; } - - if self.capabilities.get().unwrap().dynamic_watchers - && !removed_registrations.is_empty() - && let Err(err) = self.client.unregister_capability(removed_registrations).await - { - warn!("sending unregisterCapability.didChangeWatchedFiles failed: {err}"); - } self.file_system.write().await.clear(); Ok(()) diff --git a/crates/oxc_language_server/src/tests.rs b/crates/oxc_language_server/src/tests.rs index 66e5eda8cfac5..c2d0cdc9484f2 100644 --- a/crates/oxc_language_server/src/tests.rs +++ b/crates/oxc_language_server/src/tests.rs @@ -278,20 +278,6 @@ impl TestServer { assert!(shutdown_result.is_ok()); assert_eq!(shutdown_result.id(), &Id::Number(id)); } - - async fn shutdown_with_watchers(&mut self, id: i64) { - // shutdown request - self.send_request(shutdown_request(id)).await; - - // watcher unregistration expected - acknowledge_unregistrations(self).await; - - // shutdown response - let shutdown_result = self.recv_response().await; - - assert!(shutdown_result.is_ok()); - assert_eq!(shutdown_result.id(), &Id::Number(id)); - } } fn initialize_request_workspace_folders( @@ -716,24 +702,6 @@ mod test_suite { // shutdown request server.send_request(shutdown_request(2)).await; - // client/unregisterCapability request - let unregister_request = server.recv_notification().await; - assert_eq!(unregister_request.method(), "client/unregisterCapability"); - assert_eq!(unregister_request.id(), Some(&Id::Number(1))); - assert_eq!( - unregister_request.params(), - Some(&json!({ - "unregisterations": [ - { - "id": format!("watcher-FakeTool-{WORKSPACE}"), - "method": "workspace/didChangeWatchedFiles", - } - ] - })) - ); - // Acknowledge the unregistration - server.send_ack(&Id::Number(1)).await; - // shutdown response let shutdown_result = server.recv_response().await; @@ -942,7 +910,7 @@ mod test_suite { // new watcher registration expected acknowledge_registrations(&mut server).await; - server.shutdown_with_watchers(4).await; + server.shutdown(4).await; } #[tokio::test] @@ -1036,7 +1004,7 @@ mod test_suite { // Since FakeToolBuilder does not know about "unknown.file", no diagnostics or registrations are expected // Thus, no further requests or responses should occur - server.shutdown_with_watchers(3).await; + server.shutdown(3).await; } #[tokio::test] @@ -1058,7 +1026,7 @@ mod test_suite { // New watcher registration expected acknowledge_registrations(&mut server).await; - server.shutdown_with_watchers(3).await; + server.shutdown(3).await; } #[tokio::test] @@ -1093,7 +1061,7 @@ mod test_suite { format!("Fake diagnostic for content: {content}") ); - server.shutdown_with_watchers(3).await; + server.shutdown(3).await; } #[tokio::test] @@ -1117,7 +1085,7 @@ mod test_suite { // Acknowledge the refresh request acknowledge_diagnostic_refresh(&mut server).await; - server.shutdown_with_watchers(3).await; + server.shutdown(3).await; } #[tokio::test] @@ -1135,7 +1103,7 @@ mod test_suite { // 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; + server.shutdown(3).await; } #[tokio::test] @@ -1161,7 +1129,7 @@ mod test_suite { // New watcher registration expected acknowledge_registrations(&mut server).await; - server.shutdown_with_watchers(3).await; + server.shutdown(3).await; } #[tokio::test] @@ -1186,7 +1154,7 @@ mod test_suite { // New watcher registration expected acknowledge_registrations(&mut server).await; - server.shutdown_with_watchers(3).await; + server.shutdown(3).await; } #[tokio::test]