From a555d7739a4e565f5e4c8f8113096fe775ea6ff5 Mon Sep 17 00:00:00 2001 From: Graham Esau Date: Sun, 6 Oct 2019 20:22:27 +0100 Subject: [PATCH] Fix schemas for internally tagged newtype variants Fixes #2 --- schemars/src/flatten.rs | 28 ++++++-- schemars/tests/enum.rs | 15 ++++ schemars/tests/expected/enum-external.json | 79 +++++++++++++++++----- schemars/tests/expected/enum-internal.json | 71 ++++++++++++++----- schemars/tests/expected/enum-untagged.json | 39 +++++++++-- schemars_derive/src/lib.rs | 21 ++++-- 6 files changed, 199 insertions(+), 54 deletions(-) diff --git a/schemars/src/flatten.rs b/schemars/src/flatten.rs index 3a22888..bde467c 100644 --- a/schemars/src/flatten.rs +++ b/schemars/src/flatten.rs @@ -3,8 +3,13 @@ use crate::{JsonSchemaError, Map, Result, Set}; impl Schema { pub fn flatten(self, other: Self) -> Result { - let s1 = ensure_flattenable(self)?; - let s2 = ensure_flattenable(other)?; + if is_null_type(&self) { + return Ok(other) + } else if is_null_type(&other) { + return Ok(self) + } + let s1 = ensure_object_type(self)?; + let s2 = ensure_object_type(other)?; Ok(Schema::Object(s1.merge(s2))) } } @@ -111,12 +116,23 @@ impl Merge for SingleOrVec { } } -fn ensure_flattenable(schema: Schema) -> Result { +fn is_null_type(schema: &Schema) -> bool { + let s = match schema { + Schema::Object(s) => s, + _ => return false, + }; + match &s.instance_type { + Some(SingleOrVec::Single(t)) if **t == InstanceType::Null => true, + _ => false, + } +} + +fn ensure_object_type(schema: Schema) -> Result { let s = match schema { Schema::Object(s) => s, s => { return Err(JsonSchemaError::new( - "Only schemas with type `object` can be flattened.", + "Only schemas with type `object` or `null` can be flattened.", s, )) } @@ -124,13 +140,13 @@ fn ensure_flattenable(schema: Schema) -> Result { match s.instance_type { Some(SingleOrVec::Single(ref t)) if **t != InstanceType::Object => { Err(JsonSchemaError::new( - "Only schemas with type `object` can be flattened.", + "Only schemas with type `object` or `null` can be flattened.", s.into(), )) } Some(SingleOrVec::Vec(ref t)) if !t.contains(&InstanceType::Object) => { Err(JsonSchemaError::new( - "Only schemas with type `object` can be flattened.", + "Only schemas with type `object` or `null` can be flattened.", s.into(), )) } diff --git a/schemars/tests/enum.rs b/schemars/tests/enum.rs index 010419a..f0c0b9b 100644 --- a/schemars/tests/enum.rs +++ b/schemars/tests/enum.rs @@ -2,11 +2,22 @@ mod util; use schemars::{JsonSchema, Map}; use util::*; +#[derive(Debug, JsonSchema)] +pub struct UnitStruct; + +#[derive(Debug, JsonSchema)] +pub struct Struct { + foo: i32, + bar: bool, +} + #[derive(Debug, JsonSchema)] #[schemars(rename_all = "camelCase")] pub enum External { UnitOne, StringMap(Map), + UnitStructNewType(UnitStruct), + StructNewType(Struct), Struct { foo: i32, bar: bool }, UnitTwo, Tuple(i32, bool), @@ -22,6 +33,8 @@ fn enum_external_tag() -> TestResult { pub enum Internal { UnitOne, StringMap(Map), + UnitStructNewType(UnitStruct), + StructNewType(Struct), Struct { foo: i32, bar: bool }, UnitTwo, } @@ -36,6 +49,8 @@ fn enum_internal_tag() -> TestResult { pub enum Untagged { UnitOne, StringMap(Map), + UnitStructNewType(UnitStruct), + StructNewType(Struct), Struct { foo: i32, bar: bool }, Tuple(i32, bool), } diff --git a/schemars/tests/expected/enum-external.json b/schemars/tests/expected/enum-external.json index 5a3dbb5..91540ab 100644 --- a/schemars/tests/expected/enum-external.json +++ b/schemars/tests/expected/enum-external.json @@ -10,6 +10,9 @@ }, { "type": "object", + "required": [ + "stringMap" + ], "properties": { "stringMap": { "type": "object", @@ -17,16 +20,42 @@ "type": "string" } } - }, - "required": [ - "stringMap" - ] + } }, { "type": "object", + "required": [ + "unitStructNewType" + ], + "properties": { + "unitStructNewType": { + "$ref": "#/definitions/UnitStruct" + } + } + }, + { + "type": "object", + "required": [ + "structNewType" + ], + "properties": { + "structNewType": { + "$ref": "#/definitions/Struct" + } + } + }, + { + "type": "object", + "required": [ + "struct" + ], "properties": { "struct": { "type": "object", + "required": [ + "bar", + "foo" + ], "properties": { "bar": { "type": "boolean" @@ -35,19 +64,15 @@ "type": "integer", "format": "int32" } - }, - "required": [ - "bar", - "foo" - ] + } } - }, - "required": [ - "struct" - ] + } }, { "type": "object", + "required": [ + "tuple" + ], "properties": { "tuple": { "type": "array", @@ -63,10 +88,28 @@ "maxItems": 2, "minItems": 2 } - }, - "required": [ - "tuple" - ] + } } - ] + ], + "definitions": { + "Struct": { + "type": "object", + "required": [ + "bar", + "foo" + ], + "properties": { + "bar": { + "type": "boolean" + }, + "foo": { + "type": "integer", + "format": "int32" + } + } + }, + "UnitStruct": { + "type": "null" + } + } } \ No newline at end of file diff --git a/schemars/tests/expected/enum-internal.json b/schemars/tests/expected/enum-internal.json index 06c6148..003c9fd 100644 --- a/schemars/tests/expected/enum-internal.json +++ b/schemars/tests/expected/enum-internal.json @@ -4,6 +4,9 @@ "anyOf": [ { "type": "object", + "required": [ + "typeProperty" + ], "properties": { "typeProperty": { "type": "string", @@ -11,13 +14,13 @@ "UnitOne" ] } - }, - "required": [ - "typeProperty" - ] + } }, { "type": "object", + "required": [ + "typeProperty" + ], "properties": { "typeProperty": { "type": "string", @@ -26,15 +29,54 @@ ] } }, - "required": [ - "typeProperty" - ], "additionalProperties": { "type": "string" } }, { "type": "object", + "required": [ + "typeProperty" + ], + "properties": { + "typeProperty": { + "type": "string", + "enum": [ + "UnitStructNewType" + ] + } + } + }, + { + "type": "object", + "required": [ + "bar", + "foo", + "typeProperty" + ], + "properties": { + "bar": { + "type": "boolean" + }, + "foo": { + "type": "integer", + "format": "int32" + }, + "typeProperty": { + "type": "string", + "enum": [ + "StructNewType" + ] + } + } + }, + { + "type": "object", + "required": [ + "bar", + "foo", + "typeProperty" + ], "properties": { "bar": { "type": "boolean" @@ -49,15 +91,13 @@ "Struct" ] } - }, - "required": [ - "bar", - "foo", - "typeProperty" - ] + } }, { "type": "object", + "required": [ + "typeProperty" + ], "properties": { "typeProperty": { "type": "string", @@ -65,10 +105,7 @@ "UnitTwo" ] } - }, - "required": [ - "typeProperty" - ] + } } ] } \ No newline at end of file diff --git a/schemars/tests/expected/enum-untagged.json b/schemars/tests/expected/enum-untagged.json index e93c560..f3de52d 100644 --- a/schemars/tests/expected/enum-untagged.json +++ b/schemars/tests/expected/enum-untagged.json @@ -11,8 +11,18 @@ "type": "string" } }, + { + "$ref": "#/definitions/UnitStruct" + }, + { + "$ref": "#/definitions/Struct" + }, { "type": "object", + "required": [ + "bar", + "foo" + ], "properties": { "bar": { "type": "boolean" @@ -21,11 +31,7 @@ "type": "integer", "format": "int32" } - }, - "required": [ - "bar", - "foo" - ] + } }, { "type": "array", @@ -41,5 +47,26 @@ "maxItems": 2, "minItems": 2 } - ] + ], + "definitions": { + "Struct": { + "type": "object", + "required": [ + "bar", + "foo" + ], + "properties": { + "bar": { + "type": "boolean" + }, + "foo": { + "type": "integer", + "format": "int32" + } + } + }, + "UnitStruct": { + "type": "null" + } + } } \ No newline at end of file diff --git a/schemars_derive/src/lib.rs b/schemars_derive/src/lib.rs index f5875da..2554f40 100644 --- a/schemars_derive/src/lib.rs +++ b/schemars_derive/src/lib.rs @@ -174,7 +174,7 @@ fn schema_for_internal_tagged_enum<'a>( instance_type: Some(schemars::schema::InstanceType::String.into()), enum_values: Some(vec![#name.into()]), }); - let schema = wrap_schema_fields(quote! { + let tag_schema = wrap_schema_fields(quote! { instance_type: Some(schemars::schema::InstanceType::Object.into()), object: schemars::schema::ObjectValidation { properties: { @@ -190,13 +190,20 @@ fn schema_for_internal_tagged_enum<'a>( ..Default::default() }, }); - if is_unit_variant(&variant) { - schema - } else { - let sub_schema = schema_for_untagged_enum_variant(variant, cattrs); - quote! { - #schema.flatten(#sub_schema)? + let variant_schema = match variant.style { + Style::Unit => return tag_schema, + Style::Newtype => { + let field = &variant.fields[0]; + let ty = get_json_schema_type(field); + quote_spanned! {field.original.span()=> + <#ty>::json_schema(gen)? + } } + Style::Struct => schema_for_struct(&variant.fields, cattrs), + Style::Tuple => unreachable!("Internal tagged enum tuple variants will have caused serde_derive_internals to output a compile error already."), + }; + quote! { + #tag_schema.flatten(#variant_schema)? } });