Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions crates/oapi-codegen/src/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
}
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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 [
Expand Down
275 changes: 250 additions & 25 deletions crates/oapi-codegen/src/lower/all_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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<Box<Schema>>, depth: usize) -> Result<Schema> {
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(),
Expand All @@ -159,6 +161,13 @@ fn resolve(spec: &Spec, path: &str, property: &ReferenceOr<Box<Schema>>, depth:
});
}

fn resolve_reference<'a>(spec: &'a Spec, property: &'a ReferenceOr<Box<Schema>>) -> Result<&'a Schema> {
return match property {
ReferenceOr::Item(schema) => Ok(schema),
ReferenceOr::Reference { reference } => spec.resolve(reference),
};
}

fn intersect(
spec: &Spec,
path: &str,
Expand All @@ -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
{
Comment thread
dotkas marked this conversation as resolved.
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;
Expand Down Expand Up @@ -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 [
Expand Down Expand Up @@ -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"),
Expand All @@ -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});
Expand Down
Loading