diff --git a/CHANGELOG.md b/CHANGELOG.md index a8a21cd..9d3166f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Current changes (version TBC) +### Fixed: +- When deriving `JsonSchema` on structs, `Option` struct fields are no longer included in the list of required properties in the schema (https://github.com/GREsau/schemars/issues/11) + ## [0.7.0-alpha-1] - 2019-12-29 ### Changed: - **BREAKING CHANGE** - `SchemaSettings` can no longer be created using struct initialization syntax. Instead, if you need to use custom schema settings, you can use a constructor function and either: diff --git a/schemars/src/flatten.rs b/schemars/src/flatten.rs index 529b34d..d5e32a9 100644 --- a/schemars/src/flatten.rs +++ b/schemars/src/flatten.rs @@ -15,7 +15,7 @@ impl Schema { } } -trait Merge: Sized { +pub(crate) trait Merge: Sized { fn merge(self, other: Self) -> Self; } diff --git a/schemars/src/gen.rs b/schemars/src/gen.rs index 4f61bbf..17fed1d 100644 --- a/schemars/src/gen.rs +++ b/schemars/src/gen.rs @@ -1,3 +1,4 @@ +use crate::flatten::Merge; use crate::schema::*; use crate::{JsonSchema, Map}; @@ -169,9 +170,9 @@ impl SchemaGenerator { &self.settings } - /// Returns a `SchemaObject` equivalent to the given `schema` which may have validation, metadata or other properties set on it. + /// Modifies the given `SchemaObject` so that it may have validation, metadata or other properties set on it. /// - /// If `schema` is not a `$ref` schema, then this returns `schema` unmodified. Otherwise, depending on this generator's settings, + /// If `schema` is not a `$ref` schema, then this does not modify `schema`. Otherwise, depending on this generator's settings, /// this may wrap the `$ref` in another schema. This is required because in many JSON Schema implementations, a schema with `$ref` /// set may not include other properties. /// @@ -184,24 +185,19 @@ impl SchemaGenerator { /// let ref_schema = SchemaObject::new_ref("foo".to_owned()); /// assert!(ref_schema.is_ref()); /// - /// let extensible_schema = gen.make_extensible(ref_schema.clone()); + /// let mut extensible_schema = ref_schema.clone(); + /// gen.make_extensible(&mut extensible_schema); /// assert_ne!(ref_schema, extensible_schema); /// assert!(!extensible_schema.is_ref()); /// - /// let extensible_schema2 = gen.make_extensible(extensible_schema.clone()); + /// let mut extensible_schema2 = extensible_schema.clone(); + /// gen.make_extensible(&mut extensible_schema); /// assert_eq!(extensible_schema, extensible_schema2); /// ``` - pub fn make_extensible(&self, schema: SchemaObject) -> SchemaObject { + pub fn make_extensible(&self, schema: &mut SchemaObject) { if schema.is_ref() && !self.settings().allow_ref_siblings { - SchemaObject { - subschemas: Some(Box::new(SubschemaValidation { - all_of: Some(vec![schema.into()]), - ..Default::default() - })), - ..Default::default() - } - } else { - schema + let original = std::mem::replace(schema, SchemaObject::default()); + schema.subschemas().all_of = Some(vec![original.into()]); } } @@ -279,8 +275,8 @@ impl SchemaGenerator { /// add them to the `SchemaGenerator`'s schema definitions and include them in the returned `SchemaObject`'s /// [`definitions`](../schema/struct.Metadata.html#structfield.definitions) pub fn root_schema_for(&mut self) -> RootSchema { - let schema = T::json_schema(self); - let mut schema: SchemaObject = self.make_extensible(schema.into()); + let mut schema = T::json_schema(self).into(); + self.make_extensible(&mut schema); schema.metadata().title.get_or_insert_with(T::schema_name); RootSchema { meta_schema: self.settings.meta_schema.clone(), @@ -294,8 +290,8 @@ impl SchemaGenerator { /// If `T`'s schema depends on any [referenceable](JsonSchema::is_referenceable) schemas, then this method will /// include them in the returned `SchemaObject`'s [`definitions`](../schema/struct.Metadata.html#structfield.definitions) pub fn into_root_schema_for(mut self) -> RootSchema { - let schema = T::json_schema(&mut self); - let mut schema: SchemaObject = self.make_extensible(schema.into()); + let mut schema = T::json_schema(&mut self).into(); + self.make_extensible(&mut schema); schema.metadata().title.get_or_insert_with(T::schema_name); RootSchema { meta_schema: self.settings.meta_schema, @@ -346,4 +342,11 @@ impl SchemaGenerator { _ => None, } } + + // TODO should this take a Schema instead of SchemaObject? + pub(crate) fn apply_metadata(&self, schema: &mut SchemaObject, metadata: Metadata) { + self.make_extensible(schema); + // TODO get rid of the clone + schema.metadata = Some(Box::new(metadata)).merge(schema.metadata.clone()); + } } diff --git a/schemars/src/json_schema_impls/core.rs b/schemars/src/json_schema_impls/core.rs index de6975b..9f0fe7c 100644 --- a/schemars/src/json_schema_impls/core.rs +++ b/schemars/src/json_schema_impls/core.rs @@ -35,7 +35,8 @@ impl JsonSchema for Option { } } if gen.settings().option_nullable { - let mut schema_obj = gen.make_extensible(schema.into()); + let mut schema_obj = schema.into(); + gen.make_extensible(&mut schema_obj); schema_obj .extensions .insert("nullable".to_owned(), json!(true)); @@ -44,8 +45,8 @@ impl JsonSchema for Option { schema } - fn json_schema_optional(gen: &mut SchemaGenerator) -> Schema { - let mut schema = T::json_schema_optional(gen); + fn json_schema_for_flatten(gen: &mut SchemaGenerator) -> Schema { + let mut schema = T::json_schema_for_flatten(gen); if let Schema::Object(SchemaObject { object: Some(ref mut object_validation), .. @@ -55,6 +56,25 @@ impl JsonSchema for Option { } schema } + + fn add_schema_as_property( + gen: &mut SchemaGenerator, + parent: &mut SchemaObject, + name: String, + metadata: Option, + _required: bool, + ) { + let mut schema = gen.subschema_for::(); + + if let Some(metadata) = metadata { + let mut schema_obj = schema.into(); + gen.apply_metadata(&mut schema_obj, metadata); + schema = Schema::Object(schema_obj); + } + + let object = parent.object(); + object.properties.insert(name, schema); + } } fn add_null_type(instance_type: &mut SingleOrVec) { diff --git a/schemars/src/json_schema_impls/mod.rs b/schemars/src/json_schema_impls/mod.rs index 4259a53..f0a311a 100644 --- a/schemars/src/json_schema_impls/mod.rs +++ b/schemars/src/json_schema_impls/mod.rs @@ -21,8 +21,18 @@ macro_rules! forward_impl { <$target>::json_schema(gen) } - fn json_schema_optional(gen: &mut SchemaGenerator) -> Schema { - <$target>::json_schema_optional(gen) + fn json_schema_for_flatten(gen: &mut SchemaGenerator) -> Schema { + <$target>::json_schema_for_flatten(gen) + } + + fn add_schema_as_property( + gen: &mut SchemaGenerator, + parent: &mut crate::schema::SchemaObject, + name: String, + metadata: Option, + required: bool, + ) { + <$target>::add_schema_as_property(gen, parent, name, metadata, required) } } }; diff --git a/schemars/src/lib.rs b/schemars/src/lib.rs index 1568e0f..4d3f0db 100644 --- a/schemars/src/lib.rs +++ b/schemars/src/lib.rs @@ -236,7 +236,7 @@ pub use schemars_derive::*; #[doc(hidden)] pub use serde_json as _serde_json; -use schema::Schema; +use schema::{Schema, SchemaObject}; /// A type which can be described as a JSON Schema document. /// @@ -281,11 +281,39 @@ pub trait JsonSchema { /// Helper for generating schemas for flattened `Option` fields. /// - /// This should not need to be called or implemented by code outside of `schemars`. + /// This should not need to be called or implemented by code outside of `schemars`, + /// and should not be considered part of the public API. #[doc(hidden)] - fn json_schema_optional(gen: &mut gen::SchemaGenerator) -> Schema { + fn json_schema_for_flatten(gen: &mut gen::SchemaGenerator) -> Schema { Self::json_schema(gen) } + + /// Helper for generating schemas for `Option` fields. + /// + /// This should not need to be called or implemented by code outside of `schemars`, + /// and should not be considered part of the public API. + #[doc(hidden)] + fn add_schema_as_property( + gen: &mut gen::SchemaGenerator, + parent: &mut SchemaObject, + name: String, + metadata: Option, + required: bool, + ) { + let mut schema = gen.subschema_for::(); + + if let Some(metadata) = metadata { + let mut schema_obj = schema.into(); + gen.apply_metadata(&mut schema_obj, metadata); + schema = Schema::Object(schema_obj); + } + + let object = parent.object(); + if required { + object.required.insert(name.clone()); + } + object.properties.insert(name, schema); + } } #[cfg(test)] diff --git a/schemars/tests/expected/doc_comments_enum.json b/schemars/tests/expected/doc_comments_enum.json index 559ffa6..2115c37 100644 --- a/schemars/tests/expected/doc_comments_enum.json +++ b/schemars/tests/expected/doc_comments_enum.json @@ -19,9 +19,6 @@ "properties": { "Complex": { "type": "object", - "required": [ - "my_nullable_string" - ], "properties": { "my_nullable_string": { "title": "A nullable string", diff --git a/schemars/tests/expected/flatten.json b/schemars/tests/expected/flatten.json new file mode 100644 index 0000000..9e75ece --- /dev/null +++ b/schemars/tests/expected/flatten.json @@ -0,0 +1,34 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Flat", + "type": "object", + "required": [ + "b", + "f", + "s", + "v" + ], + "properties": { + "b": { + "type": "boolean" + }, + "f": { + "type": "number", + "format": "float" + }, + "os": { + "default": "", + "type": "string" + }, + "s": { + "type": "string" + }, + "v": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + } + } +} \ No newline at end of file diff --git a/schemars/tests/expected/struct-normal.json b/schemars/tests/expected/struct-normal.json index 98a5e36..c699a49 100644 --- a/schemars/tests/expected/struct-normal.json +++ b/schemars/tests/expected/struct-normal.json @@ -10,6 +10,12 @@ "bar": { "type": "boolean" }, + "baz": { + "type": [ + "string", + "null" + ] + }, "foo": { "type": "integer", "format": "int32" diff --git a/schemars/tests/expected/struct-tuple.json b/schemars/tests/expected/struct-tuple.json index 5c4893d..ced169e 100644 --- a/schemars/tests/expected/struct-tuple.json +++ b/schemars/tests/expected/struct-tuple.json @@ -9,8 +9,14 @@ }, { "type": "boolean" + }, + { + "type": [ + "string", + "null" + ] } ], - "maxItems": 2, - "minItems": 2 + "maxItems": 3, + "minItems": 3 } \ No newline at end of file diff --git a/schemars/tests/flatten.rs b/schemars/tests/flatten.rs index e445f34..d193eb5 100644 --- a/schemars/tests/flatten.rs +++ b/schemars/tests/flatten.rs @@ -1,6 +1,6 @@ mod util; -use pretty_assertions::assert_eq; -use schemars::{schema_for, JsonSchema}; +use schemars::JsonSchema; +use util::*; #[derive(Debug, JsonSchema)] struct Flat { @@ -43,8 +43,11 @@ struct Deep4 { } #[test] -fn flatten_schema() { - let flat = schema_for!(Flat); - let deep = schema_for!(Deep1); - assert_eq!(flat, deep); +fn test_flat_schema() -> TestResult { + test_default_generated_schema::("flatten") +} + +#[test] +fn test_flattened_schema() -> TestResult { + test_default_generated_schema::("flatten") } diff --git a/schemars/tests/struct.rs b/schemars/tests/struct.rs index 4082ed8..e522794 100644 --- a/schemars/tests/struct.rs +++ b/schemars/tests/struct.rs @@ -6,6 +6,7 @@ use util::*; pub struct Struct { foo: i32, bar: bool, + baz: Option, } #[test] @@ -14,7 +15,7 @@ fn struct_normal() -> TestResult { } #[derive(Debug, JsonSchema)] -pub struct Tuple(i32, bool); +pub struct Tuple(i32, bool, Option); #[test] fn struct_tuple() -> TestResult { diff --git a/schemars_derive/src/lib.rs b/schemars_derive/src/lib.rs index 042ac49..98e3810 100644 --- a/schemars_derive/src/lib.rs +++ b/schemars_derive/src/lib.rs @@ -38,7 +38,8 @@ pub fn derive_json_schema(input: proc_macro::TokenStream) -> proc_macro::TokenSt Data::Struct(Style::Struct, ref fields) => schema_for_struct(fields, Some(&cont.attrs)), Data::Enum(ref variants) => schema_for_enum(variants, &cont.attrs), }; - let schema_expr = set_metadata_on_schema_from_docs(schema_expr, &cont.original.attrs); + let doc_metadata = SchemaMetadata::from_doc_attrs(&cont.original.attrs); + let schema_expr = doc_metadata.apply_to_schema(schema_expr); let type_name = cont.ident; let type_params: Vec<_> = cont.generics.type_params().map(|ty| &ty.ident).collect(); @@ -176,7 +177,8 @@ fn schema_for_external_tagged_enum<'a>( ..Default::default() })), }); - set_metadata_on_schema_from_docs(schema_expr, &variant.original.attrs) + let doc_metadata = SchemaMetadata::from_doc_attrs(&variant.original.attrs); + doc_metadata.apply_to_schema(schema_expr) })); wrap_schema_fields(quote! { @@ -214,7 +216,8 @@ fn schema_for_internal_tagged_enum<'a>( ..Default::default() })), }); - let tag_schema = set_metadata_on_schema_from_docs(tag_schema, &variant.original.attrs); + let doc_metadata = SchemaMetadata::from_doc_attrs(&variant.original.attrs); + let tag_schema = doc_metadata.apply_to_schema(tag_schema); let variant_schema = match variant.style { Style::Unit => return tag_schema, @@ -244,7 +247,8 @@ fn schema_for_internal_tagged_enum<'a>( fn schema_for_untagged_enum<'a>(variants: impl Iterator>) -> TokenStream { let schemas = variants.map(|variant| { let schema_expr = schema_for_untagged_enum_variant(variant); - set_metadata_on_schema_from_docs(schema_expr, &variant.original.attrs) + let doc_metadata = SchemaMetadata::from_doc_attrs(&variant.original.attrs); + doc_metadata.apply_to_schema(schema_expr) }); wrap_schema_fields(quote! { @@ -288,7 +292,7 @@ fn schema_for_tuple_struct(fields: &[Field]) -> TokenStream { } fn schema_for_struct(fields: &[Field], cattrs: Option<&serde_attr::Container>) -> TokenStream { - let (flat, nested): (Vec<_>, Vec<_>) = fields + let (flattened_fields, property_fields): (Vec<_>, Vec<_>) = fields .iter() .filter(|f| !f.attrs.skip_deserializing() || !f.attrs.skip_serializing()) .partition(|f| f.attrs.flatten()); @@ -299,63 +303,50 @@ fn schema_for_struct(fields: &[Field], cattrs: Option<&serde_attr::Container>) - SerdeDefault::Path(path) => Some(quote!(let container_default = #path();)), }; - let mut required = Vec::new(); - let recurse = nested.iter().map(|field| { + let properties = property_fields.iter().map(|field| { let name = field.attrs.name().deserialize_name(); let default = field_default_expr(field, set_container_default.is_some()); - if default.is_none() { - required.push(name.clone()); - } - - let ty = get_json_schema_type(field); - let span = field.original.span(); - let schema_expr = quote_spanned! {span=> - gen.subschema_for::<#ty>() + let required = match default { + Some(_) => quote!(false), + None => quote!(true), }; - let metadata = SchemaMetadata { + let metadata = &SchemaMetadata { read_only: field.attrs.skip_deserializing(), write_only: field.attrs.skip_serializing(), default, skip_default_if: field.attrs.skip_serializing_if().cloned(), - ..get_metadata_from_docs(&field.original.attrs) + ..SchemaMetadata::from_doc_attrs(&field.original.attrs) }; - let schema_expr = set_metadata_on_schema(schema_expr, &metadata); + + let ty = get_json_schema_type(field); + let span = field.original.span(); quote_spanned! {span=> - props.insert(#name.to_owned(), #schema_expr); + <#ty>::add_schema_as_property(gen, &mut schema_object, #name.to_owned(), #metadata, #required); } }); - let schema = wrap_schema_fields(quote! { - instance_type: Some(schemars::schema::InstanceType::Object.into()), - object: Some(Box::new(schemars::schema::ObjectValidation { - properties: { - let mut props = schemars::Map::new(); - #(#recurse)* - props - }, - required: { - let mut required = schemars::Set::new(); - #(required.insert(#required.to_owned());)* - required - }, - ..Default::default() - })), - }); - - let flattens = flat.iter().map(|field| { + let flattens = flattened_fields.iter().map(|field| { let ty = get_json_schema_type(field); - quote_spanned! {field.original.span()=> - .flatten(<#ty>::json_schema_optional(gen)) + let span = field.original.span(); + + quote_spanned! {span=> + .flatten(<#ty>::json_schema_for_flatten(gen)) } }); quote! { { #set_container_default - #schema #(#flattens)* + let mut schema_object = schemars::schema::SchemaObject { + instance_type: Some(schemars::schema::InstanceType::Object.into()), + ..Default::default() + }; + #(#properties)* + schemars::schema::Schema::Object(schema_object) + #(#flattens)* } } } diff --git a/schemars_derive/src/metadata.rs b/schemars_derive/src/metadata.rs index b88f3f2..82e003c 100644 --- a/schemars_derive/src/metadata.rs +++ b/schemars_derive/src/metadata.rs @@ -1,5 +1,6 @@ use crate::attr; -use proc_macro2::TokenStream; +use proc_macro2::{Ident, Span, TokenStream}; +use quote::{ToTokens, TokenStreamExt}; use syn::{Attribute, ExprPath}; #[derive(Debug, Clone, Default)] @@ -12,74 +13,91 @@ pub struct SchemaMetadata { pub skip_default_if: Option, } -pub fn set_metadata_on_schema_from_docs( - schema_expr: TokenStream, - attrs: &[Attribute], -) -> TokenStream { - let metadata = get_metadata_from_docs(attrs); - set_metadata_on_schema(schema_expr, &metadata) -} - -pub fn get_metadata_from_docs(attrs: &[Attribute]) -> SchemaMetadata { - let (title, description) = attr::get_title_and_desc_from_doc(attrs); - SchemaMetadata { - title, - description, - ..Default::default() - } -} - -pub fn set_metadata_on_schema(schema_expr: TokenStream, metadata: &SchemaMetadata) -> TokenStream { - let mut setters = Vec::::new(); - - if let Some(title) = &metadata.title { - setters.push(quote! { - metadata.title = Some(#title.to_owned()); - }); - } - if let Some(description) = &metadata.description { - setters.push(quote! { - metadata.description = Some(#description.to_owned()); - }); - } - - if metadata.read_only { - setters.push(quote! { - metadata.read_only = true; - }); - } - if metadata.write_only { - setters.push(quote! { - metadata.write_only = true; - }); - } - - match (&metadata.default, &metadata.skip_default_if) { - (Some(default), Some(skip_if)) => setters.push(quote! { - { - let default = #default; - if !#skip_if(&default) { - metadata.default = schemars::_serde_json::value::to_value(default).ok(); - } - } - }), - (Some(default), None) => setters.push(quote! { - metadata.default = schemars::_serde_json::value::to_value(#default).ok(); - }), - _ => {} - } - - if setters.is_empty() { - return schema_expr; - } - - quote! { - { - let schema = #schema_expr.into(); - let mut schema_obj = gen.make_extensible(schema); - let mut metadata = schema_obj.metadata(); - #(#setters)* - schemars::schema::Schema::Object(schema_obj) +impl ToTokens for SchemaMetadata { + fn to_tokens(&self, tokens: &mut TokenStream) { + let setters = self.make_setters(); + if setters.is_empty() { + tokens.append(Ident::new("None", Span::call_site())) + } else { + tokens.extend(quote! { + Some({ + let mut metadata = schemars::schema::Metadata::default(); + #(#setters)* + metadata + }) + }) } } } + +impl SchemaMetadata { + pub fn from_doc_attrs(attrs: &[Attribute]) -> SchemaMetadata { + let (title, description) = attr::get_title_and_desc_from_doc(attrs); + SchemaMetadata { + title, + description, + ..Default::default() + } + } + + pub fn apply_to_schema(&self, schema_expr: TokenStream) -> TokenStream { + let setters = self.make_setters(); + + if setters.is_empty() { + return schema_expr; + } + + quote! { + { + let mut schema = #schema_expr.into(); + gen.make_extensible(&mut schema); + let mut metadata = schema.metadata(); + #(#setters)* + schemars::schema::Schema::Object(schema) + } + } + } + + fn make_setters(self: &SchemaMetadata) -> Vec { + let mut setters = Vec::::new(); + + if let Some(title) = &self.title { + setters.push(quote! { + metadata.title = Some(#title.to_owned()); + }); + } + if let Some(description) = &self.description { + setters.push(quote! { + metadata.description = Some(#description.to_owned()); + }); + } + + if self.read_only { + setters.push(quote! { + metadata.read_only = true; + }); + } + if self.write_only { + setters.push(quote! { + metadata.write_only = true; + }); + } + + match (&self.default, &self.skip_default_if) { + (Some(default), Some(skip_if)) => setters.push(quote! { + { + let default = #default; + if !#skip_if(&default) { + metadata.default = schemars::_serde_json::value::to_value(default).ok(); + } + } + }), + (Some(default), None) => setters.push(quote! { + metadata.default = schemars::_serde_json::value::to_value(#default).ok(); + }), + _ => {} + } + + setters + } +}