Skip to content

Commit 71e9288

Browse files
fix(analysis): handle {"type": "null"} variants in oneOf
oneOf with [Type, null] now reduces to the non-null type with the property marked nullable, matching how anyOf already behaves. For oneOf with 3+ variants including null, the null branch is filtered out instead of leaking through as a serde_json::Value variant that the generator would mangle into a phantom `SerdeJsonValue` type. - openapi: is_nullable_pattern / non_null_variant cover oneOf too - analysis: filter null variants in untagged unions; require &str parent_name to drop magic "Union"/"Inline" fallbacks - generator: emit serde_json::Value as a path defensively, so any future path that pushes it as a union variant target stays valid Rust instead of producing SerdeJsonValue(SerdeJsonValue) Fixes #7 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e64f1de commit 71e9288

4 files changed

Lines changed: 134 additions & 44 deletions

File tree

src/analysis.rs

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,12 @@ impl SchemaAnalyzer {
11271127
..
11281128
} => {
11291129
// Handle oneOf discriminated unions
1130-
self.analyze_oneof_union(one_of, discriminator.as_ref(), None, &mut dependencies)?
1130+
self.analyze_oneof_union(
1131+
one_of,
1132+
discriminator.as_ref(),
1133+
schema_name,
1134+
&mut dependencies,
1135+
)?
11311136
}
11321137
Schema::AllOf { all_of, .. } => {
11331138
// Handle allOf composition (schema inheritance)
@@ -1258,7 +1263,7 @@ impl SchemaAnalyzer {
12581263
let union_schema_type = self.analyze_oneof_union(
12591264
one_of,
12601265
discriminator.as_ref(),
1261-
Some(&union_type_name),
1266+
&union_type_name,
12621267
dependencies,
12631268
)?;
12641269

@@ -1627,7 +1632,7 @@ impl SchemaAnalyzer {
16271632
let oneof_result = self.analyze_oneof_union(
16281633
one_of,
16291634
discriminator.as_ref(),
1630-
Some(&union_name),
1635+
&union_name,
16311636
dependencies,
16321637
)?;
16331638

@@ -1927,9 +1932,28 @@ impl SchemaAnalyzer {
19271932
&mut self,
19281933
one_of_schemas: &[Schema],
19291934
discriminator: Option<&crate::openapi::Discriminator>,
1930-
parent_name: Option<&str>,
1935+
parent_name: &str,
19311936
dependencies: &mut HashSet<String>,
19321937
) -> Result<SchemaType> {
1938+
// Pattern: nullable [Type, null] — return the non-null type directly.
1939+
// The nullable bit is recorded at the property level via is_nullable_pattern().
1940+
if one_of_schemas.len() == 2 {
1941+
let null_count = one_of_schemas
1942+
.iter()
1943+
.filter(|s| matches!(s.schema_type(), Some(OpenApiSchemaType::Null)))
1944+
.count();
1945+
if null_count == 1 {
1946+
if let Some(non_null) = one_of_schemas
1947+
.iter()
1948+
.find(|s| !matches!(s.schema_type(), Some(OpenApiSchemaType::Null)))
1949+
{
1950+
return self
1951+
.analyze_schema_value(non_null, parent_name)
1952+
.map(|a| a.schema_type);
1953+
}
1954+
}
1955+
}
1956+
19331957
// If there's no discriminator, we should create an untagged union
19341958
if discriminator.is_none() {
19351959
// Handle untagged unions (oneOf without discriminator)
@@ -2136,9 +2160,8 @@ impl SchemaAnalyzer {
21362160
});
21372161
} else {
21382162
// Handle inline schemas by creating type aliases or using primitive types directly
2139-
let context = parent_name.unwrap_or("Union");
21402163
let inline_name = self.generate_context_aware_name(
2141-
context,
2164+
parent_name,
21422165
"InlineVariant",
21432166
variant_index,
21442167
Some(variant_schema),
@@ -2178,9 +2201,8 @@ impl SchemaAnalyzer {
21782201
}
21792202
_ => {
21802203
// For other array types, create an inline type
2181-
let context = parent_name.unwrap_or("Inline");
21822204
let inline_type_name = self.generate_context_aware_name(
2183-
context,
2205+
parent_name,
21842206
"Variant",
21852207
variant_index,
21862208
None,
@@ -2206,11 +2228,8 @@ impl SchemaAnalyzer {
22062228
}
22072229
// For other complex types, create an inline type
22082230
_ => {
2209-
let inline_type_name = format!(
2210-
"{}Variant{}",
2211-
parent_name.unwrap_or("Inline"),
2212-
variant_index + 1
2213-
);
2231+
let inline_type_name =
2232+
format!("{}Variant{}", parent_name, variant_index + 1);
22142233
self.add_inline_schema(
22152234
&inline_type_name,
22162235
variant_schema,
@@ -2246,12 +2265,27 @@ impl SchemaAnalyzer {
22462265
fn analyze_untagged_oneof_union(
22472266
&mut self,
22482267
one_of_schemas: &[Schema],
2249-
parent_name: Option<&str>,
2268+
parent_name: &str,
22502269
dependencies: &mut HashSet<String>,
22512270
) -> Result<SchemaType> {
2271+
// Drop {"type": "null"} variants. They mean "may be null" and are surfaced
2272+
// as Option<T> at the property level — including them here produces a junk
2273+
// `SerdeJsonValue(serde_json::Value)` variant.
2274+
let filtered: Vec<&Schema> = one_of_schemas
2275+
.iter()
2276+
.filter(|s| !matches!(s.schema_type(), Some(OpenApiSchemaType::Null)))
2277+
.collect();
2278+
2279+
// If filtering leaves a single variant, return its analyzed type directly.
2280+
if filtered.len() == 1 {
2281+
return self
2282+
.analyze_schema_value(filtered[0], parent_name)
2283+
.map(|a| a.schema_type);
2284+
}
2285+
22522286
let mut union_variants = Vec::new();
22532287

2254-
for (variant_index, variant_schema) in one_of_schemas.iter().enumerate() {
2288+
for (variant_index, variant_schema) in filtered.iter().copied().enumerate() {
22552289
// First check if it's a reference or recursive reference
22562290
if let Some(ref_str) = variant_schema.reference() {
22572291
if let Some(schema_name) = self.extract_schema_name(ref_str) {
@@ -2279,9 +2313,8 @@ impl SchemaAnalyzer {
22792313
});
22802314
} else {
22812315
// Handle inline schemas by creating type aliases or using primitive types directly
2282-
let context = parent_name.unwrap_or("Union");
22832316
let inline_name = self.generate_context_aware_name(
2284-
context,
2317+
parent_name,
22852318
"InlineVariant",
22862319
variant_index,
22872320
Some(variant_schema),
@@ -2340,9 +2373,8 @@ impl SchemaAnalyzer {
23402373
}
23412374
_ => {
23422375
// For deeper nesting, create an inline type
2343-
let context = parent_name.unwrap_or("Inline");
23442376
let inline_type_name = self.generate_context_aware_name(
2345-
context,
2377+
parent_name,
23462378
"Variant",
23472379
variant_index,
23482380
None,
@@ -2361,9 +2393,8 @@ impl SchemaAnalyzer {
23612393
}
23622394
_ => {
23632395
// For other array types, create an inline type
2364-
let context = parent_name.unwrap_or("Inline");
23652396
let inline_type_name = self.generate_context_aware_name(
2366-
context,
2397+
parent_name,
23672398
"Variant",
23682399
variant_index,
23692400
None,
@@ -2389,9 +2420,8 @@ impl SchemaAnalyzer {
23892420
}
23902421
// For other complex types, create an inline type
23912422
_ => {
2392-
let context = parent_name.unwrap_or("Inline");
23932423
let inline_type_name = self.generate_context_aware_name(
2394-
context,
2424+
parent_name,
23952425
"Variant",
23962426
variant_index,
23972427
None,
@@ -3122,7 +3152,12 @@ impl SchemaAnalyzer {
31223152
// Check if this is a discriminated union
31233153
if let Some(disc) = discriminator {
31243154
// This is a discriminated anyOf union, analyze it the same way as oneOf
3125-
return self.analyze_oneof_union(any_of_schemas, Some(disc), None, dependencies);
3155+
return self.analyze_oneof_union(
3156+
any_of_schemas,
3157+
Some(disc),
3158+
context_name,
3159+
dependencies,
3160+
);
31263161
}
31273162

31283163
// Auto-detect implicit discriminator from const fields across all variants
@@ -3134,7 +3169,7 @@ impl SchemaAnalyzer {
31343169
mapping: None,
31353170
extra: BTreeMap::new(),
31363171
}),
3137-
None,
3172+
context_name,
31383173
dependencies,
31393174
);
31403175
}

src/generator.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,10 @@ impl CodeGenerator {
11291129
) {
11301130
let type_ident = format_ident!("{}", variant.target);
11311131
quote! { #type_ident }
1132+
} else if variant.target == "serde_json::Value" {
1133+
// The target is a fully-qualified path; emit it as a path so
1134+
// it doesn't get mangled into a phantom `SerdeJsonValue` ident.
1135+
quote! { serde_json::Value }
11321136
} else if variant.target.starts_with("Vec<") && variant.target.ends_with(">") {
11331137
// Handle Vec types by parsing the inner type
11341138
let inner = &variant.target[4..variant.target.len() - 1];
@@ -1960,6 +1964,7 @@ impl CodeGenerator {
19601964
"u8" | "u16" | "u32" | "u64" | "u128" => return "UnsignedInteger".to_string(),
19611965
"f32" | "f64" => return "Number".to_string(),
19621966
"String" => return "String".to_string(),
1967+
"serde_json::Value" => return "Value".to_string(),
19631968
_ => {}
19641969
}
19651970

src/openapi.rs

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -288,29 +288,32 @@ impl Schema {
288288
}
289289
}
290290

291-
/// Check if this appears to be a nullable pattern (anyOf with null)
291+
/// Check if this appears to be a nullable pattern (anyOf or oneOf with null)
292292
pub fn is_nullable_pattern(&self) -> bool {
293-
match self {
294-
Schema::AnyOf { any_of, .. } => {
295-
any_of.len() == 2
296-
&& any_of
297-
.iter()
298-
.any(|s| matches!(s.schema_type(), Some(SchemaType::Null)))
299-
}
300-
_ => false,
301-
}
293+
let variants = match self {
294+
Schema::AnyOf { any_of, .. } => any_of,
295+
Schema::OneOf { one_of, .. } => one_of,
296+
_ => return false,
297+
};
298+
variants.len() == 2
299+
&& variants
300+
.iter()
301+
.any(|s| matches!(s.schema_type(), Some(SchemaType::Null)))
302302
}
303303

304304
/// Get the non-null variant from a nullable pattern
305305
pub fn non_null_variant(&self) -> Option<&Schema> {
306-
if self.is_nullable_pattern() {
307-
if let Schema::AnyOf { any_of, .. } = self {
308-
return any_of
309-
.iter()
310-
.find(|s| !matches!(s.schema_type(), Some(SchemaType::Null)));
311-
}
306+
if !self.is_nullable_pattern() {
307+
return None;
312308
}
313-
None
309+
let variants = match self {
310+
Schema::AnyOf { any_of, .. } => any_of,
311+
Schema::OneOf { one_of, .. } => one_of,
312+
_ => return None,
313+
};
314+
variants
315+
.iter()
316+
.find(|s| !matches!(s.schema_type(), Some(SchemaType::Null)))
314317
}
315318

316319
/// Infer schema type from structure if not explicitly set

tests/oneof_untagged_tests.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,11 +345,58 @@ mod tests {
345345
let generator = openapi_to_rust::CodeGenerator::new(Default::default());
346346
let types_content = generator.generate(&mut analysis).unwrap();
347347

348-
// Could be handled as Option<String> or as a union with null variant
348+
// The non-null variant should reduce to String — directly as Option<String>,
349+
// through a type alias (`pub type X = String;`), or as an untagged enum that
350+
// contains a String variant. The {"type": "null"} branch must not produce a
351+
// `serde_json::Value` / `SerdeJsonValue` variant.
349352
assert!(
350353
types_content.contains("Option<String>")
354+
|| types_content.contains("= String;")
351355
|| types_content.contains("#[serde(untagged)]"),
352-
"Should handle nullable oneOf appropriately"
356+
"Should handle nullable oneOf appropriately, got:\n{types_content}"
357+
);
358+
assert!(
359+
!types_content.contains("SerdeJsonValue"),
360+
"null variant must not become a SerdeJsonValue variant, got:\n{types_content}"
361+
);
362+
}
363+
364+
// Regression for https://github.com/gpu-cli/openapi-to-rust/issues/7:
365+
// {"type": "null"} inside oneOf with 3+ variants must be filtered out, not
366+
// emitted as a phantom `SerdeJsonValue(SerdeJsonValue)` variant.
367+
#[test]
368+
fn test_oneof_null_among_multiple_variants_is_filtered() {
369+
let spec_json = minimal_spec(json!({
370+
"NullOneOfMixed": {
371+
"type": "object",
372+
"properties": {
373+
"value": {
374+
"oneOf": [
375+
{"type": "null"},
376+
{"type": "string"},
377+
{"type": "integer"}
378+
]
379+
}
380+
}
381+
}
382+
}));
383+
384+
let mut analyzer = openapi_to_rust::SchemaAnalyzer::new(spec_json).unwrap();
385+
let mut analysis = analyzer.analyze().unwrap();
386+
let generator = openapi_to_rust::CodeGenerator::new(Default::default());
387+
let types_content = generator.generate(&mut analysis).unwrap();
388+
389+
assert!(
390+
!types_content.contains("SerdeJsonValue"),
391+
"null variant leaked through as SerdeJsonValue, got:\n{types_content}"
392+
);
393+
assert!(
394+
types_content.contains("String(String)"),
395+
"expected String variant in untagged enum, got:\n{types_content}"
396+
);
397+
assert!(
398+
types_content.contains("Integer(i64)"),
399+
"expected Integer variant in untagged enum, got:\n{types_content}"
353400
);
354401
}
355402

0 commit comments

Comments
 (0)