Default `#[schemars(deny_unknown_fields)] for json-language-server schemas (#33883)

Michael Sloan created

Followup to #33678, doing the same thing for all JSON Schema files
provided to json-language-server

Release Notes:

* Added warnings for unknown fields when editing `tasks.json` /
`snippets.json`.

Change summary

Cargo.lock                               |  1 
crates/language/src/language_settings.rs |  2 
crates/languages/src/json.rs             |  1 
crates/settings/src/keymap_file.rs       |  4 +
crates/settings/src/settings_json.rs     | 58 -------------------------
crates/settings/src/settings_store.rs    | 47 ++++++++++----------
crates/snippet_provider/src/format.rs    |  2 
crates/task/src/adapter_schema.rs        | 48 --------------------
crates/task/src/debug_format.rs          | 59 +++++++++++++++++--------
crates/task/src/task_template.rs         |  2 
crates/theme/src/settings.rs             |  3 
crates/util/Cargo.toml                   |  1 
crates/util/src/schemars.rs              | 58 +++++++++++++++++++++++++
crates/util/src/util.rs                  |  1 
14 files changed, 137 insertions(+), 150 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -17348,6 +17348,7 @@ dependencies = [
  "rand 0.8.5",
  "regex",
  "rust-embed",
+ "schemars",
  "serde",
  "serde_json",
  "serde_json_lenient",

crates/language/src/language_settings.rs 🔗

@@ -18,10 +18,10 @@ use serde::{
 
 use settings::{
     ParameterizedJsonSchema, Settings, SettingsLocation, SettingsSources, SettingsStore,
-    replace_subschema,
 };
 use shellexpand;
 use std::{borrow::Cow, num::NonZeroU32, path::Path, slice, sync::Arc};
+use util::schemars::replace_subschema;
 use util::serde::default_true;
 
 /// Initializes the language settings.

crates/languages/src/json.rs 🔗

@@ -272,6 +272,7 @@ impl JsonLspAdapter {
 #[cfg(debug_assertions)]
 fn generate_inspector_style_schema() -> serde_json_lenient::Value {
     let schema = schemars::generate::SchemaSettings::draft2019_09()
+        .with_transform(util::schemars::DefaultDenyUnknownFields)
         .into_generator()
         .root_schema_for::<gpui::StyleRefinement>();
 

crates/settings/src/keymap_file.rs 🔗

@@ -427,6 +427,10 @@ impl KeymapFile {
     }
 
     pub fn generate_json_schema_for_registered_actions(cx: &mut App) -> Value {
+        // instead of using DefaultDenyUnknownFields, actions typically use
+        // `#[serde(deny_unknown_fields)]` so that these cases are reported as parse failures. This
+        // is because the rest of the keymap will still load in these cases, whereas other settings
+        // files would not.
         let mut generator = schemars::generate::SchemaSettings::draft2019_09().into_generator();
 
         let action_schemas = cx.action_schemas(&mut generator);

crates/settings/src/settings_json.rs 🔗

@@ -1,6 +1,5 @@
 use anyhow::Result;
 use gpui::App;
-use schemars::{JsonSchema, Schema, transform::transform_subschemas};
 use serde::{Serialize, de::DeserializeOwned};
 use serde_json::Value;
 use std::{ops::Range, sync::LazyLock};
@@ -21,63 +20,6 @@ pub struct ParameterizedJsonSchema {
 
 inventory::collect!(ParameterizedJsonSchema);
 
-const DEFS_PATH: &str = "#/$defs/";
-
-/// 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: impl Fn() -> schemars::Schema,
-) -> schemars::Schema {
-    // fallback on just using the schema name, which could collide.
-    let schema_name = T::schema_name();
-    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}"))
-}
-
-/// Adds a new JSON schema definition and returns a reference to it. **Panics** if the name is
-/// already in use.
-pub fn add_new_subschema(
-    generator: &mut schemars::SchemaGenerator,
-    name: &str,
-    schema: Value,
-) -> Schema {
-    let old_definition = generator.definitions_mut().insert(name.to_string(), schema);
-    assert_eq!(old_definition, None);
-    schemars::Schema::new_ref(format!("{DEFS_PATH}{name}"))
-}
-
-/// Defaults `additionalProperties` to `true`, as if `#[schemars(deny_unknown_fields)]` was on every
-/// struct. Skips structs that have `additionalProperties` set (such as if #[serde(flatten)] is used
-/// on a map).
-#[derive(Clone)]
-pub struct DefaultDenyUnknownFields;
-
-impl schemars::transform::Transform for DefaultDenyUnknownFields {
-    fn transform(&mut self, schema: &mut schemars::Schema) {
-        if let Some(object) = schema.as_object_mut() {
-            if object.contains_key("properties")
-                && !object.contains_key("additionalProperties")
-                && !object.contains_key("unevaluatedProperties")
-            {
-                object.insert("additionalProperties".to_string(), false.into());
-            }
-        }
-        transform_subschemas(self, schema);
-    }
-}
-
 pub fn update_value_in_json_text<'a>(
     text: &mut String,
     key_path: &mut Vec<&'a str>,

