From 81425bef721bd03dc2fe89a1aee6e63adec557bc Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Thu, 16 Oct 2025 13:11:20 -0500 Subject: [PATCH] Revert deprecate code actions on format (#40409) Closes #40334 This reverts the change made in #39983, and includes a replacement migration that will transform formatter settings values consisting of only `code_action` format steps into the previously deprecated `code_actions_on_format` in an attempt to restore the behavior to what it was before the migration that deprecated `code_actions_on_format`. This PR will result in a modified order in the `code_actions_on_format` setting if it existed, however the decision was made to explicitly ignore this for now, as this PR is primarily targeting users who have already had the deprecation migration run, and no longer have the `code_actions_on_format` key Release Notes: - Fixed an issue with a settings migration that deprecated the `code_actions_on_format` setting. The `code_actions_on_format` setting has been un-deprecated, and affected users will have the bad migration rolled back with an updated migration --------- Co-authored-by: Cole Miller Co-authored-by: Mikayla Maki Co-authored-by: HactarCE <6060305+HactarCE@users.noreply.github.com> --- assets/settings/default.json | 5 +- crates/language/src/language_settings.rs | 3 + crates/migrator/src/migrations.rs | 4 +- .../src/migrations/m_2025_10_10/settings.rs | 61 --- .../src/migrations/m_2025_10_16/settings.rs | 68 ++++ crates/migrator/src/migrator.rs | 347 +++--------------- crates/project/src/lsp_store.rs | 31 ++ .../settings/src/settings_content/language.rs | 5 + crates/settings_ui/src/page_data.rs | 21 ++ docs/src/languages/javascript.md | 12 +- 10 files changed, 191 insertions(+), 366 deletions(-) delete mode 100644 crates/migrator/src/migrations/m_2025_10_10/settings.rs create mode 100644 crates/migrator/src/migrations/m_2025_10_16/settings.rs diff --git a/assets/settings/default.json b/assets/settings/default.json index 5b37e2d6c07fc348ee795eb4ea5bb043c1f2689d..f1eae6c6b47d095f1952e7acfb6a5ae5c885302e 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1529,6 +1529,7 @@ // A value of 45 preserves colorful themes while ensuring legibility. "minimum_contrast": 45 }, + "code_actions_on_format": {}, // Settings related to running tasks. "tasks": { "variables": {}, @@ -1698,7 +1699,9 @@ "preferred_line_length": 72 }, "Go": { - "formatter": [{ "code_action": "source.organizeImports" }, "language_server"], + "code_actions_on_format": { + "source.organizeImports": true + }, "debuggers": ["Delve"] }, "GraphQL": { diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 829be3bbd44dd20167fd09d2eaeeab5e24c70140..2f96e850f49dd5cda6c5d34b2b424812b23a524d 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -142,6 +142,8 @@ pub struct LanguageSettings { pub auto_indent_on_paste: bool, /// Controls how the editor handles the autoclosed characters. pub always_treat_brackets_as_autoclosed: bool, + /// Which code actions to run on save + pub code_actions_on_format: HashMap, /// Whether to perform linked edits pub linked_edits: bool, /// Task configuration for this language. @@ -576,6 +578,7 @@ impl settings::Settings for AllLanguageSettings { always_treat_brackets_as_autoclosed: settings .always_treat_brackets_as_autoclosed .unwrap(), + code_actions_on_format: settings.code_actions_on_format.unwrap(), linked_edits: settings.linked_edits.unwrap(), tasks: LanguageTaskSettings { variables: tasks.variables.unwrap_or_default(), diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index 0b35a238c60fb85a8426b0477d9b017d61513028..87a301dcad83127666888a7530ba43488ac5a72f 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -118,8 +118,8 @@ pub(crate) mod m_2025_10_03 { pub(crate) use settings::SETTINGS_PATTERNS; } -pub(crate) mod m_2025_10_10 { +pub(crate) mod m_2025_10_16 { mod settings; - pub(crate) use settings::remove_code_actions_on_format; + pub(crate) use settings::restore_code_actions_on_format; } diff --git a/crates/migrator/src/migrations/m_2025_10_10/settings.rs b/crates/migrator/src/migrations/m_2025_10_10/settings.rs deleted file mode 100644 index 694f887b5962bb1f1e0f66eb4562edd10351868a..0000000000000000000000000000000000000000 --- a/crates/migrator/src/migrations/m_2025_10_10/settings.rs +++ /dev/null @@ -1,61 +0,0 @@ -use anyhow::Result; -use serde_json::Value; - -use crate::patterns::migrate_language_setting; - -pub fn remove_code_actions_on_format(value: &mut Value) -> Result<()> { - migrate_language_setting(value, remove_code_actions_on_format_inner) -} - -fn remove_code_actions_on_format_inner(value: &mut Value, path: &[&str]) -> Result<()> { - let Some(obj) = value.as_object_mut() else { - return Ok(()); - }; - let Some(code_actions_on_format) = obj.get("code_actions_on_format").cloned() else { - return Ok(()); - }; - - fn fmt_path(path: &[&str], key: &str) -> String { - let mut path = path.to_vec(); - path.push(key); - path.join(".") - } - - anyhow::ensure!( - code_actions_on_format.is_object(), - r#"The `code_actions_on_format` setting is deprecated, but it is in an invalid state and cannot be migrated at {}. Please ensure the code_actions_on_format setting is a Map"#, - fmt_path(path, "code_actions_on_format"), - ); - - let code_actions_map = code_actions_on_format.as_object().unwrap(); - let mut code_actions = vec![]; - for (code_action, code_action_enabled) in code_actions_map { - if code_action_enabled.as_bool().map_or(false, |b| !b) { - continue; - } - code_actions.push(code_action.clone()); - } - - let mut formatter_array = vec![]; - if let Some(formatter) = obj.get("formatter") { - if let Some(array) = formatter.as_array() { - formatter_array = array.clone(); - } else { - formatter_array.insert(0, formatter.clone()); - } - }; - let found_code_actions = !code_actions.is_empty(); - formatter_array.splice( - 0..0, - code_actions - .into_iter() - .map(|code_action| serde_json::json!({"code_action": code_action})), - ); - - obj.remove("code_actions_on_format"); - if found_code_actions { - obj.insert("formatter".to_string(), Value::Array(formatter_array)); - } - - Ok(()) -} diff --git a/crates/migrator/src/migrations/m_2025_10_16/settings.rs b/crates/migrator/src/migrations/m_2025_10_16/settings.rs new file mode 100644 index 0000000000000000000000000000000000000000..c9a8b7600cbf8842453318ed8dc9b47c3a73beaf --- /dev/null +++ b/crates/migrator/src/migrations/m_2025_10_16/settings.rs @@ -0,0 +1,68 @@ +use anyhow::Result; +use serde_json::Value; + +use crate::patterns::migrate_language_setting; + +pub fn restore_code_actions_on_format(value: &mut Value) -> Result<()> { + migrate_language_setting(value, restore_code_actions_on_format_inner) +} + +fn restore_code_actions_on_format_inner(value: &mut Value, path: &[&str]) -> Result<()> { + let Some(obj) = value.as_object_mut() else { + return Ok(()); + }; + let code_actions_on_format = obj + .get("code_actions_on_format") + .cloned() + .unwrap_or_else(|| Value::Object(Default::default())); + + fn fmt_path(path: &[&str], key: &str) -> String { + let mut path = path.to_vec(); + path.push(key); + path.join(".") + } + + let Some(mut code_actions_map) = code_actions_on_format.as_object().cloned() else { + anyhow::bail!( + r#"The `code_actions_on_format` is in an invalid state and cannot be migrated at {}. Please ensure the code_actions_on_format setting is a Map"#, + fmt_path(path, "code_actions_on_format"), + ); + }; + + let Some(formatter) = obj.get("formatter") else { + return Ok(()); + }; + let formatter_array = if let Some(array) = formatter.as_array() { + array.clone() + } else { + vec![formatter.clone()] + }; + let mut code_action_formatters = Vec::new(); + for formatter in formatter_array { + let Some(code_action) = formatter.get("code_action") else { + return Ok(()); + }; + let Some(code_action_name) = code_action.as_str() else { + anyhow::bail!( + r#"The `code_action` is in an invalid state and cannot be migrated at {}. Please ensure the code_action setting is a String"#, + fmt_path(path, "formatter"), + ); + }; + code_action_formatters.push(code_action_name.to_string()); + } + + code_actions_map.extend( + code_action_formatters + .into_iter() + .rev() + .map(|code_action| (code_action, Value::Bool(true))), + ); + + obj.remove("formatter"); + obj.insert( + "code_actions_on_format".into(), + Value::Object(code_actions_map), + ); + + Ok(()) +} diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index 7b4182b00509effb0d05c64ea737b1d615c5b467..7bb5deb9516668eee699d4078eda7dc74a92be2e 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -211,7 +211,7 @@ pub fn migrate_settings(text: &str) -> Result> { migrations::m_2025_10_03::SETTINGS_PATTERNS, &SETTINGS_QUERY_2025_10_03, ), - MigrationType::Json(migrations::m_2025_10_10::remove_code_actions_on_format), + MigrationType::Json(migrations::m_2025_10_16::restore_code_actions_on_format), ]; run_migrations(text, migrations) } @@ -362,6 +362,7 @@ mod tests { pretty_assertions::assert_eq!(migrated.as_deref(), output); } + #[track_caller] fn assert_migrate_settings(input: &str, output: Option<&str>) { let migrated = migrate_settings(input).unwrap(); assert_migrated_correctly(migrated, output); @@ -1337,7 +1338,10 @@ mod tests { #[test] fn test_flatten_code_action_formatters_basic_array() { - assert_migrate_settings( + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2025_10_01::flatten_code_actions_formatters, + )], &r#"{ "formatter": [ { @@ -1368,7 +1372,10 @@ mod tests { #[test] fn test_flatten_code_action_formatters_basic_object() { - assert_migrate_settings( + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2025_10_01::flatten_code_actions_formatters, + )], &r#"{ "formatter": { "code_actions": { @@ -1522,7 +1529,10 @@ mod tests { #[test] fn test_flatten_code_action_formatters_array_with_multiple_action_blocks_in_defaults_and_multiple_languages() { - assert_migrate_settings( + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2025_10_01::flatten_code_actions_formatters, + )], &r#"{ "formatter": { "code_actions": { @@ -1990,349 +2000,94 @@ mod tests { } #[test] - fn test_code_actions_on_format_migration_basic() { + fn test_restore_code_actions_on_format() { assert_migrate_settings_with_migrations( &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, + migrations::m_2025_10_16::restore_code_actions_on_format, )], &r#"{ - "code_actions_on_format": { - "source.organizeImports": true, - "source.fixAll": true - } - }"# - .unindent(), - Some( - &r#"{ - "formatter": [ - { - "code_action": "source.organizeImports" - }, - { - "code_action": "source.fixAll" - } - ] + "formatter": { + "code_action": "foo" } - "# - .unindent(), - ), - ); - } - - #[test] - fn test_code_actions_on_format_migration_filters_false_values() { - assert_migrate_settings_with_migrations( - &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, - )], - &r#"{ - "code_actions_on_format": { - "a": true, - "b": false, - "c": true - } - }"# - .unindent(), - Some( - &r#"{ - "formatter": [ - { - "code_action": "a" - }, - { - "code_action": "c" - } - ] - } - "# - .unindent(), - ), - ); - } - - #[test] - fn test_code_actions_on_format_migration_with_existing_formatter_object() { - assert_migrate_settings_with_migrations( - &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, - )], - &r#"{ - "formatter": "prettier", - "code_actions_on_format": { - "source.organizeImports": true - } - }"# - .unindent(), - Some( - &r#"{ - "formatter": [ - { - "code_action": "source.organizeImports" - }, - "prettier" - ] - }"# - .unindent(), - ), - ); - } - - #[test] - fn test_code_actions_on_format_migration_with_existing_formatter_array() { - assert_migrate_settings_with_migrations( - &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, - )], - &r#"{ - "formatter": ["prettier", {"language_server": "eslint"}], - "code_actions_on_format": { - "source.organizeImports": true, - "source.fixAll": true - } }"# .unindent(), Some( &r#"{ - "formatter": [ - { - "code_action": "source.organizeImports" - }, - { - "code_action": "source.fixAll" - }, - "prettier", - { - "language_server": "eslint" - } - ] - }"# - .unindent(), - ), - ); - } - - #[test] - fn test_code_actions_on_format_migration_in_languages() { - assert_migrate_settings_with_migrations( - &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, - )], - &r#"{ - "languages": { - "JavaScript": { - "code_actions_on_format": { - "source.fixAll.eslint": true - } - }, - "Go": { - "code_actions_on_format": { - "source.organizeImports": true - } + "code_actions_on_format": { + "foo": true } } - }"# - .unindent(), - Some( - &r#"{ - "languages": { - "JavaScript": { - "formatter": [ - { - "code_action": "source.fixAll.eslint" - } - ] - }, - "Go": { - "formatter": [ - { - "code_action": "source.organizeImports" - } - ] - } - } - }"# + "# .unindent(), ), ); - } - #[test] - fn test_code_actions_on_format_migration_in_languages_with_existing_formatter() { assert_migrate_settings_with_migrations( &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, + migrations::m_2025_10_16::restore_code_actions_on_format, )], &r#"{ - "languages": { - "JavaScript": { - "formatter": "prettier", - "code_actions_on_format": { - "source.fixAll.eslint": true, - "source.organizeImports": false - } - } - } + "formatter": [ + { "code_action": "foo" }, + "auto" + ] }"# .unindent(), - Some( - &r#"{ - "languages": { - "JavaScript": { - "formatter": [ - { - "code_action": "source.fixAll.eslint" - }, - "prettier" - ] - } - } - }"# - .unindent(), - ), + None, ); - } - #[test] - fn test_code_actions_on_format_migration_mixed_global_and_languages() { assert_migrate_settings_with_migrations( &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, + migrations::m_2025_10_16::restore_code_actions_on_format, )], &r#"{ - "formatter": "prettier", - "code_actions_on_format": { - "source.fixAll": true - }, - "languages": { - "Rust": { - "formatter": "rust-analyzer", - "code_actions_on_format": { - "source.organizeImports": true - } + "formatter": { + "code_action": "foo" }, - "Python": { - "code_actions_on_format": { - "source.organizeImports": true, - "source.fixAll": false - } + "code_actions_on_format": { + "bar": true, + "baz": false } - } }"# .unindent(), Some( &r#"{ - "formatter": [ - { - "code_action": "source.fixAll" - }, - "prettier" - ], - "languages": { - "Rust": { - "formatter": [ - { - "code_action": "source.organizeImports" - }, - "rust-analyzer" - ] - }, - "Python": { - "formatter": [ - { - "code_action": "source.organizeImports" - } - ] + "code_actions_on_format": { + "foo": true, + "bar": true, + "baz": false } - } }"# .unindent(), ), ); - } - - #[test] - fn test_code_actions_on_format_no_migration_when_not_present() { - assert_migrate_settings_with_migrations( - &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, - )], - &r#"{ - "formatter": ["prettier"] - }"# - .unindent(), - None, - ); - } - #[test] - fn test_code_actions_on_format_migration_all_false_values() { assert_migrate_settings_with_migrations( &[MigrationType::Json( - migrations::m_2025_10_10::remove_code_actions_on_format, + migrations::m_2025_10_16::restore_code_actions_on_format, )], &r#"{ + "formatter": [ + { "code_action": "foo" }, + { "code_action": "qux" }, + ], "code_actions_on_format": { - "a": false, - "b": false - }, - "formatter": "prettier" + "bar": true, + "baz": false + } }"# .unindent(), Some( &r#"{ - "formatter": "prettier" + "code_actions_on_format": { + "foo": true, + "qux": true, + "bar": true, + "baz": false + } }"# .unindent(), ), ); } - - #[test] - fn test_code_action_formatters_issue() { - assert_migrate_settings_with_migrations( - &[MigrationType::Json( - migrations::m_2025_10_01::flatten_code_actions_formatters, - )], - &r#" - { - "languages": { - "Python": { - "language_servers": ["ruff"], - "format_on_save": "on", - "formatter": [ - { - "code_actions": { - // Fix all auto-fixable lint violations - "source.fixAll.ruff": true, - // Organize imports - "source.organizeImports.ruff": true - } - } - ] - } - } - }"# - .unindent(), - Some( - &r#" - { - "languages": { - "Python": { - "language_servers": ["ruff"], - "format_on_save": "on", - "formatter": [ - { - "code_action": "source.fixAll.ruff" - }, - { - "code_action": "source.organizeImports.ruff" - } - ] - } - } - }"# - .unindent(), - ), - ); - } } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 34ea47dbc3a4b6f1215bc1c0342f208cabfb76f0..c33f922a62d6ce702748a92063c8c2a1a2095a56 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1333,6 +1333,32 @@ impl LocalLspStore { })?; } + // Formatter for `code_actions_on_format` that runs before + // the rest of the formatters + let mut code_actions_on_format_formatters = None; + let should_run_code_actions_on_format = !matches!( + (trigger, &settings.format_on_save), + (FormatTrigger::Save, &FormatOnSave::Off) + ); + if should_run_code_actions_on_format { + let have_code_actions_to_run_on_format = settings + .code_actions_on_format + .values() + .any(|enabled| *enabled); + if have_code_actions_to_run_on_format { + zlog::trace!(logger => "going to run code actions on format"); + code_actions_on_format_formatters = Some( + settings + .code_actions_on_format + .iter() + .filter_map(|(action, enabled)| enabled.then_some(action)) + .cloned() + .map(Formatter::CodeAction) + .collect::>(), + ); + } + } + let formatters = match (trigger, &settings.format_on_save) { (FormatTrigger::Save, FormatOnSave::Off) => &[], (FormatTrigger::Manual, _) | (FormatTrigger::Save, FormatOnSave::On) => { @@ -1340,6 +1366,11 @@ impl LocalLspStore { } }; + let formatters = code_actions_on_format_formatters + .iter() + .flatten() + .chain(formatters); + for formatter in formatters { let formatter = if formatter == &Formatter::Auto { if settings.prettier.allowed { diff --git a/crates/settings/src/settings_content/language.rs b/crates/settings/src/settings_content/language.rs index 0e3f29264bcc38ce178f7e65a0f4eb08dc83f6c4..5624147369a19283f03190696f51f1393e410ed8 100644 --- a/crates/settings/src/settings_content/language.rs +++ b/crates/settings/src/settings_content/language.rs @@ -318,6 +318,11 @@ pub struct LanguageSettingsContent { /// /// Default: true pub use_on_type_format: Option, + /// Which code actions to run on save after the formatter. + /// These are not run if formatting is off. + /// + /// Default: {} (or {"source.organizeImports": true} for Go). + pub code_actions_on_format: Option>, /// Whether to perform linked edits of associated ranges, if the language server supports it. /// For example, when editing opening tag, the contents of the closing tag will be edited as well. /// diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 1330ec62763c5d0a3361c580a666d1be1f1bffd3..b54231580f269e9313eadceddacd2177d9f85fa7 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -5437,6 +5437,27 @@ fn language_settings_data() -> Vec { metadata: None, files: USER | LOCAL, }), + SettingsPageItem::SettingItem(SettingItem { + title: "Code Actions On Format", + description: "Additional Code Actions To Run When Formatting", + field: Box::new( + SettingField { + pick: |settings_content| { + language_settings_field(settings_content, |language| { + &language.code_actions_on_format + }) + }, + pick_mut: |settings_content| { + language_settings_field_mut(settings_content, |language| { + &mut language.code_actions_on_format + }) + }, + } + .unimplemented(), + ), + metadata: None, + files: USER | LOCAL, + }), SettingsPageItem::SectionHeader("Autoclose"), SettingsPageItem::SettingItem(SettingItem { title: "Use Autoclose", diff --git a/docs/src/languages/javascript.md b/docs/src/languages/javascript.md index 6c7eff5f38977d2d52417fd3491fc4af6da52e9a..77cee5d9f4ac8e87b07f778528b1cf96676eaed8 100644 --- a/docs/src/languages/javascript.md +++ b/docs/src/languages/javascript.md @@ -49,8 +49,8 @@ You can configure Zed to format code using `eslint --fix` by running the ESLint { "languages": { "JavaScript": { - "formatter": { - "code_action": "source.fixAll.eslint" + "code_actions_on_format": { + "source.fixAll.eslint": true } } } @@ -63,8 +63,8 @@ You can also only execute a single ESLint rule when using `fixAll`: { "languages": { "JavaScript": { - "formatter": { - "code_action": "source.fixAll.eslint" + "code_actions_on_format": { + "source.fixAll.eslint": true } } }, @@ -92,8 +92,8 @@ the formatter: { "languages": { "JavaScript": { - "formatter": { - "code_action": "source.fixAll.eslint" + "code_actions_on_format": { + "source.fixAll.eslint": true } } }