From fe00b6cb539b83040029d94a2674c66ada41d82b 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 | 70 ---- .../src/migrations/m_2025_10_16/settings.rs | 68 ++++ crates/migrator/src/migrator.rs | 301 ++++-------------- crates/migrator/src/patterns.rs | 1 + crates/migrator/src/patterns/settings.rs | 21 ++ 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 +- 12 files changed, 216 insertions(+), 326 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 995c3901c789912c691950ac0ea3da4855ab2cec..f9711908e465b20208ae2ec55d4778c9fc3587f2 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1527,6 +1527,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": {}, @@ -1696,7 +1697,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 1b8ede68b1eb8686325c896723d1fdc762d02b73..1936c6d7ac179c33474186f0c0e704afc3df5e00 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 1d07be71a139b60e4b362d26c68b25922f04a233..0000000000000000000000000000000000000000 --- a/crates/migrator/src/migrations/m_2025_10_10/settings.rs +++ /dev/null @@ -1,70 +0,0 @@ -use anyhow::Result; -use serde_json::Value; - -pub fn remove_code_actions_on_format(value: &mut Value) -> Result<()> { - remove_code_actions_on_format_inner(value, &[])?; - let languages = value - .as_object_mut() - .and_then(|obj| obj.get_mut("languages")) - .and_then(|languages| languages.as_object_mut()); - if let Some(languages) = languages { - for (language_name, language) in languages.iter_mut() { - let path = vec!["languages", language_name]; - remove_code_actions_on_format_inner(language, &path)?; - } - } - Ok(()) -} - -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 aea11f98c460cc6c72120138ca1068be9ea60923..0adc4d14276e8d49d44af5cc29daff13657e3b8b 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -213,7 +213,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) } @@ -367,6 +367,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); @@ -1341,7 +1342,11 @@ mod tests { #[test] fn test_flatten_code_action_formatters_basic_array() { - assert_migrate_settings( + assert_migrate_settings_with_migrations( + &[MigrationType::TreeSitter( + migrations::m_2025_10_01::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_10_01, + )], &r#"{ "formatter": [ { @@ -1368,7 +1373,11 @@ mod tests { #[test] fn test_flatten_code_action_formatters_basic_object() { - assert_migrate_settings( + assert_migrate_settings_with_migrations( + &[MigrationType::TreeSitter( + migrations::m_2025_10_01::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_10_01, + )], &r#"{ "formatter": { "code_actions": { @@ -1500,7 +1509,11 @@ 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::TreeSitter( + migrations::m_2025_10_01::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_10_01, + )], &r#"{ "formatter": { "code_actions": { @@ -1916,297 +1929,91 @@ 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" - } - ] - } - "# - .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 + "formatter": { + "code_action": "foo" } }"# .unindent(), Some( &r#"{ - "formatter": [ - { - "code_action": "a" - }, - { - "code_action": "c" - } - ] + "code_actions_on_format": { + "foo": true + } } "# .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, + migrations::m_2025_10_16::restore_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 - } + "formatter": [ + { "code_action": "foo" }, + "auto" + ] }"# .unindent(), - Some( - &r#"{ - "formatter": [ - { - "code_action": "source.organizeImports" - }, - { - "code_action": "source.fixAll" - }, - "prettier", - { - "language_server": "eslint" - } - ] - }"# - .unindent(), - ), + None, ); - } - #[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, + migrations::m_2025_10_16::restore_code_actions_on_format, )], &r#"{ - "languages": { - "JavaScript": { - "code_actions_on_format": { - "source.fixAll.eslint": true - } - }, - "Go": { - "code_actions_on_format": { - "source.organizeImports": true - } - } + "formatter": { + "code_action": "foo" + }, + "code_actions_on_format": { + "bar": true, + "baz": false } }"# .unindent(), Some( &r#"{ - "languages": { - "JavaScript": { - "formatter": [ - { - "code_action": "source.fixAll.eslint" - } - ] - }, - "Go": { - "formatter": [ - { - "code_action": "source.organizeImports" - } - ] - } + "code_actions_on_format": { + "foo": true, + "bar": true, + "baz": false } }"# .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" }, + { "code_action": "qux" }, + ], + "code_actions_on_format": { + "bar": true, + "baz": false } - } }"# .unindent(), Some( &r#"{ - "languages": { - "JavaScript": { - "formatter": [ - { - "code_action": "source.fixAll.eslint" - }, - "prettier" - ] + "code_actions_on_format": { + "foo": true, + "qux": true, + "bar": true, + "baz": false } - } - }"# - .unindent(), - ), - ); - } - - #[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, - )], - &r#"{ - "formatter": "prettier", - "code_actions_on_format": { - "source.fixAll": true - }, - "languages": { - "Rust": { - "formatter": "rust-analyzer", - "code_actions_on_format": { - "source.organizeImports": true - } - }, - "Python": { - "code_actions_on_format": { - "source.organizeImports": true, - "source.fixAll": 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" - } - ] - } - } - }"# - .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, - )], - &r#"{ - "code_actions_on_format": { - "a": false, - "b": false - }, - "formatter": "prettier" - }"# - .unindent(), - Some( - &r#"{ - "formatter": "prettier" }"# .unindent(), ), diff --git a/crates/migrator/src/patterns.rs b/crates/migrator/src/patterns.rs index 3848baf23ba0d324995f18e3a53921948291153b..4132c93d9367a8dee200200e03dcc46ee073e67f 100644 --- a/crates/migrator/src/patterns.rs +++ b/crates/migrator/src/patterns.rs @@ -10,4 +10,5 @@ pub(crate) use settings::{ SETTINGS_ASSISTANT_PATTERN, SETTINGS_ASSISTANT_TOOLS_PATTERN, SETTINGS_DUPLICATED_AGENT_PATTERN, SETTINGS_EDIT_PREDICTIONS_ASSISTANT_PATTERN, SETTINGS_LANGUAGES_PATTERN, SETTINGS_NESTED_KEY_VALUE_PATTERN, SETTINGS_ROOT_KEY_VALUE_PATTERN, + migrate_language_setting, }; diff --git a/crates/migrator/src/patterns/settings.rs b/crates/migrator/src/patterns/settings.rs index 72fd02b153a5cf6e3158790f1c5d09a9f643ebf9..a068cce23b013a3435188c03ceebe866883c4e6d 100644 --- a/crates/migrator/src/patterns/settings.rs +++ b/crates/migrator/src/patterns/settings.rs @@ -108,3 +108,24 @@ pub const SETTINGS_DUPLICATED_AGENT_PATTERN: &str = r#"(document (#eq? @agent1 "agent") (#eq? @agent2 "agent") )"#; + +/// Migrate language settings, +/// calls `migrate_fn` with the top level object as well as all language settings under the "languages" key +/// Fails early if `migrate_fn` returns an error at any point +pub fn migrate_language_setting( + value: &mut serde_json::Value, + migrate_fn: fn(&mut serde_json::Value, path: &[&str]) -> anyhow::Result<()>, +) -> anyhow::Result<()> { + migrate_fn(value, &[])?; + let languages = value + .as_object_mut() + .and_then(|obj| obj.get_mut("languages")) + .and_then(|languages| languages.as_object_mut()); + if let Some(languages) = languages { + for (language_name, language) in languages.iter_mut() { + let path = vec!["languages", language_name]; + migrate_fn(language, &path)?; + } + } + Ok(()) +} diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 80d32e16690bf1bcb7bdec9d868193dd972f3614..fe2d6d3e393e99f6c8bd304db4a156c58659a885 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 bc932b605d518d90be7fa8ab1454de09c13bbffc..b93f214249696033f00b2f9908e477aa3a30aa8f 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -5416,6 +5416,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 } } }