crates/settings/src/settings_store.rs 🔗

@@ -6,7 +6,7 @@ use futures::{FutureExt, StreamExt, channel::mpsc, future::LocalBoxFuture};
 use gpui::{App, AsyncApp, BorrowAppContext, Global, Task, UpdateGlobal};
 
 use paths::{EDITORCONFIG_NAME, local_settings_file_relative_path, task_file_name};
-use schemars::{JsonSchema, json_schema};
+use schemars::JsonSchema;
 use serde::{Deserialize, Serialize, de::DeserializeOwned};
 use serde_json::{Value, json};
 use smallvec::SmallVec;
@@ -18,14 +18,16 @@ use std::{
     str::{self, FromStr},
     sync::Arc,
 };
-
-use util::{ResultExt as _, merge_non_null_json_value_into};
+use util::{
+    ResultExt as _, merge_non_null_json_value_into,
+    schemars::{DefaultDenyUnknownFields, add_new_subschema},
+};
 
 pub type EditorconfigProperties = ec4rs::Properties;
 
 use crate::{
-    DefaultDenyUnknownFields, ParameterizedJsonSchema, SettingsJsonSchemaParams, VsCodeSettings,
-    WorktreeId, add_new_subschema, parse_json_with_comments, update_value_in_json_text,
+    ParameterizedJsonSchema, SettingsJsonSchemaParams, VsCodeSettings, WorktreeId,
+    parse_json_with_comments, update_value_in_json_text,
 };
 
 /// A value that can be defined as a user setting.
