From b9bf6839f2e948d92d6fa1672e807ca84d742aa7 Mon Sep 17 00:00:00 2001 From: Giles Westwood <201443+gilesw@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:17:37 +0100 Subject: [PATCH 1/3] fix(generator): resolve allOf members that restate the same union An allOf whose members are the same union, once annotations are ignored, failed with "intersects multiple union members". The current Cloudflare spec types its Workers Observability query filters as allOf: [$ref filter_node, anyOf: [group, leaf]], where the inline anyOf repeats filter_node's branches with added descriptions. That error stopped the whole spec from generating. Repeated union members are now dropped before composition, comparing assertions only: annotation keywords are stripped recursively, member names under properties are kept, and `type: object` on the named union is ignored. A restatement next to a single $ref becomes an alias of that schema. Unions that differ still report the diagnostic. The pinned corpus has no such schema, so its output is unchanged. --- src/analysis.rs | 81 +++++++++++++++++++ tests/allof_object_union_preservation_test.rs | 80 ++++++++++++++++++ 2 files changed, 161 insertions(+) diff --git a/src/analysis.rs b/src/analysis.rs index a0a460c..b7c4f46 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -1415,6 +1415,46 @@ 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); + } + } + "enum" | "const" | "default" | "required" | "discriminator" => {} + _ => match child { + Value::Array(items) => items.iter_mut().for_each(strip_schema_annotations), + Value::Object(_) => strip_schema_annotations(child), + _ => {} + }, + } + } +} + /// 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 +4318,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 +4689,44 @@ 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 so a named union matches an inline restatement + /// that omits it. + 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") { + 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) + } + /// 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..3d94cfa 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!({ From 5cfa18f9108df34cdb5fef2fb78c24d1013ceeb3 Mon Sep 17 00:00:00 2001 From: Giles Westwood <201443+gilesw@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:17:37 +0100 Subject: [PATCH 2/3] docs: changelog entry for restated allOf unions --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63cd447..0792ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ 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. ## [0.22.0] - 2026-10-06 From 0ac4b337c89052fc78eae1fa4ccddaa29bed066e Mon Sep 17 00:00:00 2001 From: Giles Westwood <201443+gilesw@users.noreply.github.com> Date: Sat, 17 Oct 2026 00:21:03 +0100 Subject: [PATCH 3/3] fix: preserve assertions when deduplicating allOf unions --- CHANGELOG.md | 4 +- src/analysis.rs | 67 +++++++++++++++++-- tests/allof_object_union_preservation_test.rs | 53 +++++++++++++++ 3 files changed, 116 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0792ea3..91dac65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,9 @@ when correcting output that was wrong or incomplete on the wire. 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. + 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 b7c4f46..5fabaac 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -1445,12 +1445,29 @@ fn strip_schema_annotations(value: &mut Value) { members.values_mut().for_each(strip_schema_annotations); } } - "enum" | "const" | "default" | "required" | "discriminator" => {} - _ => match child { + "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), - Value::Object(_) => strip_schema_annotations(child), - _ => {} + _ => 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. + _ => {} } } } @@ -4707,8 +4724,8 @@ impl SchemaAnalyzer { } /// The assertions of a member that is only a union, following one `$ref`. - /// `type: object` is ignored so a named union matches an inline restatement - /// that omits it. + /// `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)?)?, @@ -4717,7 +4734,19 @@ impl SchemaAnalyzer { 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") { + 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"); @@ -4727,6 +4756,30 @@ impl SchemaAnalyzer { (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 3d94cfa..4258c6f 100644 --- a/tests/allof_object_union_preservation_test.rs +++ b/tests/allof_object_union_preservation_test.rs @@ -577,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")); + } +}