Don't allow formatters in format on save (#39400)

Ben Kunkle and Smit created

Closes #ISSUE



Release Notes:

- settings: Removed support for having format steps in both the
`format_on_save` and `formatter` settings for languages.
`format_on_save` is now restricted to the values of `"on"` and `"off"`,
and all format steps should be set under the `formatter` key. If you
were using `format_on_save` but not `formatter` this will be migrated
for you, otherwise it will require a manual migration.

---------

Co-authored-by: Smit <smit@zed.dev>

Change summary

crates/migrator/src/migrations.rs                       |   6 
crates/migrator/src/migrations/m_2025_10_02/settings.rs |  50 +++
crates/migrator/src/migrator.rs                         | 177 ++++++++++
crates/onboarding/src/editing_page.rs                   |   2 
crates/project/src/lsp_store.rs                         |   1 
crates/settings/src/settings_content/language.rs        |  93 -----
6 files changed, 229 insertions(+), 100 deletions(-)

Detailed changes

crates/migrator/src/migrations.rs 🔗

@@ -105,3 +105,9 @@ pub(crate) mod m_2025_10_01 {
 
     pub(crate) use settings::SETTINGS_PATTERNS;
 }
+
+pub(crate) mod m_2025_10_02 {
+    mod settings;
+
+    pub(crate) use settings::remove_formatters_on_save;
+}

crates/migrator/src/migrations/m_2025_10_02/settings.rs 🔗

@@ -0,0 +1,50 @@
+use anyhow::Result;
+use serde_json::Value;
+
+pub fn remove_formatters_on_save(value: &mut Value) -> Result<()> {
+    remove_formatters_on_save_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_formatters_on_save_inner(language, &path)?;
+        }
+    }
+    Ok(())
+}
+
+fn remove_formatters_on_save_inner(value: &mut Value, path: &[&str]) -> Result<()> {
+    let Some(obj) = value.as_object_mut() else {
+        return Ok(());
+    };
+    let Some(format_on_save) = obj.get("format_on_save").cloned() else {
+        return Ok(());
+    };
+    let is_format_on_save_set_to_formatter = format_on_save
+        .as_str()
+        .map_or(true, |s| s != "on" && s != "off");
+    if !is_format_on_save_set_to_formatter {
+        return Ok(());
+    }
+
+    fn fmt_path(path: &[&str], key: &str) -> String {
+        let mut path = path.to_vec();
+        path.push(key);
+        path.join(".")
+    }
+
+    anyhow::ensure!(
+        obj.get("formatter").is_none(),
+        r#"Setting formatters in both "format_on_save" and "formatter" is deprecated. Please migrate the formatters from {} into {}"#,
+        fmt_path(path, "format_on_save"),
+        fmt_path(path, "formatter")
+    );
+
+    obj.insert("format_on_save".to_string(), serde_json::json!("on"));
+    obj.insert("formatter".to_string(), format_on_save);
+
+    Ok(())
+}

crates/migrator/src/migrator.rs 🔗

@@ -76,7 +76,7 @@ fn run_migrations(text: &str, migrations: &[MigrationType]) -> Result<Option<Str
                     settings::parse_json_with_comments(&current_text)?;
                 let old_value = serde_json::to_value(&old_content).unwrap();
                 let mut new_value = old_value.clone();
