Disambiguate duplicate enum variant names instead of panicking#1036
Open
danieleades wants to merge 1 commit into
Open
Disambiguate duplicate enum variant names instead of panicking#1036danieleades wants to merge 1 commit into
danieleades wants to merge 1 commit into
Conversation
When enum values sanitize to identical identifiers (e.g. "=", ">", "≥"), generation panicked. Instead, disambiguate deterministically: in variant order, the first variant with a given name keeps it and each subsequent duplicate gets the smallest numeric suffix (starting at 2) that doesn't collide with any other variant name. Serialized names are unaffected since serde renames and the Display/FromStr impls are generated from the raw variant names. Fixes oxidecomputer#948
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Schemas whose enum values sanitize to identical Rust identifiers cause a panic during generation:
Variant naming makes two attempts (plain sanitization, then replacing non-identifier characters with
X), and if names still collide — as with the comparison-operator enum above, where=,>,<,≥,≤, and≠all reduce toX— it panics.Fixes #948
Fix
Replace the panic with a final, deterministic disambiguation pass: 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 (guarding against, say, a pre-existing literal
X2variant). Three variants that all sanitize toXbecomeX,X2,X3.Serialization is unaffected: serde renames and the
Display/FromStrimpls are all generated from the variant's raw name, so suffixed variants round-trip through the original strings. The confusing duplicate-counting bookkeeping that fed the panic message is removed along the way.Trade-offs
Schemas that previously generated successfully produce identical output; the new pass only runs where generation used to panic, so the suffixed names introduce no compatibility concern.
Tests
typify/tests/schemas/colliding-variant-names.{json,rs}covering the same schema end to end (including trybuild compilation).