diff --git a/CHANGELOG.md b/CHANGELOG.md index 63cd447..91dac65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ when correcting output that was wrong or incomplete on the wire. optionality. Cloudflare's DNS record responses list `data` as required although only some record types carry it, so A and MX records failed to deserialize, and `name`, `content` and `ttl` were untyped. +- An `allOf` whose members repeat the same union, differing only in + annotations such as `description`, resolves to that union instead of failing + with "intersects multiple union members". The current Cloudflare spec + restates its Workers Observability filter union inline next to a `$ref` to + it, which stopped the whole spec from generating. Members that are different + unions still fail with that diagnostic. Assertion data such as + `dependentRequired` is preserved, and `type: object` is ignored only when + every union branch explicitly requires an object. ## [0.22.0] - 2026-10-06 diff --git a/src/analysis.rs b/src/analysis.rs index a0a460c..5fabaac 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -1415,6 +1415,63 @@ fn schema_is_annotation_only(schema: &crate::openapi::Schema) -> bool { }) } +fn is_annotation_keyword(key: &str) -> bool { + matches!( + key, + "title" + | "description" + | "deprecated" + | "readOnly" + | "writeOnly" + | "examples" + | "example" + | "externalDocs" + | "xml" + | "$comment" + ) || key.starts_with("x-") +} + +/// Remove annotation keywords from a schema and its subschemas, leaving the +/// assertions. Member names under `properties` and similar maps are kept. +fn strip_schema_annotations(value: &mut Value) { + let Some(object) = value.as_object_mut() else { + return; + }; + object.retain(|key, _| !is_annotation_keyword(key)); + for (key, child) in object.iter_mut() { + match key.as_str() { + "properties" | "patternProperties" | "$defs" | "definitions" | "dependentSchemas" => { + if let Some(members) = child.as_object_mut() { + members.values_mut().for_each(strip_schema_annotations); + } + } + "allOf" | "anyOf" | "oneOf" | "prefixItems" => { + if let Some(items) = child.as_array_mut() { + items.iter_mut().for_each(strip_schema_annotations); + } + } + "items" => match child { + Value::Array(items) => items.iter_mut().for_each(strip_schema_annotations), + _ => strip_schema_annotations(child), + }, + "additionalProperties" + | "additionalItems" + | "unevaluatedProperties" + | "unevaluatedItems" + | "contains" + | "propertyNames" + | "not" + | "if" + | "then" + | "else" + | "contentSchema" => strip_schema_annotations(child), + // Other keywords contain data, not schemas (notably dependentRequired). + // Preserve unknown keywords conservatively as well. + _ => {} + } + } +} + /// Load an extension file and parse it into the JSON representation used by /// the analyzer. YAML extensions follow the same conversion policy as YAML /// OpenAPI documents; every other extension is parsed as JSON. @@ -4278,6 +4335,9 @@ impl SchemaAnalyzer { all_of_schemas: &[Schema], dependencies: &mut HashSet, ) -> Result { + let without_restated = self.without_restated_unions(all_of_schemas); + let all_of_schemas = without_restated.as_deref().unwrap_or(all_of_schemas); + // A scalar/array carrier intersected only with annotation-only // siblings keeps its wire type. This is deliberately narrower than // "pick the first non-object": multiple carriers or assertion-bearing @@ -4646,6 +4706,80 @@ impl SchemaAnalyzer { } } + /// Drop `allOf` members that repeat an earlier member's union, compared + /// without annotations. Returns `None` when nothing repeats. + fn without_restated_unions(&self, members: &[Schema]) -> Option> { + let mut seen = Vec::new(); + let mut kept = Vec::with_capacity(members.len()); + for member in members { + if let Some(signature) = self.union_signature(member) { + if seen.contains(&signature) { + continue; + } + seen.push(signature); + } + kept.push(member.clone()); + } + (kept.len() < members.len()).then_some(kept) + } + + /// The assertions of a member that is only a union, following one `$ref`. + /// `type: object` is ignored only when every branch already requires an + /// object, so a named object union can match an inline restatement. + fn union_signature(&self, member: &Schema) -> Option { + let schema = match member.reference() { + Some(reference) => self.schemas.get(self.extract_schema_name(reference)?)?, + None => member, + }; + let mut value = serde_json::to_value(schema).ok()?; + strip_schema_annotations(&mut value); + let object = value.as_object_mut()?; + if object.get("type").and_then(Value::as_str) == Some("object") + && ["anyOf", "oneOf"].iter().any(|keyword| { + object + .get(*keyword) + .and_then(Value::as_array) + .is_some_and(|branches| { + !branches.is_empty() + && branches + .iter() + .all(|branch| self.union_branch_requires_object(branch)) + }) + }) + { + object.remove("type"); + } + let is_union = object.contains_key("anyOf") || object.contains_key("oneOf"); + let only_union = object + .keys() + .all(|key| matches!(key.as_str(), "anyOf" | "oneOf" | "discriminator")); + (is_union && only_union).then_some(value) + } + + /// A conservative proof: require an explicit object type, optionally + /// following one reference. Do not infer a type from object-only keywords + /// such as properties/required, which also accept scalars in JSON Schema. + fn union_branch_requires_object(&self, branch: &Value) -> bool { + let resolved; + let branch = if let Some(reference) = branch.get("$ref").and_then(Value::as_str) { + let Some(schema) = self + .extract_schema_name(reference) + .and_then(|name| self.schemas.get(name)) + else { + return false; + }; + let Ok(value) = serde_json::to_value(schema) else { + return false; + }; + resolved = value; + &resolved + } else { + branch + }; + branch.get("type").and_then(Value::as_str) == Some("object") + && branch.get("nullable").and_then(Value::as_bool) != Some(true) + } + /// Property names declared by any object reachable through the branches /// of an analyzed union, including nested unions and flattened variants. fn union_declared_property_names(&mut self, target: &str) -> Result> { diff --git a/tests/allof_object_union_preservation_test.rs b/tests/allof_object_union_preservation_test.rs index 6ce2704..4258c6f 100644 --- a/tests/allof_object_union_preservation_test.rs +++ b/tests/allof_object_union_preservation_test.rs @@ -1,3 +1,5 @@ +#![recursion_limit = "256"] + use openapi_to_rust::analysis::SchemaType; use openapi_to_rust::{CodeGenerator, GeneratorConfig, SchemaAnalyzer}; use serde_json::{Value, json}; @@ -188,6 +190,61 @@ fn object_union_allof_spec() -> Value { { "$ref": "#/components/schemas/RecordWithData" } ] }, + "FilterLeaf": { + "type": "object", + "required": ["key", "operation"], + "properties": { + "key": { "type": "string" }, + "operation": { "type": "string" }, + "value": { "type": "string" } + } + }, + "FilterNode": { + "description": "A leaf filter or a nested group.", + "type": "object", + "anyOf": [ + { + "type": "object", + "required": ["kind", "filters"], + "properties": { + "kind": { "type": "string", "enum": ["group"] }, + "filters": { + "type": "array", + "items": { "$ref": "#/components/schemas/FilterNode" } + } + } + }, + { "$ref": "#/components/schemas/FilterLeaf" } + ] + }, + "RestatedFilter": { + "allOf": [ + { "$ref": "#/components/schemas/FilterNode" }, + { + "description": "Supports nested groups.", + "anyOf": [ + { + "description": "A group of child filters.", + "type": "object", + "required": ["kind", "filters"], + "properties": { + "kind": { + "description": "Marks a group.", + "type": "string", + "enum": ["group"] + }, + "filters": { + "description": "Child filters.", + "type": "array", + "items": { "$ref": "#/components/schemas/FilterNode" } + } + } + }, + { "$ref": "#/components/schemas/FilterLeaf" } + ] + } + ] + }, "GitpodOutputSpec": { "type": "object", "additionalProperties": false, @@ -445,6 +502,17 @@ fn main() { "boolean": {} })); round_trip::(serde_json::json!({})); + round_trip::(serde_json::json!({ + "kind": "group", + "filters": [ + { "key": "service", "operation": "eq", "value": "api" }, + { "kind": "group", "filters": [] } + ] + })); + round_trip::(serde_json::json!({ + "key": "status", + "operation": "exists" + })); } "##, ) @@ -468,6 +536,18 @@ fn main() { ); } +#[test] +fn allof_restating_a_union_up_to_annotations_resolves_to_that_union() { + let analysis = analyze(); + let SchemaType::Reference { target } = &analysis.schemas["RestatedFilter"].schema_type else { + panic!( + "RestatedFilter should alias FilterNode, got {:?}", + analysis.schemas["RestatedFilter"].schema_type + ); + }; + assert_eq!(target, "FilterNode"); +} + #[test] fn multiple_union_intersections_are_reported_instead_of_silently_dropped() { let spec = json!({ @@ -497,3 +577,56 @@ fn multiple_union_intersections_are_reported_instead_of_silently_dropped() { "unexpected error: {error}" ); } + +fn assert_distinct_unions_are_rejected(first: Value, second: Value, witness: Value) { + assert!(jsonschema::is_valid(&first, &witness)); + assert!(!jsonschema::is_valid(&second, &witness)); + for members in [json!([first, second]), json!([second, first])] { + let spec = json!({ + "openapi": "3.1.0", + "info": { "title": "distinct unions", "version": "1" }, + "paths": {}, + "components": { "schemas": { "Intersection": { "allOf": members } } } + }); + let error = SchemaAnalyzer::new(spec) + .unwrap() + .analyze() + .expect_err("different union assertions must not be deduplicated"); + assert!( + error + .to_string() + .contains("intersects multiple union members"), + "unexpected error: {error}" + ); + } +} + +#[test] +fn union_restatements_preserve_dependent_required_data() { + for keyword in ["anyOf", "oneOf"] { + let first = json!({keyword: [ + {"type": "object", "dependentRequired": {"description": ["kind"]}}, + {"type": "string"} + ]}); + let second = json!({keyword: [ + {"type": "object", "dependentRequired": {"description": ["value"]}}, + {"type": "string"} + ]}); + assert_distinct_unions_are_rejected( + first, + second, + json!({"description": "x", "kind": "y"}), + ); + } +} + +#[test] +fn union_restatements_preserve_object_type_for_mixed_branches() { + for keyword in ["anyOf", "oneOf"] { + let first = json!({keyword: [{"type": "object"}, {"type": "string"}]}); + let second = json!({"type": "object", keyword: [ + {"type": "object"}, {"type": "string"} + ]}); + assert_distinct_unions_are_rejected(first, second, json!("scalar")); + } +}