-                callback(&mut new_value);
+                callback(&mut new_value)?;
                 if new_value != old_value {
                     let mut current = current_text.clone();
                     let mut edits = vec![];
@@ -134,8 +134,7 @@ pub fn migrate_keymap(text: &str) -> Result<Option<String>> {
 
 enum MigrationType<'a> {
     TreeSitter(MigrationPatterns, &'a Query),
-    #[allow(unused)]
-    Json(fn(&mut serde_json::Value)),
+    Json(fn(&mut serde_json::Value) -> Result<()>),
 }
 
 pub fn migrate_settings(text: &str) -> Result<Option<String>> {
@@ -200,6 +199,7 @@ pub fn migrate_settings(text: &str) -> Result<Option<String>> {
             migrations::m_2025_10_01::SETTINGS_PATTERNS,
             &SETTINGS_QUERY_2025_10_01,
         ),
+        MigrationType::Json(migrations::m_2025_10_02::remove_formatters_on_save),
     ];
     run_migrations(text, migrations)
 }
@@ -360,7 +360,7 @@ mod tests {
         output: Option<&str>,
     ) {
         let migrated = run_migrations(input, migrations).unwrap();
-        pretty_assertions::assert_eq!(migrated.as_deref(), output);
+        assert_migrated_correctly(migrated, output);
     }
 
     #[test]
@@ -1486,7 +1486,7 @@ mod tests {
                         "default-3": true,
                         "default-4": true,
                     }
-                }
+                },
                 "languages": {
                     "Rust": {
                         "formatter": [
@@ -1549,7 +1549,7 @@ mod tests {
                       { "code_action": "default-2" },
                       { "code_action": "default-3" },
                       { "code_action": "default-4" }
-                    ]
+                    ],
                     "languages": {
                         "Rust": {
                             "formatter": [
@@ -1580,7 +1580,11 @@ mod tests {
 
     #[test]
     fn test_flatten_code_action_formatters_array_with_format_on_save_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": {
@@ -1728,4 +1732,163 @@ mod tests {
             ),
         );
     }
+
+    #[test]
+    fn test_format_on_save_formatter_migration_basic() {
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                  "format_on_save": "prettier"
+              }"#
+            .unindent(),
+            Some(
+                &r#"{
+                      "formatter": "prettier",
+                      "format_on_save": "on"
+                  }"#
+                .unindent(),
+            ),
+        );
+    }
+
+    #[test]
+    fn test_format_on_save_formatter_migration_array() {
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                "format_on_save": ["prettier", {"language_server": "eslint"}]
+            }"#
+            .unindent(),
+            Some(
+                &r#"{
+                    "formatter": [
+                        "prettier",
+                        {
+                            "language_server": "eslint"
+                        }
+                    ],
+                    "format_on_save": "on"
+                }"#
+                .unindent(),
+            ),
+        );
+    }
+
+    #[test]
+    fn test_format_on_save_on_off_unchanged() {
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                "format_on_save": "on"
+            }"#
+            .unindent(),
+            None,
+        );
+
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                "format_on_save": "off"
+            }"#
+            .unindent(),
+            None,
+        );
+    }
+
+    #[test]
+    fn test_format_on_save_formatter_migration_in_languages() {
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                "languages": {
+                    "Rust": {
+                        "format_on_save": "rust-analyzer"
+                    },
+                    "Python": {
+                        "format_on_save": ["ruff", "black"]
+                    }
+                }
+            }"#
+            .unindent(),
+            Some(
+                &r#"{
+                    "languages": {
+                        "Rust": {
+                            "formatter": "rust-analyzer",
+                            "format_on_save": "on"
+                        },
+                        "Python": {
+                            "formatter": [
+                                "ruff",
+                                "black"
+                            ],
+                            "format_on_save": "on"
+                        }
+                    }
+                }"#
+                .unindent(),
+            ),
+        );
+    }
+
+    #[test]
+    fn test_format_on_save_formatter_migration_mixed_global_and_languages() {
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                "format_on_save": "prettier",
+                "languages": {
+                    "Rust": {
+                        "format_on_save": "rust-analyzer"
+                    },
+                    "Python": {
+                        "format_on_save": "on"
+                    }
+                }
+            }"#
+            .unindent(),
+            Some(
+                &r#"{
+                    "formatter": "prettier",
+                    "format_on_save": "on",
+                    "languages": {
+                        "Rust": {
+                            "formatter": "rust-analyzer",
+                            "format_on_save": "on"
+                        },
+                        "Python": {
+                            "format_on_save": "on"
+                        }
+                    }
+                }"#
+                .unindent(),
+            ),
+        );
+    }
+
+    #[test]
+    fn test_format_on_save_no_migration_when_no_format_on_save() {
+        assert_migrate_settings_with_migrations(
+            &[MigrationType::Json(
+                migrations::m_2025_10_02::remove_formatters_on_save,
+            )],
+            &r#"{
+                "formatter": ["prettier"]
+            }"#
+            .unindent(),
+            None,
+        );
+    }
 }

