From ed7552d3e3f4e3a07ea8805c3a88085507994aed Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Thu, 3 Jul 2025 18:57:43 -0600 Subject: [PATCH] Default `#[schemars(deny_unknown_fields)] for json-language-server schemas (#33883) 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`. --- 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 | 45 +++++++++--------- 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, 136 insertions(+), 149 deletions(-) create mode 100644 crates/util/src/schemars.rs diff --git a/Cargo.lock b/Cargo.lock index d7e645131d..baed77a49f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17348,6 +17348,7 @@ dependencies = [ "rand 0.8.5", "regex", "rust-embed", + "schemars", "serde", "serde_json", "serde_json_lenient", diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 1caa6eceec..9b0abb1537 100644 --- a/crates/language/src/language_settings.rs +++ b/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. diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs index 6f51cdadba..7a3300eb01 100644 --- a/crates/languages/src/json.rs +++ b/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::(); diff --git a/crates/settings/src/keymap_file.rs b/crates/settings/src/keymap_file.rs index 0548a69b55..4c4ceee49b 100644 --- a/crates/settings/src/keymap_file.rs +++ b/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); diff --git a/crates/settings/src/settings_json.rs b/crates/settings/src/settings_json.rs index d78043a335..f569a18769 100644 --- a/crates/settings/src/settings_json.rs +++ b/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( - 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>, diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 0ba516ad7d..0d23385a68 100644 --- a/crates/settings/src/settings_store.rs +++ b/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!({}) - }; + let meta_schema = generator + .settings() + .meta_schema + .as_ref() + .expect("meta_schema should be present in schemars settings") + .to_string(); - // "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!([ + 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( diff --git a/crates/snippet_provider/src/format.rs b/crates/snippet_provider/src/format.rs index 0d06cbbc88..1a390aa2e1 100644 --- a/crates/snippet_provider/src/format.rs +++ b/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::(); diff --git a/crates/task/src/adapter_schema.rs b/crates/task/src/adapter_schema.rs index 111f555ca5..2c58bc0eab 100644 --- a/crates/task/src/adapter_schema.rs +++ b/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); - -impl AdapterSchemas { - pub fn generate_json_schema(&self) -> Result { - 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::>(); - - 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)?) - } -} diff --git a/crates/task/src/debug_format.rs b/crates/task/src/debug_format.rs index 2d24098bbb..f20f55975e 100644 --- a/crates/task/src/debug_format.rs +++ b/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); 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::(); - 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::>(); - 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), }) } } diff --git a/crates/task/src/task_template.rs b/crates/task/src/task_template.rs index 65424eeed4..cc36b28e4b 100644 --- a/crates/task/src/task_template.rs +++ b/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::(); diff --git a/crates/theme/src/settings.rs b/crates/theme/src/settings.rs index ca59eba766..1c4c90a475 100644 --- a/crates/theme/src/settings.rs +++ b/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; diff --git a/crates/util/Cargo.toml b/crates/util/Cargo.toml index 6a874fd329..825d6471b2 100644 --- a/crates/util/Cargo.toml +++ b/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 diff --git a/crates/util/src/schemars.rs b/crates/util/src/schemars.rs new file mode 100644 index 0000000000..4d8ab530dd --- /dev/null +++ b/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( + 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); + } +} diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index eb07d3e5e5..86bee7ffd1 100644 --- a/crates/util/src/util.rs +++ b/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;