use inferred union variant names in more places

arlyon · arlyon · commit ac77c4bfa25d · 2025-09-15T16:54:52.000+01:00
diff --git a/typify-impl/src/enums.rs b/typify-impl/src/enums.rs
@@ -1,6 +1,6 @@
 // Copyright 2025 Oxide Computer Company
 
-use std::collections::{BTreeMap, BTreeSet, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 
 use heck::{ToKebabCase, ToPascalCase};
 use proc_macro2::TokenStream;
@@ -609,33 +609,52 @@ impl TypeSpace {
             // account for types such as Uuid whose names come from outside of
             // the schema... but you can't win them all.
             .map(schema_is_named)
-            .collect::<Option<Vec<_>>>()
-            // Prune the common prefixes from all variant names. If this
-            // results in any of them being empty, we don't use these names.
-            .and_then(|variant_names| {
-                let common_prefix = variant_names
-                    .iter()
-                    .cloned()
-                    .reduce(|a, b| get_common_prefix(&a, &b))
-                    .unwrap();
-                variant_names
-                    .into_iter()
-                    .map(|var_name| {
-                        let var_name = &var_name[common_prefix.len()..];
-                        if var_name.is_empty() {
-                            None
-                        } else {
-                            Some(var_name.to_string())
-                        }
-                    })
-                    .collect::<Option<Vec<_>>>()
+            .collect::<Vec<_>>();
+
+        let common_prefix = variant_names
+            .iter()
+            .cloned()
+            .filter_map(|v| v)
+            .reduce(|a, b| get_common_prefix(&a, &b))
+            .unwrap_or_else(String::new);
+
+        // Store seen, un-stripped names here. Any item for which their
+        // name appears more than once, ALL the items with that clashing
+        // name will be converted to the `VariantN` scheme.
+        let duplicate_names = variant_names
+            .iter()
+            .cloned()
+            .filter_map(|v| v)
+            .fold(HashMap::new(), |mut counter: HashMap<_, u8>, name| {
+                *counter.entry(name).or_default() += 1;
+                counter
             })
-            // Fall back to `VariantN` naming.
-            .unwrap_or_else(|| {
-                (0..subschemas.len())
-                    .map(|idx| format!("Variant{}", idx))
-                    .collect()
-            });
+            .into_iter()
+            .filter_map(|(name, count)| if count > 1 { Some(name) } else { None })
+            .collect::<HashSet<_>>();
+
+        // Prune the common prefixes from all variant names. If this
+        // results in any of them being empty, we don't use these names.
+        //
+        // Fall back to VariantN if the variant name cannot be derived from
+        // the schema.
+        let variant_names = variant_names
+            .into_iter()
+            .enumerate()
+            .map(|(idx, var_name)| {
+                match var_name {
+                    None => None,
+                    var_name => {
+                        var_name.filter(|var_name| !duplicate_names.contains(var_name.as_str()))
+                    }
+                }
+                .map(|var_name| var_name[common_prefix.len()..].to_string())
+                .filter(|var_name| !var_name.is_empty())
+                .map(|var_name| var_name.to_string())
+                // Fall back to `VariantN` naming.
+                .unwrap_or_else(|| format!("Variant{}", idx))
+            })
+            .collect::<Vec<_>>();
 
         // Gather the variant details along with its name.
         let variant_details = subschemas
diff --git a/typify-impl/src/util.rs b/typify-impl/src/util.rs
@@ -622,6 +622,20 @@ pub(crate) fn schema_is_named(schema: &Schema) -> Option<String> {
             extensions: _,
         }) => singleton_subschema(subschemas).and_then(schema_is_named),
 
+        // Best-effort fallback for things with raw types that can be easily inferred
+        Schema::Object(SchemaObject {
+            instance_type: Some(SingleOrVec::Single(single)),
+            ..
+        }) => match **single {
+            InstanceType::Boolean => Some("Bool".to_string()),
+            InstanceType::Integer => Some("Integer".to_string()),
+            InstanceType::Number => Some("Number".to_string()),
+            InstanceType::String => Some("String".to_string()),
+            InstanceType::Array => Some("Array".to_string()),
+            InstanceType::Object => Some("Object".to_string()),
+            InstanceType::Null => Some("Null".to_string()),
+        },
+
         _ => None,
     }?;
 
diff --git a/typify/tests/schemas/rust-collisions.rs b/typify/tests/schemas/rust-collisions.rs
@@ -383,7 +383,7 @@ impl ::std::convert::TryFrom<::std::string::String> for FormatCollision {
 #[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
 #[serde(untagged)]
 pub enum KeywordFieldsEnum {
-    Variant0 {
+    Object {
         #[serde(rename = "impl")]
         impl_: ::std::string::String,
         #[serde(rename = "match")]
@@ -393,7 +393,7 @@ pub enum KeywordFieldsEnum {
         #[serde(rename = "type")]
         type_: ::std::string::String,
     },
-    Variant1([::std::string::String; 2usize]),
+    Array([::std::string::String; 2usize]),
 }
 impl ::std::convert::From<&Self> for KeywordFieldsEnum {
     fn from(value: &KeywordFieldsEnum) -> Self {
@@ -402,7 +402,7 @@ impl ::std::convert::From<&Self> for KeywordFieldsEnum {
 }
 impl ::std::convert::From<[::std::string::String; 2usize]> for KeywordFieldsEnum {
     fn from(value: [::std::string::String; 2usize]) -> Self {
-        Self::Variant1(value)
+        Self::Array(value)
     }
 }
 #[doc = "`MapOfKeywords`"]
diff --git a/typify/tests/schemas/various-enums.json b/typify/tests/schemas/various-enums.json
@@ -26,6 +26,47 @@
         }
       ]
     },
+    "one-of-raw-type": {
+      "oneOf": [
+        {
+          "type": "string"
+        },
+        {
+          "type": "integer"
+        }
+      ]
+    },
+    "one-of-missing-title": {
+      "type": "object",
+      "oneOf": [
+        {
+          "title": "A",
+          "properties": {
+            "foo": {
+              "type": "string"
+            }
+          }
+        },
+        {
+          "title": "B",
+          "properties": {
+            "bar": {
+              "type": "integer"
+            }
+          }
+        },
+        {
+          "properties": {
+            "bar": {
+              "type": "integer"
+            },
+            "baz": {
+              "type": "integer"
+            }
+          }
+        }        
+      ]
+    },
     "IpNet": {
       "$comment": "we want to see *nice* variant names in the output",
       "oneOf": [
diff --git a/typify/tests/schemas/various-enums.rs b/typify/tests/schemas/various-enums.rs
@@ -863,7 +863,7 @@ impl<'de> ::serde::Deserialize<'de> for Ipv6Net {
 pub enum JankNames {
     Variant0(::std::string::String),
     Variant1(::std::collections::HashMap<::std::string::String, ::std::string::String>),
-    Variant2(::std::collections::HashMap<::std::string::String, i64>),
+    Object(::std::collections::HashMap<::std::string::String, i64>),
 }
 impl ::std::convert::From<&Self> for JankNames {
     fn from(value: &JankNames) -> Self {
@@ -881,7 +881,7 @@ impl ::std::convert::From<::std::collections::HashMap<::std::string::String, ::s
 }
 impl ::std::convert::From<::std::collections::HashMap<::std::string::String, i64>> for JankNames {
     fn from(value: ::std::collections::HashMap<::std::string::String, i64>) -> Self {
-        Self::Variant2(value)
+        Self::Object(value)
     }
 }
 #[doc = "`Never`"]
@@ -1097,6 +1097,108 @@ impl ::std::convert::TryFrom<::std::string::String> for NullStringEnumWithUnknow
         value.parse()
     }
 }
+#[doc = "`OneOfMissingTitle`"]
+#[doc = r""]
+#[doc = r" <details><summary>JSON schema</summary>"]
+#[doc = r""]
+#[doc = r" ```json"]
+#[doc = "{"]
+#[doc = "  \"type\": \"object\","]
+#[doc = "  \"oneOf\": ["]
+#[doc = "    {"]
+#[doc = "      \"title\": \"A\","]
+#[doc = "      \"properties\": {"]
+#[doc = "        \"foo\": {"]
+#[doc = "          \"type\": \"string\""]
+#[doc = "        }"]
+#[doc = "      }"]
+#[doc = "    },"]
+#[doc = "    {"]
+#[doc = "      \"title\": \"B\","]
+#[doc = "      \"properties\": {"]
+#[doc = "        \"bar\": {"]
+#[doc = "          \"type\": \"integer\""]
+#[doc = "        }"]
+#[doc = "      }"]
+#[doc = "    },"]
+#[doc = "    {"]
+#[doc = "      \"properties\": {"]
+#[doc = "        \"bar\": {"]
+#[doc = "          \"type\": \"integer\""]
+#[doc = "        },"]
+#[doc = "        \"baz\": {"]
+#[doc = "          \"type\": \"integer\""]
+#[doc = "        }"]
+#[doc = "      }"]
+#[doc = "    }"]
+#[doc = "  ]"]
+#[doc = "}"]
+#[doc = r" ```"]
+#[doc = r" </details>"]
+#[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
+#[serde(untagged)]
+pub enum OneOfMissingTitle {
+    A {
+        #[serde(default, skip_serializing_if = "::std::option::Option::is_none")]
+        foo: ::std::option::Option<::std::string::String>,
+    },
+    B {
+        #[serde(default, skip_serializing_if = "::std::option::Option::is_none")]
+        bar: ::std::option::Option<i64>,
+    },
+    Variant2 {
+        #[serde(default, skip_serializing_if = "::std::option::Option::is_none")]
+        bar: ::std::option::Option<i64>,
+        #[serde(default, skip_serializing_if = "::std::option::Option::is_none")]
+        baz: ::std::option::Option<i64>,
+    },
+}
+impl ::std::convert::From<&Self> for OneOfMissingTitle {
+    fn from(value: &OneOfMissingTitle) -> Self {
+        value.clone()
+    }
+}
+#[doc = "`OneOfRawType`"]
+#[doc = r""]
+#[doc = r" <details><summary>JSON schema</summary>"]
+#[doc = r""]
+#[doc = r" ```json"]
+#[doc = "{"]
+#[doc = "  \"oneOf\": ["]
+#[doc = "    {"]
+#[doc = "      \"type\": \"string\""]
+#[doc = "    },"]
+#[doc = "    {"]
+#[doc = "      \"type\": \"integer\""]
+#[doc = "    }"]
+#[doc = "  ]"]
+#[doc = "}"]
+#[doc = r" ```"]
+#[doc = r" </details>"]
+#[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
+#[serde(untagged)]
+pub enum OneOfRawType {
+    String(::std::string::String),
+    Integer(i64),
+}
+impl ::std::convert::From<&Self> for OneOfRawType {
+    fn from(value: &OneOfRawType) -> Self {
+        value.clone()
+    }
+}
+impl ::std::fmt::Display for OneOfRawType {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+        match self {
+            Self::String(x) => x.fmt(f),
+            Self::Integer(x) => x.fmt(f),
+        }
+    }
+}
+impl ::std::convert::From<i64> for OneOfRawType {
+    fn from(value: i64) -> Self {
+        Self::Integer(value)
+    }
+}
 #[doc = "`OneOfTypes`"]
 #[doc = r""]
 #[doc = r" <details><summary>JSON schema</summary>"]
@@ -1486,8 +1588,8 @@ impl ::std::fmt::Display for ReferenceDef {
 #[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
 #[serde(untagged)]
 pub enum References {
-    Variant0(::std::vec::Vec<::std::string::String>),
-    Variant1(::std::collections::HashMap<::std::string::String, ReferencesVariant1Value>),
+    Array(::std::vec::Vec<::std::string::String>),
+    Object(::std::collections::HashMap<::std::string::String, ReferencesObjectValue>),
 }
 impl ::std::convert::From<&Self> for References {
     fn from(value: &References) -> Self {
@@ -1496,21 +1598,19 @@ impl ::std::convert::From<&Self> for References {
 }
 impl ::std::convert::From<::std::vec::Vec<::std::string::String>> for References {
     fn from(value: ::std::vec::Vec<::std::string::String>) -> Self {
-        Self::Variant0(value)
+        Self::Array(value)
     }
 }
-impl
-    ::std::convert::From<
-        ::std::collections::HashMap<::std::string::String, ReferencesVariant1Value>,
-    > for References
+impl ::std::convert::From<::std::collections::HashMap<::std::string::String, ReferencesObjectValue>>
+    for References
 {
     fn from(
-        value: ::std::collections::HashMap<::std::string::String, ReferencesVariant1Value>,
+        value: ::std::collections::HashMap<::std::string::String, ReferencesObjectValue>,
     ) -> Self {
-        Self::Variant1(value)
+        Self::Object(value)
     }
 }
-#[doc = "`ReferencesVariant1Value`"]
+#[doc = "`ReferencesObjectValue`"]
 #[doc = r""]
 #[doc = r" <details><summary>JSON schema</summary>"]
 #[doc = r""]
@@ -1529,29 +1629,29 @@ impl
 #[doc = r" </details>"]
 #[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
 #[serde(untagged)]
-pub enum ReferencesVariant1Value {
+pub enum ReferencesObjectValue {
     StringVersion(StringVersion),
     ReferenceDef(ReferenceDef),
 }
-impl ::std::convert::From<&Self> for ReferencesVariant1Value {
-    fn from(value: &ReferencesVariant1Value) -> Self {
+impl ::std::convert::From<&Self> for ReferencesObjectValue {
+    fn from(value: &ReferencesObjectValue) -> Self {
         value.clone()
     }
 }
-impl ::std::fmt::Display for ReferencesVariant1Value {
+impl ::std::fmt::Display for ReferencesObjectValue {
     fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
         match self {
             Self::StringVersion(x) => x.fmt(f),
             Self::ReferenceDef(x) => x.fmt(f),
         }
     }
 }
-impl ::std::convert::From<StringVersion> for ReferencesVariant1Value {
+impl ::std::convert::From<StringVersion> for ReferencesObjectValue {
     fn from(value: StringVersion) -> Self {
         Self::StringVersion(value)
     }
 }
-impl ::std::convert::From<ReferenceDef> for ReferencesVariant1Value {
+impl ::std::convert::From<ReferenceDef> for ReferencesObjectValue {
     fn from(value: ReferenceDef) -> Self {
         Self::ReferenceDef(value)
     }
