From 619a2267a4d857d54a5268d0a24665d4edd7477b Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Fri, 21 Nov 2025 15:22:46 +0000 Subject: [PATCH] fix(oxlint/lsp): don't register `textDocument/formatting` capability (#15882) Refactored the language server, so the `ServerCapability` can modified by each tool. Before, we just registered the `textDocument/formatting` capability, if the client supports it dynamically. Now we tell the client at the beginning if the request is supported, skipping the dynamic registration. Improving integration with client that does only support static registration. And only of course when we use the formatter tool ;) Skipping it for `oxlint --lsp` --- crates/oxc_language_server/src/backend.rs | 27 ++- .../oxc_language_server/src/capabilities.rs | 211 ++--------------- .../src/formatter/server_formatter.rs | 23 +- .../src/linter/server_linter.rs | 220 +++++++++++++++++- crates/oxc_language_server/src/tool.rs | 17 +- 5 files changed, 274 insertions(+), 224 deletions(-) diff --git a/crates/oxc_language_server/src/backend.rs b/crates/oxc_language_server/src/backend.rs index 6c24e62e71f0f..a1ca20265c746 100644 --- a/crates/oxc_language_server/src/backend.rs +++ b/crates/oxc_language_server/src/backend.rs @@ -12,14 +12,16 @@ use tower_lsp_server::{ DidChangeConfigurationParams, DidChangeTextDocumentParams, DidChangeWatchedFilesParams, DidChangeWorkspaceFoldersParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, DocumentFormattingParams, ExecuteCommandParams, - InitializeParams, InitializeResult, InitializedParams, Registration, ServerInfo, TextEdit, - Uri, + InitializeParams, InitializeResult, InitializedParams, ServerInfo, TextEdit, Uri, }, }; use crate::{ - ConcurrentHashMap, ToolBuilder, capabilities::Capabilities, file_system::LSPFileSystem, - options::WorkspaceOption, worker::WorkspaceWorker, + ConcurrentHashMap, ToolBuilder, + capabilities::{Capabilities, server_capabilities}, + file_system::LSPFileSystem, + options::WorkspaceOption, + worker::WorkspaceWorker, }; /// The Backend implements the LanguageServer trait to handle LSP requests and notifications. @@ -133,7 +135,12 @@ impl LanguageServer for Backend { *self.workspace_workers.write().await = workers; - self.capabilities.set(capabilities.clone()).map_err(|err| { + let mut server_capabilities = server_capabilities(); + for tool_builder in &self.tool_builders { + tool_builder.server_capabilities(&mut server_capabilities); + } + + self.capabilities.set(capabilities).map_err(|err| { let message = match err { SetError::AlreadyInitializedError(_) => { "capabilities are already initialized".into() @@ -150,7 +157,7 @@ impl LanguageServer for Backend { version: Some(server_version.to_string()), }), offset_encoding: None, - capabilities: capabilities.server_capabilities(&self.tool_builders), + capabilities: server_capabilities, }) } @@ -200,14 +207,6 @@ impl LanguageServer for Backend { } } - if capabilities.dynamic_formatting { - registrations.push(Registration { - id: "dynamic-formatting".to_string(), - method: "textDocument/formatting".to_string(), - register_options: None, - }); - } - if registrations.is_empty() { return; } diff --git a/crates/oxc_language_server/src/capabilities.rs b/crates/oxc_language_server/src/capabilities.rs index 0503b1016173d..f69dc75f8f7b8 100644 --- a/crates/oxc_language_server/src/capabilities.rs +++ b/crates/oxc_language_server/src/capabilities.rs @@ -1,17 +1,12 @@ use tower_lsp_server::lsp_types::{ - ClientCapabilities, CodeActionKind, CodeActionOptions, CodeActionProviderCapability, - ExecuteCommandOptions, OneOf, SaveOptions, ServerCapabilities, TextDocumentSyncCapability, + ClientCapabilities, OneOf, SaveOptions, ServerCapabilities, TextDocumentSyncCapability, TextDocumentSyncKind, TextDocumentSyncOptions, TextDocumentSyncSaveOptions, - WorkDoneProgressOptions, WorkspaceFoldersServerCapabilities, WorkspaceServerCapabilities, + WorkspaceFoldersServerCapabilities, WorkspaceServerCapabilities, }; -use crate::ToolBuilder; - #[derive(Clone, Default)] pub struct Capabilities { - pub code_action_provider: bool, pub workspace_apply_edit: bool, - pub workspace_execute_command: bool, pub workspace_configuration: bool, pub dynamic_watchers: bool, pub dynamic_formatting: bool, @@ -19,18 +14,8 @@ pub struct Capabilities { impl From for Capabilities { fn from(value: ClientCapabilities) -> Self { - // check if the client support some code action literal support - let code_action_provider = value.text_document.as_ref().is_some_and(|capability| { - capability.code_action.as_ref().is_some_and(|code_action| { - code_action.code_action_literal_support.as_ref().is_some_and(|literal_support| { - !literal_support.code_action_kind.value_set.is_empty() - }) - }) - }); let workspace_apply_edit = value.workspace.as_ref().is_some_and(|workspace| workspace.apply_edit.is_some()); - let workspace_execute_command = - value.workspace.as_ref().is_some_and(|workspace| workspace.execute_command.is_some()); let workspace_configuration = value .workspace .as_ref() @@ -46,193 +31,39 @@ impl From for Capabilities { }) }); - Self { - code_action_provider, - workspace_apply_edit, - workspace_execute_command, - workspace_configuration, - dynamic_watchers, - dynamic_formatting, - } + Self { workspace_apply_edit, workspace_configuration, dynamic_watchers, dynamic_formatting } } } -impl Capabilities { - pub fn server_capabilities(&self, tools: &[Box]) -> ServerCapabilities { - let code_action_kinds: Vec = - tools.iter().flat_map(|tool| tool.provided_code_action_kinds()).collect(); - - let commands: Vec = - tools.iter().flat_map(|tool| tool.provided_commands()).collect(); - - ServerCapabilities { - text_document_sync: Some(TextDocumentSyncCapability::Options( - TextDocumentSyncOptions { - change: Some(TextDocumentSyncKind::FULL), - open_close: Some(true), - save: Some(TextDocumentSyncSaveOptions::SaveOptions(SaveOptions { - include_text: Some(false), - })), - ..Default::default() - }, - )), - workspace: Some(WorkspaceServerCapabilities { - workspace_folders: Some(WorkspaceFoldersServerCapabilities { - supported: Some(true), - change_notifications: Some(OneOf::Left(true)), - }), - file_operations: None, +pub fn server_capabilities() -> ServerCapabilities { + ServerCapabilities { + text_document_sync: Some(TextDocumentSyncCapability::Options(TextDocumentSyncOptions { + change: Some(TextDocumentSyncKind::FULL), + open_close: Some(true), + save: Some(TextDocumentSyncSaveOptions::SaveOptions(SaveOptions { + include_text: Some(false), + })), + ..Default::default() + })), + workspace: Some(WorkspaceServerCapabilities { + workspace_folders: Some(WorkspaceFoldersServerCapabilities { + supported: Some(true), + change_notifications: Some(OneOf::Left(true)), }), - code_action_provider: if self.code_action_provider && !code_action_kinds.is_empty() { - Some(CodeActionProviderCapability::Options(CodeActionOptions { - code_action_kinds: Some(code_action_kinds), - work_done_progress_options: WorkDoneProgressOptions { - work_done_progress: None, - }, - resolve_provider: None, - })) - } else { - None - }, - execute_command_provider: if self.workspace_execute_command && !commands.is_empty() { - Some(ExecuteCommandOptions { commands, ..Default::default() }) - } else { - None - }, - // the server supports formatting, but it will tell the client if he enabled the setting - document_formatting_provider: None, - ..ServerCapabilities::default() - } + file_operations: None, + }), + ..ServerCapabilities::default() } } #[cfg(test)] mod test { use tower_lsp_server::lsp_types::{ - ClientCapabilities, CodeActionClientCapabilities, CodeActionKindLiteralSupport, - CodeActionLiteralSupport, DidChangeWatchedFilesClientCapabilities, - DynamicRegistrationClientCapabilities, TextDocumentClientCapabilities, - WorkspaceClientCapabilities, + ClientCapabilities, DidChangeWatchedFilesClientCapabilities, WorkspaceClientCapabilities, }; use super::Capabilities; - #[test] - fn test_code_action_provider_vscode() { - let client_capabilities = ClientCapabilities { - text_document: Some(TextDocumentClientCapabilities { - code_action: Some(CodeActionClientCapabilities { - code_action_literal_support: Some(CodeActionLiteralSupport { - code_action_kind: CodeActionKindLiteralSupport { - // this is from build (see help, about): - // Version: 1.95.3 (user setup) - // Commit: f1a4fb101478ce6ec82fe9627c43efbf9e98c813 - value_set: vec![ - #[expect(clippy::manual_string_new)] - "".into(), - "quickfix".into(), - "refactor".into(), - "refactor.extract".into(), - "refactor.inline".into(), - "refactor.rewrite".into(), - "source".into(), - "source.organizeImports".into(), - ], - }, - }), - ..CodeActionClientCapabilities::default() - }), - ..TextDocumentClientCapabilities::default() - }), - ..ClientCapabilities::default() - }; - - let capabilities = Capabilities::from(client_capabilities); - - assert!(capabilities.code_action_provider); - } - - #[test] - fn test_code_action_provider_intellij() { - let client_capabilities = ClientCapabilities { - text_document: Some(TextDocumentClientCapabilities { - code_action: Some(CodeActionClientCapabilities { - code_action_literal_support: Some(CodeActionLiteralSupport { - code_action_kind: CodeActionKindLiteralSupport { - // this is from build (see help, about): - // Build #IU-243.22562.145, built on December 8, 2024 - value_set: vec![ - "quickfix".into(), - #[expect(clippy::manual_string_new)] - "".into(), - "source".into(), - "refactor".into(), - ], - }, - }), - ..CodeActionClientCapabilities::default() - }), - ..TextDocumentClientCapabilities::default() - }), - ..ClientCapabilities::default() - }; - - let capabilities = Capabilities::from(client_capabilities); - - assert!(capabilities.code_action_provider); - } - - #[test] - fn test_code_action_provider_nvim() { - let client_capabilities = ClientCapabilities { - text_document: Some(TextDocumentClientCapabilities { - code_action: Some(CodeActionClientCapabilities { - code_action_literal_support: Some(CodeActionLiteralSupport { - code_action_kind: CodeActionKindLiteralSupport { - // nvim 0.10.3 - value_set: vec![ - #[expect(clippy::manual_string_new)] - "".into(), - "quickfix".into(), - "refactor".into(), - "refactor.extract".into(), - "refactor.inline".into(), - "refactor.rewrite".into(), - "source".into(), - "source.organizeImports".into(), - ], - }, - }), - ..CodeActionClientCapabilities::default() - }), - ..TextDocumentClientCapabilities::default() - }), - ..ClientCapabilities::default() - }; - - let capabilities = Capabilities::from(client_capabilities); - - assert!(capabilities.code_action_provider); - } - - // This tests code, intellij and neovim (at least nvim 0.10.0+), as they all support dynamic registration. - #[test] - fn test_workspace_execute_command() { - let client_capabilities = ClientCapabilities { - workspace: Some(WorkspaceClientCapabilities { - execute_command: Some(DynamicRegistrationClientCapabilities { - dynamic_registration: Some(true), - }), - ..WorkspaceClientCapabilities::default() - }), - ..ClientCapabilities::default() - }; - - let capabilities = Capabilities::from(client_capabilities); - - assert!(capabilities.workspace_execute_command); - } - #[test] fn test_workspace_edit_nvim() { let client_capabilities = ClientCapabilities { diff --git a/crates/oxc_language_server/src/formatter/server_formatter.rs b/crates/oxc_language_server/src/formatter/server_formatter.rs index fd5f4963fbcb9..bd4921f72e7d3 100644 --- a/crates/oxc_language_server/src/formatter/server_formatter.rs +++ b/crates/oxc_language_server/src/formatter/server_formatter.rs @@ -11,7 +11,7 @@ use oxc_formatter::{ use oxc_parser::Parser; use tower_lsp_server::{ UriExt, - lsp_types::{Pattern, Position, Range, TextEdit, Uri}, + lsp_types::{Pattern, Position, Range, ServerCapabilities, TextEdit, Uri}, }; use crate::{ @@ -67,6 +67,10 @@ impl ServerFormatterBuilder { } impl ToolBuilder for ServerFormatterBuilder { + fn server_capabilities(&self, capabilities: &mut ServerCapabilities) { + capabilities.document_formatting_provider = + Some(tower_lsp_server::lsp_types::OneOf::Left(true)); + } fn build_boxed(&self, root_uri: &Uri, options: serde_json::Value) -> Box { Box::new(ServerFormatterBuilder::build(root_uri, options)) } @@ -380,6 +384,23 @@ fn load_ignore_paths(cwd: &Path) -> Vec { .collect::>() } +#[cfg(test)] +mod tests_builder { + use crate::{ServerFormatterBuilder, ToolBuilder}; + + #[test] + fn test_server_capabilities() { + use tower_lsp_server::lsp_types::{OneOf, ServerCapabilities}; + + let builder = ServerFormatterBuilder; + let mut capabilities = ServerCapabilities::default(); + + builder.server_capabilities(&mut capabilities); + + assert_eq!(capabilities.document_formatting_provider, Some(OneOf::Left(true))); + } +} + #[cfg(test)] mod tests { use serde_json::json; diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index b1b0ea6703401..d9f53a8761c95 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -9,7 +9,9 @@ use tower_lsp_server::{ UriExt, jsonrpc::ErrorCode, lsp_types::{ - CodeActionKind, CodeActionOrCommand, Diagnostic, Pattern, Range, Uri, WorkspaceEdit, + CodeActionKind, CodeActionOptions, CodeActionOrCommand, CodeActionProviderCapability, + Diagnostic, ExecuteCommandOptions, Pattern, Range, ServerCapabilities, Uri, + WorkDoneProgressOptions, WorkspaceEdit, }, }; @@ -143,11 +145,63 @@ impl ServerLinterBuilder { } impl ToolBuilder for ServerLinterBuilder { - fn provided_code_action_kinds(&self) -> Vec { - vec![CodeActionKind::QUICKFIX, CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC] - } - fn provided_commands(&self) -> Vec { - vec![FIX_ALL_COMMAND_ID.to_string()] + fn server_capabilities(&self, capabilities: &mut ServerCapabilities) { + let mut code_action_kinds = capabilities + .code_action_provider + .as_ref() + .and_then(|cap| match cap { + CodeActionProviderCapability::Simple(_) => None, + CodeActionProviderCapability::Options(options) => options.code_action_kinds.clone(), + }) + .unwrap_or_default(); + + if !code_action_kinds.contains(&CodeActionKind::QUICKFIX) { + code_action_kinds.push(CodeActionKind::QUICKFIX); + } + if !code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC) { + code_action_kinds.push(CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC); + } + + // override code action kinds if the code action provider is already set + capabilities.code_action_provider = + Some(CodeActionProviderCapability::Options(CodeActionOptions { + code_action_kinds: Some(code_action_kinds), + work_done_progress_options: capabilities + .code_action_provider + .as_ref() + .and_then(|cap| match cap { + CodeActionProviderCapability::Simple(_) => None, + CodeActionProviderCapability::Options(options) => { + Some(options.work_done_progress_options) + } + }) + .unwrap_or_default(), + resolve_provider: capabilities.code_action_provider.as_ref().and_then(|cap| { + match cap { + CodeActionProviderCapability::Simple(_) => None, + CodeActionProviderCapability::Options(options) => options.resolve_provider, + } + }), + })); + + let mut commands = capabilities + .execute_command_provider + .as_ref() + .map_or(vec![], |opts| opts.commands.clone()); + + if !commands.contains(&FIX_ALL_COMMAND_ID.to_string()) { + commands.push(FIX_ALL_COMMAND_ID.to_string()); + } + + capabilities.execute_command_provider = Some(ExecuteCommandOptions { + commands, + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: capabilities + .execute_command_provider + .as_ref() + .and_then(|provider| provider.work_done_progress_options.work_done_progress), + }, + }); } fn build_boxed(&self, root_uri: &Uri, options: serde_json::Value) -> Box { Box::new(ServerLinterBuilder::build(root_uri, options)) @@ -607,6 +661,160 @@ fn range_overlaps(a: Range, b: Range) -> bool { a.start <= b.end && a.end >= b.start } +#[cfg(test)] +mod tests_builder { + use tower_lsp_server::lsp_types::{ + CodeActionKind, CodeActionOptions, CodeActionProviderCapability, ExecuteCommandOptions, + ServerCapabilities, WorkDoneProgressOptions, + }; + + use crate::{ + ServerLinterBuilder, ToolBuilder, + linter::{code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, commands::FIX_ALL_COMMAND_ID}, + }; + + #[test] + fn test_server_capabilities_empty_capabilities() { + let builder = ServerLinterBuilder; + let mut capabilities = ServerCapabilities::default(); + + builder.server_capabilities(&mut capabilities); + + // Should set code action provider with quickfix and source fix all kinds + match &capabilities.code_action_provider { + Some(CodeActionProviderCapability::Options(options)) => { + let code_action_kinds = options.code_action_kinds.as_ref().unwrap(); + assert!(code_action_kinds.contains(&CodeActionKind::QUICKFIX)); + assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); + assert_eq!(code_action_kinds.len(), 2); + } + _ => panic!("Expected code action provider options"), + } + + // Should set execute command provider with fix all command + let execute_command_provider = capabilities.execute_command_provider.as_ref().unwrap(); + assert!(execute_command_provider.commands.contains(&FIX_ALL_COMMAND_ID.to_string())); + assert_eq!(execute_command_provider.commands.len(), 1); + } + + #[test] + fn test_server_capabilities_with_existing_code_action_kinds() { + let builder = ServerLinterBuilder; + let mut capabilities = ServerCapabilities { + code_action_provider: Some(CodeActionProviderCapability::Options(CodeActionOptions { + code_action_kinds: Some(vec![CodeActionKind::REFACTOR]), + work_done_progress_options: WorkDoneProgressOptions::default(), + resolve_provider: Some(true), + })), + ..Default::default() + }; + + builder.server_capabilities(&mut capabilities); + + match &capabilities.code_action_provider { + Some(CodeActionProviderCapability::Options(options)) => { + let code_action_kinds = options.code_action_kinds.as_ref().unwrap(); + assert!(code_action_kinds.contains(&CodeActionKind::REFACTOR)); + assert!(code_action_kinds.contains(&CodeActionKind::QUICKFIX)); + assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); + assert_eq!(code_action_kinds.len(), 3); + assert_eq!(options.resolve_provider, Some(true)); + } + _ => panic!("Expected code action provider options"), + } + } + + #[test] + fn test_server_capabilities_with_existing_quickfix_kind() { + let builder = ServerLinterBuilder; + let mut capabilities = ServerCapabilities { + code_action_provider: Some(CodeActionProviderCapability::Options(CodeActionOptions { + code_action_kinds: Some(vec![CodeActionKind::QUICKFIX]), + work_done_progress_options: WorkDoneProgressOptions::default(), + resolve_provider: None, + })), + ..Default::default() + }; + + builder.server_capabilities(&mut capabilities); + + match &capabilities.code_action_provider { + Some(CodeActionProviderCapability::Options(options)) => { + let code_action_kinds = options.code_action_kinds.as_ref().unwrap(); + assert!(code_action_kinds.contains(&CodeActionKind::QUICKFIX)); + assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); + assert_eq!(code_action_kinds.len(), 2); + } + _ => panic!("Expected code action provider options"), + } + } + + #[test] + fn test_server_capabilities_with_simple_code_action_provider() { + let builder = ServerLinterBuilder; + let mut capabilities = ServerCapabilities { + code_action_provider: Some(CodeActionProviderCapability::Simple(true)), + ..Default::default() + }; + + builder.server_capabilities(&mut capabilities); + + // Should override with options + match &capabilities.code_action_provider { + Some(CodeActionProviderCapability::Options(options)) => { + let code_action_kinds = options.code_action_kinds.as_ref().unwrap(); + assert!(code_action_kinds.contains(&CodeActionKind::QUICKFIX)); + assert!(code_action_kinds.contains(&CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC)); + assert_eq!(code_action_kinds.len(), 2); + } + _ => panic!("Expected code action provider options"), + } + } + + #[test] + fn test_server_capabilities_with_existing_commands() { + let builder = ServerLinterBuilder; + let mut capabilities = ServerCapabilities { + execute_command_provider: Some(ExecuteCommandOptions { + commands: vec!["existing.command".to_string()], + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: Some(true), + }, + }), + ..Default::default() + }; + + builder.server_capabilities(&mut capabilities); + + let execute_command_provider = capabilities.execute_command_provider.as_ref().unwrap(); + assert!(execute_command_provider.commands.contains(&"existing.command".to_string())); + assert!(execute_command_provider.commands.contains(&FIX_ALL_COMMAND_ID.to_string())); + assert_eq!(execute_command_provider.commands.len(), 2); + assert_eq!( + execute_command_provider.work_done_progress_options.work_done_progress, + Some(true) + ); + } + + #[test] + fn test_server_capabilities_with_existing_fix_all_command() { + let builder = ServerLinterBuilder; + let mut capabilities = ServerCapabilities { + execute_command_provider: Some(ExecuteCommandOptions { + commands: vec![FIX_ALL_COMMAND_ID.to_string()], + work_done_progress_options: WorkDoneProgressOptions::default(), + }), + ..Default::default() + }; + + builder.server_capabilities(&mut capabilities); + + let execute_command_provider = capabilities.execute_command_provider.as_ref().unwrap(); + assert!(execute_command_provider.commands.contains(&FIX_ALL_COMMAND_ID.to_string())); + assert_eq!(execute_command_provider.commands.len(), 1); + } +} + #[cfg(test)] mod test { use std::path::{Path, PathBuf}; diff --git a/crates/oxc_language_server/src/tool.rs b/crates/oxc_language_server/src/tool.rs index 4026f21d27c03..7187869668ad0 100644 --- a/crates/oxc_language_server/src/tool.rs +++ b/crates/oxc_language_server/src/tool.rs @@ -1,23 +1,14 @@ use tower_lsp_server::{ jsonrpc::ErrorCode, lsp_types::{ - CodeActionKind, CodeActionOrCommand, Diagnostic, Pattern, Range, TextEdit, Uri, - WorkspaceEdit, + CodeActionKind, CodeActionOrCommand, Diagnostic, Pattern, Range, ServerCapabilities, + TextEdit, Uri, WorkspaceEdit, }, }; pub trait ToolBuilder: Send + Sync { - /// Get the commands provided by this tool. - /// This will be used to register the commands with the LSP Client. - fn provided_commands(&self) -> Vec { - Vec::new() - } - - /// Get the code action kinds provided by this tool. - /// This will be used to register the code action kinds with the LSP Client. - fn provided_code_action_kinds(&self) -> Vec { - Vec::new() - } + /// Modify the server capabilities to include capabilities provided by this tool. + fn server_capabilities(&self, _capabilities: &mut ServerCapabilities) {} /// Build a boxed instance of the tool for the given root URI and options. fn build_boxed(&self, root_uri: &Uri, options: serde_json::Value) -> Box;