Fix remote server (ssh) crash when editing json (#33818)

Michael Sloan and Cole created 5 months ago

Closes #33807

Release Notes:

- (Preview Only) Fixes a remote server (ssh) crash when editing json
files

---------

Co-authored-by: Cole <[e-mail]>

Change summary

crates/language/src/language_settings.rs |  5 +--
crates/settings/src/settings_json.rs     | 39 ++++++++++----------------
crates/theme/src/settings.rs             | 21 ++++++-------
3 files changed, 27 insertions(+), 38 deletions(-)

Detailed changes

crates/language/src/language_settings.rs

@@ -321,7 +321,7 @@ inventory::submit! {
             let language_settings_content_ref = generator
                 .subschema_for::<LanguageSettingsContent>()
                 .to_value();
-            let schema = json_schema!({
+            replace_subschema::<LanguageToSettingsMap>(generator, || json_schema!({
                 "type": "object",
                 "properties": params
                     .language_names
@@ -333,8 +333,7 @@ inventory::submit! {
                         )
                     })
                     .collect::<serde_json::Map<_, _>>()
-            });
-            replace_subschema::<LanguageToSettingsMap>(generator, schema)
+            }))
         }
     }
 }

crates/settings/src/settings_json.rs

@@ -23,35 +23,26 @@ inventory::collect!(ParameterizedJsonSchema);
 
 const DEFS_PATH: &str = "#/$defs/";
 
-/// Replaces the JSON schema definition for some type, and returns a reference to it.
+/// Replaces the JSON schema definition for some type if it is in use (in the definitions list), and
+/// returns a reference to it.
+///
+/// This asserts that JsonSchema::schema_name() + "2" does not exist because this indicates that
+/// there are multiple types that use this name, and unfortunately schemars APIs do not support
+/// resolving this ambiguity - see https://github.com/GREsau/schemars/issues/449
+///
+/// This takes a closure for `schema` because some settings types are not available on the remote
+/// server, and so will crash when attempting to access e.g. GlobalThemeRegistry.
 pub fn replace_subschema<T: JsonSchema>(
     generator: &mut schemars::SchemaGenerator,
-    schema: schemars::Schema,
+    schema: impl Fn() -> schemars::Schema,
 ) -> schemars::Schema {
-    // The key in definitions may not match T::schema_name() if multiple types have the same name.
-    // This is a workaround for there being no straightforward way to get the key used for a type -
-    // see https://github.com/GREsau/schemars/issues/449
-    let ref_schema = generator.subschema_for::<T>();
-    if let Some(serde_json::Value::String(definition_pointer)) = ref_schema.get("$ref") {
-        if let Some(definition_name) = definition_pointer.strip_prefix(DEFS_PATH) {
-            generator
-                .definitions_mut()
-                .insert(definition_name.to_string(), schema.to_value());
-            return ref_schema;
-        } else {
-            log::error!(
-                "bug: expected `$ref` field to start with {DEFS_PATH}, \
-                got {definition_pointer}"
-            );
-        }
-    } else {
-        log::error!("bug: expected `$ref` field in result of `subschema_for`");
-    }
     // fallback on just using the schema name, which could collide.
     let schema_name = T::schema_name();
-    generator
-        .definitions_mut()
-        .insert(schema_name.to_string(), schema.to_value());
+    let definitions = generator.definitions_mut();
+    assert!(!definitions.contains_key(&format!("{schema_name}2")));
+    if definitions.contains_key(schema_name.as_ref()) {
+        definitions.insert(schema_name.to_string(), schema().to_value());
+    }
     Schema::new_ref(format!("{DEFS_PATH}{schema_name}"))
 }
 

crates/theme/src/settings.rs

@@ -978,11 +978,10 @@ pub struct ThemeName(pub Arc<str>);
 inventory::submit! {
     ParameterizedJsonSchema {
         add_and_get_ref: |generator, _params, cx| {
-            let schema = json_schema!({
+            replace_subschema::<ThemeName>(generator, || json_schema!({
                 "type": "string",
                 "enum": ThemeRegistry::global(cx).list_names(),
-            });
-            replace_subschema::<ThemeName>(generator, schema)
+            }))
         }
     }
 }
@@ -996,15 +995,14 @@ pub struct IconThemeName(pub Arc<str>);
 inventory::submit! {
     ParameterizedJsonSchema {
         add_and_get_ref: |generator, _params, cx| {
-            let schema = json_schema!({
+            replace_subschema::<IconThemeName>(generator, || json_schema!({
                 "type": "string",
                 "enum": ThemeRegistry::global(cx)
                     .list_icon_themes()
                     .into_iter()
                     .map(|icon_theme| icon_theme.name)
                     .collect::<Vec<SharedString>>(),
-            });
-            replace_subschema::<IconThemeName>(generator, schema)
+            }))
         }
     }
 }
@@ -1018,11 +1016,12 @@ pub struct FontFamilyName(pub Arc<str>);
 inventory::submit! {
     ParameterizedJsonSchema {
         add_and_get_ref: |generator, params, _cx| {
-            let schema = json_schema!({
-                "type": "string",
-                "enum": params.font_names,
-            });
-            replace_subschema::<FontFamilyName>(generator, schema)
+            replace_subschema::<FontFamilyName>(generator, || {
+                json_schema!({
+                    "type": "string",
+                    "enum": params.font_names,
+                })
+            })
         }
     }
 }