diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b0adea3..d693c99f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - MSRV is now 1.70 - [The `example` attribute](https://graham.cool/schemars/deriving/attributes/#example) value is now an arbitrary expression, rather than a string literal identifying a function to call. To avoid silent behaviour changes, the expression must not be a string literal where the value can be parsed as a function path - e.g. `#[schemars(example = "foo")]` is now a compile error, but `#[schemars(example = foo())]` is allowed (as is `#[schemars(example = &"foo")]` if you want the the literal string value `"foo"` to be the example). +- For newtype variants of internally-tagged enums, prefer referencing the inner type's schema via `$ref` instead of always inlining the schema (https://github.com/GREsau/schemars/pull/355) ### Fixed diff --git a/schemars/src/_private/mod.rs b/schemars/src/_private/mod.rs index 3989f496..aa5c6fe5 100644 --- a/schemars/src/_private/mod.rs +++ b/schemars/src/_private/mod.rs @@ -1,5 +1,5 @@ use crate::_alloc_prelude::*; -use crate::transform::transform_immediate_subschemas; +use crate::transform::{transform_immediate_subschemas, Transform}; use crate::{JsonSchema, Schema, SchemaGenerator}; use serde::Serialize; use serde_json::{json, map::Entry, Map, Value}; @@ -12,6 +12,39 @@ pub extern crate serde_json; pub use rustdoc::get_title_and_description; +pub fn json_schema_for_internally_tagged_enum_newtype_variant( + generator: &mut SchemaGenerator, +) -> Schema { + let mut schema = T::json_schema(generator); + + // Inline the newtype's inner schema if any of: + // - The type specifies that its schema should always be inlined + // - The generator settings specify that all schemas should be inlined + // - The inner type is a unit struct, which would cause an unsatisfiable schema due to mismatched `type`. + // In this case, we replace its type with "object" in `apply_internal_enum_variant_tag` + // - The inner schema specified `"additionalProperties": false` or `"unevaluatedProperties": false`, + // since that would disallow the variant tag. If additional/unevaluatedProperties is in the top-level + // schema, then we can leave it there, because it will "see" the variant tag property. But if it is + // nested e.g. in an `allOf`, then it must be removed, which is why we run `AllowUnknownProperties` + // but only on immediate subschemas. + + let mut transform = AllowUnknownProperties::default(); + transform_immediate_subschemas(&mut transform, &mut schema); + + if T::always_inline_schema() + || generator.settings().inline_subschemas + || schema.get("type").and_then(Value::as_str) == Some("null") + || schema.get("additionalProperties").and_then(Value::as_bool) == Some(false) + || schema.get("unevaluatedProperties").and_then(Value::as_bool) == Some(false) + || transform.did_modify + { + return schema; + } + + // ...otherwise, we can freely refer to the schema via a `$ref` + generator.subschema_for::() +} + // Helper for generating schemas for flattened `Option` fields. pub fn json_schema_for_flatten( generator: &mut SchemaGenerator, @@ -25,20 +58,29 @@ pub fn json_schema_for_flatten( // Always allow aditional/unevaluated properties, because the outer struct determines // whether it denies unknown fields. - allow_unknown_properties(&mut schema); + AllowUnknownProperties::default().transform(&mut schema); schema } -fn allow_unknown_properties(schema: &mut Schema) { - if schema.get("additionalProperties").and_then(Value::as_bool) == Some(false) { - schema.remove("additionalProperties"); - } - if schema.get("unevaluatedProperties").and_then(Value::as_bool) == Some(false) { - schema.remove("unevaluatedProperties"); - } +#[derive(Default)] +struct AllowUnknownProperties { + did_modify: bool, +} - transform_immediate_subschemas(&mut allow_unknown_properties, schema); +impl Transform for AllowUnknownProperties { + fn transform(&mut self, schema: &mut Schema) { + if schema.get("additionalProperties").and_then(Value::as_bool) == Some(false) { + schema.remove("additionalProperties"); + self.did_modify = true; + } + if schema.get("unevaluatedProperties").and_then(Value::as_bool) == Some(false) { + schema.remove("unevaluatedProperties"); + self.did_modify = true; + } + + transform_immediate_subschemas(self, schema); + } } /// Hack to simulate specialization: diff --git a/schemars/tests/integration/enums_deny_unknown_fields.rs b/schemars/tests/integration/enums_deny_unknown_fields.rs index 9eb79bfb..e44b4f24 100644 --- a/schemars/tests/integration/enums_deny_unknown_fields.rs +++ b/schemars/tests/integration/enums_deny_unknown_fields.rs @@ -15,6 +15,10 @@ macro_rules! fn_values { foo: 123, bar: true, }), + Self::StructDenyUnknownFieldsNewType(StructDenyUnknownFields { + baz: 123, + foobar: true, + }), Self::Struct { foo: 123, bar: true, @@ -30,12 +34,20 @@ struct Struct { bar: bool, } +#[derive(JsonSchema, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] +struct StructDenyUnknownFields { + baz: i32, + foobar: bool, +} + #[derive(JsonSchema, Deserialize, Serialize)] #[serde(deny_unknown_fields)] enum External { Unit, StringMap(BTreeMap), StructNewType(Struct), + StructDenyUnknownFieldsNewType(StructDenyUnknownFields), Struct { foo: i32, bar: bool }, } @@ -49,6 +61,7 @@ enum Internal { Unit, StringMap(BTreeMap), StructNewType(Struct), + StructDenyUnknownFieldsNewType(StructDenyUnknownFields), Struct { foo: i32, bar: bool }, } @@ -62,6 +75,7 @@ enum Adjacent { Unit, StringMap(BTreeMap), StructNewType(Struct), + StructDenyUnknownFieldsNewType(StructDenyUnknownFields), Struct { foo: i32, bar: bool }, } @@ -75,6 +89,7 @@ enum Untagged { Unit, StringMap(BTreeMap), StructNewType(Struct), + StructDenyUnknownFieldsNewType(StructDenyUnknownFields), Struct { foo: i32, bar: bool }, } @@ -88,13 +103,22 @@ fn externally_tagged_enum() { .assert_snapshot() .assert_allows_ser_roundtrip(External::values()) .assert_matches_de_roundtrip(arbitrary_values()) - .assert_rejects_de([json!({ - "Struct": { - "foo": 123, - "bar": true, - "extra": null - } - })]) + .assert_rejects_de([ + json!({ + "Struct": { + "foo": 123, + "bar": true, + "extra": null + } + }), + json!({ + "StructDenyUnknownFieldsNewType": { + "baz": 123, + "foobar": true, + "extra": null + } + }), + ]) .assert_allows_de_roundtrip([json!({ "StructNewType": { "foo": 123, @@ -110,12 +134,20 @@ fn internally_tagged_enum() { .assert_snapshot() .assert_allows_ser_roundtrip(Internal::values()) .assert_matches_de_roundtrip(arbitrary_values()) - .assert_rejects_de([json!({ - "tag": "Struct", - "foo": 123, - "bar": true, - "extra": null - })]) + .assert_rejects_de([ + json!({ + "tag": "Struct", + "foo": 123, + "bar": true, + "extra": null + }), + json!({ + "tag": "StructDenyUnknownFieldsNewType", + "baz": 123, + "foobar": true, + "extra": null + }), + ]) .assert_allows_de_roundtrip([json!({ "tag": "StructNewType", "foo": 123, @@ -130,14 +162,24 @@ fn adjacently_tagged_enum() { .assert_snapshot() .assert_allows_ser_roundtrip(Adjacent::values()) .assert_matches_de_roundtrip(arbitrary_values()) - .assert_rejects_de([json!({ - "tag": "Struct", - "content": { - "foo": 123, - "bar": true, - "extra": null - } - })]) + .assert_rejects_de([ + json!({ + "tag": "Struct", + "content": { + "foo": 123, + "bar": true, + "extra": null + } + }), + json!({ + "tag": "StructDenyUnknownFieldsNewType", + "content": { + "baz": 123, + "foobar": true, + "extra": null + } + }), + ]) .assert_allows_de_roundtrip([json!({ "tag": "StructNewType", "content": { @@ -154,6 +196,11 @@ fn untagged_enum() { .assert_snapshot() .assert_allows_ser_roundtrip(Untagged::values()) .assert_matches_de_roundtrip(arbitrary_values()) + .assert_rejects_de([json!({ + "baz": 123, + "foobar": true, + "extra": null + })]) .assert_allows_de_roundtrip([json!({ "foo": 123, "bar": true, diff --git a/schemars/tests/integration/snapshots/schemars/tests/integration/enums.rs~internally_tagged_enum.json b/schemars/tests/integration/snapshots/schemars/tests/integration/enums.rs~internally_tagged_enum.json index e91dcb6a..219be7ae 100644 --- a/schemars/tests/integration/snapshots/schemars/tests/integration/enums.rs~internally_tagged_enum.json +++ b/schemars/tests/integration/snapshots/schemars/tests/integration/enums.rs~internally_tagged_enum.json @@ -44,22 +44,14 @@ { "type": "object", "properties": { - "foo": { - "type": "integer", - "format": "int32" - }, - "bar": { - "type": "boolean" - }, "tag": { "type": "string", "const": "StructNewType" } }, + "$ref": "#/$defs/Struct", "required": [ - "tag", - "foo", - "bar" + "tag" ] }, { @@ -95,5 +87,23 @@ "tag" ] } - ] + ], + "$defs": { + "Struct": { + "type": "object", + "properties": { + "foo": { + "type": "integer", + "format": "int32" + }, + "bar": { + "type": "boolean" + } + }, + "required": [ + "foo", + "bar" + ] + } + } } \ No newline at end of file diff --git a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~adjacently_tagged_enum.json b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~adjacently_tagged_enum.json index 20f2aed4..9c4b6d9b 100644 --- a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~adjacently_tagged_enum.json +++ b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~adjacently_tagged_enum.json @@ -52,6 +52,23 @@ ], "additionalProperties": false }, + { + "type": "object", + "properties": { + "tag": { + "type": "string", + "const": "StructDenyUnknownFieldsNewType" + }, + "content": { + "$ref": "#/$defs/StructDenyUnknownFields" + } + }, + "required": [ + "tag", + "content" + ], + "additionalProperties": false + }, { "type": "object", "properties": { @@ -100,6 +117,23 @@ "foo", "bar" ] + }, + "StructDenyUnknownFields": { + "type": "object", + "properties": { + "baz": { + "type": "integer", + "format": "int32" + }, + "foobar": { + "type": "boolean" + } + }, + "additionalProperties": false, + "required": [ + "baz", + "foobar" + ] } } } \ No newline at end of file diff --git a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~externally_tagged_enum.json b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~externally_tagged_enum.json index 819e0e46..911428ba 100644 --- a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~externally_tagged_enum.json +++ b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~externally_tagged_enum.json @@ -35,6 +35,18 @@ ], "additionalProperties": false }, + { + "type": "object", + "properties": { + "StructDenyUnknownFieldsNewType": { + "$ref": "#/$defs/StructDenyUnknownFields" + } + }, + "required": [ + "StructDenyUnknownFieldsNewType" + ], + "additionalProperties": false + }, { "type": "object", "properties": { @@ -78,6 +90,23 @@ "foo", "bar" ] + }, + "StructDenyUnknownFields": { + "type": "object", + "properties": { + "baz": { + "type": "integer", + "format": "int32" + }, + "foobar": { + "type": "boolean" + } + }, + "additionalProperties": false, + "required": [ + "baz", + "foobar" + ] } } } \ No newline at end of file diff --git a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~internally_tagged_enum.json b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~internally_tagged_enum.json index 2e5b8c88..ab5e53a8 100644 --- a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~internally_tagged_enum.json +++ b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~internally_tagged_enum.json @@ -33,22 +33,36 @@ { "type": "object", "properties": { - "foo": { + "tag": { + "type": "string", + "const": "StructNewType" + } + }, + "$ref": "#/$defs/Struct", + "required": [ + "tag" + ] + }, + { + "type": "object", + "properties": { + "baz": { "type": "integer", "format": "int32" }, - "bar": { + "foobar": { "type": "boolean" }, "tag": { "type": "string", - "const": "StructNewType" + "const": "StructDenyUnknownFieldsNewType" } }, + "additionalProperties": false, "required": [ "tag", - "foo", - "bar" + "baz", + "foobar" ] }, { @@ -73,5 +87,23 @@ "bar" ] } - ] + ], + "$defs": { + "Struct": { + "type": "object", + "properties": { + "foo": { + "type": "integer", + "format": "int32" + }, + "bar": { + "type": "boolean" + } + }, + "required": [ + "foo", + "bar" + ] + } + } } \ No newline at end of file diff --git a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~untagged_enum.json b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~untagged_enum.json index 9b581362..1e6a3671 100644 --- a/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~untagged_enum.json +++ b/schemars/tests/integration/snapshots/schemars/tests/integration/enums_deny_unknown_fields.rs~untagged_enum.json @@ -14,6 +14,9 @@ { "$ref": "#/$defs/Struct" }, + { + "$ref": "#/$defs/StructDenyUnknownFields" + }, { "type": "object", "properties": { @@ -48,6 +51,23 @@ "foo", "bar" ] + }, + "StructDenyUnknownFields": { + "type": "object", + "properties": { + "baz": { + "type": "integer", + "format": "int32" + }, + "foobar": { + "type": "boolean" + } + }, + "additionalProperties": false, + "required": [ + "baz", + "foobar" + ] } } } \ No newline at end of file diff --git a/schemars_derive/src/schema_exprs.rs b/schemars_derive/src/schema_exprs.rs index 0b30e2e6..77211038 100644 --- a/schemars_derive/src/schema_exprs.rs +++ b/schemars_derive/src/schema_exprs.rs @@ -112,23 +112,24 @@ pub fn expr_for_repr(cont: &Container) -> Result { Ok(schema_expr) } -fn expr_for_field(field: &Field, allow_ref: bool) -> SchemaExpr { +fn expr_for_field(field: &Field, is_internal_tagged_enum_newtype: bool) -> SchemaExpr { let (ty, type_def) = type_for_field_schema(field); let span = field.original.span(); - let mut schema_expr = SchemaExpr::from(if field.attrs.validation.required { + let schema_expr = if field.attrs.validation.required { quote_spanned! {span=> <#ty as schemars::JsonSchema>::_schemars_private_non_optional_json_schema(#GENERATOR) } - } else if allow_ref { + } else if is_internal_tagged_enum_newtype { quote_spanned! {span=> - #GENERATOR.subschema_for::<#ty>() + schemars::_private::json_schema_for_internally_tagged_enum_newtype_variant::<#ty>(#GENERATOR) } } else { quote_spanned! {span=> - <#ty as schemars::JsonSchema>::json_schema(#GENERATOR) + #GENERATOR.subschema_for::<#ty>() } - }); + }; + let mut schema_expr = SchemaExpr::from(schema_expr); schema_expr.definitions.extend(type_def); field.add_mutators(&mut schema_expr.mutators); @@ -407,7 +408,7 @@ fn expr_for_untagged_enum_variant(variant: &Variant, deny_unknown_fields: bool) match variant.style { Style::Unit => expr_for_unit_struct(), - Style::Newtype => expr_for_field(&variant.fields[0], true), + Style::Newtype => expr_for_field(&variant.fields[0], false), Style::Tuple => expr_for_tuple_struct(&variant.fields), Style::Struct => expr_for_struct(&variant.fields, &SerdeDefault::None, deny_unknown_fields), } @@ -430,7 +431,7 @@ fn expr_for_internal_tagged_enum_variant( match variant.style { Style::Unit => expr_for_unit_struct(), - Style::Newtype => expr_for_field(&variant.fields[0], false), + Style::Newtype => expr_for_field(&variant.fields[0], true), Style::Tuple => expr_for_tuple_struct(&variant.fields), Style::Struct => expr_for_struct(&variant.fields, &SerdeDefault::None, deny_unknown_fields), } @@ -444,14 +445,14 @@ fn expr_for_unit_struct() -> SchemaExpr { } fn expr_for_newtype_struct(field: &Field) -> SchemaExpr { - expr_for_field(field, true) + expr_for_field(field, false) } fn expr_for_tuple_struct(fields: &[Field]) -> SchemaExpr { let fields: Vec<_> = fields .iter() .map(|f| { - let field_expr = expr_for_field(f, true); + let field_expr = expr_for_field(f, false); f.with_contract_check(quote! { prefix_items.push((#field_expr).to_value()); })