From 42c91872f7a410570326a824e0d83481dc98e77f Mon Sep 17 00:00:00 2001 From: Svetlin Ralchev Date: Fri, 2 Oct 2026 16:54:55 +0400 Subject: [PATCH] fix(analysis): requiredness branches may restate `type: object` A union whose branches only list required properties constrains the object; it isn't a variant of it. A branch that also says `type: object` was read as an empty object variant, though only an object has required properties, so it says nothing the requiredness doesn't. An `allOf` of two such unions then failed as intersecting multiple union members, and one beside a real union competed with it for the variant. The corpus doesn't change: no pinned spec has such a branch. Fixes #88 --- CHANGELOG.md | 9 ++++ src/analysis.rs | 44 +++++++++-------- tests/recoverable_typing_test.rs | 81 ++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d33c89..49cfbe3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ when correcting output that was wrong or incomplete on the wire. ## [Unreleased] +### Fixed + +- A union whose branches only list required properties is a constraint on the + object, not a variant of it, also when a branch restates `type: object`. + An `allOf` of two of them (Cloudflare's "at least one of `to`, `cc` or + `bcc`, and of `text` or `html`") no longer fails generation as intersecting + multiple union members, and one beside a real union leaves that union the + variant (#88). + ## [0.19.0] - 2026-09-26 ### Added diff --git a/src/analysis.rs b/src/analysis.rs index e95613b..3ab7bb5 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -6392,30 +6392,36 @@ impl SchemaAnalyzer { /// Requiredness formulas can nest through `anyOf`/`oneOf` and `not`, as /// in protobuf-generated "at most one field" schemas. They constrain /// presence but add no payload shape for a Rust field to carry. + /// + /// A branch may restate `type: object`, as Cloudflare's "at least one of + /// `to`, `cc` or `bcc`" does: only an object has required properties, so + /// it says nothing the requiredness doesn't. fn schema_only_constrains_requiredness(schema: &Schema) -> bool { let keys_are_requiredness_or_annotations = serde_json::to_value(schema) .ok() .and_then(|value| value.as_object().cloned()) .is_some_and(|object| { - object.keys().all(|key| { - matches!( - key.as_str(), - "required" - | "not" - | "anyOf" - | "oneOf" - | "title" - | "description" - | "deprecated" - | "readOnly" - | "writeOnly" - | "examples" - | "example" - | "default" - | "externalDocs" - | "xml" - | "$comment" - ) || key.starts_with("x-") + object.iter().all(|(key, value)| { + (key == "type" && value == "object") + || matches!( + key.as_str(), + "required" + | "not" + | "anyOf" + | "oneOf" + | "title" + | "description" + | "deprecated" + | "readOnly" + | "writeOnly" + | "examples" + | "example" + | "default" + | "externalDocs" + | "xml" + | "$comment" + ) + || key.starts_with("x-") }) }); if !keys_are_requiredness_or_annotations { diff --git a/tests/recoverable_typing_test.rs b/tests/recoverable_typing_test.rs index 2706765..fa465a2 100644 --- a/tests/recoverable_typing_test.rs +++ b/tests/recoverable_typing_test.rs @@ -372,6 +372,87 @@ fn a_union_that_only_alternates_requiredness_is_the_object_it_describes() { ); } +#[test] +fn requiredness_branches_that_restate_type_object_are_still_only_requiredness() { + // Cloudflare's Email Sending: "at least one of to or cc, and at least one + // of text or html". Each branch also says `type: object`, which only + // restates what having required properties means, so the two unions are + // constraints on one object, not two variants it can't be both of. + assert_types( + spec_with_schemas(json!({ + "Email": { + "type": "object", + "required": ["from"], + "properties": { + "from": { "type": "string" }, + "to": { "type": "string" }, + "cc": { "type": "string" }, + "text": { "type": "string" }, + "html": { "type": "string" } + }, + "allOf": [ + { "anyOf": [ + { "type": "object", "required": ["to"] }, + { "type": "object", "required": ["cc"] } + ]}, + { "anyOf": [ + { "type": "object", "required": ["text"] }, + { "type": "object", "required": ["html"] } + ]} + ] + } + })), + &[ + "pub struct Email", + "pub from: String", + "pub to: Option", + "pub html: Option", + ], + ); +} + +#[test] +fn a_requiredness_union_beside_a_real_union_leaves_that_union_the_variant() { + // Cloudflare's Magic WAN: an app is an account app or a managed app, and + // sets breakout, priority or both. Only the first is a variant. + let generated = generate(spec_with_schemas(json!({ + "AppConfig": { + "type": "object", + "allOf": [ + { "oneOf": [ + { "type": "object", "required": ["account_app_id"], + "properties": { "account_app_id": { "type": "string" } } }, + { "type": "object", "required": ["managed_app_id"], + "properties": { "managed_app_id": { "type": "string" } } } + ]}, + { "anyOf": [ + { "type": "object", "required": ["breakout"] }, + { "type": "object", "required": ["priority"] } + ]}, + { "type": "object", "properties": { + "breakout": { "type": "boolean" }, + "priority": { "type": "integer" } + }} + ] + } + }))); + for want in [ + "pub struct AppConfig", + "pub breakout: Option", + "pub priority: Option", + "pub variant: AppConfigAllOfVariant1", + ] { + assert!( + generated.contains(want), + "expected `{want}` in generated output:\n{generated}" + ); + } + assert!( + !generated.contains("AppConfigAllOfVariant2"), + "the requiredness union is not a variant:\n{generated}" + ); +} + #[test] fn union_branches_that_are_deep_pointers_are_expanded() { // A component-root prefix must not make these look like two references to