diff --git a/crates/oxc_language_server/src/formatter/mod.rs b/crates/oxc_language_server/src/formatter/mod.rs index 367deda9b6f76..4fac84212e425 100644 --- a/crates/oxc_language_server/src/formatter/mod.rs +++ b/crates/oxc_language_server/src/formatter/mod.rs @@ -3,6 +3,6 @@ mod server_formatter; #[cfg(test)] mod tester; -pub use server_formatter::{ServerFormatter, ServerFormatterBuilder}; +pub use server_formatter::ServerFormatterBuilder; const FORMAT_CONFIG_FILES: &[&str; 2] = &[".oxfmtrc.json", ".oxfmtrc.jsonc"]; diff --git a/crates/oxc_language_server/src/formatter/server_formatter.rs b/crates/oxc_language_server/src/formatter/server_formatter.rs index c8fd247a762ad..4ef3e81ac503e 100644 --- a/crates/oxc_language_server/src/formatter/server_formatter.rs +++ b/crates/oxc_language_server/src/formatter/server_formatter.rs @@ -103,6 +103,9 @@ pub struct ServerFormatter { } impl Tool for ServerFormatter { + fn name(&self) -> &'static str { + "formatter" + } /// # Panics /// Panics if the root URI cannot be converted to a file path. fn handle_configuration_change( @@ -110,7 +113,7 @@ impl Tool for ServerFormatter { root_uri: &Uri, old_options_json: &serde_json::Value, new_options_json: serde_json::Value, - ) -> ToolRestartChanges { + ) -> ToolRestartChanges { let old_option = match serde_json::from_value::(old_options_json.clone()) { Ok(opts) => opts, @@ -145,7 +148,7 @@ impl Tool for ServerFormatter { ServerFormatterBuilder::new(root_uri.clone(), new_options_json.clone()).build(); let watch_patterns = new_formatter.get_watcher_patterns(new_options_json); ToolRestartChanges { - tool: Some(new_formatter), + tool: Some(Box::new(new_formatter)), diagnostic_reports: None, watch_patterns: Some(watch_patterns), } @@ -178,7 +181,7 @@ impl Tool for ServerFormatter { _changed_uri: &Uri, root_uri: &Uri, options: serde_json::Value, - ) -> ToolRestartChanges { + ) -> ToolRestartChanges { if !self.should_run { return ToolRestartChanges { tool: None, @@ -192,7 +195,7 @@ impl Tool for ServerFormatter { let new_formatter = ServerFormatterBuilder::new(root_uri.clone(), options).build(); ToolRestartChanges { - tool: Some(new_formatter), + tool: Some(Box::new(new_formatter)), diagnostic_reports: None, // TODO: update watch patterns if config_path changed watch_patterns: None, diff --git a/crates/oxc_language_server/src/lib.rs b/crates/oxc_language_server/src/lib.rs index 6a3e2888d7423..7db518f23e079 100644 --- a/crates/oxc_language_server/src/lib.rs +++ b/crates/oxc_language_server/src/lib.rs @@ -11,7 +11,7 @@ mod utils; mod worker; pub use crate::backend::Backend; -pub use crate::linter::ServerLinter; -pub use crate::worker::WorkspaceWorker; +pub use crate::tool::{Tool, ToolBuilder, ToolRestartChanges, ToolShutdownChanges}; +pub use crate::worker::WorkspaceWorker; // TODO: remove pub use, `Backend` docs need it for now pub type ConcurrentHashMap = papaya::HashMap; diff --git a/crates/oxc_language_server/src/linter/mod.rs b/crates/oxc_language_server/src/linter/mod.rs index 20e0e48a8f4bb..9d787eb3019f5 100644 --- a/crates/oxc_language_server/src/linter/mod.rs +++ b/crates/oxc_language_server/src/linter/mod.rs @@ -10,6 +10,6 @@ mod tester; pub use code_actions::CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC; pub use commands::FIX_ALL_COMMAND_ID; -pub use server_linter::{ServerLinter, ServerLinterBuilder}; +pub use server_linter::ServerLinterBuilder; const LINT_CONFIG_FILE: &str = ".oxlintrc.json"; diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index e1c677fa55ee1..7847e4481717f 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -18,7 +18,6 @@ use oxc_linter::{ LintIgnoreMatcher, LintOptions, Oxlintrc, }; -use crate::tool::{Tool, ToolBuilder, ToolShutdownChanges}; use crate::{ ConcurrentHashMap, linter::{ @@ -30,7 +29,7 @@ use crate::{ isolated_lint_handler::{IsolatedLintHandler, IsolatedLintHandlerOptions}, options::{LintOptions as LSPLintOptions, Run, UnusedDisableDirectives}, }, - tool::ToolRestartChanges, + tool::{Tool, ToolBuilder, ToolRestartChanges, ToolShutdownChanges}, utils::normalize_path, }; @@ -244,6 +243,10 @@ pub struct ServerLinter { } impl Tool for ServerLinter { + fn name(&self) -> &'static str { + "linter" + } + fn shutdown(&self) -> ToolShutdownChanges { ToolShutdownChanges { uris_to_clear_diagnostics: Some(self.get_cached_files_of_diagnostics()), @@ -257,7 +260,7 @@ impl Tool for ServerLinter { root_uri: &Uri, old_options_json: &serde_json::Value, new_options_json: serde_json::Value, - ) -> ToolRestartChanges { + ) -> ToolRestartChanges { let old_option = match serde_json::from_value::(old_options_json.clone()) { Ok(opts) => opts, Err(e) => { @@ -303,7 +306,7 @@ impl Tool for ServerLinter { }; ToolRestartChanges { - tool: Some(new_linter), + tool: Some(Box::new(new_linter)), diagnostic_reports: diagnostics, watch_patterns: patterns, } @@ -341,7 +344,7 @@ impl Tool for ServerLinter { _changed_uri: &Uri, root_uri: &Uri, options: serde_json::Value, - ) -> ToolRestartChanges { + ) -> ToolRestartChanges { // TODO: Check if the changed file is actually a config file (including extended paths) let new_linter = ServerLinterBuilder::new(root_uri.clone(), options).build(); @@ -350,7 +353,7 @@ impl Tool for ServerLinter { let diagnostics = Some(new_linter.revalidate_diagnostics(cached_files)); ToolRestartChanges { - tool: Some(new_linter), + tool: Some(Box::new(new_linter)), diagnostic_reports: diagnostics, // TODO: update watch patterns if config_path changed, or the extended paths changed watch_patterns: None, diff --git a/crates/oxc_language_server/src/linter/tester.rs b/crates/oxc_language_server/src/linter/tester.rs index 82ab13dbaacea..d90d6ccce9395 100644 --- a/crates/oxc_language_server/src/linter/tester.rs +++ b/crates/oxc_language_server/src/linter/tester.rs @@ -9,7 +9,7 @@ use tower_lsp_server::{ }; use crate::{ - linter::{ServerLinter, ServerLinterBuilder}, + linter::{ServerLinterBuilder, server_linter::ServerLinter}, tool::{Tool, ToolBuilder}, }; diff --git a/crates/oxc_language_server/src/tool.rs b/crates/oxc_language_server/src/tool.rs index 368de5799eb7f..55315147c9e9d 100644 --- a/crates/oxc_language_server/src/tool.rs +++ b/crates/oxc_language_server/src/tool.rs @@ -11,7 +11,10 @@ pub trait ToolBuilder { fn build(&self) -> T; } -pub trait Tool: Sized { +pub trait Tool: Send + Sync { + /// Get the name of the tool. + fn name(&self) -> &'static str; + /// The Server has new configuration changes. /// Returns a [ToolRestartChanges] indicating what changes were made for the Tool. fn handle_configuration_change( @@ -19,7 +22,7 @@ pub trait Tool: Sized { root_uri: &Uri, old_options_json: &serde_json::Value, new_options_json: serde_json::Value, - ) -> ToolRestartChanges; + ) -> ToolRestartChanges; /// Get the file watcher patterns for this tool based on the provided options. /// These patterns will be used to watch for file changes relevant to the tool. @@ -33,7 +36,7 @@ pub trait Tool: Sized { changed_uri: &Uri, root_uri: &Uri, options: serde_json::Value, - ) -> ToolRestartChanges; + ) -> ToolRestartChanges; /// Check if this tool is responsible for handling the given command. fn is_responsible_for_command(&self, _command: &str) -> bool { @@ -45,6 +48,9 @@ pub trait Tool: Sized { /// If the command is recognized and executed it can return: /// - `Ok(Some(WorkspaceEdit))` if the command was executed successfully and produced a workspace edit. /// - `Ok(None)` if the command was executed successfully but did not produce any workspace edit. + /// + /// # Errors + /// If there was an error executing the command, returns an `Err(ErrorCode)`. fn execute_command( &self, _command: &str, @@ -116,10 +122,10 @@ pub trait Tool: Sized { } } -pub struct ToolRestartChanges { +pub struct ToolRestartChanges { /// The tool that was restarted (linter, formatter). /// If None, no tool was restarted. - pub tool: Option, + pub tool: Option>, /// The diagnostic reports that need to be revalidated after the tool restart pub diagnostic_reports: Option)>>, /// The patterns that were added during the tool restart diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs index 16acc52954716..518d2e41b624f 100644 --- a/crates/oxc_language_server/src/worker.rs +++ b/crates/oxc_language_server/src/worker.rs @@ -12,8 +12,8 @@ use tower_lsp_server::{ }; use crate::{ - formatter::{ServerFormatter, ServerFormatterBuilder}, - linter::{ServerLinter, ServerLinterBuilder}, + formatter::ServerFormatterBuilder, + linter::ServerLinterBuilder, tool::{Tool, ToolBuilder}, }; @@ -24,8 +24,7 @@ use crate::{ /// The [`Backend`](crate::backend::Backend) is responsible to target the correct worker for a given file URI. pub struct WorkspaceWorker { root_uri: Uri, - server_linter: RwLock>, - server_formatter: RwLock>, + tools: RwLock>>, // Initialized options from the client // If None, the worker has not been initialized yet options: Mutex>, @@ -36,12 +35,7 @@ impl WorkspaceWorker { /// This will not start any programs, use [`start_worker`](Self::start_worker) for that. /// Depending on the client, we need to request the workspace configuration in `initialized. pub fn new(root_uri: Uri) -> Self { - Self { - root_uri, - server_linter: RwLock::new(None), - server_formatter: RwLock::new(None), - options: Mutex::new(None), - } + Self { root_uri, tools: RwLock::new(vec![]), options: Mutex::new(None) } } /// Get the root URI of the worker @@ -67,17 +61,10 @@ impl WorkspaceWorker { /// Start all programs (linter, formatter) for the worker. /// This should be called after the client has sent the workspace configuration. pub async fn start_worker(&self, options: serde_json::Value) { - tokio::join!( - async { - *self.server_linter.write().await = - Some(ServerLinterBuilder::new(self.root_uri.clone(), options.clone()).build()); - }, - async { - *self.server_formatter.write().await = Some( - ServerFormatterBuilder::new(self.root_uri.clone(), options.clone()).build(), - ); - }, - ); + *self.tools.write().await = vec![ + Box::new(ServerLinterBuilder::new(self.root_uri.clone(), options.clone()).build()), + Box::new(ServerFormatterBuilder::new(self.root_uri.clone(), options.clone()).build()), + ]; *self.options.lock().await = Some(options); } @@ -86,49 +73,22 @@ impl WorkspaceWorker { /// These watchers are used to watch for changes in the lint configuration files. /// The returned watchers will be registered to the client. pub async fn init_watchers(&self) -> Vec { - let mut registrations = Vec::new(); - // clone the options to avoid locking the mutex let options_json = { self.options.lock().await.clone().unwrap_or_default() }; - let (lint_patterns, format_patterns) = tokio::join!( - async { - self.server_linter - .read() - .await - .as_ref() - .map(|linter| linter.get_watcher_patterns(options_json.clone())) - }, - async { - self.server_formatter - .read() - .await - .as_ref() - .map(|formatter| formatter.get_watcher_patterns(options_json.clone())) - } - ); - - if let Some(lint_patterns) = lint_patterns - && !lint_patterns.is_empty() - { - registrations.push(registration_tool_watcher_id( - "linter", - &self.root_uri, - lint_patterns, - )); - } - - if let Some(format_patterns) = format_patterns - && !format_patterns.is_empty() - { - registrations.push(registration_tool_watcher_id( - "formatter", - &self.root_uri, - format_patterns, - )); - } - - registrations + self.tools + .read() + .await + .iter() + .filter_map(|tool| { + let patterns = tool.get_watcher_patterns(options_json.clone()); + if patterns.is_empty() { + None + } else { + Some(registration_tool_watcher_id(tool.name(), &self.root_uri, patterns)) + } + }) + .collect() } /// Check if the worker needs to be initialized with options @@ -138,11 +98,9 @@ impl WorkspaceWorker { /// Remove all diagnostics for the given URI pub async fn remove_diagnostics(&self, uri: &Uri) { - let server_linter_guard = self.server_linter.read().await; - let Some(server_linter) = server_linter_guard.as_ref() else { - return; - }; - server_linter.remove_diagnostics(uri); + self.tools.read().await.iter().for_each(|tool| { + tool.remove_diagnostics(uri); + }); } /// Run different tools to collect diagnostics. @@ -151,11 +109,15 @@ impl WorkspaceWorker { uri: &Uri, content: Option<&str>, ) -> Option> { - let Some(server_linter) = &*self.server_linter.read().await else { - return None; - }; - - server_linter.run_diagnostic(uri, content) + let mut diagnostics = Vec::new(); + let mut found = false; + for tool in self.tools.read().await.iter() { + if let Some(tool_diagnostics) = tool.run_diagnostic(uri, content) { + diagnostics.extend(tool_diagnostics); + found = true; + } + } + if found { Some(diagnostics) } else { None } } /// Run different tools to collect diagnostics on change. @@ -164,11 +126,15 @@ impl WorkspaceWorker { uri: &Uri, content: Option<&str>, ) -> Option> { - let Some(server_linter) = &*self.server_linter.read().await else { - return None; - }; - - server_linter.run_diagnostic_on_change(uri, content) + let mut diagnostics = Vec::new(); + let mut found = false; + for tool in self.tools.read().await.iter() { + if let Some(tool_diagnostics) = tool.run_diagnostic_on_change(uri, content) { + diagnostics.extend(tool_diagnostics); + found = true; + } + } + if found { Some(diagnostics) } else { None } } /// Run different tools to collect diagnostics on save. @@ -177,22 +143,27 @@ impl WorkspaceWorker { uri: &Uri, content: Option<&str>, ) -> Option> { - let Some(server_linter) = &*self.server_linter.read().await else { - return None; - }; - - server_linter.run_diagnostic_on_save(uri, content) + let mut diagnostics = Vec::new(); + let mut found = false; + for tool in self.tools.read().await.iter() { + if let Some(tool_diagnostics) = tool.run_diagnostic_on_save(uri, content) { + diagnostics.extend(tool_diagnostics); + found = true; + } + } + if found { Some(diagnostics) } else { None } } /// Format a file with the current formatter /// - If no file is not formattable or ignored, [`None`] is returned /// - If the file is formattable, but no changes are made, an empty vector is returned pub async fn format_file(&self, uri: &Uri, content: Option<&str>) -> Option> { - let Some(server_formatter) = &*self.server_formatter.read().await else { - return None; - }; - - server_formatter.run_format(uri, content) + for tool in self.tools.read().await.iter() { + if let Some(edits) = tool.run_format(uri, content) { + return Some(edits); + } + } + None } /// Shutdown the worker and return any necessary changes to be made after shutdown. @@ -206,36 +177,37 @@ impl WorkspaceWorker { Vec, ) { let mut uris_to_clear_diagnostics = Vec::new(); - - if let Some(server_linter) = &*self.server_linter.read().await - && let Some(uris) = server_linter.shutdown().uris_to_clear_diagnostics - { - uris_to_clear_diagnostics.extend(uris); + let mut watchers_to_unregister = Vec::new(); + for tool in self.tools.read().await.iter() { + let shutdown_changes = tool.shutdown(); + if let Some(uris) = shutdown_changes.uris_to_clear_diagnostics { + uris_to_clear_diagnostics.extend(uris); + } + watchers_to_unregister + .push(unregistration_tool_watcher_id(tool.name(), &self.root_uri)); } - ( - uris_to_clear_diagnostics, - vec![ - unregistration_tool_watcher_id("linter", &self.root_uri), - unregistration_tool_watcher_id("formatter", &self.root_uri), - ], - ) + (uris_to_clear_diagnostics, watchers_to_unregister) } - /// Get code actions or commands for the given range - /// It uses the [`ServerLinter`] cached diagnostics if available, otherwise it will lint the file - /// If `is_source_fix_all_oxc` is true, it will return a single code action that applies all fixes + /// Get code actions or commands for the given range. + /// It calls all tools and collects their code actions or commands. + /// If `only_code_action_kinds` is provided, only code actions of the specified kinds are returned. pub async fn get_code_actions_or_commands( &self, uri: &Uri, range: &Range, only_code_action_kinds: Option>, ) -> Vec { - let Some(server_linter) = &*self.server_linter.read().await else { - return vec![]; - }; - - server_linter.get_code_actions_or_commands(uri, range, only_code_action_kinds) + let mut actions = Vec::new(); + for tool in self.tools.read().await.iter() { + actions.extend(tool.get_code_actions_or_commands( + uri, + range, + only_code_action_kinds.clone(), + )); + } + actions } /// Handle file changes that are watched by the client @@ -260,54 +232,31 @@ impl WorkspaceWorker { let mut registrations = vec![]; let mut unregistrations = vec![]; - let mut diagnostics = None; - - let mut new_formatter = None; - if let Some(formatter) = self.server_formatter.read().await.as_ref() { - let format_change = formatter.handle_watched_file_change( - &file_event.uri, - &self.root_uri, - options.clone(), - ); - new_formatter = format_change.tool; - - if let Some(patterns) = format_change.watch_patterns { - unregistrations.push(unregistration_tool_watcher_id("formatter", &self.root_uri)); - if !patterns.is_empty() { - registrations.push(registration_tool_watcher_id( - "formatter", - &self.root_uri, - patterns, - )); + let mut diagnostics: Option)>> = None; + + for tool in self.tools.write().await.iter_mut() { + let change = + tool.handle_watched_file_change(&file_event.uri, &self.root_uri, options.clone()); + if let Some(reports) = change.diagnostic_reports { + if let Some(existing_diagnostics) = &mut diagnostics { + existing_diagnostics.extend(reports); + } else { + diagnostics = Some(reports); } } - } - if let Some(new_formatter) = new_formatter { - *self.server_formatter.write().await = Some(new_formatter); - } - - let mut new_linter = None; - if let Some(linter) = self.server_linter.read().await.as_ref() { - let lint_change = - linter.handle_watched_file_change(&file_event.uri, &self.root_uri, options); - - new_linter = lint_change.tool; - diagnostics = lint_change.diagnostic_reports; - - if let Some(patterns) = lint_change.watch_patterns { - unregistrations.push(unregistration_tool_watcher_id("linter", &self.root_uri)); + if let Some(patterns) = change.watch_patterns { + unregistrations.push(unregistration_tool_watcher_id(tool.name(), &self.root_uri)); if !patterns.is_empty() { registrations.push(registration_tool_watcher_id( - "linter", + tool.name(), &self.root_uri, patterns, )); } } - } - - if let Some(new_linter) = new_linter { - *self.server_linter.write().await = Some(new_linter); + if let Some(replaced_tool) = change.tool { + *tool = replaced_tool; + } } (diagnostics, registrations, unregistrations) @@ -348,57 +297,34 @@ impl WorkspaceWorker { let mut registrations = vec![]; let mut unregistrations = vec![]; - let mut diagnostics = None; + let mut diagnostics: Option)>> = None; - let mut new_formatter = None; - if let Some(formatter) = self.server_formatter.read().await.as_ref() { - let format_change = formatter.handle_configuration_change( + for tool in self.tools.write().await.iter_mut() { + let change = tool.handle_configuration_change( &self.root_uri, &old_options, changed_options_json.clone(), ); - - new_formatter = format_change.tool; - - if let Some(patterns) = format_change.watch_patterns { - unregistrations.push(unregistration_tool_watcher_id("formatter", &self.root_uri)); - if !patterns.is_empty() { - registrations.push(registration_tool_watcher_id( - "formatter", - &self.root_uri, - patterns, - )); + if let Some(reports) = change.diagnostic_reports { + if let Some(existing_diagnostics) = &mut diagnostics { + existing_diagnostics.extend(reports); + } else { + diagnostics = Some(reports); } } - } - if let Some(new_formatter) = new_formatter { - *self.server_formatter.write().await = Some(new_formatter); - } - - let mut new_linter = None; - if let Some(linter) = self.server_linter.read().await.as_ref() { - let lint_change = linter.handle_configuration_change( - &self.root_uri, - &old_options, - changed_options_json, - ); - new_linter = lint_change.tool; - diagnostics = lint_change.diagnostic_reports; - - if let Some(patterns) = lint_change.watch_patterns { - unregistrations.push(unregistration_tool_watcher_id("linter", &self.root_uri)); + if let Some(patterns) = change.watch_patterns { + unregistrations.push(unregistration_tool_watcher_id(tool.name(), &self.root_uri)); if !patterns.is_empty() { registrations.push(registration_tool_watcher_id( - "linter", + tool.name(), &self.root_uri, patterns, )); } } - } - - if let Some(new_linter) = new_linter { - *self.server_linter.write().await = Some(new_linter); + if let Some(replaced_tool) = change.tool { + *tool = replaced_tool; + } } (diagnostics, registrations, unregistrations) @@ -414,15 +340,12 @@ impl WorkspaceWorker { command: &str, arguments: Vec, ) -> Result, ErrorCode> { - let Some(server_linter) = &*self.server_linter.read().await else { - return Ok(None); - }; - - if !server_linter.is_responsible_for_command(command) { - return Ok(None); + for tool in self.tools.read().await.iter() { + if tool.is_responsible_for_command(command) { + return tool.execute_command(command, arguments); + } } - - server_linter.execute_command(command, arguments) + Ok(None) } }