Fix language settings formatter regression - formatter list can be a single formatter not wrapped in an array (#33721)

Michael Sloan created

Fixes a regression from #33635

Release Notes:

- N/A

Change summary

crates/collab/src/tests/integration_tests.rs                  | 18 
crates/collab/src/tests/remote_editing_collaboration_tests.rs | 10 
crates/editor/src/editor_tests.rs                             | 24 
crates/language/src/language_settings.rs                      | 67 +++-
crates/project/src/lsp_store.rs                               |  4 
crates/project/src/prettier_store.rs                          |  1 
6 files changed, 72 insertions(+), 52 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs 🔗

@@ -22,7 +22,9 @@ use gpui::{
 use language::{
     Diagnostic, DiagnosticEntry, DiagnosticSourceKind, FakeLspAdapter, Language, LanguageConfig,
     LanguageMatcher, LineEnding, OffsetRangeExt, Point, Rope,
-    language_settings::{AllLanguageSettings, Formatter, PrettierSettings, SelectedFormatter},
+    language_settings::{
+        AllLanguageSettings, Formatter, FormatterList, PrettierSettings, SelectedFormatter,
+    },
     tree_sitter_rust, tree_sitter_typescript,
 };
 use lsp::{LanguageServerId, OneOf};
@@ -4589,13 +4591,14 @@ async fn test_formatting_buffer(
         cx_a.update(|cx| {
             SettingsStore::update_global(cx, |store, cx| {
                 store.update_user_settings::<AllLanguageSettings>(cx, |file| {
-                    file.defaults.formatter =
-                        Some(SelectedFormatter::List(vec![Formatter::External {
+                    file.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Single(
+                        Formatter::External {
                             command: "awk".into(),
                             arguments: Some(
                                 vec!["{sub(/two/,\"{buffer_path}\")}1".to_string()].into(),
                             ),
-                        }]));
+                        },
+                    )));
                 });
             });
         });
@@ -4695,10 +4698,9 @@ async fn test_prettier_formatting_buffer(
     cx_b.update(|cx| {
         SettingsStore::update_global(cx, |store, cx| {
             store.update_user_settings::<AllLanguageSettings>(cx, |file| {
-                file.defaults.formatter =
-                    Some(SelectedFormatter::List(vec![Formatter::LanguageServer {
-                        name: None,
-                    }]));
+                file.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Single(
+                    Formatter::LanguageServer { name: None },
+                )));
                 file.defaults.prettier = Some(PrettierSettings {
                     allowed: true,
                     ..PrettierSettings::default()

crates/collab/src/tests/remote_editing_collaboration_tests.rs 🔗

@@ -14,7 +14,8 @@ use http_client::BlockedHttpClient;
 use language::{
     FakeLspAdapter, Language, LanguageConfig, LanguageMatcher, LanguageRegistry,
     language_settings::{
-        AllLanguageSettings, Formatter, PrettierSettings, SelectedFormatter, language_settings,
+        AllLanguageSettings, Formatter, FormatterList, PrettierSettings, SelectedFormatter,
+        language_settings,
     },
     tree_sitter_typescript,
 };
@@ -504,10 +505,9 @@ async fn test_ssh_collaboration_formatting_with_prettier(
     cx_b.update(|cx| {
         SettingsStore::update_global(cx, |store, cx| {
             store.update_user_settings::<AllLanguageSettings>(cx, |file| {
-                file.defaults.formatter =
-                    Some(SelectedFormatter::List(vec![Formatter::LanguageServer {
-                        name: None,
-                    }]));
+                file.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Single(
+                    Formatter::LanguageServer { name: None },
+                )));
                 file.defaults.prettier = Some(PrettierSettings {
                     allowed: true,
                     ..PrettierSettings::default()

crates/editor/src/editor_tests.rs 🔗

@@ -25,8 +25,8 @@ use language::{
     DiagnosticSourceKind, FakeLspAdapter, LanguageConfig, LanguageConfigOverride, LanguageMatcher,
     LanguageName, Override, Point,
     language_settings::{
-        AllLanguageSettings, AllLanguageSettingsContent, CompletionSettings,
-        LanguageSettingsContent, LspInsertMode, PrettierSettings,
+        AllLanguageSettings, AllLanguageSettingsContent, CompletionSettings, FormatterList,
+        LanguageSettingsContent, LspInsertMode, PrettierSettings, SelectedFormatter,
     },
     tree_sitter_python,
 };
@@ -10012,9 +10012,9 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
 #[gpui::test]
 async fn test_document_format_manual_trigger(cx: &mut TestAppContext) {
     init_test(cx, |settings| {
-        settings.defaults.formatter = Some(language_settings::SelectedFormatter::List(vec![
+        settings.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Single(
             Formatter::LanguageServer { name: None },
-        ]))
+        )))
     });
 
     let fs = FakeFs::new(cx.executor());
@@ -10141,7 +10141,7 @@ async fn test_document_format_manual_trigger(cx: &mut TestAppContext) {
 async fn test_multiple_formatters(cx: &mut TestAppContext) {
     init_test(cx, |settings| {
         settings.defaults.remove_trailing_whitespace_on_save = Some(true);
-        settings.defaults.formatter = Some(language_settings::SelectedFormatter::List(vec![
+        settings.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Vec(vec![
             Formatter::LanguageServer { name: None },
             Formatter::CodeActions(
                 [
@@ -10151,7 +10151,7 @@ async fn test_multiple_formatters(cx: &mut TestAppContext) {
                 .into_iter()
                 .collect(),
             ),
-        ]))
+        ])))
     });
 
     let fs = FakeFs::new(cx.executor());
@@ -10403,9 +10403,9 @@ async fn test_multiple_formatters(cx: &mut TestAppContext) {
 #[gpui::test]
 async fn test_organize_imports_manual_trigger(cx: &mut TestAppContext) {
     init_test(cx, |settings| {
-        settings.defaults.formatter = Some(language_settings::SelectedFormatter::List(vec![
+        settings.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Vec(vec![
             Formatter::LanguageServer { name: None },
-        ]))
+        ])))
     });
 
     let fs = FakeFs::new(cx.executor());
@@ -10611,7 +10611,7 @@ async fn test_concurrent_format_requests(cx: &mut TestAppContext) {
 #[gpui::test]
 async fn test_strip_whitespace_and_format_via_lsp(cx: &mut TestAppContext) {
     init_test(cx, |settings| {
-        settings.defaults.formatter = Some(language_settings::SelectedFormatter::Auto)
+        settings.defaults.formatter = Some(SelectedFormatter::Auto)
     });
 
     let mut cx = EditorLspTestContext::new_rust(
@@ -15878,9 +15878,9 @@ fn completion_menu_entries(menu: &CompletionsMenu) -> Vec<String> {
 #[gpui::test]
 async fn test_document_format_with_prettier(cx: &mut TestAppContext) {
     init_test(cx, |settings| {
-        settings.defaults.formatter = Some(language_settings::SelectedFormatter::List(vec![
+        settings.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Single(
             Formatter::Prettier,
-        ]))
+        )))
     });
 
     let fs = FakeFs::new(cx.executor());
@@ -15950,7 +15950,7 @@ async fn test_document_format_with_prettier(cx: &mut TestAppContext) {
     );
 
     update_test_language_settings(cx, |settings| {
-        settings.defaults.formatter = Some(language_settings::SelectedFormatter::Auto)
+        settings.defaults.formatter = Some(SelectedFormatter::Auto)
     });
     let format = editor.update_in(cx, |editor, window, cx| {
         editor.perform_format(

crates/language/src/language_settings.rs 🔗

@@ -21,7 +21,7 @@ use settings::{
     replace_subschema,
 };
 use shellexpand;
-use std::{borrow::Cow, num::NonZeroU32, path::Path, sync::Arc};
+use std::{borrow::Cow, num::NonZeroU32, path::Path, slice, sync::Arc};
 use util::serde::default_true;
 
 /// Initializes the language settings.
@@ -673,7 +673,7 @@ pub enum FormatOnSave {
     On,
     /// Files should not be formatted on save.
     Off,
-    List(Vec<Formatter>),
+    List(FormatterList),
 }
 
 impl JsonSchema for FormatOnSave {
@@ -692,7 +692,7 @@ impl JsonSchema for FormatOnSave {
                 },
                 {
                     "type": "string",
-                    "enum": ["on", "off", "prettier", "language_server"]
+                    "enum": ["on", "off", "language_server"]
                 },
                 formatter_schema
             ]
@@ -735,11 +735,11 @@ impl<'de> Deserialize<'de> for FormatOnSave {
                 } else if v == "off" {
                     Ok(Self::Value::Off)
                 } else if v == "language_server" {
-                    Ok(Self::Value::List(vec![Formatter::LanguageServer {
-                        name: None,
-                    }]))
+                    Ok(Self::Value::List(FormatterList::Single(
+                        Formatter::LanguageServer { name: None },
+                    )))
                 } else {
-                    let ret: Result<Vec<Formatter>, _> =
+                    let ret: Result<FormatterList, _> =
                         Deserialize::deserialize(v.into_deserializer());
                     ret.map(Self::Value::List)
                 }
@@ -748,7 +748,7 @@ impl<'de> Deserialize<'de> for FormatOnSave {
             where
                 A: MapAccess<'d>,
             {
-                let ret: Result<Vec<Formatter>, _> =
+                let ret: Result<FormatterList, _> =
                     Deserialize::deserialize(de::value::MapAccessDeserializer::new(map));
                 ret.map(Self::Value::List)
             }
@@ -756,7 +756,7 @@ impl<'de> Deserialize<'de> for FormatOnSave {
             where
                 A: SeqAccess<'d>,
             {
-                let ret: Result<Vec<Formatter>, _> =
+                let ret: Result<FormatterList, _> =
                     Deserialize::deserialize(de::value::SeqAccessDeserializer::new(map));
                 ret.map(Self::Value::List)
             }
@@ -793,7 +793,7 @@ pub enum SelectedFormatter {
     /// or falling back to formatting via language server.
     #[default]
     Auto,
-    List(Vec<Formatter>),
+    List(FormatterList),
 }
 
 impl JsonSchema for SelectedFormatter {
@@ -812,7 +812,7 @@ impl JsonSchema for SelectedFormatter {
                 },
                 {
                     "type": "string",
-                    "enum": ["auto", "prettier", "language_server"]
+                    "enum": ["auto", "language_server"]
                 },
                 formatter_schema
             ]
@@ -852,11 +852,11 @@ impl<'de> Deserialize<'de> for SelectedFormatter {
                 if v == "auto" {
                     Ok(Self::Value::Auto)
                 } else if v == "language_server" {
-                    Ok(Self::Value::List(vec![Formatter::LanguageServer {
-                        name: None,
-                    }]))
+                    Ok(Self::Value::List(FormatterList::Single(
+                        Formatter::LanguageServer { name: None },
+                    )))
                 } else {
-                    let ret: Result<Vec<Formatter>, _> =
+                    let ret: Result<FormatterList, _> =
                         Deserialize::deserialize(v.into_deserializer());
                     ret.map(SelectedFormatter::List)
                 }
@@ -865,7 +865,7 @@ impl<'de> Deserialize<'de> for SelectedFormatter {
             where
                 A: MapAccess<'d>,
             {
-                let ret: Result<Vec<Formatter>, _> =
+                let ret: Result<FormatterList, _> =
                     Deserialize::deserialize(de::value::MapAccessDeserializer::new(map));
                 ret.map(SelectedFormatter::List)
             }
@@ -873,7 +873,7 @@ impl<'de> Deserialize<'de> for SelectedFormatter {
             where
                 A: SeqAccess<'d>,
             {
-                let ret: Result<Vec<Formatter>, _> =
+                let ret: Result<FormatterList, _> =
                     Deserialize::deserialize(de::value::SeqAccessDeserializer::new(map));
                 ret.map(SelectedFormatter::List)
             }
@@ -882,6 +882,23 @@ impl<'de> Deserialize<'de> for SelectedFormatter {
     }
 }
 
+/// Controls which formatters should be used when formatting code.
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
+#[serde(untagged)]
+pub enum FormatterList {
+    Single(Formatter),
+    Vec(Vec<Formatter>),
+}
+
+impl AsRef<[Formatter]> for FormatterList {
+    fn as_ref(&self) -> &[Formatter] {
+        match &self {
+            Self::Single(single) => slice::from_ref(single),
+            Self::Vec(v) => v,
+        }
+    }
+}
+
 /// Controls which formatter should be used when formatting code. If there are multiple formatters, they are executed in the order of declaration.
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
 #[serde(rename_all = "snake_case")]
@@ -1612,26 +1629,26 @@ mod tests {
         let settings: LanguageSettingsContent = serde_json::from_str(raw).unwrap();
         assert_eq!(
             settings.formatter,
-            Some(SelectedFormatter::List(vec![Formatter::LanguageServer {
-                name: None
-            }]))
+            Some(SelectedFormatter::List(FormatterList::Single(
+                Formatter::LanguageServer { name: None }
+            )))
         );
         let raw = "{\"formatter\": [{\"language_server\": {\"name\": null}}]}";
         let settings: LanguageSettingsContent = serde_json::from_str(raw).unwrap();
         assert_eq!(
             settings.formatter,
-            Some(SelectedFormatter::List(vec![Formatter::LanguageServer {
-                name: None
-            }]))
+            Some(SelectedFormatter::List(FormatterList::Vec(vec![
+                Formatter::LanguageServer { name: None }
+            ])))
         );
         let raw = "{\"formatter\": [{\"language_server\": {\"name\": null}}, \"prettier\"]}";
         let settings: LanguageSettingsContent = serde_json::from_str(raw).unwrap();
         assert_eq!(
             settings.formatter,
-            Some(SelectedFormatter::List(vec![
+            Some(SelectedFormatter::List(FormatterList::Vec(vec![
                 Formatter::LanguageServer { name: None },
                 Formatter::Prettier
-            ]))
+            ])))
         );
     }
 

crates/project/src/lsp_store.rs 🔗

@@ -1405,7 +1405,7 @@ impl LocalLspStore {
 
         let formatters = match (trigger, &settings.format_on_save) {
             (FormatTrigger::Save, FormatOnSave::Off) => &[],
-            (FormatTrigger::Save, FormatOnSave::List(formatters)) => formatters.as_slice(),
+            (FormatTrigger::Save, FormatOnSave::List(formatters)) => formatters.as_ref(),
             (FormatTrigger::Manual, _) | (FormatTrigger::Save, FormatOnSave::On) => {
                 match &settings.formatter {
                     SelectedFormatter::Auto => {
@@ -1417,7 +1417,7 @@ impl LocalLspStore {
                             std::slice::from_ref(&Formatter::LanguageServer { name: None })
                         }
                     }
-                    SelectedFormatter::List(formatter_list) => formatter_list.as_slice(),
+                    SelectedFormatter::List(formatter_list) => formatter_list.as_ref(),
                 }
             }
         };

crates/project/src/prettier_store.rs 🔗

@@ -705,6 +705,7 @@ pub fn prettier_plugins_for_language(
         SelectedFormatter::Auto => Some(&language_settings.prettier.plugins),
 
         SelectedFormatter::List(list) => list
+            .as_ref()
             .contains(&Formatter::Prettier)
             .then_some(&language_settings.prettier.plugins),
     }