migrator: Do some cleanup (#24687)

This PR does some clean up of the `migrator` crate:

- Remove `.unwrap`s
- Don't suppress `rustfmt`

Release Notes:

- N/A

Cargo.lock

@@ -7878,6 +7878,7 @@ dependencies = [
 name = "migrator"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "collections",
  "convert_case 0.7.1",
  "log",

crates/migrator/Cargo.toml

@@ -13,6 +13,7 @@ path = "src/migrator.rs"
 doctest = false
 
 [dependencies]
+anyhow.workspace = true
 collections.workspace = true
 convert_case.workspace = true
 log.workspace = true

crates/migrator/src/migrator.rs

@@ -1,15 +1,16 @@
+use anyhow::{Context, Result};
 use collections::HashMap;
 use convert_case::{Case, Casing};
 use std::{cmp::Reverse, ops::Range, sync::LazyLock};
 use streaming_iterator::StreamingIterator;
 use tree_sitter::{Query, QueryMatch};
 
-fn migrate(text: &str, patterns: MigrationPatterns, query: &Query) -> Option<String> {
+fn migrate(text: &str, patterns: MigrationPatterns, query: &Query) -> Result<Option<String>> {
     let mut parser = tree_sitter::Parser::new();
-    parser
+    parser.set_language(&tree_sitter_json::LANGUAGE.into())?;
-        .set_language(&tree_sitter_json::LANGUAGE.into())
+    let syntax_tree = parser
-        .unwrap();
+        .parse(&text, None)
-    let syntax_tree = parser.parse(&text, None).unwrap();
+        .context("failed to parse settings")?;
 
     let mut cursor = tree_sitter::QueryCursor::new();
     let mut matches = cursor.matches(query, syntax_tree.root_node(), text.as_bytes());
@@ -27,7 +28,7 @@ fn migrate(text: &str, patterns: MigrationPatterns, query: &Query) -> Option<Str
     });
 
     if edits.is_empty() {
-        None
+        Ok(None)
     } else {
         let mut new_text = text.to_string();
         for (range, replacement) in edits.iter().rev() {
@@ -38,28 +39,28 @@ fn migrate(text: &str, patterns: MigrationPatterns, query: &Query) -> Option<Str
                 "Edits computed for configuration migration do not cause a change: {:?}",
                 edits
             );
-            None
+            Ok(None)
         } else {
-            Some(new_text)
+            Ok(Some(new_text))
         }
     }
 }
 
-pub fn migrate_keymap(text: &str) -> Option<String> {
+pub fn migrate_keymap(text: &str) -> Result<Option<String>> {
     let transformed_text = migrate(
         text,
         KEYMAP_MIGRATION_TRANSFORMATION_PATTERNS,
         &KEYMAP_MIGRATION_TRANSFORMATION_QUERY,
-    );
+    )?;
     let replacement_text = migrate(
         &transformed_text.as_ref().unwrap_or(&text.to_string()),
         KEYMAP_MIGRATION_REPLACEMENT_PATTERNS,
         &KEYMAP_MIGRATION_REPLACEMENT_QUERY,
-    );
+    )?;
-    replacement_text.or(transformed_text)
+    Ok(replacement_text.or(transformed_text))
 }
 