crates/onboarding/src/editing_page.rs 🔗

@@ -167,7 +167,7 @@ fn write_font_ligatures(enabled: bool, cx: &mut App) {
 
 fn read_format_on_save(cx: &App) -> bool {
     match AllLanguageSettings::get_global(cx).defaults.format_on_save {
-        FormatOnSave::On | FormatOnSave::List(_) => true,
+        FormatOnSave::On => true,
         FormatOnSave::Off => false,
     }
 }

crates/project/src/lsp_store.rs 🔗

@@ -1366,7 +1366,6 @@ impl LocalLspStore {
 
         let formatters = match (trigger, &settings.format_on_save) {
             (FormatTrigger::Save, FormatOnSave::Off) => &[],
-            (FormatTrigger::Save, FormatOnSave::List(formatters)) => formatters.as_ref(),
             (FormatTrigger::Manual, _) | (FormatTrigger::Save, FormatOnSave::On) => {
                 match &settings.formatter {
                     SelectedFormatter::Auto => {

crates/settings/src/settings_content/language.rs 🔗

@@ -569,102 +569,13 @@ pub struct PrettierSettingsContent {
 }
 
 /// Controls the behavior of formatting files when they are saved.
-#[derive(Debug, Clone, PartialEq, Eq, MergeFrom)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, MergeFrom)]
+#[serde(rename_all = "lowercase")]
 pub enum FormatOnSave {
     /// Files should be formatted on save.
     On,
     /// Files should not be formatted on save.
     Off,
-    List(FormatterList),
-}
-
-impl JsonSchema for FormatOnSave {
-    fn schema_name() -> Cow<'static, str> {
-        "OnSaveFormatter".into()
-    }
-
-    fn json_schema(generator: &mut schemars::SchemaGenerator) -> schemars::Schema {
-        let formatter_schema = Formatter::json_schema(generator);
-
-        json_schema!({
-            "oneOf": [
-                {
-                    "type": "array",
-                    "items": formatter_schema
-                },
-                {
-                    "type": "string",
-                    "enum": ["on", "off", "language_server"]
-                },
-                formatter_schema
-            ]
-        })
-    }
-}
-
-impl Serialize for FormatOnSave {
-    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        match self {
-            Self::On => serializer.serialize_str("on"),
-            Self::Off => serializer.serialize_str("off"),
-            Self::List(list) => list.serialize(serializer),
-        }
-    }
-}
-
-impl<'de> Deserialize<'de> for FormatOnSave {
-    fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        struct FormatDeserializer;
-
-        impl<'d> Visitor<'d> for FormatDeserializer {
-            type Value = FormatOnSave;
-
-            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-                formatter.write_str("a valid on-save formatter kind")
-            }
-            fn visit_str<E>(self, v: &str) -> std::result::Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                if v == "on" {
-                    Ok(Self::Value::On)
-                } else if v == "off" {
-                    Ok(Self::Value::Off)
-                } else if v == "language_server" {
-                    Ok(Self::Value::List(FormatterList::Single(
-                        Formatter::LanguageServer { name: None },
-                    )))
-                } else {
-                    let ret: Result<FormatterList, _> =
-                        Deserialize::deserialize(v.into_deserializer());
-                    ret.map(Self::Value::List)
-                }
-            }
-            fn visit_map<A>(self, map: A) -> Result<Self::Value, A::Error>
-            where
-                A: MapAccess<'d>,
-            {
-                let ret: Result<FormatterList, _> =
-                    Deserialize::deserialize(de::value::MapAccessDeserializer::new(map));
-                ret.map(Self::Value::List)
-            }
-            fn visit_seq<A>(self, map: A) -> Result<Self::Value, A::Error>
-            where
-                A: SeqAccess<'d>,
-            {
-                let ret: Result<FormatterList, _> =
-                    Deserialize::deserialize(de::value::SeqAccessDeserializer::new(map));
-                ret.map(Self::Value::List)
-            }
-        }
-        deserializer.deserialize_any(FormatDeserializer)
-    }
 }
 
 /// Controls which formatter should be used when formatting code.