Simplify generated enum code (#286)

* simplify the code generated for unit enums
* simplify generated code for validating object properties
* optimize internal and externally tagged enums

---------

Co-authored-by: Robin Appelman <robin@icewind.nl>

schemars/src/_private.rs

@@ -1,7 +1,6 @@
-use crate::flatten::Merge;
 use crate::gen::SchemaGenerator;
-use crate::schema::{Metadata, Schema, SchemaObject};
+use crate::schema::{InstanceType, ObjectValidation, Schema, SchemaObject};
-use crate::JsonSchema;
+use crate::{JsonSchema, Map, Set};
 use serde::Serialize;
 use serde_json::Value;
 
@@ -25,16 +24,6 @@ pub fn json_schema_for_flatten<T: ?Sized + JsonSchema>(
     schema
 }
 
-pub fn apply_metadata(schema: Schema, metadata: Metadata) -> Schema {
-    if metadata == Metadata::default() {
-        schema
-    } else {
-        let mut schema_obj = schema.into_object();
-        schema_obj.metadata = Some(Box::new(metadata)).merge(schema_obj.metadata);
-        Schema::Object(schema_obj)
-    }
-}
-
 /// Hack to simulate specialization:
 /// `MaybeSerializeWrapper(x).maybe_to_value()` will resolve to either
 /// - The inherent method `MaybeSerializeWrapper::maybe_to_value(...)` if x is `Serialize`
@@ -65,3 +54,118 @@ impl<T: Serialize> MaybeSerializeWrapper<T> {
         serde_json::value::to_value(self.0).ok()
     }
 }
+
+/// Create a schema for a unit enum
+pub fn new_unit_enum(variant: &str) -> Schema {
+    Schema::Object(SchemaObject {
+        instance_type: Some(InstanceType::String.into()),
+        enum_values: Some(vec![variant.into()]),
+        ..SchemaObject::default()
+    })
+}
+
+/// Create a schema for an externally tagged enum
+pub fn new_externally_tagged_enum(variant: &str, sub_schema: Schema) -> Schema {
+    Schema::Object(SchemaObject {
+        instance_type: Some(InstanceType::Object.into()),
+        object: Some(Box::new(ObjectValidation {
+            properties: {
+                let mut props = Map::new();
+                props.insert(variant.to_owned(), sub_schema);
+                props
+            },
+            required: {
+                let mut required = Set::new();
+                required.insert(variant.to_owned());
+                required
+            },
+            // Externally tagged variants must prohibit additional
+            // properties irrespective of the disposition of
+            // `deny_unknown_fields`. If additional properties were allowed
+            // one could easily construct an object that validated against
+            // multiple variants since here it's the properties rather than
+            // the values of a property that distingish between variants.
+            additional_properties: Some(Box::new(false.into())),
+            ..Default::default()
+        })),
+        ..SchemaObject::default()
+    })
+}
+
+/// Create a schema for an internally tagged enum
+pub fn new_internally_tagged_enum(
+    tag_name: &str,
+    variant: &str,
+    deny_unknown_fields: bool,
+) -> Schema {
+    let tag_schema = Schema::Object(SchemaObject {
+        instance_type: Some(InstanceType::String.into()),
+        enum_values: Some(vec![variant.into()]),
+        ..Default::default()
+    });
+    Schema::Object(SchemaObject {
+        instance_type: Some(InstanceType::Object.into()),
+        object: Some(Box::new(ObjectValidation {
+            properties: {
+                let mut props = Map::new();
+                props.insert(tag_name.to_owned(), tag_schema);
+                props
+            },
+            required: {
+                let mut required = Set::new();
+                required.insert(tag_name.to_owned());
+                required
+            },
+            additional_properties: deny_unknown_fields.then(|| Box::new(false.into())),
+            ..Default::default()
+        })),
+        ..SchemaObject::default()
+    })
+}
+
+pub fn insert_object_property<T: ?Sized + JsonSchema>(
+    obj: &mut ObjectValidation,
+    key: &str,
+    has_default: bool,
+    required: bool,
+    schema: Schema,
+) {
+    obj.properties.insert(key.to_owned(), schema);
+    if required || !(has_default || T::_schemars_private_is_option()) {
+        obj.required.insert(key.to_owned());
+    }
+}
+
+pub mod metadata {
+    use crate::Schema;
+    use serde_json::Value;
+
+    macro_rules! add_metadata_fn {
+        ($method:ident, $name:ident, $ty:ty) => {
+            pub fn $method(schema: Schema, $name: impl Into<$ty>) -> Schema {
+                let value = $name.into();
+                if value == <$ty>::default() {
+                    schema
+                } else {
+                    let mut schema_obj = schema.into_object();
+                    schema_obj.metadata().$name = value.into();
+                    Schema::Object(schema_obj)
+                }
+            }
+        };
+    }
+
+    add_metadata_fn!(add_description, description, String);
+    add_metadata_fn!(add_id, id, String);
+    add_metadata_fn!(add_title, title, String);
+    add_metadata_fn!(add_deprecated, deprecated, bool);
+    add_metadata_fn!(add_read_only, read_only, bool);
+    add_metadata_fn!(add_write_only, write_only, bool);
+    add_metadata_fn!(add_default, default, Value);
+
+    pub fn add_examples<I: IntoIterator<Item = Value>>(schema: Schema, examples: I) -> Schema {
+        let mut schema_obj = schema.into_object();
+        schema_obj.metadata().examples.extend(examples);
+        Schema::Object(schema_obj)
+    }
+}