@@ -1019,19 +1021,19 @@ impl SettingsStore {
             .unwrap()
             .remove("additionalProperties");
 
-        let mut root_schema = if let Some(meta_schema) = generator.settings().meta_schema.as_ref() {
-            json_schema!({ "$schema": meta_schema.to_string() })
-        } else {
-            json_schema!({})
-        };
-
-        // "unevaluatedProperties: false" to report unknown fields.
-        root_schema.insert("unevaluatedProperties".to_string(), false.into());
-
-        // Settings file contents matches ZedSettings + overrides for each release stage.
-        root_schema.insert(
-            "allOf".to_string(),
-            json!([
+        let meta_schema = generator
+            .settings()
+            .meta_schema
+            .as_ref()
+            .expect("meta_schema should be present in schemars settings")
+            .to_string();
+
+        json!({
+            "$schema": meta_schema,
+            "title": "Zed Settings",
+            "unevaluatedProperties": false,
+            // ZedSettings + settings overrides for each release stage
+            "allOf": [
                 zed_settings_ref,
                 {
                     "properties": {
@@ -1041,12 +1043,9 @@ impl SettingsStore {
                         "preview": zed_release_stage_settings_ref,
                     }
                 }
-            ]),
-        );
-
-        root_schema.insert("$defs".to_string(), definitions.into());
-
-        root_schema.to_value()
+            ],
+            "$defs": definitions,
+        })
     }
 
     fn recompute_values(

crates/snippet_provider/src/format.rs 🔗

@@ -3,6 +3,7 @@ use schemars::{JsonSchema, json_schema};
 use serde::Deserialize;
 use serde_json_lenient::Value;
 use std::borrow::Cow;
+use util::schemars::DefaultDenyUnknownFields;
 
 #[derive(Deserialize)]
 pub struct VsSnippetsFile {
@@ -13,6 +14,7 @@ pub struct VsSnippetsFile {
 impl VsSnippetsFile {
     pub fn generate_json_schema() -> Value {
         let schema = schemars::generate::SchemaSettings::draft2019_09()
+            .with_transform(DefaultDenyUnknownFields)
             .into_generator()
             .root_schema_for::<Self>();
 

crates/task/src/adapter_schema.rs 🔗

@@ -1,10 +1,8 @@
-use anyhow::Result;
 use gpui::SharedString;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use serde_json::json;
 
-/// Represents a schema for a specific adapter
+/// JSON schema for a specific adapter
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
 pub struct AdapterSchema {
     /// The adapter name identifier
@@ -16,47 +14,3 @@ pub struct AdapterSchema {
 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
 #[serde(transparent)]
 pub struct AdapterSchemas(pub Vec<AdapterSchema>);
-
-impl AdapterSchemas {
-    pub fn generate_json_schema(&self) -> Result<serde_json_lenient::Value> {
-        let adapter_conditions = self
-            .0
-            .iter()
-            .map(|adapter_schema| {
-                let adapter_name = adapter_schema.adapter.to_string();
-                json!({
-                    "if": {
-                        "properties": {
-                            "adapter": { "const": adapter_name }
-                        }
-                    },
-                    "then": adapter_schema.schema
-                })
-            })
-            .collect::<Vec<_>>();
-
-        let schema = serde_json_lenient::json!({
-            "$schema": "http://json-schema.org/draft-07/schema#",
-            "title": "Debug Adapter Configurations",
-            "description": "Configuration for debug adapters. Schema changes based on the selected adapter.",
-            "type": "array",
-            "items": {
-                "type": "object",
-                "required": ["adapter", "label"],
-                "properties": {
-                    "adapter": {
-                        "type": "string",
-                        "description": "The name of the debug adapter"
-                    },
-                    "label": {
-                        "type": "string",
-                        "description": "The name of the debug configuration"
-                    },
-                },
-                "allOf": adapter_conditions
-            }
-        });
-
-        Ok(serde_json_lenient::to_value(schema)?)
-    }
-}

crates/task/src/debug_format.rs 🔗

@@ -6,7 +6,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::net::Ipv4Addr;
 use std::path::PathBuf;
-use util::debug_panic;
+use util::{debug_panic, schemars::add_new_subschema};
 
 use crate::{TaskTemplate, adapter_schema::AdapterSchemas};
 
@@ -286,11 +286,10 @@ pub struct DebugScenario {
 pub struct DebugTaskFile(pub Vec<DebugScenario>);
 
 impl DebugTaskFile {
-    pub fn generate_json_schema(schemas: &AdapterSchemas) -> serde_json_lenient::Value {
+    pub fn generate_json_schema(schemas: &AdapterSchemas) -> serde_json::Value {
         let mut generator = schemars::generate::SchemaSettings::draft2019_09().into_generator();
-        let build_task_schema = generator.root_schema_for::<BuildTaskDefinition>();
-        let mut build_task_value =
-            serde_json_lenient::to_value(&build_task_schema).unwrap_or_default();
+
+        let mut build_task_value = BuildTaskDefinition::json_schema(&mut generator).to_value();
 
         if let Some(template_object) = build_task_value
             .get_mut("anyOf")
@@ -322,32 +321,54 @@ impl DebugTaskFile {
             );
         }
 
-        let task_definitions = build_task_value.get("$defs").cloned().unwrap_or_default();
-
         let adapter_conditions = schemas
             .0
             .iter()
             .map(|adapter_schema| {
                 let adapter_name = adapter_schema.adapter.to_string();
-                serde_json::json!({
-                    "if": {
-                        "properties": {
-                            "adapter": { "const": adapter_name }
-                        }
-                    },
-                    "then": adapter_schema.schema
-                })
+                add_new_subschema(
+                    &mut generator,
+                    &format!("{adapter_name}DebugSettings"),
+                    serde_json::json!({
+                        "if": {
+                            "properties": {
+                                "adapter": { "const": adapter_name }
+                            }
+                        },
+                        "then": adapter_schema.schema
+                    }),
+                )
             })
             .collect::<Vec<_>>();
 
-        serde_json_lenient::json!({
-            "$schema": "http://json-schema.org/draft-07/schema#",
+        let build_task_definition_ref = add_new_subschema(
+            &mut generator,
+            BuildTaskDefinition::schema_name().as_ref(),
+            build_task_value,
+        );
+
+        let meta_schema = generator
+            .settings()
+            .meta_schema
+            .as_ref()
+            .expect("meta_schema should be present in schemars settings")
+            .to_string();
+
+        serde_json::json!({
+            "$schema": meta_schema,
             "title": "Debug Configurations",
             "description": "Configuration for debug scenarios",
             "type": "array",
             "items": {
                 "type": "object",
                 "required": ["adapter", "label"],
+                // TODO: Uncommenting this will cause json-language-server to provide warnings for
+                // unrecognized properties. It should be enabled if/when there's an adapter JSON
+                // schema that's comprehensive. In order to not get warnings for the other schemas,
+                // `additionalProperties` or `unevaluatedProperties` (to handle "allOf" etc style
+                // schema combinations) could be set to `true` for that schema.
+                //
+                // "unevaluatedProperties": false,
                 "properties": {
                     "adapter": {
                         "type": "string",
@@ -357,7 +378,7 @@ impl DebugTaskFile {
                         "type": "string",
                         "description": "The name of the debug configuration"
                     },
-                    "build": build_task_value,
+                    "build": build_task_definition_ref,
                     "tcp_connection": {
                         "type": "object",
                         "description": "Optional TCP connection information for connecting to an already running debug adapter",
@@ -380,7 +401,7 @@ impl DebugTaskFile {
                 },
                 "allOf": adapter_conditions
             },
-            "$defs": task_definitions
+            "$defs": generator.take_definitions(true),
         })
     }
 }

crates/task/src/task_template.rs 🔗

@@ -4,6 +4,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 use std::path::PathBuf;
+use util::schemars::DefaultDenyUnknownFields;
 use util::serde::default_true;
 use util::{ResultExt, truncate_and_remove_front};
 
@@ -116,6 +117,7 @@ impl TaskTemplates {
     /// Generates JSON schema of Tasks JSON template format.
     pub fn generate_json_schema() -> serde_json_lenient::Value {
         let schema = schemars::generate::SchemaSettings::draft2019_09()
+            .with_transform(DefaultDenyUnknownFields)
             .into_generator()
             .root_schema_for::<Self>();
 

crates/theme/src/settings.rs 🔗

@@ -12,9 +12,10 @@ use gpui::{
 use refineable::Refineable;
 use schemars::{JsonSchema, json_schema};
 use serde::{Deserialize, Serialize};
-use settings::{ParameterizedJsonSchema, Settings, SettingsSources, replace_subschema};
+use settings::{ParameterizedJsonSchema, Settings, SettingsSources};
 use std::sync::Arc;
 use util::ResultExt as _;
+use util::schemars::replace_subschema;
 
 const MIN_FONT_SIZE: Pixels = px(6.0);
 const MIN_LINE_HEIGHT: f32 = 1.0;

crates/util/Cargo.toml 🔗

@@ -30,6 +30,7 @@ log.workspace = true
 rand = { workspace = true, optional = true }
 regex.workspace = true
 rust-embed.workspace = true
+schemars.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 serde_json_lenient.workspace = true

crates/util/src/schemars.rs 🔗

@@ -0,0 +1,58 @@
+use schemars::{JsonSchema, transform::transform_subschemas};
+
+const DEFS_PATH: &str = "#/$defs/";
+
+/// 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: impl Fn() -> schemars::Schema,
+) -> schemars::Schema {
+    // fallback on just using the schema name, which could collide.
+    let schema_name = T::schema_name();
+    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());
+    }
+    schemars::Schema::new_ref(format!("{DEFS_PATH}{schema_name}"))
+}
+
+/// Adds a new JSON schema definition and returns a reference to it. **Panics** if the name is
+/// already in use.
+pub fn add_new_subschema(
+    generator: &mut schemars::SchemaGenerator,
+    name: &str,
+    schema: serde_json::Value,
+) -> schemars::Schema {
+    let old_definition = generator.definitions_mut().insert(name.to_string(), schema);
+    assert_eq!(old_definition, None);
+    schemars::Schema::new_ref(format!("{DEFS_PATH}{name}"))
+}
+
+/// Defaults `additionalProperties` to `true`, as if `#[schemars(deny_unknown_fields)]` was on every
+/// struct. Skips structs that have `additionalProperties` set (such as if #[serde(flatten)] is used
+/// on a map).
+#[derive(Clone)]
+pub struct DefaultDenyUnknownFields;
+
+impl schemars::transform::Transform for DefaultDenyUnknownFields {
+    fn transform(&mut self, schema: &mut schemars::Schema) {
+        if let Some(object) = schema.as_object_mut() {
+            if object.contains_key("properties")
+                && !object.contains_key("additionalProperties")
+                && !object.contains_key("unevaluatedProperties")
+            {
+                object.insert("additionalProperties".to_string(), false.into());
+            }
+        }
+        transform_subschemas(self, schema);
+    }
+}

crates/util/src/util.rs 🔗

@@ -5,6 +5,7 @@ pub mod fs;
 pub mod markdown;
 pub mod paths;
 pub mod redact;
+pub mod schemars;
 pub mod serde;
 pub mod shell_env;
 pub mod size;