diff --git a/crates/oapi-codegen/src/coverage.rs b/crates/oapi-codegen/src/coverage.rs index ac52e2e..0755efe 100644 --- a/crates/oapi-codegen/src/coverage.rs +++ b/crates/oapi-codegen/src/coverage.rs @@ -652,18 +652,6 @@ impl Sweep<'_> { ); } } - if kind == Some("object") - && value.get("additionalProperties").and_then(Value::as_bool) == Some(false) - && value - .get("properties") - .and_then(Value::as_mapping) - .is_none_or(serde_yaml::Mapping::is_empty) - { - self.warn( - &pointer(path, "additionalProperties"), - "an object without declared properties becomes a map and does not reject additional properties", - ); - } } } @@ -887,7 +875,6 @@ security: [{arbitrary: [custom]}] ("{type: string, nullable: true, default: null}", "default"), ("{oneOf: [{type: string}, {type: integer}]}", "oneOf"), ("{anyOf: [{type: string}, {type: integer}]}", "anyOf"), - ("{type: object, additionalProperties: false}", "additionalProperties"), ] { let yaml = format!("components: {{schemas: {{Widget: {schema}}}}}"); let sweep = inspect_yaml(&yaml); @@ -908,6 +895,13 @@ security: [{arbitrary: [custom]}] } } + #[test] + fn empty_closed_objects_have_no_map_warning() { + let sweep = inspect_yaml("components: {schemas: {Empty: {type: object, additionalProperties: false}}}"); + assert!(sweep.problems.is_empty()); + assert!(sweep.warnings.is_empty()); + } + #[test] fn nullable_limitations_warn_before_the_typed_parse() { for (schema, message) in [ diff --git a/crates/oapi-codegen/src/lower/all_of.rs b/crates/oapi-codegen/src/lower/all_of.rs index 85f6f12..79336bf 100644 --- a/crates/oapi-codegen/src/lower/all_of.rs +++ b/crates/oapi-codegen/src/lower/all_of.rs @@ -89,20 +89,7 @@ fn collect( ReferenceOr::Item(schema) => schema, ReferenceOr::Reference { reference } => spec.resolve(reference)?, }; - let data = &schema.schema_data; - if data.nullable - || data.read_only - || data.write_only - || data.deprecated - || data.default.is_some() - || !data.extensions.is_empty() - || data.discriminator.is_some() - { - return Err(unsupported( - path, - "member nullability, access, deprecation, defaults, extensions, or discriminators cannot be flattened", - )); - } + check_member_metadata(path, schema)?; match &schema.schema_kind { SchemaKind::Type(Type::Object(object)) => { if object.min_properties.is_some() || object.max_properties.is_some() { @@ -126,17 +113,32 @@ fn collect( return Ok(()); } +pub(super) fn check_member_metadata(path: &str, schema: &Schema) -> Result<()> { + let data = &schema.schema_data; + if data.nullable + || data.read_only + || data.write_only + || data.deprecated + || data.default.is_some() + || !data.extensions.is_empty() + || data.discriminator.is_some() + { + return Err(unsupported( + path, + "member nullability, access, deprecation, defaults, extensions, or discriminators cannot be flattened", + )); + } + return Ok(()); +} + fn resolve(spec: &Spec, path: &str, property: &ReferenceOr>, depth: usize) -> Result { - let mut schema = match property { - ReferenceOr::Item(schema) => *schema.clone(), - ReferenceOr::Reference { reference } => spec.resolve(reference)?.clone(), - }; + let mut schema = resolve_reference(spec, property)?.clone(); for _ in depth..MAX_SCHEMA_DEPTH { let SchemaKind::AllOf { all_of } = &schema.schema_kind else { return Ok(schema); }; let [member] = all_of.as_slice() else { - return Err(unsupported(path, "overlapping composed properties are not supported")); + return Ok(schema); }; let mut target = match member { ReferenceOr::Item(schema) => schema.clone(), @@ -159,6 +161,13 @@ fn resolve(spec: &Spec, path: &str, property: &ReferenceOr>, depth: }); } +fn resolve_reference<'a>(spec: &'a Spec, property: &'a ReferenceOr>) -> Result<&'a Schema> { + return match property { + ReferenceOr::Item(schema) => Ok(schema), + ReferenceOr::Reference { reference } => spec.resolve(reference), + }; +} + fn intersect( spec: &Spec, path: &str, @@ -174,8 +183,35 @@ fn intersect( { return Ok(left.clone()); } - let mut left = resolve(spec, path, left, depth)?; - let mut right = resolve(spec, path, right, depth)?; + let resolved_left = resolve_reference(spec, left)?; + let resolved_right = resolve_reference(spec, right)?; + if matches!(&resolved_left.schema_kind, SchemaKind::AllOf { all_of } if !all_of.is_empty()) + && resolved_left == resolved_right + { + return Ok(match (left, right) { + (ReferenceOr::Reference { .. }, _) => left.clone(), + (_, ReferenceOr::Reference { .. }) => right.clone(), + _ => left.clone(), + }); + } + let normalized_left = resolve(spec, path, left, depth)?; + let normalized_right = resolve(spec, path, right, depth)?; + if matches!(normalized_left.schema_kind, SchemaKind::AllOf { .. }) + || matches!(normalized_right.schema_kind, SchemaKind::AllOf { .. }) + { + if normalized_left == normalized_right + && matches!(&normalized_left.schema_kind, SchemaKind::AllOf { all_of } if !all_of.is_empty()) + { + return Ok(match (left, right) { + (ReferenceOr::Reference { .. }, _) => left.clone(), + (_, ReferenceOr::Reference { .. }) => right.clone(), + _ => ReferenceOr::Item(Box::new(normalized_left)), + }); + } + return Err(unsupported(path, "overlapping composed properties are not supported")); + } + let mut left = normalized_left; + let mut right = normalized_right; let nullable = left.schema_data.nullable && right.schema_data.nullable; left.schema_data.nullable = nullable; right.schema_data.nullable = nullable; @@ -379,6 +415,192 @@ mod tests { return json!({"type": "object", "properties": {"value": property}}); } + #[test] + fn equivalent_composite_aliases_reuse_named_types() { + let composed = json!({"allOf":[ + {"type":"object","required":["count"],"properties":{"count":{"type":"integer","minimum":2_i64}}}, + {"type":"object","properties":{"count":{"type":"integer","maximum":8_i64}}} + ]}); + let direct = json!({"$ref":"#/components/schemas/A"}); + let alias = json!({"$ref":"#/components/schemas/B"}); + let chain = json!({"$ref":"#/components/schemas/C"}); + for (left, right) in [ + (&direct, &alias), + (&alias, &chain), + (&direct, &chain), + (&composed, &chain), + ] { + for (left, right) in [(left, right), (right, left)] { + let spec = spec(json!({ + "A":composed, "B":direct, "C":alias, + "Test":{"allOf":[object(left.clone()),object(right.clone())]}, + })); + let names = crate::lower::rename::type_renames(&spec, None).expect("names"); + let module = crate::lower::schema::generate_models(&spec, &names).expect("equivalent composites"); + let strukt = module + .items + .iter() + .find_map(|item| { + return match item { + crate::ir::Item::Struct(strukt) if strukt.name.logical() == "Test" => Some(strukt), + _ => None, + }; + }) + .expect("Test struct"); + assert!(matches!( + &strukt.fields[0].ty, + crate::ir::RustType::Option(inner) if matches!(inner.as_ref(), crate::ir::RustType::Named(_)) + )); + let left = serde_json::from_value(left.clone()).expect("left property"); + let right = serde_json::from_value(right.clone()).expect("right property"); + let expected = if matches!(left, ReferenceOr::Reference { .. }) { + &left + } else { + &right + }; + assert_eq!( + &intersect(&spec, "Test.value", &left, &right, 0).expect("intersection"), + expected + ); + } + } + for changed in [ + json!({"allOf":[{"type":"object"},{"type":"object"}]}), + json!({"nullable":true,"allOf":composed["allOf"]}), + json!({"description":"different","allOf":composed["allOf"]}), + json!({"x-rust-type":"serde_json::Value","allOf":composed["allOf"]}), + ] { + let spec = spec(json!({"A":composed,"B":changed})); + let left = serde_json::from_value(direct.clone()).expect("left"); + let right = serde_json::from_value(alias.clone()).expect("right"); + for (left, right) in [(&left, &right), (&right, &left)] { + assert!(intersect(&spec, "Test.value", left, right, 0).is_err()); + } + } + } + + #[test] + fn equivalent_singleton_composite_wrappers_reuse_references() { + let composite = json!({"allOf":[ + {"type":"object","properties":{"count":{"type":"integer","minimum":2_i64}}}, + {"type":"object","properties":{"count":{"type":"integer","maximum":8_i64}}} + ]}); + let wrapper = json!({"allOf":[{"$ref":"#/components/schemas/Composite"}]}); + let reference_a = json!({"$ref":"#/components/schemas/A"}); + let reference_b = json!({"$ref":"#/components/schemas/B"}); + let chain = json!({"$ref":"#/components/schemas/Chain"}); + let direct = json!({"$ref":"#/components/schemas/Composite"}); + let nested = json!({"allOf":[wrapper.clone()]}); + for (left, right) in [ + (&reference_a, &reference_b), + (&reference_a, &chain), + (&wrapper, &chain), + (&direct, &reference_a), + (&direct, &chain), + (&direct, &nested), + (&reference_a, &nested), + ] { + for (left, right) in [(left, right), (right, left)] { + let spec = spec(json!({ + "Composite":composite, "A":wrapper, "B":wrapper, "Chain":reference_b, + "Test":{"allOf":[object(left.clone()),object(right.clone())]}, + })); + let names = crate::lower::rename::type_renames(&spec, None).expect("names"); + crate::lower::schema::generate_models(&spec, &names).expect("equal wrappers"); + let left = serde_json::from_value(left.clone()).expect("left"); + let right = serde_json::from_value(right.clone()).expect("right"); + let expected = if matches!(left, ReferenceOr::Reference { .. }) { + &left + } else { + &right + }; + assert_eq!( + &intersect(&spec, "Test.value", &left, &right, 0).expect("intersection"), + expected + ); + } + } + for changed in [ + json!({"description":"different","allOf":wrapper["allOf"]}), + json!({"nullable":true,"allOf":wrapper["allOf"]}), + json!({"x-rust-type":"serde_json::Value","allOf":wrapper["allOf"]}), + ] { + let spec = spec(json!({"Composite":composite,"A":wrapper,"B":changed})); + let left = serde_json::from_value(reference_a.clone()).expect("left"); + let right = serde_json::from_value(reference_b.clone()).expect("right"); + for (left, right) in [(&left, &right), (&right, &left)] { + assert!(intersect(&spec, "Test.value", left, right, 0).is_err()); + } + } + let spec = spec(json!({"A":{"allOf":[]},"B":{"allOf":[]}})); + let left = serde_json::from_value(reference_a).expect("left"); + let right = serde_json::from_value(reference_b).expect("right"); + for (left, right) in [(&left, &right), (&right, &left)] { + assert!(intersect(&spec, "Test.value", left, right, 0).is_err()); + } + assert!(lower(json!({"allOf":[]})).is_err()); + assert!(lower(json!({"allOf":[object(json!({"allOf":[]})),object(json!({"allOf":[]}))]})).is_err()); + } + + #[test] + fn map_value_hints_do_not_duplicate_container_names() { + for value in [ + json!({"type":"object","properties":{"label":{"type":"string"}}}), + json!({"type":"string","enum":["red","blue"]}), + json!({"type":"integer","enum":[1_i64,2_i64]}), + ] { + let map = json!({"type":"object","additionalProperties":value}); + for schema in [ + map.clone(), + json!({"allOf":[map]}), + json!({"type":"object","properties":{"id":{"type":"string"}},"additionalProperties":value}), + ] { + let module = lower(schema).expect("map with inline value"); + let names: Vec<_> = module.items.iter().map(crate::ir::Item::name).collect(); + assert_eq!(names, ["Test", "TestValue"]); + } + } + } + + #[test] + fn singleton_maps_keep_types_and_reject_unenforced_constraints() { + for (additional, expected) in [ + (json!({"type":"string"}), crate::ir::RustType::String), + (json!(true), crate::ir::RustType::Value), + (serde_json::Value::Null, crate::ir::RustType::Value), + ] { + let mut map = json!({"type":"object"}); + if !additional.is_null() { + map["additionalProperties"] = additional; + } + let wrapper = json!({"allOf":[map.clone()]}); + let module = lower(wrapper.clone()).expect("named map"); + assert!(matches!( + &module.items[0], + crate::ir::Item::Alias(alias) if alias.ty == crate::ir::RustType::Map(Box::new(expected.clone())) + )); + let module = lower(object(wrapper)).expect("inline map"); + assert!(matches!( + &module.items[0], + crate::ir::Item::Struct(strukt) if strukt.fields[0].ty + == crate::ir::RustType::Option(Box::new(crate::ir::RustType::Map(Box::new(expected)))) + )); + for (keyword, value) in [ + ("minProperties", json!(1_i64)), + ("maxProperties", json!(2_i64)), + ("required", json!(["missing"])), + ] { + let mut constrained = map.clone(); + constrained[keyword] = value; + let wrapper = json!({"allOf":[constrained]}); + for schema in [wrapper.clone(), object(wrapper)] { + let error = lower(schema).expect_err("unsupported map restriction").to_string(); + assert!(error.contains("allOf intersection"), "{error}"); + } + } + } + } + #[test] fn incompatible_properties_report_context_in_both_orders() { for (left, right, reason) in [ @@ -450,10 +672,6 @@ mod tests { #[test] fn unsupported_member_restrictions_are_not_discarded() { for (member, reason) in [ - ( - json!({"type":"object","additionalProperties":{"type":"string"}}), - "schema-valued additionalProperties", - ), (json!({"type":"object","minProperties":1_i64}), "property-count"), (json!({"type":"object","nullable":true}), "nullability"), (json!({"type":"object","default":{}}), "defaults"), @@ -464,6 +682,13 @@ mod tests { .to_string(); assert!(error.contains("Test") && error.contains(reason), "{error}"); } + let error = lower(json!({"allOf":[ + {"type":"object","additionalProperties":{"type":"string"}}, + {"type":"object"} + ]})) + .expect_err("map composition") + .to_string(); + assert!(error.contains("schema-valued additionalProperties"), "{error}"); for required in [json!([]), json!(["value"])] { let other = json!({"type":"object","required":required,"properties":{"value":{"type":"string"}}}); let closed = json!({"type":"object","additionalProperties":false}); diff --git a/crates/oapi-codegen/src/lower/paths.rs b/crates/oapi-codegen/src/lower/paths.rs index 4461be3..b6ef229 100644 --- a/crates/oapi-codegen/src/lower/paths.rs +++ b/crates/oapi-codegen/src/lower/paths.rs @@ -1805,6 +1805,58 @@ fn trimmed(text: &str) -> Option { mod tests { use super::*; + #[test] + fn all_of_inline_json_body_maps_keep_value_types_and_nullability() { + let document = serde_json::from_value(serde_json::json!({ + "openapi":"3.0.3", "info":{"title":"maps","version":"1"}, "paths":{} + })) + .expect("document"); + let spec = Spec::from_parts(document, "maps.yaml".into()); + let import_mapping = BTreeMap::new(); + let lowerer = Lowerer { + spec: &spec, + import_mapping: &import_mapping, + response_type_suffix: "", + }; + for (member, expected) in [ + ( + serde_json::json!({"type":"object","additionalProperties":{"type":"string"}}), + RustType::String, + ), + (serde_json::json!({"type":"object"}), RustType::Value), + ( + serde_json::json!({"type":"object","additionalProperties":true}), + RustType::Value, + ), + ] { + for nullable in [false, true] { + let schema = serde_json::from_value(serde_json::json!({"nullable":nullable,"allOf":[member]})) + .expect("map schema"); + let map = RustType::Map(Box::new(expected.clone())); + let expected = if nullable { + RustType::Nullable(Box::new(map)) + } else { + map + }; + assert_eq!( + lowerer + .inline_body_type("/maps", "post", None, &schema) + .expect("map body"), + expected + ); + } + } + let closed = serde_json::from_value(serde_json::json!({ + "allOf":[{"type":"object","additionalProperties":false}] + })) + .expect("closed schema"); + let error = lowerer + .inline_body_type("/maps", "post", None, &closed) + .expect_err("closed inline body requires a named schema") + .to_string(); + assert!(error.contains("closed object bodies"), "{error}"); + } + #[test] fn valid_header_names_accept_tokens_and_reject_separators() { // Real header names with `-` and digits and `.` are accepted. diff --git a/crates/oapi-codegen/src/lower/schema.rs b/crates/oapi-codegen/src/lower/schema.rs index 986fe4c..7e2a2fa 100644 --- a/crates/oapi-codegen/src/lower/schema.rs +++ b/crates/oapi-codegen/src/lower/schema.rs @@ -201,7 +201,18 @@ impl Mapper<'_> { deprecated: deprecation_of(data, name)?, ty: RustType::Named(target), }), - None => Item::Struct(self.merge_all_of(name, all_of, data)?), + None => { + if let Some(ty) = self.single_all_of_map(name, all_of)? { + Item::Alias(Alias { + name: self.type_name_ident(name), + doc: doc_of(data), + deprecated: deprecation_of(data, name)?, + ty, + }) + } else { + Item::Struct(self.merge_all_of(name, all_of, data)?) + } + } }, SchemaKind::Type(_) => { let ty = self.type_from_schema(name, schema)?; @@ -228,10 +239,9 @@ impl Mapper<'_> { return Ok(item); } - /// Lower an object schema: a struct when it has properties, otherwise a map - /// alias. + /// Lower closed objects and objects with properties as structs. Other objects become map aliases. fn object_to_item(&mut self, name: &str, obj: &ObjectType, data: &SchemaData) -> Result { - if obj.properties.is_empty() { + if obj.properties.is_empty() && !matches!(obj.additional_properties, Some(AdditionalProperties::Any(false))) { let element = self.additional_properties_type(name, obj)?; return Ok(Item::Alias(Alias { name: self.type_name_ident(name), @@ -258,10 +268,7 @@ impl Mapper<'_> { let fields = sort_by_order(ordered); let additional_properties = match &obj.additional_properties { - Some(AdditionalProperties::Schema(schema)) => { - let ty = self.type_from_ref_schema(name, schema.as_ref())?; - Some(ty) - } + Some(AdditionalProperties::Schema(_)) => Some(self.additional_properties_type(name, obj)?), Some(AdditionalProperties::Any(true)) => Some(RustType::Value), Some(AdditionalProperties::Any(false)) | None => None, }; @@ -450,6 +457,9 @@ impl Mapper<'_> { /// than synthesizing a duplicate struct. Multi-member `allOf` is genuine /// composition and returns `None` so the caller merges it as before. fn collapse_single_all_of(&mut self, hint: &str, members: &[ReferenceOr]) -> Result> { + if let Some(ty) = self.single_all_of_map(hint, members)? { + return Ok(Some(ty)); + } let [only] = members else { return Ok(None); }; @@ -466,6 +476,25 @@ impl Mapper<'_> { return Ok(Some(ty)); } + fn single_all_of_map(&mut self, hint: &str, members: &[ReferenceOr]) -> Result> { + let [ReferenceOr::Item(schema)] = members else { + return Ok(None); + }; + let SchemaKind::Type(Type::Object(object)) = &schema.schema_kind else { + return Ok(None); + }; + if !object.properties.is_empty() + || matches!(object.additional_properties, Some(AdditionalProperties::Any(false))) + || object.min_properties.is_some() + || object.max_properties.is_some() + || !object.required.is_empty() + { + return Ok(None); + } + super::all_of::check_member_metadata(hint, schema)?; + return self.type_from_schema(hint, schema).map(Some); + } + /// Merge an `allOf` into a single flat struct, resolving `$ref` members to /// pull in their properties (matching oapi-codegen's behaviour). fn merge_all_of(&mut self, name: &str, members: &[ReferenceOr], data: &SchemaData) -> Result { @@ -788,10 +817,9 @@ impl Mapper<'_> { return Ok(ty); } - /// Map an inline object: hoist a struct when it has properties, otherwise a - /// map of its additionalProperties element type. + /// Hoist closed objects and objects with properties as structs. Other objects become maps. fn inline_object_type(&mut self, hint: &str, obj: &ObjectType, data: &SchemaData) -> Result { - if obj.properties.is_empty() { + if obj.properties.is_empty() && !matches!(obj.additional_properties, Some(AdditionalProperties::Any(false))) { let element = self.additional_properties_type(hint, obj)?; return Ok(RustType::Map(Box::new(element))); } @@ -816,7 +844,9 @@ impl Mapper<'_> { /// Element type for an object used purely as a map (`additionalProperties`). fn additional_properties_type(&mut self, hint: &str, obj: &ObjectType) -> Result { let element = match &obj.additional_properties { - Some(AdditionalProperties::Schema(schema)) => self.type_from_ref_schema(hint, schema.as_ref())?, + Some(AdditionalProperties::Schema(schema)) => { + self.type_from_ref_schema(&format!("{hint}_value"), schema.as_ref())? + } Some(AdditionalProperties::Any(_)) | None => RustType::Value, }; return Ok(element); diff --git a/crates/oapi-codegen/tests/coverage.rs b/crates/oapi-codegen/tests/coverage.rs index dac76c4..c52e963 100644 --- a/crates/oapi-codegen/tests/coverage.rs +++ b/crates/oapi-codegen/tests/coverage.rs @@ -2584,7 +2584,7 @@ fn every_generated_struct_is_braced() { // else. A tuple struct opens `(` and a unit struct ends at `;`. let tail = rest.trim_end(); assert!( - tail.ends_with('{'), + tail.ends_with('{') || tail.ends_with("{}"), "`{}` in {} is not a braced struct, which would take a prelude value name", tail, path.display(), diff --git a/crates/oapi-codegen/tests/fixtures/allof_merge.yaml b/crates/oapi-codegen/tests/fixtures/allof_merge.yaml index c636309..e359f04 100644 --- a/crates/oapi-codegen/tests/fixtures/allof_merge.yaml +++ b/crates/oapi-codegen/tests/fixtures/allof_merge.yaml @@ -140,3 +140,132 @@ components: enum: [2, 3] EnumReversed: allOf: [*enum_narrow, *enum_wide] + CompositeAlias: + $ref: "#/components/schemas/Intersection" + CompositeChain: + $ref: "#/components/schemas/CompositeAlias" + AliasedComposites: + allOf: + - &composite_direct + type: object + required: [value] + properties: + value: + $ref: "#/components/schemas/Intersection" + - &composite_alias + type: object + properties: + value: + $ref: "#/components/schemas/CompositeChain" + AliasedCompositesReversed: + allOf: [*composite_alias, *composite_direct] + InlineRefComposites: + allOf: + - type: object + required: [value] + properties: + value: *composed + - *composite_alias + WrapperA: &composite_wrapper + allOf: + - $ref: "#/components/schemas/Intersection" + WrapperB: *composite_wrapper + WrapperChain: + $ref: "#/components/schemas/WrapperB" + WrappedComposites: + allOf: + - &wrapper_a + type: object + required: [value] + properties: + value: + $ref: "#/components/schemas/WrapperA" + - &wrapper_b + type: object + properties: + value: + $ref: "#/components/schemas/WrapperChain" + - *composite_direct + WrappedCompositesReversed: + allOf: [*wrapper_b, *wrapper_a, *composite_direct] + ObjectMap: + allOf: + - &object_map + type: object + additionalProperties: + type: object + required: [label] + properties: + label: + type: string + EnumMap: + allOf: + - &enum_map + type: object + additionalProperties: + type: string + enum: [red, blue] + NestedMap: + allOf: + - type: object + additionalProperties: + allOf: [*object_map] + ReferencedMap: + allOf: + - type: object + additionalProperties: + $ref: "#/components/schemas/Base" + StringMap: + allOf: + - &string_map + type: object + additionalProperties: + type: string + AnyMap: + allOf: + - &any_map + type: object + ExplicitAnyMap: + allOf: + - type: object + additionalProperties: true + NullableMap: + nullable: true + allOf: + - *string_map + EmptyClosed: + allOf: + - &empty_closed + type: object + additionalProperties: false + ClosedTarget: *empty_closed + ClosedAlias: + allOf: + - $ref: "#/components/schemas/ClosedTarget" + InlineMaps: + type: object + required: [typed, arbitrary, nullable, closed, named] + properties: + typed: + allOf: [*string_map] + arbitrary: + allOf: [*any_map] + nullable: + nullable: true + allOf: [*string_map] + closed: + allOf: [*empty_closed] + named: + $ref: "#/components/schemas/StringMap" + optional: + allOf: [*string_map] + objects: + allOf: [*object_map] + enums: + allOf: [*enum_map] + nested: + type: object + required: [value] + properties: + value: + allOf: [*string_map] diff --git a/crates/oapi-codegen/tests/generated.rs b/crates/oapi-codegen/tests/generated.rs index c764e66..c6662f3 100644 --- a/crates/oapi-codegen/tests/generated.rs +++ b/crates/oapi-codegen/tests/generated.rs @@ -11,6 +11,117 @@ //! attribute that turns off every lint that is about first-party source, and //! `include!` cannot carry one. So this file needs no lint exceptions of its own. +#[test] +fn all_of_maps_preserve_entries_and_closed_objects_reject_them() { + use generated::allof_merge::*; + + fn roundtrip(input: &str) { + let value: T = serde_json::from_str(input).expect("accepted payload"); + assert_eq!( + serde_json::to_value(value).expect("serialized payload"), + serde_json::from_str::(input).expect("input JSON"), + ); + } + + roundtrip::(r#"{"first":"hello","second":"world"}"#); + roundtrip::(r#"{"first":{"label":"hello"},"second":{"label":"world"}}"#); + roundtrip::(r#"{"first":"red","second":"blue"}"#); + roundtrip::(r#"{"outer":{"inner":{"label":"hello"}}}"#); + roundtrip::(r#"{"first":{"id":"hello"}}"#); + let _: ObjectMapValue = serde_json::from_str(r#"{"label":"hello"}"#).expect("object value type"); + let _: EnumMapValue = serde_json::from_str(r#""red""#).expect("enum value type"); + for input in [r#"{"key":{}}"#, r#"{"key":{"label":42}}"#, r#"{"key":null}"#] { + assert!(serde_json::from_str::(input).is_err(), "{input}"); + } + for input in [r#"{"key":"green"}"#, r#"{"key":42}"#, r#"{"key":null}"#] { + assert!(serde_json::from_str::(input).is_err(), "{input}"); + } + assert!(serde_json::from_str::(r#"{"outer":{"inner":{}}}"#).is_err()); + roundtrip::(r#"{"number":7,"nested":{"list":[true,null,"text"]}}"#); + roundtrip::(r#"{"number":7,"nested":{"list":[true,null,"text"]}}"#); + roundtrip::("null"); + roundtrip::(r#"{"key":"value"}"#); + roundtrip::("{}"); + roundtrip::("{}"); + let input = r#"{"typed":{"a":"b"},"arbitrary":{"n":3,"list":[false,null]},"nullable":null,"closed":{},"named":{"c":"d"},"nested":{"value":{"e":"f"}},"objects":{"a":{"label":"hello"}},"enums":{"a":"blue"}}"#; + roundtrip::(input); + for input in [r#"{"key":42}"#, r#"{"key":null}"#, "[]", "null"] { + assert!(serde_json::from_str::(input).is_err(), "{input}"); + } + assert!(serde_json::from_str::(r#"{"extra":true}"#).is_err()); + assert!(serde_json::from_str::(r#"{"extra":true}"#).is_err()); + for (field, invalid) in [ + ("typed", serde_json::json!({"a": 42_i64})), + ("named", serde_json::json!({"a": false})), + ("nullable", serde_json::json!({"a": 42_i64})), + ("closed", serde_json::json!({"extra": true})), + ("nested", serde_json::json!({"value": {"a": 42_i64}})), + ("optional", serde_json::json!({"a": 42_i64})), + ("objects", serde_json::json!({"a": {}})), + ("enums", serde_json::json!({"a": "green"})), + ] { + let mut value: serde_json::Value = serde_json::from_str(input).expect("base payload"); + value[field] = invalid; + assert!(serde_json::from_value::(value).is_err(), "{field}"); + } + for required in ["typed", "arbitrary", "nullable", "closed", "named"] { + let mut value: serde_json::Value = serde_json::from_str(input).expect("base payload"); + value.as_object_mut().expect("object").remove(required); + assert!(serde_json::from_value::(value).is_err(), "{required}"); + } +} + +#[test] +fn all_of_aliased_composites_preserve_constraints() { + use generated::allof_merge::AliasedComposites; + use generated::allof_merge::AliasedCompositesReversed; + use generated::allof_merge::InlineRefComposites; + use generated::allof_merge::WrappedComposites; + use generated::allof_merge::WrappedCompositesReversed; + + for (input, accepted) in [ + (r#"{"value":{"count":6,"label":"valid","color":"blue"}}"#, true), + (r#"{"value":{"count":5,"label":"valid","color":"blue"}}"#, false), + (r#"{"value":{"count":10,"label":"valid","color":"blue"}}"#, false), + (r#"{"value":{"count":6,"label":"no","color":"blue"}}"#, false), + (r#"{"value":{"count":6,"label":"UPPER","color":"blue"}}"#, false), + (r#"{"value":{"count":6,"label":"valid","color":"red"}}"#, false), + ( + r#"{"value":{"count":6,"label":"valid","color":"blue","extra":0}}"#, + false, + ), + (r#"{"value":{"count":null,"label":"valid","color":"blue"}}"#, false), + (r#"{"value":{"count":6,"color":"blue"}}"#, false), + (r#"{}"#, false), + ] { + assert_eq!( + serde_json::from_str::(input).is_ok(), + accepted, + "{input}" + ); + assert_eq!( + serde_json::from_str::(input).is_ok(), + accepted, + "{input}" + ); + assert_eq!( + serde_json::from_str::(input).is_ok(), + accepted, + "{input}" + ); + assert_eq!( + serde_json::from_str::(input).is_ok(), + accepted, + "{input}" + ); + assert_eq!( + serde_json::from_str::(input).is_ok(), + accepted, + "{input}" + ); + } +} + #[test] fn all_of_enum_intersections_have_stable_variants() { use generated::allof_merge::EnumIntersection; diff --git a/crates/oapi-codegen/tests/generated/allof_merge.rs b/crates/oapi-codegen/tests/generated/allof_merge.rs index e8316b9..1beb775 100644 --- a/crates/oapi-codegen/tests/generated/allof_merge.rs +++ b/crates/oapi-codegen/tests/generated/allof_merge.rs @@ -333,6 +333,171 @@ pub struct EnumReversed { pub count: EnumReversedCount, } +pub type CompositeAlias = Intersection; + +pub type CompositeChain = CompositeAlias; + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AliasedComposites { + pub value: Intersection, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct AliasedCompositesReversed { + pub value: CompositeChain, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct InlineRefComposites { + pub value: CompositeChain, +} + +pub type WrapperA = Intersection; + +pub type WrapperB = Intersection; + +pub type WrapperChain = WrapperB; + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct WrappedComposites { + pub value: WrapperA, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct WrappedCompositesReversed { + pub value: WrapperChain, +} + +pub type ObjectMap = std::collections::HashMap; + +pub type EnumMap = std::collections::HashMap; + +pub type NestedMap = std::collections::HashMap< + String, + std::collections::HashMap, +>; + +pub type ReferencedMap = std::collections::HashMap; + +pub type StringMap = std::collections::HashMap; + +pub type AnyMap = std::collections::HashMap; + +pub type ExplicitAnyMap = std::collections::HashMap; + +pub type NullableMap = Nullable>; + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +#[serde(deny_unknown_fields)] +pub struct EmptyClosed {} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +#[serde(deny_unknown_fields)] +pub struct ClosedTarget {} + +pub type ClosedAlias = ClosedTarget; + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct InlineMaps { + pub typed: std::collections::HashMap, + pub arbitrary: std::collections::HashMap, + pub nullable: Nullable>, + pub closed: InlineMapsClosed, + pub named: StringMap, + #[serde( + skip_serializing_if = "Option::is_none", + deserialize_with = "InlineMaps::validate_optional", + default + )] + pub optional: Option>, + #[serde( + skip_serializing_if = "Option::is_none", + deserialize_with = "InlineMaps::validate_objects", + default + )] + pub objects: Option>, + #[serde( + skip_serializing_if = "Option::is_none", + deserialize_with = "InlineMaps::validate_enums", + default + )] + pub enums: Option>, + #[serde( + skip_serializing_if = "Option::is_none", + deserialize_with = "InlineMaps::validate_nested", + default + )] + pub nested: Option, +} +impl InlineMaps { + /// The rules the document gives `optional`, checked on the way in. + fn validate_optional<'de, D>( + deserializer: D, + ) -> ::core::result::Result< + Option>, + D::Error, + > + where + D: serde::Deserializer<'de>, + { + let value = Some( + as serde::Deserialize>::deserialize(deserializer)?, + ); + return Ok(value); + } + /// The rules the document gives `objects`, checked on the way in. + fn validate_objects<'de, D>( + deserializer: D, + ) -> ::core::result::Result< + Option>, + D::Error, + > + where + D: serde::Deserializer<'de>, + { + let value = Some( + as serde::Deserialize>::deserialize(deserializer)?, + ); + return Ok(value); + } + /// The rules the document gives `enums`, checked on the way in. + fn validate_enums<'de, D>( + deserializer: D, + ) -> ::core::result::Result< + Option>, + D::Error, + > + where + D: serde::Deserializer<'de>, + { + let value = Some( + as serde::Deserialize>::deserialize(deserializer)?, + ); + return Ok(value); + } + /// The rules the document gives `nested`, checked on the way in. + fn validate_nested<'de, D>( + deserializer: D, + ) -> ::core::result::Result, D::Error> + where + D: serde::Deserializer<'de>, + { + let value = Some( + ::deserialize(deserializer)?, + ); + return Ok(value); + } +} + #[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] pub enum IntersectionColor { #[serde(rename = "blue")] @@ -591,3 +756,43 @@ impl TryFrom for EnumReversedCount { }; } } + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct ObjectMapValue { + pub label: String, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub enum EnumMapValue { + #[serde(rename = "red")] + Red, + #[serde(rename = "blue")] + Blue, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct NestedMapValueValue { + pub label: String, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +#[serde(deny_unknown_fields)] +pub struct InlineMapsClosed {} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct InlineMapsObjectsValue { + pub label: String, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub enum InlineMapsEnumsValue { + #[serde(rename = "red")] + Red, + #[serde(rename = "blue")] + Blue, +} + +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq)] +pub struct InlineMapsNested { + pub value: std::collections::HashMap, +} diff --git a/docs/design.md b/docs/design.md index 0b8e646..0ed1f94 100644 --- a/docs/design.md +++ b/docs/design.md @@ -356,6 +356,9 @@ fails, including when the forbidden property is optional. The merged struct preserves `additionalProperties: false`. Schema-valued additional properties and object property-count constraints produce an error during a merge. +An unconstrained or typed map inside a single-member `allOf` retains its map representation. +Empty closed objects remain structs and reject additional properties. +References through different aliases can share the same composite definition. An absent format, pattern, or `multipleOf` retains the other member's constraint. Conflicting specified values, property types, or metadata produce an error