-pub fn migrate_settings(text: &str) -> Option<String> {
+pub fn migrate_settings(text: &str) -> Result<Option<String>> {
     migrate(
         &text,
         SETTINGS_MIGRATION_PATTERNS,
@@ -72,7 +73,7 @@ type MigrationPatterns = &'static [(
     fn(&str, &QueryMatch, &Query) -> Option<(Range<usize>, String)>,
 )];
 
-static KEYMAP_MIGRATION_TRANSFORMATION_PATTERNS: MigrationPatterns = &[
+const KEYMAP_MIGRATION_TRANSFORMATION_PATTERNS: MigrationPatterns = &[
     (ACTION_ARRAY_PATTERN, replace_array_with_single_string),
     (
         ACTION_ARGUMENT_OBJECT_PATTERN,
@@ -120,9 +121,9 @@ fn replace_array_with_single_string(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let array_ix = query.capture_index_for_name("array").unwrap();
+    let array_ix = query.capture_index_for_name("array")?;
-    let action_name_ix = query.capture_index_for_name("action_name").unwrap();
+    let action_name_ix = query.capture_index_for_name("action_name")?;
-    let argument_ix = query.capture_index_for_name("argument").unwrap();
+    let argument_ix = query.capture_index_for_name("argument")?;
 
     let action_name = contents.get(
         mat.nodes_for_capture_index(action_name_ix)
@@ -142,50 +143,109 @@ fn replace_array_with_single_string(
     Some((range_to_replace, replacement_as_string))
 }
 
-#[rustfmt::skip]
 static TRANSFORM_ARRAY: LazyLock<HashMap<(&str, &str), &str>> = LazyLock::new(|| {
     HashMap::from_iter([
         // activate
-        (("workspace::ActivatePaneInDirection", "Up"), "workspace::ActivatePaneUp"),
+        (
-        (("workspace::ActivatePaneInDirection", "Down"), "workspace::ActivatePaneDown"),
+            ("workspace::ActivatePaneInDirection", "Up"),
-        (("workspace::ActivatePaneInDirection", "Left"), "workspace::ActivatePaneLeft"),
+            "workspace::ActivatePaneUp",
-        (("workspace::ActivatePaneInDirection", "Right"), "workspace::ActivatePaneRight"),
+        ),
+        (
+            ("workspace::ActivatePaneInDirection", "Down"),
+            "workspace::ActivatePaneDown",
+        ),
+        (
+            ("workspace::ActivatePaneInDirection", "Left"),
+            "workspace::ActivatePaneLeft",
+        ),
+        (
+            ("workspace::ActivatePaneInDirection", "Right"),
+            "workspace::ActivatePaneRight",
+        ),
         // swap
-        (("workspace::SwapPaneInDirection", "Up"), "workspace::SwapPaneUp"),
+        (
-        (("workspace::SwapPaneInDirection", "Down"), "workspace::SwapPaneDown"),
+            ("workspace::SwapPaneInDirection", "Up"),
-        (("workspace::SwapPaneInDirection", "Left"), "workspace::SwapPaneLeft"),
+            "workspace::SwapPaneUp",
-        (("workspace::SwapPaneInDirection", "Right"), "workspace::SwapPaneRight"),
+        ),
+        (
+            ("workspace::SwapPaneInDirection", "Down"),
+            "workspace::SwapPaneDown",
+        ),
+        (
+            ("workspace::SwapPaneInDirection", "Left"),
+            "workspace::SwapPaneLeft",
+        ),
+        (
+            ("workspace::SwapPaneInDirection", "Right"),
+            "workspace::SwapPaneRight",
+        ),
         // menu
-        (("app_menu::NavigateApplicationMenuInDirection", "Left"), "app_menu::ActivateMenuLeft"),
+        (
-        (("app_menu::NavigateApplicationMenuInDirection", "Right"), "app_menu::ActivateMenuRight"),
+            ("app_menu::NavigateApplicationMenuInDirection", "Left"),
+            "app_menu::ActivateMenuLeft",
+        ),
+        (
+            ("app_menu::NavigateApplicationMenuInDirection", "Right"),
+            "app_menu::ActivateMenuRight",
+        ),
         // vim push
         (("vim::PushOperator", "Change"), "vim::PushChange"),
         (("vim::PushOperator", "Delete"), "vim::PushDelete"),
         (("vim::PushOperator", "Yank"), "vim::PushYank"),
         (("vim::PushOperator", "Replace"), "vim::PushReplace"),
-        (("vim::PushOperator", "DeleteSurrounds"), "vim::PushDeleteSurrounds"),
+        (
+            ("vim::PushOperator", "DeleteSurrounds"),
+            "vim::PushDeleteSurrounds",
+        ),
         (("vim::PushOperator", "Mark"), "vim::PushMark"),
         (("vim::PushOperator", "Indent"), "vim::PushIndent"),
         (("vim::PushOperator", "Outdent"), "vim::PushOutdent"),
         (("vim::PushOperator", "AutoIndent"), "vim::PushAutoIndent"),
         (("vim::PushOperator", "Rewrap"), "vim::PushRewrap"),
-        (("vim::PushOperator", "ShellCommand"), "vim::PushShellCommand"),
+        (
+            ("vim::PushOperator", "ShellCommand"),
+            "vim::PushShellCommand",
+        ),
         (("vim::PushOperator", "Lowercase"), "vim::PushLowercase"),
         (("vim::PushOperator", "Uppercase"), "vim::PushUppercase"),
-        (("vim::PushOperator", "OppositeCase"), "vim::PushOppositeCase"),
+        (
+            ("vim::PushOperator", "OppositeCase"),
+            "vim::PushOppositeCase",
+        ),
         (("vim::PushOperator", "Register"), "vim::PushRegister"),
-        (("vim::PushOperator", "RecordRegister"), "vim::PushRecordRegister"),
+        (
-        (("vim::PushOperator", "ReplayRegister"), "vim::PushReplayRegister"),
+            ("vim::PushOperator", "RecordRegister"),
-        (("vim::PushOperator", "ReplaceWithRegister"), "vim::PushReplaceWithRegister"),
+            "vim::PushRecordRegister",
-        (("vim::PushOperator", "ToggleComments"), "vim::PushToggleComments"),
+        ),
+        (
+            ("vim::PushOperator", "ReplayRegister"),
+            "vim::PushReplayRegister",
+        ),
+        (
+            ("vim::PushOperator", "ReplaceWithRegister"),
+            "vim::PushReplaceWithRegister",
+        ),
+        (
+            ("vim::PushOperator", "ToggleComments"),
+            "vim::PushToggleComments",
+        ),
         // vim switch
         (("vim::SwitchMode", "Normal"), "vim::SwitchToNormalMode"),
         (("vim::SwitchMode", "Insert"), "vim::SwitchToInsertMode"),
         (("vim::SwitchMode", "Replace"), "vim::SwitchToReplaceMode"),
         (("vim::SwitchMode", "Visual"), "vim::SwitchToVisualMode"),
-        (("vim::SwitchMode", "VisualLine"), "vim::SwitchToVisualLineMode"),
+        (
-        (("vim::SwitchMode", "VisualBlock"), "vim::SwitchToVisualBlockMode"),
+            ("vim::SwitchMode", "VisualLine"),
-        (("vim::SwitchMode", "HelixNormal"), "vim::SwitchToHelixNormalMode"),
+            "vim::SwitchToVisualLineMode",
+        ),
+        (
+            ("vim::SwitchMode", "VisualBlock"),
+            "vim::SwitchToVisualBlockMode",
+        ),
+        (
+            ("vim::SwitchMode", "HelixNormal"),
+            "vim::SwitchToHelixNormalMode",
+        ),
         // vim resize
         (("vim::ResizePane", "Widen"), "vim::ResizePaneRight"),
         (("vim::ResizePane", "Narrow"), "vim::ResizePaneLeft"),
@@ -225,10 +285,10 @@ fn replace_action_argument_object_with_single_value(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let array_ix = query.capture_index_for_name("array").unwrap();
+    let array_ix = query.capture_index_for_name("array")?;
-    let action_name_ix = query.capture_index_for_name("action_name").unwrap();
+    let action_name_ix = query.capture_index_for_name("action_name")?;
-    let action_key_ix = query.capture_index_for_name("action_key").unwrap();
+    let action_key_ix = query.capture_index_for_name("action_key")?;
-    let argument_ix = query.capture_index_for_name("argument").unwrap();
+    let argument_ix = query.capture_index_for_name("argument")?;
 
     let action_name = contents.get(
         mat.nodes_for_capture_index(action_name_ix)
@@ -253,7 +313,7 @@ fn replace_action_argument_object_with_single_value(
     Some((range_to_replace, replacement))
 }
 
-// "ctrl-k ctrl-1": [ "editor::PushOperator", { "Object": {} } ] -> [ "editor::vim::PushObject", {} ]
+/// "ctrl-k ctrl-1": [ "editor::PushOperator", { "Object": {} } ] -> [ "editor::vim::PushObject", {} ]
 static UNWRAP_OBJECTS: LazyLock<HashMap<&str, HashMap<&str, &str>>> = LazyLock::new(|| {
     HashMap::from_iter([
         (
@@ -278,7 +338,7 @@ static UNWRAP_OBJECTS: LazyLock<HashMap<&str, HashMap<&str, &str>>> = LazyLock::
     ])
 });
 
-static KEYMAP_MIGRATION_REPLACEMENT_PATTERNS: MigrationPatterns = &[(
+const KEYMAP_MIGRATION_REPLACEMENT_PATTERNS: MigrationPatterns = &[(
     ACTION_ARGUMENT_SNAKE_CASE_PATTERN,
     action_argument_snake_case,
 )];
@@ -318,7 +378,7 @@ fn rename_string_action(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let action_name_ix = query.capture_index_for_name("action_name").unwrap();
+    let action_name_ix = query.capture_index_for_name("action_name")?;
     let action_name_range = mat
         .nodes_for_capture_index(action_name_ix)
         .next()?
@@ -328,17 +388,31 @@ fn rename_string_action(
     Some((action_name_range, new_action_name.to_string()))
 }
 
-// "ctrl-k ctrl-1": "inline_completion::ToggleMenu" -> "edit_prediction::ToggleMenu"
+/// "ctrl-k ctrl-1": "inline_completion::ToggleMenu" -> "edit_prediction::ToggleMenu"
-#[rustfmt::skip]
 static STRING_REPLACE: LazyLock<HashMap<&str, &str>> = LazyLock::new(|| {
     HashMap::from_iter([
-        ("inline_completion::ToggleMenu", "edit_prediction::ToggleMenu"),
+        (
+            "inline_completion::ToggleMenu",
+            "edit_prediction::ToggleMenu",
+        ),
         ("editor::NextInlineCompletion", "editor::NextEditPrediction"),
-        ("editor::PreviousInlineCompletion", "editor::PreviousEditPrediction"),
+        (
-        ("editor::AcceptPartialInlineCompletion", "editor::AcceptPartialEditPrediction"),
+            "editor::PreviousInlineCompletion",
+            "editor::PreviousEditPrediction",
+        ),
+        (
+            "editor::AcceptPartialInlineCompletion",
+            "editor::AcceptPartialEditPrediction",
+        ),
         ("editor::ShowInlineCompletion", "editor::ShowEditPrediction"),
-        ("editor::AcceptInlineCompletion", "editor::AcceptEditPrediction"),
+        (
-        ("editor::ToggleInlineCompletions", "editor::ToggleEditPrediction"),
+            "editor::AcceptInlineCompletion",
+            "editor::AcceptEditPrediction",
+        ),
+        (
+            "editor::ToggleInlineCompletions",
+            "editor::ToggleEditPrediction",
+        ),
     ])
 });
 
@@ -359,7 +433,7 @@ fn rename_context_key(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let context_predicate_ix = query.capture_index_for_name("context_predicate").unwrap();
+    let context_predicate_ix = query.capture_index_for_name("context_predicate")?;
     let context_predicate_range = mat
         .nodes_for_capture_index(context_predicate_ix)
         .next()?
@@ -415,10 +489,10 @@ fn action_argument_snake_case(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let array_ix = query.capture_index_for_name("array").unwrap();
+    let array_ix = query.capture_index_for_name("array")?;
-    let action_name_ix = query.capture_index_for_name("action_name").unwrap();
+    let action_name_ix = query.capture_index_for_name("action_name")?;
-    let argument_key_ix = query.capture_index_for_name("argument_key").unwrap();
+    let argument_key_ix = query.capture_index_for_name("argument_key")?;
-    let argument_value_ix = query.capture_index_for_name("argument_value").unwrap();
+    let argument_value_ix = query.capture_index_for_name("argument_value")?;
     let action_name = contents.get(
         mat.nodes_for_capture_index(action_name_ix)
             .next()?
@@ -463,7 +537,7 @@ fn action_argument_snake_case(
     Some((range_to_replace, replacement))
 }
 
-// "context": "Editor && inline_completion && !showing_completions" -> "Editor && edit_prediction && !showing_completions"
+/// "context": "Editor && inline_completion && !showing_completions" -> "Editor && edit_prediction && !showing_completions"
 pub static CONTEXT_REPLACE: LazyLock<HashMap<&str, &str>> = LazyLock::new(|| {
     HashMap::from_iter([
         ("inline_completion", "edit_prediction"),
@@ -474,7 +548,7 @@ pub static CONTEXT_REPLACE: LazyLock<HashMap<&str, &str>> = LazyLock::new(|| {
     ])
 });
 
-static SETTINGS_MIGRATION_PATTERNS: MigrationPatterns = &[
+const SETTINGS_MIGRATION_PATTERNS: MigrationPatterns = &[
     (SETTINGS_STRING_REPLACE_QUERY, replace_setting_name),
     (SETTINGS_REPLACE_NESTED_KEY, replace_setting_nested_key),
     (
@@ -494,7 +568,7 @@ static SETTINGS_MIGRATION_QUERY: LazyLock<Query> = LazyLock::new(|| {
     .unwrap()
 });
 
-static SETTINGS_STRING_REPLACE_QUERY: &str = r#"(document
+const SETTINGS_STRING_REPLACE_QUERY: &str = r#"(document
     (object
         (pair
             key: (string (string_content) @name)
@@ -508,7 +582,7 @@ fn replace_setting_name(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let setting_capture_ix = query.capture_index_for_name("name").unwrap();
+    let setting_capture_ix = query.capture_index_for_name("name")?;
     let setting_name_range = mat
         .nodes_for_capture_index(setting_capture_ix)
         .next()?
@@ -518,17 +592,23 @@ fn replace_setting_name(
     Some((setting_name_range, new_setting_name.to_string()))
 }
 
-#[rustfmt::skip]
+pub static SETTINGS_STRING_REPLACE: LazyLock<HashMap<&'static str, &'static str>> =
-pub static SETTINGS_STRING_REPLACE: LazyLock<HashMap<&'static str, &'static str>> = LazyLock::new(|| {
+    LazyLock::new(|| {
-    HashMap::from_iter([
+        HashMap::from_iter([
-        ("show_inline_completions_in_menu", "show_edit_predictions_in_menu"),
+            (
-        ("show_inline_completions", "show_edit_predictions"),
+                "show_inline_completions_in_menu",
-        ("inline_completions_disabled_in", "edit_predictions_disabled_in"),
+                "show_edit_predictions_in_menu",
-        ("inline_completions", "edit_predictions")
+            ),
-    ])
+            ("show_inline_completions", "show_edit_predictions"),
-});
+            (
+                "inline_completions_disabled_in",
+                "edit_predictions_disabled_in",
+            ),
+            ("inline_completions", "edit_predictions"),
+        ])
+    });
 
-static SETTINGS_REPLACE_NESTED_KEY: &str = r#"
+const SETTINGS_REPLACE_NESTED_KEY: &str = r#"
 (object
   (pair
     key: (string (string_content) @parent_key)
@@ -547,14 +627,14 @@ fn replace_setting_nested_key(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let parent_object_capture_ix = query.capture_index_for_name("parent_key").unwrap();
+    let parent_object_capture_ix = query.capture_index_for_name("parent_key")?;
     let parent_object_range = mat
         .nodes_for_capture_index(parent_object_capture_ix)
         .next()?
         .byte_range();
     let parent_object_name = contents.get(parent_object_range.clone())?;
 
-    let setting_name_ix = query.capture_index_for_name("setting_name").unwrap();
+    let setting_name_ix = query.capture_index_for_name("setting_name")?;
     let setting_range = mat
         .nodes_for_capture_index(setting_name_ix)
         .next()?
@@ -568,9 +648,11 @@ fn replace_setting_nested_key(
     Some((setting_range, new_setting_name.to_string()))
 }
 
-// "features": {
+/// ```json
-//   "inline_completion_provider": "copilot"
+/// "features": {
-// },
+///   "inline_completion_provider": "copilot"
+/// },
+/// ```
 pub static SETTINGS_NESTED_STRING_REPLACE: LazyLock<
     HashMap<&'static str, HashMap<&'static str, &'static str>>,
 > = LazyLock::new(|| {
@@ -580,7 +662,7 @@ pub static SETTINGS_NESTED_STRING_REPLACE: LazyLock<
     )])
 });
 
-static SETTINGS_REPLACE_IN_LANGUAGES_QUERY: &str = r#"
+const SETTINGS_REPLACE_IN_LANGUAGES_QUERY: &str = r#"
 (object
   (pair
     key: (string (string_content) @languages)
@@ -604,7 +686,7 @@ fn replace_setting_in_languages(
     mat: &QueryMatch,
     query: &Query,
 ) -> Option<(Range<usize>, String)> {
-    let setting_capture_ix = query.capture_index_for_name("setting_name").unwrap();
+    let setting_capture_ix = query.capture_index_for_name("setting_name")?;
     let setting_name_range = mat
         .nodes_for_capture_index(setting_capture_ix)
         .next()?
@@ -615,27 +697,28 @@ fn replace_setting_in_languages(
     Some((setting_name_range, new_setting_name.to_string()))
 }
 
-#[rustfmt::skip]
+static LANGUAGE_SETTINGS_REPLACE: LazyLock<HashMap<&'static str, &'static str>> =
-static LANGUAGE_SETTINGS_REPLACE: LazyLock<
+    LazyLock::new(|| {
-    HashMap<&'static str, &'static str>,
+        HashMap::from_iter([
-> = LazyLock::new(|| {
+            ("show_inline_completions", "show_edit_predictions"),
-    HashMap::from_iter([
+            (
-        ("show_inline_completions", "show_edit_predictions"),
+                "inline_completions_disabled_in",
-        ("inline_completions_disabled_in", "edit_predictions_disabled_in"),
+                "edit_predictions_disabled_in",
-    ])
+            ),
-});
+        ])
+    });
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
     fn assert_migrate_keymap(input: &str, output: Option<&str>) {
-        let migrated = migrate_keymap(&input);
+        let migrated = migrate_keymap(&input).unwrap();
         pretty_assertions::assert_eq!(migrated.as_deref(), output);
     }
 
     fn assert_migrate_settings(input: &str, output: Option<&str>) {
-        let migrated = migrate_settings(&input);
+        let migrated = migrate_settings(&input).unwrap();
         pretty_assertions::assert_eq!(migrated.as_deref(), output);
     }
 

crates/zed/src/zed/migrate.rs

@@ -8,14 +8,17 @@ pub fn should_migrate_settings(settings: &serde_json::Value) -> bool {
     let Ok(old_text) = serde_json::to_string(settings) else {
         return false;
     };
-    migrator::migrate_settings(&old_text).is_some()
+    migrator::migrate_settings(&old_text)
+        .ok()
+        .flatten()
+        .is_some()
 }
 
 pub fn migrate_settings(fs: Arc<dyn Fs>, cx: &mut gpui::App) {
     cx.background_executor()
         .spawn(async move {
             let old_text = SettingsStore::load_settings(&fs).await?;
-            let Some(new_text) = migrator::migrate_settings(&old_text) else {
+            let Some(new_text) = migrator::migrate_settings(&old_text)? else {
                 return anyhow::Ok(());
             };
             let settings_path = paths::settings_file().as_path();
@@ -49,12 +52,12 @@ pub fn should_migrate_keymap(keymap_file: KeymapFile) -> bool {
     let Ok(old_text) = serde_json::to_string(&keymap_file) else {
         return false;
     };
-    migrator::migrate_keymap(&old_text).is_some()
+    migrator::migrate_keymap(&old_text).ok().flatten().is_some()
 }
 
 pub async fn migrate_keymap(fs: Arc<dyn Fs>) -> anyhow::Result<()> {
     let old_text = KeymapFile::load_keymap_file(&fs).await?;
-    let Some(new_text) = migrator::migrate_keymap(&old_text) else {
+    let Some(new_text) = migrator::migrate_keymap(&old_text)? else {
         return Ok(());
     };
     let keymap_path = paths::keymap_file().as_path();