Disambiguate duplicate enum variant names instead of panicking

typify-impl/src/enums.rs

@@ -1237,6 +1237,47 @@ mod tests {
         }
     }
 
+    // See https://github.com/oxidecomputer/typify/issues/948
+    #[test]
+    fn test_variant_name_collisions() {
+        let schema_json = r##"
+        {
+            "type": "string",
+            "enum": ["=", ">", "<", "≥", ">=", "≤", "<=", "≠", "!="]
+        }
+        "##;
+
+        let schema: schemars::schema::Schema = serde_json::from_str(schema_json).unwrap();
+
+        let mut type_space = TypeSpace::default();
+        let (type_entry, _) = type_space
+            .convert_schema(Name::Required("ComparisonOperator".to_string()), &schema)
+            .unwrap();
+
+        let TypeEntryDetails::Enum(TypeEntryEnum { variants, .. }) = &type_entry.details else {
+            panic!("expected an enum");
+        };
+
+        // The raw names--used for serialization--are preserved, in order.
+        let raw_names = variants
+            .iter()
+            .map(|variant| variant.raw_name.as_str())
+            .collect::<Vec<_>>();
+        assert_eq!(raw_names, ["=", ">", "<", "≥", ">=", "≤", "<=", "≠", "!="]);
+
+        // The identifier names are disambiguated with deterministic numeric
+        // suffixes: the first variant with a given name keeps it and each
+        // subsequent duplicate gets the next available suffix.
+        let ident_names = variants
+            .iter()
+            .map(|variant| variant.ident_name.as_deref().unwrap())
+            .collect::<Vec<_>>();
+        assert_eq!(
+            ident_names,
+            ["X", "X2", "X3", "X4", "Xx", "X5", "Xx2", "X6", "Xx3"]
+        );
+    }
+
     #[test]
     fn test_maybe_option() {
         let subschemas = vec![

typify-impl/src/type_entry.rs

@@ -1,6 +1,6 @@
 // Copyright 2025 Oxide Computer Company
 
-use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::collections::{BTreeMap, BTreeSet};
 
 use proc_macro2::{Punct, Spacing, TokenStream, TokenTree};
 use quote::{format_ident, quote, ToTokens};
@@ -268,24 +268,44 @@ impl TypeEntryEnum {
             });
         }
 
-        // If variants still aren't unique, we fail: we'd rather not emit code
-        // that can't compile
+        // If variants still aren't unique, we disambiguate deterministically:
+        // proceeding in variant order, the first variant with a given name
+        // keeps it unchanged, and each subsequent duplicate gets the smallest
+        // numeric suffix (starting at 2) that doesn't collide with any other
+        // variant name. For example, three variants that all sanitize to `X`
+        // become `X`, `X2`, and `X3`. Serialized names are unaffected: serde
+        // renames and Display/FromStr impls are all generated from the
+        // variant's raw name.
         if !variants_unique(&variants) {
-            let mut counts = HashMap::new();
-            variants.iter().for_each(|variant| {
-                counts
-                    .entry(variant.ident_name.as_ref().unwrap())
-                    .and_modify(|xxx| *xxx += 1)
-                    .or_insert(0);
-            });
-            let dups = variants
+            // Names of all variants prior to disambiguation; a suffixed name
+            // must not collide with any of these (e.g. an actual `X2`
+            // variant that appears later in the list).
+            let original_names = variants
                 .iter()
-                .filter(|variant| *counts.get(variant.ident_name.as_ref().unwrap()).unwrap() > 0)
-                .map(|variant| variant.raw_name.as_str())
-                .collect::<Vec<_>>()
-                .join(",");
-            panic!("Failed to make unique variant names for [{}]", dups);
+                .map(|variant| variant.ident_name.as_ref().unwrap().clone())
+                .collect::<BTreeSet<_>>();
+
+            // Names assigned so far: first occurrences and suffixed names.
+            let mut assigned_names = BTreeSet::new();
+            for variant in variants.iter_mut() {
+                let name = variant.ident_name.as_ref().unwrap().clone();
+
+                // The first variant with a given name keeps it.
+                if assigned_names.insert(name.clone()) {
+                    continue;
+                }
+
+                let new_name = (2..)
+                    .map(|ii| format!("{}{}", name, ii))
+                    .find(|candidate| {
+                        !original_names.contains(candidate) && !assigned_names.contains(candidate)
+                    })
+                    .unwrap();
+                assigned_names.insert(new_name.clone());
+                variant.ident_name = Some(new_name);
+            }
         }
+        debug_assert!(variants_unique(&variants));
 
         let name = get_type_name(&type_name, metadata).unwrap();
         let rename = None;

typify/tests/schemas/colliding-variant-names.json

@@ -0,0 +1,18 @@
+{
+  "definitions": {
+    "comparison-operator": {
+      "type": "string",
+      "enum": [
+        "=",
+        ">",
+        "<",
+        "≥",
+        ">=",
+        "≤",
+        "<=",
+        "≠",
+        "!="
+      ]
+    }
+  }
+}

