Fix up schema naming - nightly no longer required!

schemars/rust-toolchain

@@ -1 +0,0 @@
-nightly

schemars/src/gen.rs

@@ -1,9 +1,7 @@
-use crate::make_schema::{MakeSchema, SchemaTypeId};
+use crate::make_schema::MakeSchema;
 use crate::schema::*;
 use crate::{MakeSchemaError, Result};
 use std::collections::BTreeMap as Map;
-use std::collections::BTreeSet as Set;
-use std::iter::FromIterator;
 
 #[derive(Debug, PartialEq, Clone)]
 pub struct SchemaSettings {
@@ -54,8 +52,7 @@ impl SchemaSettings {
 #[derive(Debug, Default, Clone)]
 pub struct SchemaGenerator {
     settings: SchemaSettings,
-    names: Set<String>,
+    definitions: Map<String, Schema>,
-    definitions: Map<SchemaTypeId, (String, Schema)>,
 }
 
 impl SchemaGenerator {
@@ -94,52 +91,37 @@ impl SchemaGenerator {
             return T::make_schema(self);
         }
 
-        let type_id = T::schema_type_id();
+        let name = T::schema_name();
-        let name = self
+        if !self.definitions.contains_key(&name) {
-            .definitions
+            self.insert_new_subschema_for::<T>(name.clone())?;
-            .get(&type_id)
+        }
-            .map(|(n, _)| Ok(n.clone()))
-            .unwrap_or_else(|| {
-                let name = self.make_unique_name::<T>();
-                self.insert_new_subschema_for::<T>(type_id, name.clone())?;
-                Ok(name)
-            })?;
         let reference = format!("{}{}", self.settings().definitions_path, name);
         Ok(SchemaRef { reference }.into())
     }
 
-    fn insert_new_subschema_for<T: ?Sized + MakeSchema>(
+    fn insert_new_subschema_for<T: ?Sized + MakeSchema>(&mut self, name: String) -> Result<()> {
-        &mut self,
-        type_id: SchemaTypeId,
-        name: String,
-    ) -> Result<String> {
-        self.names.insert(name.clone());
         let dummy = Schema::Bool(false);
         // insert into definitions BEFORE calling make_schema to avoid infinite recursion
-        self.definitions
+        self.definitions.insert(name.clone(), dummy);
-            .insert(type_id.clone(), (name.clone(), dummy));
 
         match T::make_schema(self) {
             Ok(schema) => {
-                self.definitions
+                self.definitions.insert(name.clone(), schema);
-                    .entry(type_id)
+                Ok(())
-                    .and_modify(|(_, s)| *s = schema);
-                Ok(name)
             }
             Err(e) => {
-                self.names.remove(&name);
+                self.definitions.remove(&name);
-                self.definitions.remove(&type_id);
                 Err(e)
             }
         }
     }
 
-    pub fn definitions(&self) -> Map<String, Schema> {
+    pub fn definitions(&self) -> &Map<String, Schema> {
-        Map::from_iter(self.definitions.values().cloned())
+        &self.definitions
     }
 
     pub fn into_definitions(self) -> Map<String, Schema> {
-        Map::from_iter(self.definitions.into_iter().map(|(_, v)| v))
+        self.definitions
     }
 
     pub fn root_schema_for<T: ?Sized + MakeSchema>(&mut self) -> Result {
@@ -148,7 +130,7 @@ impl SchemaGenerator {
             Schema::Object(mut o) => {
                 o.schema = Some("http://json-schema.org/draft-07/schema#".to_owned());
                 o.title = Some(T::schema_name());
-                o.definitions.extend(self.definitions());
+                o.definitions.extend(self.definitions().clone());
                 Schema::Object(o)
             }
             schema => schema,
@@ -187,34 +169,25 @@ impl SchemaGenerator {
                             Schema::Ref(r.clone()),
                         )
                     })?;
-                    // FIXME this is pretty inefficient
-                    schema = self
-                        .definitions
-                        .values()
-                        .filter(|(n, _)| n == name)
-                        .map(|(_, s)| s)
-                        .next()
-                        .ok_or_else(|| {
-                            MakeSchemaError::new(
-                                "Could not find referenced schema.",
-                                Schema::Ref(r.clone()),
-                            )
-                        })?;
-                }
-            }
-        }
-    }
 
