Make Option<T> fields optional in generated schemas (#16)

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Current changes (version TBC)
+### Fixed:
+- When deriving `JsonSchema` on structs, `Option<T>` 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:

schemars/src/flatten.rs

@@ -15,7 +15,7 @@ impl Schema {
     }
 }
 
-trait Merge: Sized {
+pub(crate) trait Merge: Sized {
     fn merge(self, other: Self) -> Self;
 }
 

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<T: ?Sized + JsonSchema>(&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<T: ?Sized + JsonSchema>(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());
+    }
 }

schemars/src/json_schema_impls/core.rs

@@ -35,7 +35,8 @@ impl<T: JsonSchema> JsonSchema for Option<T> {
             }
         }
         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<T: JsonSchema> JsonSchema for Option<T> {
         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<T: JsonSchema> JsonSchema for Option<T> {
         }
         schema
     }
+
+    fn add_schema_as_property(
+        gen: &mut SchemaGenerator,
+        parent: &mut SchemaObject,
+        name: String,
+        metadata: Option<Metadata>,
+        _required: bool,
+    ) {
+        let mut schema = gen.subschema_for::<Self>();
+
+        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<InstanceType>) {

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<crate::schema::Metadata>,
+                required: bool,
+            ) {
+                <$target>::add_schema_as_property(gen, parent, name, metadata, required)
             }
         }
     };

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<schema::Metadata>,
+        required: bool,
+    ) {
+        let mut schema = gen.subschema_for::<Self>();
+
+        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)]

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",

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"
+      }
+    }
+  }
+}

schemars/tests/expected/struct-normal.json

@@ -10,6 +10,12 @@
     "bar": {
       "type": "boolean"
     },
+    "baz": {
+      "type": [
+        "string",
+        "null"
+      ]
+    },
     "foo": {
       "type": "integer",
       "format": "int32"

schemars/tests/expected/struct-tuple.json

@@ -9,8 +9,14 @@
     },
     {
       "type": "boolean"
+    },
+    {
+      "type": [
+        "string",
+        "null"
+      ]
     }
   ],
-  "maxItems": 2,
-  "minItems": 2
+  "maxItems": 3,
+  "minItems": 3
 }

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::<Flat>("flatten")
+}
+
+#[test]
+fn test_flattened_schema() -> TestResult {
+    test_default_generated_schema::<Deep1>("flatten")
 }

schemars/tests/struct.rs

@@ -6,6 +6,7 @@ use util::*;
 pub struct Struct {
     foo: i32,
     bar: bool,
+    baz: Option<String>,
 }
 
 #[test]
@@ -14,7 +15,7 @@ fn struct_normal() -> TestResult {
 }
 
 #[derive(Debug, JsonSchema)]
-pub struct Tuple(i32, bool);
+pub struct Tuple(i32, bool, Option<String>);
 
 #[test]
 fn struct_tuple() -> TestResult {