Fix confusing keymap json errors and hovers for nonexistent actions (#23098)

Release Notes:

- N/A

crates/settings/src/keymap_file.rs

@@ -1,10 +1,10 @@
 use crate::{settings_store::parse_json_with_comments, SettingsAssets};
 use anyhow::{anyhow, Context, Result};
 use collections::{BTreeMap, HashMap};
-use gpui::{Action, AppContext, KeyBinding, SharedString};
+use gpui::{Action, AppContext, KeyBinding, NoAction, SharedString};
 use schemars::{
     gen::{SchemaGenerator, SchemaSettings},
-    schema::{ArrayValidation, InstanceType, Metadata, Schema, SchemaObject, SubschemaValidation},
+    schema::{ArrayValidation, InstanceType, Schema, SchemaObject, SubschemaValidation},
     JsonSchema,
 };
 use serde::Deserialize;
@@ -161,21 +161,68 @@ impl KeymapFile {
             Some(input.into())
         }
 
-        fn add_deprecation_notice(schema_object: &mut SchemaObject, new_name: &SharedString) {
+        fn add_deprecation(schema_object: &mut SchemaObject, message: String) {
             schema_object.extensions.insert(
                 // deprecationMessage is not part of the JSON Schema spec,
                 // but json-language-server recognizes it.
                 "deprecationMessage".to_owned(),
-                format!("Deprecated, use {new_name}").into(),
+                Value::String(message),
             );
         }
 
+        fn add_deprecation_preferred_name(schema_object: &mut SchemaObject, new_name: &str) {
+            add_deprecation(schema_object, format!("Deprecated, use {new_name}"));
+        }
+
+        fn add_description(schema_object: &mut SchemaObject, description: String) {
+            schema_object
+                .metadata
+                .get_or_insert(Default::default())
+                .description = Some(description);
+        }
+
         let empty_object: SchemaObject = SchemaObject {
             instance_type: set(InstanceType::Object),
             ..Default::default()
         };
 
-        let mut keymap_action_alternatives = Vec::new();
+        // This is a workaround for a json-language-server issue where it matches the first
+        // alternative that matches the value's shape and uses that for documentation.
+        //
+        // In the case of the array validations, it would even provide an error saying that the name
+        // must match the name of the first alternative.
+        let mut plain_action = SchemaObject {
+            instance_type: set(InstanceType::String),
+            const_value: Some(Value::String("".to_owned())),
+            ..Default::default()
+        };
+        let no_action_message = "No action named this.";
+        add_description(&mut plain_action, no_action_message.to_owned());
+        add_deprecation(&mut plain_action, no_action_message.to_owned());
+        let mut matches_action_name = SchemaObject {
+            const_value: Some(Value::String("".to_owned())),
+            ..Default::default()
+        };
+        let no_action_message = "No action named this that takes input.";
+        add_description(&mut matches_action_name, no_action_message.to_owned());
+        add_deprecation(&mut matches_action_name, no_action_message.to_owned());
+        let action_with_input = SchemaObject {
+            instance_type: set(InstanceType::Array),
+            array: set(ArrayValidation {
+                items: set(vec![
+                    matches_action_name.into(),
+                    // Accept any value, as we want this to be the preferred match when there is a
+                    // typo in the name.
+                    Schema::Bool(true),
+                ]),
+                min_items: Some(2),
+                max_items: Some(2),
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+        let mut keymap_action_alternatives = vec![plain_action.into(), action_with_input.into()];
+
         for (name, action_schema) in action_schemas.iter() {
             let schema = if let Some(Schema::Object(schema)) = action_schema {
                 Some(schema.clone())
@@ -183,60 +230,61 @@ impl KeymapFile {
                 None
             };
 
-            // If the type has a description, also apply it to the value. Ideally it would be
-            // removed and applied to the overall array, but `json-language-server` does not show
-            // these descriptions.
             let description = schema.as_ref().and_then(|schema| {
                 schema
                     .metadata
                     .as_ref()
-                    .and_then(|metadata| metadata.description.as_ref())
+                    .and_then(|metadata| metadata.description.clone())
             });
-            let mut matches_action_name = SchemaObject {
+
-                const_value: Some(Value::String(name.to_string())),
+            let deprecation = if name == NoAction.name() {
-                ..Default::default()
+                Some("null")
+            } else {
+                deprecations.get(name).map(|new_name| new_name.as_ref())
             };
-            if let Some(description) = description {
-                matches_action_name.metadata = set(Metadata {
-                    description: Some(description.clone()),
-                    ..Default::default()
-                });
-            }
 
             // Add an alternative for plain action names.
-            let deprecation = deprecations.get(name);
             let mut plain_action = SchemaObject {
                 instance_type: set(InstanceType::String),
                 const_value: Some(Value::String(name.to_string())),
                 ..Default::default()
             };
             if let Some(new_name) = deprecation {
-                add_deprecation_notice(&mut plain_action, new_name);
+                add_deprecation_preferred_name(&mut plain_action, new_name);
+            }
+            if let Some(description) = description.clone() {
+                add_description(&mut plain_action, description);
             }
             keymap_action_alternatives.push(plain_action.into());
 
-            // When all fields are skipped or an empty struct is added with impl_actions! /
+            // Add an alternative for actions with data specified as a [name, data] array.
-            // impl_actions_as! an empty struct is produced. The action should be invoked without
+            //
-            // data in this case.
+            // When a struct with no deserializable fields is added with impl_actions! /
+            // impl_actions_as! an empty object schema is produced. The action should be invoked
+            // without data in this case.
             if let Some(schema) = schema {
                 if schema != empty_object {
-                    let mut action_with_data = SchemaObject {
+                    let mut matches_action_name = SchemaObject {
-                        instance_type: set(InstanceType::Array),
+                        const_value: Some(Value::String(name.to_string())),
-                        array: Some(
-                            ArrayValidation {
-                                items: set(vec![matches_action_name.into(), schema.into()]),
-                                min_items: Some(2),
-                                max_items: Some(2),
-                                ..Default::default()
-                            }
-                            .into(),
-                        ),
                         ..Default::default()
                     };
-                    if let Some(new_name) = deprecation {
+                    if let Some(description) = description.clone() {
-                        add_deprecation_notice(&mut action_with_data, new_name);
+                        add_description(&mut matches_action_name, description.to_string());
                     }
-                    keymap_action_alternatives.push(action_with_data.into());
+                    if let Some(new_name) = deprecation {
+                        add_deprecation_preferred_name(&mut matches_action_name, new_name);
+                    }
+                    let action_with_input = SchemaObject {
+                        instance_type: set(InstanceType::Array),
+                        array: set(ArrayValidation {
+                            items: set(vec![matches_action_name.into(), schema.into()]),
+                            min_items: Some(2),
+                            max_items: Some(2),
+                            ..Default::default()
+                        }),
+                        ..Default::default()
+                    };
+                    keymap_action_alternatives.push(action_with_input.into());
                 }
             }
         }