From ec8c8d86f469c4ef2eb0dde3c4bd0f294f3f0989 Mon Sep 17 00:00:00 2001 From: Jacob MacKenzie-Websdale Date: Wed, 19 Aug 2026 08:45:31 +0200 Subject: [PATCH] fix: resolve issue where multi-typed schemas are presumed to be nullable The existing fallback code just took the primary schema and did a fallback to `Option` but this wasn't necessarily correct. --- src/analysis.rs | 257 ++++++++++++++---- src/openapi.rs | 24 ++ ...ers__array_member_of_type_array_union.snap | 26 ++ ...helpers__nullable_shorthand_collapses.snap | 18 ++ ...pers__top_level_two_scalar_type_array.snap | 19 ++ ..._helpers__two_scalar_type_array_order.snap | 24 ++ ...lpers__two_scalar_type_array_property.snap | 24 ++ tests/typed_multi_union_test.rs | 167 ++++++++++++ 8 files changed, 511 insertions(+), 48 deletions(-) create mode 100644 src/snapshots/openapi_to_rust__test_helpers__array_member_of_type_array_union.snap create mode 100644 src/snapshots/openapi_to_rust__test_helpers__nullable_shorthand_collapses.snap create mode 100644 src/snapshots/openapi_to_rust__test_helpers__top_level_two_scalar_type_array.snap create mode 100644 src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_order.snap create mode 100644 src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_property.snap create mode 100644 tests/typed_multi_union_test.rs diff --git a/src/analysis.rs b/src/analysis.rs index 344fdd6..107a442 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -338,7 +338,7 @@ pub struct DependencyGraph { pub struct DetectedPatterns { /// Schemas that should use tagged enums (discriminated unions) pub tagged_enum_schemas: HashSet, - /// Schemas that should use untagged enums (simple unions) + /// Schemas that should use untagged enums (simple unions) pub untagged_enum_schemas: HashSet, /// Auto-detected type mappings for discriminated unions pub type_mappings: BTreeMap>, @@ -1758,54 +1758,24 @@ impl SchemaAnalyzer { } } Schema::Typed { .. } | Schema::TypedMulti { .. } => { - let primary = schema - .schema_type() - .cloned() - .unwrap_or(OpenApiSchemaType::Object); - let format = details.format.as_deref(); - match primary { - OpenApiSchemaType::String => { - if let Some(values) = details.string_enum_values() { - SchemaType::StringEnum { values } - } else { - SchemaType::Primitive { - rust_type: self.type_mapper.string_format(format).rust_type, - serde_with: None, - } - } - } - OpenApiSchemaType::Integer => SchemaType::Primitive { - rust_type: self.type_mapper.integer_format(format).rust_type, - serde_with: None, - }, - OpenApiSchemaType::Number => SchemaType::Primitive { - rust_type: self.type_mapper.number_format(format).rust_type, - serde_with: None, - }, - OpenApiSchemaType::Boolean => SchemaType::Primitive { - rust_type: self.type_mapper.boolean().rust_type, - serde_with: None, - }, - OpenApiSchemaType::Array => { - // Analyze array item type - self.analyze_array_schema(schema, schema_name, &mut dependencies)? - } - OpenApiSchemaType::Object => { - // Check if this is a dynamic JSON object - if self.should_use_dynamic_json(schema) { - SchemaType::Primitive { - rust_type: self.type_mapper.dynamic_json().rust_type, - serde_with: None, - } - } else { - // Analyze object properties - self.analyze_object_schema(schema, &mut dependencies)? - } + if let Some(non_null_types) = schema.non_null_schema_types() { + let mut variants = Vec::with_capacity(non_null_types.len()); + for t in non_null_types { + variants.push(self.build_typed_multi_union_variant( + t, + schema, + schema_name, + &mut dependencies, + )?); } - _ => SchemaType::Primitive { - rust_type: self.type_mapper.dynamic_json().rust_type, - serde_with: None, - }, + SchemaType::Union { variants } + } else { + self.analyze_single_typed_schema( + schema, + schema_name, + details, + &mut dependencies, + )? } } Schema::AnyOf { @@ -1882,6 +1852,66 @@ impl SchemaAnalyzer { }) } + /// Resolve a `Schema::Typed`/`Schema::TypedMulti` schema that carries a + /// single effective type (the 3.1 nullable shorthand already collapses + /// to this via `schema_type()`). Proper multi-type unions are handled in + /// [Self::analyze_schema_value] via + /// [Self::build_typed_multi_union_variant]. + fn analyze_single_typed_schema( + &mut self, + schema: &Schema, + schema_name: &str, + details: &crate::openapi::SchemaDetails, + dependencies: &mut HashSet, + ) -> Result { + let primary = schema + .schema_type() + .cloned() + .unwrap_or(OpenApiSchemaType::Object); + let format = details.format.as_deref(); + Ok(match primary { + OpenApiSchemaType::String => { + if let Some(values) = details.string_enum_values() { + SchemaType::StringEnum { values } + } else { + SchemaType::Primitive { + rust_type: self.type_mapper.string_format(format).rust_type, + serde_with: None, + } + } + } + OpenApiSchemaType::Integer => SchemaType::Primitive { + rust_type: self.type_mapper.integer_format(format).rust_type, + serde_with: None, + }, + OpenApiSchemaType::Number => SchemaType::Primitive { + rust_type: self.type_mapper.number_format(format).rust_type, + serde_with: None, + }, + OpenApiSchemaType::Boolean => SchemaType::Primitive { + rust_type: self.type_mapper.boolean().rust_type, + serde_with: None, + }, + OpenApiSchemaType::Array => { + self.analyze_array_schema(schema, schema_name, dependencies)? + } + OpenApiSchemaType::Object => { + if self.should_use_dynamic_json(schema) { + SchemaType::Primitive { + rust_type: self.type_mapper.dynamic_json().rust_type, + serde_with: None, + } + } else { + self.analyze_object_schema(schema, dependencies)? + } + } + _ => SchemaType::Primitive { + rust_type: self.type_mapper.dynamic_json().rust_type, + serde_with: None, + }, + }) + } + fn analyze_object_schema( &mut self, schema: &Schema, @@ -2148,6 +2178,76 @@ impl SchemaAnalyzer { }) } + /// Build one union variant for a genuine `type: [X, Y, ...]` member. + /// All members of a `TypedMulti` share a single `SchemaDetails`, so + /// `array`/`object` members carry the *same* `items`/`properties` as + /// the union schema itself — routing them through `TypeMapper::map` + /// (as the scalar members are) would discard that shape and collapse + /// to generic `Vec` / `serde_json::Value`. + /// + /// This just properly handles array and object types before passing on to + /// the type mapper. + fn build_typed_multi_union_variant( + &mut self, + member_type: OpenApiSchemaType, + schema: &Schema, + union_type_name: &str, + dependencies: &mut HashSet, + ) -> Result { + match member_type { + OpenApiSchemaType::Array => { + let array_type_name = format!("{union_type_name}Array"); + let array_type = + self.analyze_array_schema(schema, &array_type_name, dependencies)?; + self.resolved_cache.insert( + array_type_name.clone(), + AnalyzedSchema { + name: array_type_name.clone(), + original: serde_json::to_value(schema).unwrap_or(Value::Null), + schema_type: array_type, + dependencies: HashSet::new(), + nullable: false, + description: Some("Array variant in union".to_string()), + default: None, + }, + ); + dependencies.insert(array_type_name.clone()); + Ok(SchemaRef { + target: array_type_name, + nullable: false, + }) + } + OpenApiSchemaType::Object => { + let object_type_name = format!("{union_type_name}Object"); + let object_type = self.analyze_object_schema(schema, dependencies)?; + self.resolved_cache.insert( + object_type_name.clone(), + AnalyzedSchema { + name: object_type_name.clone(), + original: serde_json::to_value(schema).unwrap_or(Value::Null), + schema_type: object_type, + dependencies: dependencies.clone(), + nullable: false, + description: schema.details().description.clone(), + default: None, + }, + ); + dependencies.insert(object_type_name.clone()); + Ok(SchemaRef { + target: object_type_name, + nullable: false, + }) + } + _ => Ok(SchemaRef { + target: self + .type_mapper + .map(member_type, schema.details()) + .rust_type, + nullable: false, + }), + } + } + fn analyze_property_schema_with_context( &mut self, schema: &Schema, @@ -2181,6 +2281,67 @@ impl SchemaAnalyzer { } } + // Genuine multi-scalar `type: [X, Y]` union (not the 3.1 nullable + // shorthand `[X, "null"]`, which `schema_type()` already collapses). + // Give it a named enum, same as an anyOf/oneOf union property below. + if let Some(non_null_types) = schema.non_null_schema_types() { + let context_name = self + .current_schema_name + .clone() + .unwrap_or_else(|| "Unknown".to_string()); + let prop_pascal = property_name + .map(|name| self.to_pascal_case(name)) + .unwrap_or_default(); + let mut union_type_name = format!("{context_name}{prop_pascal}"); + if self.schemas.contains_key(&union_type_name) + || self.resolved_cache.contains_key(&union_type_name) + { + let mut suffix = 2; + loop { + let candidate = format!("{union_type_name}Union{suffix}"); + if !self.schemas.contains_key(&candidate) + && !self.resolved_cache.contains_key(&candidate) + { + union_type_name = candidate; + break; + } + suffix += 1; + if suffix > 1000 { + break; + } + } + } + + let details = schema.details(); + let mut variants = Vec::with_capacity(non_null_types.len()); + for t in non_null_types { + variants.push(self.build_typed_multi_union_variant( + t, + schema, + &union_type_name, + dependencies, + )?); + } + + self.resolved_cache.insert( + union_type_name.clone(), + AnalyzedSchema { + name: union_type_name.clone(), + original: serde_json::to_value(schema).unwrap_or(Value::Null), + schema_type: SchemaType::Union { variants }, + dependencies: HashSet::new(), + nullable: false, + description: details.description.clone(), + default: None, + }, + ); + + dependencies.insert(union_type_name.clone()); + return Ok(SchemaType::Reference { + target: union_type_name, + }); + } + if let Some(schema_type) = schema.schema_type() { match schema_type { OpenApiSchemaType::String => { diff --git a/src/openapi.rs b/src/openapi.rs index 32b05b6..e998da9 100644 --- a/src/openapi.rs +++ b/src/openapi.rs @@ -664,6 +664,9 @@ impl Schema { /// Get the schema type if explicitly set. For `Schema::TypedMulti` the /// "primary" non-null type is returned; if the array contained only `null` /// then `Some(&SchemaType::Null)` is returned. + /// + /// If the other non-null variants are important, consider where you should + /// instead use [non_null_schema_types][Self::non_null_schema_types]. pub fn schema_type(&self) -> Option<&SchemaType> { match self { Schema::Typed { schema_type, .. } => Some(schema_type), @@ -675,6 +678,27 @@ impl Schema { } } + /// Gets all non-null types from `type: [...]` (unlike + /// [Schema::schema_type] that handles the null value). Returns `None` if + /// the given [Schema::TypedMulti] has either only one non-null value, or if + /// this isn't a `Schema::TypedMulti`. + /// + /// This also removes duplicates. + pub fn non_null_schema_types(&self) -> Option> { + match self { + Schema::TypedMulti { schema_types, .. } => { + let mut non_null = Vec::new(); + for t in schema_types { + if *t != SchemaType::Null && !non_null.contains(t) { + non_null.push(t.clone()); + } + } + (non_null.len() > 1).then_some(non_null) + } + _ => None, + } + } + /// True when the schema's type set explicitly contains `null`. /// (3.1 canonical nullability via `type: ["X", "null"]`.) pub fn type_array_contains_null(&self) -> bool { diff --git a/src/snapshots/openapi_to_rust__test_helpers__array_member_of_type_array_union.snap b/src/snapshots/openapi_to_rust__test_helpers__array_member_of_type_array_union.snap new file mode 100644 index 0000000..1808f54 --- /dev/null +++ b/src/snapshots/openapi_to_rust__test_helpers__array_member_of_type_array_union.snap @@ -0,0 +1,26 @@ +--- +source: src/test_helpers.rs +expression: "&generated_code" +--- +//! Generated types from OpenAPI specification +//! +//! This file contains all the generated types for the API. +//! Do not edit manually - regenerate using the appropriate script. +#![allow(clippy::large_enum_variant)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] +#![allow(unreachable_patterns)] +use serde::{Deserialize, Serialize}; +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +pub struct Widget { + #[serde(skip_serializing_if = "Option::is_none")] + pub tags_or_tag: Option, +} +///Array variant in union +pub type WidgetTagsOrTagArray = Vec; +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(untagged)] +pub enum WidgetTagsOrTag { + WidgetTagsOrTagArray(WidgetTagsOrTagArray), + String(String), +} diff --git a/src/snapshots/openapi_to_rust__test_helpers__nullable_shorthand_collapses.snap b/src/snapshots/openapi_to_rust__test_helpers__nullable_shorthand_collapses.snap new file mode 100644 index 0000000..cb9b54b --- /dev/null +++ b/src/snapshots/openapi_to_rust__test_helpers__nullable_shorthand_collapses.snap @@ -0,0 +1,18 @@ +--- +source: src/test_helpers.rs +expression: "&generated_code" +--- +//! Generated types from OpenAPI specification +//! +//! This file contains all the generated types for the API. +//! Do not edit manually - regenerate using the appropriate script. +#![allow(clippy::large_enum_variant)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] +#![allow(unreachable_patterns)] +use serde::{Deserialize, Serialize}; +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +pub struct Widget { + #[serde(skip_serializing_if = "Option::is_none")] + pub maybe_name: Option, +} diff --git a/src/snapshots/openapi_to_rust__test_helpers__top_level_two_scalar_type_array.snap b/src/snapshots/openapi_to_rust__test_helpers__top_level_two_scalar_type_array.snap new file mode 100644 index 0000000..d4c475b --- /dev/null +++ b/src/snapshots/openapi_to_rust__test_helpers__top_level_two_scalar_type_array.snap @@ -0,0 +1,19 @@ +--- +source: src/test_helpers.rs +expression: "&generated_code" +--- +//! Generated types from OpenAPI specification +//! +//! This file contains all the generated types for the API. +//! Do not edit manually - regenerate using the appropriate script. +#![allow(clippy::large_enum_variant)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] +#![allow(unreachable_patterns)] +use serde::{Deserialize, Serialize}; +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(untagged)] +pub enum IdOrCode { + Integer(i64), + String(String), +} diff --git a/src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_order.snap b/src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_order.snap new file mode 100644 index 0000000..97a9177 --- /dev/null +++ b/src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_order.snap @@ -0,0 +1,24 @@ +--- +source: src/test_helpers.rs +expression: "&generated_code" +--- +//! Generated types from OpenAPI specification +//! +//! This file contains all the generated types for the API. +//! Do not edit manually - regenerate using the appropriate script. +#![allow(clippy::large_enum_variant)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] +#![allow(unreachable_patterns)] +use serde::{Deserialize, Serialize}; +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +pub struct Widget { + #[serde(skip_serializing_if = "Option::is_none")] + pub id_or_code: Option, +} +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(untagged)] +pub enum WidgetIdOrCode { + String(String), + Integer(i64), +} diff --git a/src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_property.snap b/src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_property.snap new file mode 100644 index 0000000..45a4dae --- /dev/null +++ b/src/snapshots/openapi_to_rust__test_helpers__two_scalar_type_array_property.snap @@ -0,0 +1,24 @@ +--- +source: src/test_helpers.rs +expression: "&generated_code" +--- +//! Generated types from OpenAPI specification +//! +//! This file contains all the generated types for the API. +//! Do not edit manually - regenerate using the appropriate script. +#![allow(clippy::large_enum_variant)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] +#![allow(unreachable_patterns)] +use serde::{Deserialize, Serialize}; +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +pub struct Widget { + #[serde(skip_serializing_if = "Option::is_none")] + pub id_or_code: Option, +} +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(untagged)] +pub enum WidgetIdOrCode { + Integer(i64), + String(String), +} diff --git a/tests/typed_multi_union_test.rs b/tests/typed_multi_union_test.rs new file mode 100644 index 0000000..f4803ae --- /dev/null +++ b/tests/typed_multi_union_test.rs @@ -0,0 +1,167 @@ +#![cfg(feature = "test-helpers")] + +//! A JSON-Schema-2020-12-style `type: [X, Y]` array with two *non-null* +//! scalar types must generate an untagged enum covering both branches, +//! not silently collapse to whichever type is listed first. + +use openapi_to_rust::test_helpers::*; +use serde_json::json; + +#[test] +fn two_scalar_type_array_property_becomes_untagged_enum() { + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0"}, + "components": { + "schemas": { + "Widget": { + "type": "object", + "properties": { + "id_or_code": {"type": ["integer", "string"]} + } + } + } + } + }); + + let result = + test_generation("two_scalar_type_array_property", spec).expect("Generation failed"); + + assert!( + result.contains("pub id_or_code: Option"), + "a two-scalar type array property must reference a named union enum, got:\n{result}" + ); + assert!( + result.contains("enum WidgetIdOrCode"), + "the named union enum must be generated, got:\n{result}" + ); + assert!( + result.contains("Integer(i64)") && result.contains("String(String)"), + "the union enum must cover both declared types, got:\n{result}" + ); + assert!( + result.contains("#[serde(untagged)]"), + "the union enum must be untagged so either wire shape deserializes, got:\n{result}" + ); +} + +#[test] +fn two_scalar_type_array_order_is_preserved_not_first_wins() { + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0"}, + "components": { + "schemas": { + "Widget": { + "type": "object", + "properties": { + "id_or_code": {"type": ["string", "integer"]} + } + } + } + } + }); + + let result = test_generation("two_scalar_type_array_order", spec).expect("Generation failed"); + + let enum_start = result + .find("enum WidgetIdOrCode") + .expect("union enum must be generated"); + let string_variant = result[enum_start..].find("String(String)").unwrap(); + let integer_variant = result[enum_start..].find("Integer(i64)").unwrap(); + assert!( + string_variant < integer_variant, + "variant order must follow the declared type order, got:\n{result}" + ); +} + +#[test] +fn top_level_two_scalar_type_array_becomes_untagged_enum() { + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0"}, + "components": { + "schemas": { + "IdOrCode": {"type": ["integer", "string"]} + } + } + }); + + let result = + test_generation("top_level_two_scalar_type_array", spec).expect("Generation failed"); + + assert!( + result.contains("enum IdOrCode") + && result.contains("Integer(i64)") + && result.contains("String(String)"), + "a top-level two-scalar type array schema must become an untagged enum, got:\n{result}" + ); +} + +/// The 3.1 nullable shorthand (`[X, "null"]`) must keep collapsing to +/// `Option` — only genuine multi-scalar unions get an enum. +#[test] +fn nullable_shorthand_still_collapses_to_option() { + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0"}, + "components": { + "schemas": { + "Widget": { + "type": "object", + "properties": { + "maybe_name": {"type": ["string", "null"]} + } + } + } + } + }); + + let result = test_generation("nullable_shorthand_collapses", spec).expect("Generation failed"); + + assert!( + result.contains("pub maybe_name: Option"), + "the nullable shorthand must stay a plain Option, got:\n{result}" + ); + assert!( + !result.contains("enum WidgetMaybeName"), + "the nullable shorthand must not synthesize a union enum, got:\n{result}" + ); +} + +/// A multi-type union member that's `"array"` shares its `items` schema +/// with the other members (`TypedMulti` carries one `SchemaDetails` for +/// the whole `type: [...]` list). The array variant must keep that item +/// type instead of collapsing to `Vec`. +#[test] +fn array_member_of_type_array_union_keeps_item_type() { + let spec = json!({ + "openapi": "3.1.0", + "info": {"title": "Test", "version": "1.0"}, + "components": { + "schemas": { + "Widget": { + "type": "object", + "properties": { + "tags_or_tag": { + "type": ["array", "string"], + "items": {"type": "string"} + } + } + } + } + } + }); + + let result = + test_generation("array_member_of_type_array_union", spec).expect("Generation failed"); + + assert!( + result.contains("pub type WidgetTagsOrTagArray = Vec"), + "the array variant must keep its declared item type, got:\n{result}" + ); + assert!( + !result.contains("Vec"), + "the array variant must not degrade to a generic JSON array, got:\n{result}" + ); +}