schemars_derive/src/metadata.rs

@@ -13,47 +13,32 @@ pub struct SchemaMetadata<'a> {
 
 impl<'a> SchemaMetadata<'a> {
     pub fn apply_to_schema(&self, schema_expr: &mut TokenStream) {
-        let setters = self.make_setters();
-        if !setters.is_empty() {
-            *schema_expr = quote! {{
-                let schema = #schema_expr;
-                schemars::_private::apply_metadata(schema, schemars::schema::Metadata {
-                    #(#setters)*
-                    ..Default::default()
-                })
-            }}
-        }
-    }
-
-    fn make_setters(&self) -> Vec<TokenStream> {
-        let mut setters = Vec::<TokenStream>::new();
-
         if let Some(title) = &self.title {
-            setters.push(quote! {
+            *schema_expr = quote! {
-                title: Some(#title.to_owned()),
+                schemars::_private::metadata::add_title(#schema_expr, #title)
-            });
+            };
         }
         if let Some(description) = &self.description {
-            setters.push(quote! {
+            *schema_expr = quote! {
-                description: Some(#description.to_owned()),
+                schemars::_private::metadata::add_description(#schema_expr, #description)
-            });
+            };
         }
 
         if self.deprecated {
-            setters.push(quote! {
+            *schema_expr = quote! {
-                deprecated: true,
+                schemars::_private::metadata::add_deprecated(#schema_expr, true)
-            });
+            };
         }
 
         if self.read_only {
-            setters.push(quote! {
+            *schema_expr = quote! {
-                read_only: true,
+                schemars::_private::metadata::add_read_only(#schema_expr, true)
-            });
+            };
         }
         if self.write_only {
-            setters.push(quote! {
+            *schema_expr = quote! {
-                write_only: true,
+                schemars::_private::metadata::add_write_only(#schema_expr, true)
-            });
+            };
         }
 
         if !self.examples.is_empty() {
@@ -62,17 +47,16 @@ impl<'a> SchemaMetadata<'a> {
                     schemars::_serde_json::value::to_value(#eg())
                 }
             });
-            setters.push(quote! {
+
-                examples: vec![#(#examples),*].into_iter().flatten().collect(),
+            *schema_expr = quote! {
-            });
+                schemars::_private::metadata::add_examples(#schema_expr,  [#(#examples),*].into_iter().flatten())
+            };
         }
 
         if let Some(default) = &self.default {
-            setters.push(quote! {
+            *schema_expr = quote! {
-                default: #default.and_then(|d| schemars::_schemars_maybe_to_value!(d)),
+                schemars::_private::metadata::add_default(#schema_expr, #default.and_then(|d| schemars::_schemars_maybe_to_value!(d)))
-            });
+            };
         }
-
-        setters
     }
 }

schemars_derive/src/schema_exprs.rs

@@ -162,7 +162,7 @@ fn expr_for_external_tagged_enum<'a>(
     let unit_names = unit_variants.iter().map(|v| v.name());
     let unit_schema = schema_object(quote! {
         instance_type: Some(schemars::schema::InstanceType::String.into()),
-        enum_values: Some(vec![#(#unit_names.into()),*]),
+        enum_values: Some([#(#unit_names),*].into_iter().map(|v| v.into()).collect()),
     });
 
     if complex_variants.is_empty() {
@@ -178,35 +178,14 @@ fn expr_for_external_tagged_enum<'a>(
         let name = variant.name();
 
         let mut schema_expr = if variant.is_unit() && variant.attrs.with.is_none() {
-            schema_object(quote! {
+            quote! {
-                instance_type: Some(schemars::schema::InstanceType::String.into()),
+                schemars::_private::new_unit_enum(#name)
-                enum_values: Some(vec![#name.into()]),
+            }
-            })
         } else {
             let sub_schema = expr_for_untagged_enum_variant(variant, deny_unknown_fields);
-            schema_object(quote! {
+            quote! {
-                instance_type: Some(schemars::schema::InstanceType::Object.into()),
+                schemars::_private::new_externally_tagged_enum(#name, #sub_schema)
-                object: Some(Box::new(schemars::schema::ObjectValidation {
+            }
-                    properties: {
-                        let mut props = schemars::Map::new();
-                        props.insert(#name.to_owned(), #sub_schema);
-                        props
-                    },
-                    required: {
-                        let mut required = schemars::Set::new();
-                        required.insert(#name.to_owned());
-                        required
-                    },
-                    // Externally tagged variants must prohibit additional
-                    // properties irrespective of the disposition of
-                    // `deny_unknown_fields`. If additional properties were allowed
-                    // one could easily construct an object that validated against
-                    // multiple variants since here it's the properties rather than
-                    // the values of a property that distingish between variants.
-                    additional_properties: Some(Box::new(false.into())),
-                    ..Default::default()
-                })),
-            })
         };
 
         variant
@@ -227,43 +206,16 @@ fn expr_for_internal_tagged_enum<'a>(
 ) -> TokenStream {
     let mut unique_names = HashSet::new();
     let mut count = 0;
-    let set_additional_properties = if deny_unknown_fields {
-        quote! {
-            additional_properties: Some(Box::new(false.into())),
-        }
-    } else {
-        TokenStream::new()
-    };
     let variant_schemas = variants
         .map(|variant| {
             unique_names.insert(variant.name());
             count += 1;
 
             let name = variant.name();
-            let type_schema = schema_object(quote! {
-                instance_type: Some(schemars::schema::InstanceType::String.into()),
-                enum_values: Some(vec![#name.into()]),
-            });
 
-            let mut tag_schema = schema_object(quote! {
+            let mut tag_schema = quote! {
-                instance_type: Some(schemars::schema::InstanceType::Object.into()),
+                schemars::_private::new_internally_tagged_enum(#tag_name, #name, #deny_unknown_fields)
-                object: Some(Box::new(schemars::schema::ObjectValidation {
+            };
-                    properties: {
-                        let mut props = schemars::Map::new();
-                        props.insert(#tag_name.to_owned(), #type_schema);
-                        props
-                    },
-                    required: {
-                        let mut required = schemars::Set::new();
-                        required.insert(#tag_name.to_owned());
-                        required
-                    },
-                    // As we're creating a "wrapper" object, we can honor the
-                    // disposition of deny_unknown_fields.
-                    #set_additional_properties
-                    ..Default::default()
-                })),
-            });
 
             variant.attrs.as_metadata().apply_to_schema(&mut tag_schema);
 
@@ -498,19 +450,8 @@ fn expr_for_struct(
 
             let (ty, type_def) = type_for_field_schema(field);
 
-            let maybe_insert_required = match (&default, field.validation_attrs.required()) {
+            let has_default = default.is_some();
-                (Some(_), _) => TokenStream::new(),
+            let required = field.validation_attrs.required();
-                (None, false) => {
-                    quote! {
-                        if !<#ty as schemars::JsonSchema>::_schemars_private_is_option() {
-                            object_validation.required.insert(#name.to_owned());
-                        }
-                    }
-                }
-                (None, true) => quote! {
-                    object_validation.required.insert(#name.to_owned());
-                },
-            };
 
             let metadata = SchemaMetadata {
                 read_only: field.serde_attrs.skip_deserializing(),
@@ -536,8 +477,7 @@ fn expr_for_struct(
             quote! {
                 {
                     #type_def
-                    object_validation.properties.insert(#name.to_owned(), #schema_expr);
+                    schemars::_private::insert_object_property::<#ty>(object_validation, #name, #has_default, #required, #schema_expr);
-                    #maybe_insert_required
                 }
             }
         })