-    fn make_unique_name<T: ?Sized + MakeSchema>(&mut self) -> String {
+                    schema = self.definitions.get(name).ok_or_else(|| {
-        let base_name = T::schema_name();
+                        MakeSchemaError::new(
-        if self.names.contains(&base_name) {
+                            "Could not find referenced schema.",
-            for i in 2.. {
+                            Schema::Ref(r.clone()),
-                let name = format!("{}{}", base_name, i);
+                        )
-                if !self.names.contains(&name) {
+                    })?;
-                    return name;
+
+                    match schema {
+                        Schema::Ref(r2) if r2 == r => {
+                            return Err(MakeSchemaError::new(
+                                "Schema is referencing itself.",
+                                schema.clone(),
+                            ));
+                        }
+                        _ => {}
+                    }
                 }
             }
         }
-        base_name
     }
 }

schemars/src/make_schema.rs

@@ -4,36 +4,14 @@ use crate::Result;
 use serde_json::json;
 use std::collections::BTreeMap as Map;
 
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
-pub struct SchemaTypeId(&'static str);
-
 pub trait MakeSchema {
-    fn schema_type_id() -> SchemaTypeId {
-        // FIXME schema name might not be unique!
-        SchemaTypeId(core::any::type_name::<Self>())
-    }
-
-    fn schema_name() -> String {
-        // TODO this requires nightly
-        // It's probably worth removing the default implemenation,
-        //  then make every impl in this file set an explicit name
-        // Or maybe hide it under feature flag?
-        core::any::type_name::<Self>().replace(|c: char| !c.is_ascii_alphanumeric(), "_")
-    }
-
     fn is_referenceable() -> bool {
         true
     }
 
-    fn make_schema(gen: &mut SchemaGenerator) -> Result;
+    fn schema_name() -> String;
-}
 
-macro_rules! no_ref_schema {
+    fn make_schema(gen: &mut SchemaGenerator) -> Result;
-    () => {
-        fn is_referenceable() -> bool {
-            false
-        }
-    };
 }
 
 // TODO any other serde/json types other than serde_json value?
@@ -46,13 +24,25 @@ macro_rules! no_ref_schema {
 // NonZeroU8 etc., ArcWeak, RcWeak, BTreeMap, HashMap, (!)?, Bound?, Range?, RangeInclusive?,
 // PhantomData?, CString?, CStr?, fmt::Arguments?
 
+macro_rules! no_ref_schema {
+    () => {
+        fn is_referenceable() -> bool {
+            false
+        }
+    };
+}
+
 ////////// PRIMITIVES //////////
 
 macro_rules! simple_impl {
-    ($type:tt => $instance_type:tt) => {
+    ($type:tt => $instance_type:ident) => {
         impl MakeSchema for $type {
             no_ref_schema!();
 
+            fn schema_name() -> String {
+                stringify!($instance_type).to_owned()
+            }
+
             fn make_schema(_: &mut SchemaGenerator) -> Result {
                 Ok(SchemaObject {
                     instance_type: Some(InstanceType::$instance_type.into()),
@@ -86,6 +76,10 @@ simple_impl!(() => Null);
 impl MakeSchema for char {
     no_ref_schema!();
 
+    fn schema_name() -> String {
+        "Character".to_owned()
+    }
+
     fn make_schema(_: &mut SchemaGenerator) -> Result {
         let mut extensions = Map::new();
         extensions.insert("minLength".to_owned(), json!(1));
@@ -105,6 +99,10 @@ impl MakeSchema for char {
 impl<T> MakeSchema for [T; 0] {
     no_ref_schema!();
 
+    fn schema_name() -> String {
+        "Empty_Array".to_owned()
+    }
+
     fn make_schema(_: &mut SchemaGenerator) -> Result {
         let mut extensions = Map::new();
         extensions.insert("maxItems".to_owned(), json!(0));
@@ -123,6 +121,10 @@ macro_rules! array_impls {
             impl<T: MakeSchema> MakeSchema for [T; $len] {
                 no_ref_schema!();
 
+                fn schema_name() -> String {
+                    format!("Array_Size_{}_Of_{}", $len, T::schema_name())
+                }
+
                 fn make_schema(gen: &mut SchemaGenerator) -> Result {
                     let mut extensions = Map::new();
                     extensions.insert("minItems".to_owned(), json!($len));
@@ -154,6 +156,10 @@ macro_rules! tuple_impls {
             impl<$($name: MakeSchema),+> MakeSchema for ($($name,)+) {
                 no_ref_schema!();
 
+                fn schema_name() -> String {
+                    ["Tuple_Of".to_owned()$(, $name::schema_name())+].join("_And_")
+                }
+
                 fn make_schema(gen: &mut SchemaGenerator) -> Result {
                     let mut extensions = Map::new();
                     extensions.insert("minItems".to_owned(), json!($len));
@@ -202,6 +208,10 @@ macro_rules! seq_impl {
         {
             no_ref_schema!();
 
+            fn schema_name() -> String {
+                format!("Array_Of_{}", T::schema_name())
+            }
+
             fn make_schema(gen: &mut SchemaGenerator) -> Result {
                 Ok(SchemaObject {
                     instance_type: Some(InstanceType::Array.into()),
@@ -231,6 +241,10 @@ macro_rules! map_impl {
         {
             no_ref_schema!();
 
+            fn schema_name() -> String {
+                format!("Map_Of_{}", V::schema_name())
+            }
+
             fn make_schema(gen: &mut SchemaGenerator) -> Result {
                 let subschema = gen.subschema_for::<V>()?;
                 let make_schema_bool = gen.settings().bool_schemas == BoolSchemas::AdditionalPropertiesOnly
@@ -260,13 +274,15 @@ map_impl!(<K: Eq + core::hash::Hash, V, H: core::hash::BuildHasher> MakeSchema f
 ////////// OPTION //////////
 
 impl<T: MakeSchema> MakeSchema for Option<T> {
-    fn is_referenceable() -> bool {
+    no_ref_schema!();
-        // TODO only really needs to be referenceable with option_nullable enabled.
+
-        // TODO what if T is Box<U> and U is referenceable?
+    fn schema_name() -> String {
-        T::is_referenceable()
+        format!("Nullable_{}", T::schema_name())
     }
 
     fn make_schema(gen: &mut SchemaGenerator) -> Result {
+        // FIXME this may produce a subschema that is not referenced in the final schema,
+        // e.g. SingleOrVec_For_InstanceType in schema-openapi3.json
         let mut schema = gen.subschema_for::<T>()?;
         if gen.settings().option_add_null_type {
             schema = match schema {
@@ -296,10 +312,16 @@ macro_rules! deref_impl {
         where
             T: MakeSchema,
         {
-            no_ref_schema!();
+            fn is_referenceable() -> bool {
+                T::is_referenceable()
+            }
+
+            fn schema_name() -> String {
+                T::schema_name()
+            }
 
             fn make_schema(gen: &mut SchemaGenerator) -> Result {
-                gen.subschema_for::<T>()
+                T::make_schema(gen)
             }
         }
     };
@@ -317,6 +339,10 @@ deref_impl!(<'a, T: ToOwned> MakeSchema for std::borrow::Cow<'a, T>);
 impl MakeSchema for serde_json::Value {
     no_ref_schema!();
 
+    fn schema_name() -> String {
+        "Any_Value".to_owned()
+    }
+
     fn make_schema(gen: &mut SchemaGenerator) -> Result {
         Ok(gen.schema_for_any())
     }

schemars/tests/schema-openapi3.json

@@ -1,47 +1,19 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "title": "schemars__schema__Schema",
+  "title": "Schema",
   "anyOf": [
     {
       "type": "boolean"
     },
     {
-      "$ref": "#/components/schemas/schemars__schema__SchemaRef"
+      "$ref": "#/components/schemas/SchemaRef"
     },
     {
-      "$ref": "#/components/schemas/schemars__schema__SchemaObject"
+      "$ref": "#/components/schemas/SchemaObject"
     }
   ],
   "definitions": {
-    "core__option__Option_schemars__schema__SingleOrVec_schemars__schema__InstanceType__": {
+    "InstanceType": {
-      "anyOf": [
-        {
-          "$ref": "#/components/schemas/schemars__schema__InstanceType"
-        },
-        {
-          "type": "array",
-          "items": {
-            "$ref": "#/components/schemas/schemars__schema__InstanceType"
-          }
-        }
-      ],
-      "nullable": true
-    },
-    "core__option__Option_schemars__schema__SingleOrVec_schemars__schema__Schema__": {
-      "anyOf": [
-        {
-          "$ref": "#/components/schemas/schemars__schema__Schema"
-        },
-        {
-          "type": "array",
-          "items": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
-          }
-        }
-      ],
-      "nullable": true
-    },
-    "schemars__schema__InstanceType": {
       "enum": [
         "null",
         "boolean",
@@ -52,20 +24,20 @@
         "integer"
       ]
     },
-    "schemars__schema__Schema": {
+    "Schema": {
       "anyOf": [
         {
           "type": "boolean"
         },
         {
-          "$ref": "#/components/schemas/schemars__schema__SchemaRef"
+          "$ref": "#/components/schemas/SchemaRef"
         },
         {
-          "$ref": "#/components/schemas/schemars__schema__SchemaObject"
+          "$ref": "#/components/schemas/SchemaObject"
         }
       ]
     },
-    "schemars__schema__SchemaObject": {
+    "SchemaObject": {
       "properties": {
         "$id": {
           "type": "string",
@@ -78,21 +50,21 @@
         "allOf": {
           "type": "array",
           "items": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
+            "$ref": "#/components/schemas/Schema"
           },
           "nullable": true
         },
         "anyOf": {
           "type": "array",
           "items": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
+            "$ref": "#/components/schemas/Schema"
           },
           "nullable": true
         },
         "definitions": {
           "type": "object",
           "additionalProperties": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
+            "$ref": "#/components/schemas/Schema"
           }
         },
         "description": {
@@ -109,7 +81,18 @@
           "additionalProperties": true
         },
         "items": {
-          "$ref": "#/components/schemas/core__option__Option_schemars__schema__SingleOrVec_schemars__schema__Schema__"
+          "anyOf": [
+            {
+              "$ref": "#/components/schemas/Schema"
+            },
+            {
+              "type": "array",
+              "items": {
+                "$ref": "#/components/schemas/Schema"
+              }
+            }
+          ],
+          "nullable": true
         },
         "not": {
           "anyOf": [
@@ -117,10 +100,10 @@
               "type": "boolean"
             },
             {
-              "$ref": "#/components/schemas/schemars__schema__SchemaRef"
+              "$ref": "#/components/schemas/SchemaRef"
             },
             {
-              "$ref": "#/components/schemas/schemars__schema__SchemaObject"
+              "$ref": "#/components/schemas/SchemaObject"
             }
           ],
           "nullable": true
@@ -128,14 +111,14 @@
         "oneOf": {
           "type": "array",
           "items": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
+            "$ref": "#/components/schemas/Schema"
           },
           "nullable": true
         },
         "properties": {
           "type": "object",
           "additionalProperties": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
+            "$ref": "#/components/schemas/Schema"
           }
         },
         "required": {
@@ -150,39 +133,50 @@
           "nullable": true
         },
         "type": {
-          "$ref": "#/components/schemas/core__option__Option_schemars__schema__SingleOrVec_schemars__schema__InstanceType__"
+          "anyOf": [
+            {
+              "$ref": "#/components/schemas/InstanceType"
+            },
+            {
+              "type": "array",
+              "items": {
+                "$ref": "#/components/schemas/InstanceType"
+              }
+            }
+          ],
+          "nullable": true
         }
       }
     },
-    "schemars__schema__SchemaRef": {
+    "SchemaRef": {
       "properties": {
         "$ref": {
           "type": "string"
         }
       }
     },
-    "schemars__schema__SingleOrVec_schemars__schema__InstanceType_": {
+    "SingleOrVec_For_InstanceType": {
       "anyOf": [
         {
-          "$ref": "#/components/schemas/schemars__schema__InstanceType"
+          "$ref": "#/components/schemas/InstanceType"
         },
         {
           "type": "array",
           "items": {
-            "$ref": "#/components/schemas/schemars__schema__InstanceType"
+            "$ref": "#/components/schemas/InstanceType"
           }
         }
       ]
     },
-    "schemars__schema__SingleOrVec_schemars__schema__Schema_": {
+    "SingleOrVec_For_Schema": {
       "anyOf": [
         {
-          "$ref": "#/components/schemas/schemars__schema__Schema"
+          "$ref": "#/components/schemas/Schema"
         },
         {
           "type": "array",
           "items": {
-            "$ref": "#/components/schemas/schemars__schema__Schema"
+            "$ref": "#/components/schemas/Schema"
           }
         }
       ]

schemars/tests/schema.json

@@ -1,39 +1,19 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "title": "schemars__schema__Schema",
+  "title": "Schema",
   "anyOf": [
     {
       "type": "boolean"
     },
     {
-      "$ref": "#/definitions/schemars__schema__SchemaRef"
+      "$ref": "#/definitions/SchemaRef"
     },
     {
-      "$ref": "#/definitions/schemars__schema__SchemaObject"
+      "$ref": "#/definitions/SchemaObject"
     }
   ],
   "definitions": {
-    "core__option__Option_schemars__schema__SingleOrVec_schemars__schema__InstanceType__": {
+    "InstanceType": {
-      "anyOf": [
-        {
-          "$ref": "#/definitions/schemars__schema__SingleOrVec_schemars__schema__InstanceType_"
-        },
-        {
-          "type": "null"
-        }
-      ]
-    },
-    "core__option__Option_schemars__schema__SingleOrVec_schemars__schema__Schema__": {
-      "anyOf": [
-        {
-          "$ref": "#/definitions/schemars__schema__SingleOrVec_schemars__schema__Schema_"
-        },
-        {
-          "type": "null"
-        }
-      ]
-    },
-    "schemars__schema__InstanceType": {
       "enum": [
         "null",
         "boolean",
@@ -44,20 +24,20 @@
         "integer"
       ]
     },
-    "schemars__schema__Schema": {
+    "Schema": {
       "anyOf": [
         {
           "type": "boolean"
         },
         {
-          "$ref": "#/definitions/schemars__schema__SchemaRef"
+          "$ref": "#/definitions/SchemaRef"
         },
         {
-          "$ref": "#/definitions/schemars__schema__SchemaObject"
+          "$ref": "#/definitions/SchemaObject"
         }
       ]
     },
-    "schemars__schema__SchemaObject": {
+    "SchemaObject": {
       "properties": {
         "$id": {
           "anyOf": [
@@ -84,7 +64,7 @@
             {
               "type": "array",
               "items": {
-                "$ref": "#/definitions/schemars__schema__Schema"
+                "$ref": "#/definitions/Schema"
               }
             },
             {
@@ -97,7 +77,7 @@
             {
               "type": "array",
               "items": {
-                "$ref": "#/definitions/schemars__schema__Schema"
+                "$ref": "#/definitions/Schema"
               }
             },
             {
@@ -108,7 +88,7 @@
         "definitions": {
           "type": "object",
           "additionalProperties": {
-            "$ref": "#/definitions/schemars__schema__Schema"
+            "$ref": "#/definitions/Schema"
           }
         },
         "description": {
@@ -137,12 +117,19 @@
           "additionalProperties": true
         },
         "items": {
-          "$ref": "#/definitions/core__option__Option_schemars__schema__SingleOrVec_schemars__schema__Schema__"
+          "anyOf": [
+            {
+              "$ref": "#/definitions/SingleOrVec_For_Schema"
+            },
+            {
+              "type": "null"
+            }
+          ]
         },
         "not": {
           "anyOf": [
             {
-              "$ref": "#/definitions/schemars__schema__Schema"
+              "$ref": "#/definitions/Schema"
             },
             {
               "type": "null"
@@ -154,7 +141,7 @@
             {
               "type": "array",
               "items": {
-                "$ref": "#/definitions/schemars__schema__Schema"
+                "$ref": "#/definitions/Schema"
               }
             },
             {
@@ -165,7 +152,7 @@
         "properties": {
           "type": "object",
           "additionalProperties": {
-            "$ref": "#/definitions/schemars__schema__Schema"
+            "$ref": "#/definitions/Schema"
           }
         },
         "required": {
@@ -192,39 +179,46 @@
           ]
         },
         "type": {
-          "$ref": "#/definitions/core__option__Option_schemars__schema__SingleOrVec_schemars__schema__InstanceType__"
+          "anyOf": [
+            {
+              "$ref": "#/definitions/SingleOrVec_For_InstanceType"
+            },
+            {
+              "type": "null"
+            }
+          ]
         }
       }
     },
-    "schemars__schema__SchemaRef": {
+    "SchemaRef": {
       "properties": {
         "$ref": {
           "type": "string"
         }
       }
     },
-    "schemars__schema__SingleOrVec_schemars__schema__InstanceType_": {
+    "SingleOrVec_For_InstanceType": {
       "anyOf": [
         {
-          "$ref": "#/definitions/schemars__schema__InstanceType"
+          "$ref": "#/definitions/InstanceType"
         },
         {
           "type": "array",
           "items": {
-            "$ref": "#/definitions/schemars__schema__InstanceType"
+            "$ref": "#/definitions/InstanceType"
           }
         }
       ]
     },
-    "schemars__schema__SingleOrVec_schemars__schema__Schema_": {
+    "SingleOrVec_For_Schema": {
       "anyOf": [
         {
-          "$ref": "#/definitions/schemars__schema__Schema"
+          "$ref": "#/definitions/Schema"
         },
         {
           "type": "array",
           "items": {
-            "$ref": "#/definitions/schemars__schema__Schema"
+            "$ref": "#/definitions/Schema"
           }
         }
       ]

schemars_derive/src/lib.rs

@@ -23,18 +23,29 @@ pub fn derive_make_schema(input: proc_macro::TokenStream) -> proc_macro::TokenSt
         return compile_error(input.span(), e).into();
     }
 
-    let name = cont.ident;
-    let (impl_generics, ty_generics, where_clause) = cont.generics.split_for_impl();
-
     let schema = match cont.data {
         Data::Struct(Style::Struct, ref fields) => schema_for_struct(fields),
         Data::Enum(ref variants) => schema_for_enum(variants, &cont.attrs),
         _ => unimplemented!("work in progress!"),
     };
 
+    let name = cont.ident;
+    let type_params: Vec<_> = cont.generics.type_params().map(|ty| &ty.ident).collect();
+    let type_param_fmt = match type_params.len() {
+        0 => "{}".to_owned(),
+        1 => "{}_For_{}".to_owned(),
+        n => format!("{{}}_For_{{}}_And{}", "_{}".repeat(n - 1)),
+    };
+
+    let (impl_generics, ty_generics, where_clause) = cont.generics.split_for_impl();
+
     let impl_block = quote! {
         #[automatically_derived]
         impl #impl_generics schemars::MakeSchema for #name #ty_generics #where_clause {
+            fn schema_name() -> String {
+                format!(#type_param_fmt, stringify!(#name) #(,#type_params::schema_name())*)
+            }
+
             fn make_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::Result {
                 #schema
             }