From f8959834c4fec9a4848e008af810fd32feb42881 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 28 Feb 2024 13:55:20 +0100 Subject: [PATCH] Add ability to run ESLint (and other non-primary language server) code actions on format (#8496) This PR does two things to fix https://github.com/zed-industries/zed/issues/4325: 1. It changes the way `code_actions_on_format` works to send the possibly configured code actions to _all_ (and not just the primary) languages servers. That means configured code actions can now be sent to ESLint, tailwind, ... and other language servers. 2. It enables `codeActionsOnSave` by default for ESLint. That does **not** mean that by default we will run something on save, but only that we enable it for ESLint. Users can then configure their Zed to run the `eslint` code action on format. Example, for JavaScript: ```json { "languages": { "JavaScript": { "code_actions_on_format": { "source.fixAll.eslint": true } }, } } ``` Release Notes: - Added ability to run ESLint fixes when formatting a buffer. Code actions configured in [`code_actions_on_format`](https://zed.dev/docs/configuring-zed#code-actions-on-format) are now being sent to _all_ language servers connected to a buffer, not just the primary one. So if a user now sets `"code_actions_on_format": { "source.fixAll.eslint": true }` in their Zed settings, the `source.fixAll.eslint` code action will be sent to ESLint, which is not a primary language server. Since the formatter (prettier, or external commands, or another language server, ...) still runs, it's important that these code actions and the formatter don't clash. ([#4325](https://github.com/zed-industries/zed/issues/4325)). Demo: https://github.com/zed-industries/zed/assets/1185253/9ef03ad5-1f5c-4d46-b72a-eef611e32f39 --- crates/languages/src/typescript.rs | 5 ++++ crates/project/src/project.rs | 46 +++++++++++++++++------------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index 4de20a1337..0e99adfa7c 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -241,6 +241,11 @@ impl LspAdapter for EsLintLspAdapter { .unwrap_or_else(|| workspace_root.as_os_str()), }, "problems": {}, + "codeActionOnSave": { + // We enable this, but without also configuring `code_actions_on_format` + // in the Zed configuration, it doesn't have an effect. + "enable": true, + }, "experimental": { "useFlatConfig": workspace_root.join("eslint.config.js").is_file(), }, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index aed89ef241..af142d4ad6 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4129,17 +4129,13 @@ impl Project { cx: &mut ModelContext, ) -> Task> { if self.is_local() { - let mut buffers_with_paths_and_servers = buffers + let mut buffers_with_paths = buffers .into_iter() .filter_map(|buffer_handle| { let buffer = buffer_handle.read(cx); let file = File::from_dyn(buffer.file())?; let buffer_abs_path = file.as_local().map(|f| f.abs_path(cx)); - let (adapter, server) = self - .primary_language_server_for_buffer(buffer, cx) - .map(|(a, s)| (Some(a.clone()), Some(s.clone()))) - .unwrap_or((None, None)); - Some((buffer_handle, buffer_abs_path, adapter, server)) + Some((buffer_handle, buffer_abs_path)) }) .collect::>(); @@ -4147,7 +4143,7 @@ impl Project { // Do not allow multiple concurrent formatting requests for the // same buffer. project.update(&mut cx, |this, cx| { - buffers_with_paths_and_servers.retain(|(buffer, _, _, _)| { + buffers_with_paths.retain(|(buffer, _)| { this.buffers_being_formatted .insert(buffer.read(cx).remote_id()) }); @@ -4156,10 +4152,10 @@ impl Project { let _cleanup = defer({ let this = project.clone(); let mut cx = cx.clone(); - let buffers = &buffers_with_paths_and_servers; + let buffers = &buffers_with_paths; move || { this.update(&mut cx, |this, cx| { - for (buffer, _, _, _) in buffers { + for (buffer, _) in buffers { this.buffers_being_formatted .remove(&buffer.read(cx).remote_id()); } @@ -4169,9 +4165,14 @@ impl Project { }); let mut project_transaction = ProjectTransaction::default(); - for (buffer, buffer_abs_path, lsp_adapter, language_server) in - &buffers_with_paths_and_servers - { + for (buffer, buffer_abs_path) in &buffers_with_paths { + let adapters_and_servers: Vec<_> = project.update(&mut cx, |project, cx| { + project + .language_servers_for_buffer(&buffer.read(cx), cx) + .map(|(adapter, lsp)| (adapter.clone(), lsp.clone())) + .collect() + })?; + let settings = buffer.update(&mut cx, |buffer, cx| { language_settings(buffer.language(), buffer.file(), cx).clone() })?; @@ -4202,9 +4203,7 @@ impl Project { buffer.end_transaction(cx) })?; - if let (Some(lsp_adapter), Some(language_server)) = - (lsp_adapter, language_server) - { + for (lsp_adapter, language_server) in adapters_and_servers.iter() { // Apply the code actions on let code_actions: Vec = settings .code_actions_on_format @@ -4241,6 +4240,7 @@ impl Project { if edit.changes.is_none() && edit.document_changes.is_none() { continue; } + let new = Self::deserialize_workspace_edit( project .upgrade() @@ -4284,17 +4284,23 @@ impl Project { } } - // Apply language-specific formatting using either a language server + // Apply language-specific formatting using either the primary language server // or external command. + let primary_language_server = adapters_and_servers + .first() + .cloned() + .map(|(_, lsp)| lsp.clone()); + let server_and_buffer = primary_language_server + .as_ref() + .zip(buffer_abs_path.as_ref()); + let mut format_operation = None; match (&settings.formatter, &settings.format_on_save) { (_, FormatOnSave::Off) if trigger == FormatTrigger::Save => {} (Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off) | (_, FormatOnSave::LanguageServer) => { - if let Some((language_server, buffer_abs_path)) = - language_server.as_ref().zip(buffer_abs_path.as_ref()) - { + if let Some((language_server, buffer_abs_path)) = server_and_buffer { format_operation = Some(FormatOperation::Lsp( Self::format_via_lsp( &project, @@ -4338,7 +4344,7 @@ impl Project { { format_operation = Some(new_operation); } else if let Some((language_server, buffer_abs_path)) = - language_server.as_ref().zip(buffer_abs_path.as_ref()) + server_and_buffer { format_operation = Some(FormatOperation::Lsp( Self::format_via_lsp(