From a52177fd398038641e032144489472565756085b Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 28 Feb 2024 17:04:36 +0100 Subject: [PATCH] Allow users to configure ESLint `codeActionOnSave` settings (#8537) This fixes #8533 by allowing users to specify the settings that are passed to ESLint on workspace initialization. Example Zed `settings.json` to enable `fixAll` for eslint when saving/formatting, but only for the `import/order` rule: ```json { "languages": { "JavaScript": { "code_actions_on_format": { "source.fixAll.eslint": true } } }, "lsp": { "eslint": { "settings": { "codeActionOnSave": { "rules": ["import/order"] } } }, } } ``` The possible settings are described in the README of `vscode-eslint` here: https://github.com/Microsoft/vscode-eslint?tab=readme-ov-file#settings-options - `eslint.codeActionsOnSave.enable` (default: `true`, config key in Zed: `lsp.eslint.settings.codeActionOnSave.enable`) - `eslint.codeActionsOnSave.mode` (default: not set by Zed, config key in Zed: `lsp.eslint.settings.codeActionOnSave.mode`) - `eslint.codeActionsOnSave.rules` (default: `[]`, config key in Zed: `lsp.eslint.settings.codeActionOnSave.rules`) Yes, in the readme it's plural: `codeActionsOnSave`, but since `eslint-vscode` we're using this old release: https://github.com/microsoft/vscode-eslint/releases/tag/release%2F2.2.20-Insider We use the singular version: https://github.com/microsoft/vscode-eslint/blob/release/2.2.20-Insider/server/src/eslintServer.ts#L461 Our schema looks like this: ```json { "lsp": { "eslint": { "settings": { "codeActionOnSave": { "enable": true, "rules": ["import/order"], "mode": "all" } } }, } } ``` We should probably fix this and upgrade to the newest version of ESLint. Release Notes: - Added ability for users to configure settings for ESLint's `codeActionOnSave`, e.g. specifying `rules` that should be respected when also using `"code_actions_on_format": {"source.fixAll.eslint": true}`. These settings can be passed to ESLint as part of the `"lsp"` part of the Zed settings. Example: `{"lsp": {"eslint": {"settings": {"codeActionOnSave": { "rules": ["import/order"] }}}}}` ([#8533](https://github.com/zed-industries/zed/issues/8533)). Demo: https://github.com/zed-industries/zed/assets/1185253/5c0cf900-9acb-4a70-b89d-49b6eeb6f0e4 --- crates/editor/src/editor_tests.rs | 4 ++ crates/languages/src/typescript.rs | 41 +++++++++++++++++---- crates/project_core/src/project_settings.rs | 1 + 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 1c9c5f1db6224f0488c662789d7ea463affa32a8..a942686fc7449923128a6b25a8415a8dea51c97d 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8098,6 +8098,7 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::Test project_settings.lsp.insert( "Some other server name".into(), LspSettings { + settings: None, initialization_options: Some(json!({ "some other init value": false })), @@ -8115,6 +8116,7 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::Test project_settings.lsp.insert( language_server_name.into(), LspSettings { + settings: None, initialization_options: Some(json!({ "anotherInitValue": false })), @@ -8132,6 +8134,7 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::Test project_settings.lsp.insert( language_server_name.into(), LspSettings { + settings: None, initialization_options: Some(json!({ "anotherInitValue": false })), @@ -8149,6 +8152,7 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::Test project_settings.lsp.insert( language_server_name.into(), LspSettings { + settings: None, initialization_options: None, }, ); diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index 0e99adfa7c9d76616141577874849215f807f951..395900df937511c4cd5b4deb75a0342c4311b603 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -7,7 +7,9 @@ use gpui::AppContext; use language::{LanguageServerName, LspAdapter, LspAdapterDelegate}; use lsp::{CodeActionKind, LanguageServerBinary}; use node_runtime::NodeRuntime; +use project::project_settings::ProjectSettings; use serde_json::{json, Value}; +use settings::Settings; use smol::{fs, io::BufReader, stream::StreamExt}; use std::{ any::Any, @@ -219,6 +221,7 @@ pub struct EsLintLspAdapter { impl EsLintLspAdapter { const SERVER_PATH: &'static str = "vscode-eslint/server/out/eslintServer.js"; + const SERVER_NAME: &'static str = "eslint"; pub fn new(node: Arc) -> Self { EsLintLspAdapter { node } @@ -227,7 +230,35 @@ impl EsLintLspAdapter { #[async_trait] impl LspAdapter for EsLintLspAdapter { - fn workspace_configuration(&self, workspace_root: &Path, _: &mut AppContext) -> Value { + fn workspace_configuration(&self, workspace_root: &Path, cx: &mut AppContext) -> Value { + let eslint_user_settings = ProjectSettings::get_global(cx) + .lsp + .get(Self::SERVER_NAME) + .and_then(|s| s.settings.clone()) + .unwrap_or_default(); + + let mut code_action_on_save = json!({ + // We enable this, but without also configuring `code_actions_on_format` + // in the Zed configuration, it doesn't have an effect. + "enable": true, + "rules": [] + }); + + if let Some(code_action_settings) = eslint_user_settings + .get("codeActionOnSave") + .and_then(|settings| settings.as_object()) + { + if let Some(enable) = code_action_settings.get("enable") { + code_action_on_save["enable"] = enable.clone(); + } + if let Some(mode) = code_action_settings.get("mode") { + code_action_on_save["mode"] = mode.clone(); + } + if let Some(rules) = code_action_settings.get("rules") { + code_action_on_save["rules"] = rules.clone(); + } + } + json!({ "": { "validate": "on", @@ -241,11 +272,7 @@ 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, - }, + "codeActionOnSave": code_action_on_save, "experimental": { "useFlatConfig": workspace_root.join("eslint.config.js").is_file(), }, @@ -254,7 +281,7 @@ impl LspAdapter for EsLintLspAdapter { } fn name(&self) -> LanguageServerName { - LanguageServerName("eslint".into()) + LanguageServerName(Self::SERVER_NAME.into()) } fn short_name(&self) -> &'static str { diff --git a/crates/project_core/src/project_settings.rs b/crates/project_core/src/project_settings.rs index d26209043e9b0f61025de1ea6ebe710d072882e7..835ca61e7d34b9e91c9b5d77adbed272094fa7c4 100644 --- a/crates/project_core/src/project_settings.rs +++ b/crates/project_core/src/project_settings.rs @@ -64,6 +64,7 @@ pub enum GitGutterSetting { #[serde(rename_all = "snake_case")] pub struct LspSettings { pub initialization_options: Option, + pub settings: Option, } impl Settings for ProjectSettings {