Fix skip_serializing_if/serialize_with handling

Previously whenever a field with a default value has both `skip_serializing_if` and `with`/`serialize_with` attributes, the value would be converted to a type that performs the custom serialization before checking if it should be serialized. This would cause the wrong type to be given to the skip_serializing_if function, causing a compile error.

Issue #26

schemars/src/gen.rs

@@ -343,12 +343,22 @@ impl SchemaGenerator {
         }
     }
 
-    pub(crate) fn apply_metadata(&self, schema: Schema, metadata: Metadata) -> Schema {
+    /// This function is only public for use by schemars_derive.
-        let mut schema_obj = schema.into();
+    ///
+    /// It should not be considered part of the public API.
+    #[doc(hidden)]
+    pub fn apply_metadata(&self, schema: Schema, metadata: Option<Metadata>) -> Schema {
+        match metadata {
+            None => return schema,
+            Some(metadata) if metadata == Metadata::default() => return schema,
+            Some(metadata) => {
+                let mut schema_obj = schema.into();
 
-        self.make_extensible(&mut schema_obj);
+                self.make_extensible(&mut schema_obj);
-        schema_obj.metadata = Some(Box::new(metadata)).merge(schema_obj.metadata);
+                schema_obj.metadata = Some(Box::new(metadata)).merge(schema_obj.metadata);
 
-        Schema::Object(schema_obj)
+                Schema::Object(schema_obj)
+            }
+        }
     }
 }

schemars/src/json_schema_impls/core.rs

@@ -65,10 +65,7 @@ impl<T: JsonSchema> JsonSchema for Option<T> {
         _required: bool,
     ) {
         let mut schema = gen.subschema_for::<Self>();
-
+        schema = gen.apply_metadata(schema, metadata);
-        if let Some(metadata) = metadata {
-            schema = gen.apply_metadata(schema, metadata);
-        }
 
         let object = parent.object();
         object.properties.insert(name, schema);

schemars/src/lib.rs

@@ -300,10 +300,7 @@ pub trait JsonSchema {
         required: bool,
     ) {
         let mut schema = gen.subschema_for::<Self>();
-
+        schema = gen.apply_metadata(schema, metadata);
-        if let Some(metadata) = metadata {
-            schema = gen.apply_metadata(schema, metadata);
-        }
 
         let object = parent.object();
         if required {

schemars/tests/default.rs

@@ -3,6 +3,10 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use util::*;
 
+fn is_default<T: Default + PartialEq>(value: &T) -> bool {
+    value == &T::default()
+}
+
 fn ten_and_true() -> MyStruct2 {
     MyStruct2 {
         my_int: 10,
@@ -28,9 +32,14 @@ pub struct MyStruct {
     pub my_bool: bool,
     #[serde(serialize_with = "custom_serialize")]
     pub my_struct2: MyStruct2,
+    #[serde(
+        serialize_with = "custom_serialize",
+        skip_serializing_if = "is_default"
+    )]
+    pub my_struct2_default_skipped: MyStruct2,
 }
 
-#[derive(Default, Deserialize, Serialize, JsonSchema, Debug)]
+#[derive(Default, Deserialize, Serialize, JsonSchema, Debug, PartialEq)]
 #[serde(default = "ten_and_true")]
 pub struct MyStruct2 {
     #[serde(default = "six")]

schemars/tests/expected/default.json

@@ -19,6 +19,9 @@
           "$ref": "#/definitions/MyStruct2"
         }
       ]
+    },
+    "my_struct2_default_skipped": {
+      "$ref": "#/definitions/MyStruct2"
     }
   },
   "definitions": {

schemars_derive/src/lib.rs

@@ -316,7 +316,6 @@ fn schema_for_struct(fields: &[Field], cattrs: Option<&serde_attr::Container>) -
             read_only: field.attrs.skip_deserializing(),
             write_only: field.attrs.skip_serializing(),
             default,
-            skip_default_if: field.attrs.skip_serializing_if().cloned(),
             ..SchemaMetadata::from_doc_attrs(&field.original.attrs)
         };
 
@@ -367,6 +366,22 @@ fn field_default_expr(field: &Field, container_has_default: bool) -> Option<Toke
         SerdeDefault::Path(path) => quote!(#path()),
     };
 
+    let default_expr = match field.attrs.skip_serializing_if() {
+        Some(skip_if) => {
+            quote! {
+                {
+                    let default = #default_expr;
+                    if #skip_if(&default) {
+                        None
+                    } else {
+                        Some(default)
+                    }
+                }
+            }
+        }
+        None => quote!(Some(#default_expr)),
+    };
+
     Some(if let Some(ser_with) = field.attrs.serialize_with() {
         quote! {
             {
@@ -382,7 +397,7 @@ fn field_default_expr(field: &Field, container_has_default: bool) -> Option<Toke
                     }
                 }
 
-                _SchemarsDefaultSerialize(#default_expr)
+                #default_expr.map(|d| _SchemarsDefaultSerialize(d))
             }
         }
     } else {

schemars_derive/src/metadata.rs

@@ -1,7 +1,7 @@
 use crate::attr;
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::{ToTokens, TokenStreamExt};
-use syn::{Attribute, ExprPath};
+use syn::Attribute;
 
 #[derive(Debug, Clone, Default)]
 pub struct SchemaMetadata {
@@ -10,7 +10,6 @@ pub struct SchemaMetadata {
     pub read_only: bool,
     pub write_only: bool,
     pub default: Option<TokenStream>,
-    pub skip_default_if: Option<ExprPath>,
 }
 
 impl ToTokens for SchemaMetadata {
@@ -41,19 +40,10 @@ impl SchemaMetadata {
     }
 
     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();
+                let schema = #schema_expr;
-                gen.make_extensible(&mut schema);
+                gen.apply_metadata(schema, #self)
-                let mut metadata = schema.metadata();
-                #(#setters)*
-                schemars::schema::Schema::Object(schema)
             }
         }
     }
@@ -83,19 +73,10 @@ impl SchemaMetadata {
             });
         }
 
-        match (&self.default, &self.skip_default_if) {
+        if let Some(default) = &self.default {
-            (Some(default), Some(skip_if)) => setters.push(quote! {
+            setters.push(quote! {
-                {
+                metadata.default = #default.and_then(|d| schemars::_serde_json::value::to_value(d).ok());
-                    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