typify/tests/schemas/colliding-variant-names.rs

@@ -0,0 +1,135 @@
+#![deny(warnings)]
+#[doc = r" Error types."]
+pub mod error {
+    #[doc = r" Error from a `TryFrom` or `FromStr` implementation."]
+    pub struct ConversionError(::std::borrow::Cow<'static, str>);
+    impl ::std::error::Error for ConversionError {}
+    impl ::std::fmt::Display for ConversionError {
+        fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> Result<(), ::std::fmt::Error> {
+            ::std::fmt::Display::fmt(&self.0, f)
+        }
+    }
+    impl ::std::fmt::Debug for ConversionError {
+        fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> Result<(), ::std::fmt::Error> {
+            ::std::fmt::Debug::fmt(&self.0, f)
+        }
+    }
+    impl From<&'static str> for ConversionError {
+        fn from(value: &'static str) -> Self {
+            Self(value.into())
+        }
+    }
+    impl From<String> for ConversionError {
+        fn from(value: String) -> Self {
+            Self(value.into())
+        }
+    }
+}
+#[doc = "`ComparisonOperator`"]
+#[doc = r""]
+#[doc = r" <details><summary>JSON schema</summary>"]
+#[doc = r""]
+#[doc = r" ```json"]
+#[doc = "{"]
+#[doc = "  \"type\": \"string\","]
+#[doc = "  \"enum\": ["]
+#[doc = "    \"=\","]
+#[doc = "    \">\","]
+#[doc = "    \"<\","]
+#[doc = "    \"≥\","]
+#[doc = "    \">=\","]
+#[doc = "    \"≤\","]
+#[doc = "    \"<=\","]
+#[doc = "    \"≠\","]
+#[doc = "    \"!=\""]
+#[doc = "  ]"]
+#[doc = "}"]
+#[doc = r" ```"]
+#[doc = r" </details>"]
+#[derive(
+    :: serde :: Deserialize,
+    :: serde :: Serialize,
+    Clone,
+    Copy,
+    Debug,
+    Eq,
+    Hash,
+    Ord,
+    PartialEq,
+    PartialOrd,
+)]
+pub enum ComparisonOperator {
+    #[serde(rename = "=")]
+    X,
+    #[serde(rename = ">")]
+    X2,
+    #[serde(rename = "<")]
+    X3,
+    #[serde(rename = "≥")]
+    X4,
+    #[serde(rename = ">=")]
+    Xx,
+    #[serde(rename = "≤")]
+    X5,
+    #[serde(rename = "<=")]
+    Xx2,
+    #[serde(rename = "≠")]
+    X6,
+    #[serde(rename = "!=")]
+    Xx3,
+}
+impl ::std::fmt::Display for ComparisonOperator {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
+        match *self {
+            Self::X => f.write_str("="),
+            Self::X2 => f.write_str(">"),
+            Self::X3 => f.write_str("<"),
+            Self::X4 => f.write_str("≥"),
+            Self::Xx => f.write_str(">="),
+            Self::X5 => f.write_str("≤"),
+            Self::Xx2 => f.write_str("<="),
+            Self::X6 => f.write_str("≠"),
+            Self::Xx3 => f.write_str("!="),
+        }
+    }
+}
+impl ::std::str::FromStr for ComparisonOperator {
+    type Err = self::error::ConversionError;
+    fn from_str(value: &str) -> ::std::result::Result<Self, self::error::ConversionError> {
+        match value {
+            "=" => Ok(Self::X),
+            ">" => Ok(Self::X2),
+            "<" => Ok(Self::X3),
+            "≥" => Ok(Self::X4),
+            ">=" => Ok(Self::Xx),
+            "≤" => Ok(Self::X5),
+            "<=" => Ok(Self::Xx2),
+            "≠" => Ok(Self::X6),
+            "!=" => Ok(Self::Xx3),
+            _ => Err("invalid value".into()),
+        }
+    }
+}
+impl ::std::convert::TryFrom<&str> for ComparisonOperator {
+    type Error = self::error::ConversionError;
+    fn try_from(value: &str) -> ::std::result::Result<Self, self::error::ConversionError> {
+        value.parse()
+    }
+}
+impl ::std::convert::TryFrom<&::std::string::String> for ComparisonOperator {
+    type Error = self::error::ConversionError;
+    fn try_from(
+        value: &::std::string::String,
+    ) -> ::std::result::Result<Self, self::error::ConversionError> {
+        value.parse()
+    }
+}
+impl ::std::convert::TryFrom<::std::string::String> for ComparisonOperator {
+    type Error = self::error::ConversionError;
+    fn try_from(
+        value: ::std::string::String,
+    ) -> ::std::result::Result<Self, self::error::ConversionError> {
+        value.parse()
+    }
+}
+fn main() {}