Fix enum-variant class names that collide with a same-named type - #151
Merged
Conversation
DenisovAV
force-pushed
the
pr-fix-enum-collision
branch
from
August 8, 2026 14:23
fb0d4ec to
904e1ec
Compare
chavic
approved these changes
Aug 22, 2026
chavic
left a comment
Contributor
There was a problem hiding this comment.
Nice work, thank you! One note for a possible follow-up, not for this PR: the reserved set does not contain the class names of other variants. Because of this, two variant classes from different enums can still collide, for example FieldA + Cond and Field + ACond. This case is rare, and the loop can cover it later.
Contributor
|
The one failed check is the flaky |
A data-carrying enum variant's Dart class is named `{Variant}{Enum}`. When that
concatenation equals a real top-level type name the file ends up with two
same-named declarations — a compile error. Real case: enum `Condition` variant
`Field` holding a record `FieldCondition` produces variant class `FieldCondition`,
clashing with the record; likewise a `Named` variant over a `NamedVector` type.
Collect the names of every type that emits a top-level Dart class — records,
enums, objects, and callback interfaces — and, only on collision, suffix the
variant class with `Variant`, looping (`Variant`, `Variant2`, ...) so a type
literally named `{base}Variant` can't reintroduce the duplicate. The name is
produced by one shared closure used both where the class is defined and where the
FfiConverter's `read` dispatches to it, so the two never drift.
Adds an `enum_variant_collision` fixture covering: a record collision that loops
past a second-order clash, a variant colliding with an enum type, and a
non-colliding control. It fails to compile without the fix and round-trips with it.
DenisovAV
force-pushed
the
pr-fix-enum-collision
branch
from
August 23, 2026 00:33
904e1ec to
32e90aa
Compare
Contributor
|
@DenisovAV, thanks for the fix |
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
A data-carrying enum variant is rendered as a Dart class named
{Variant}{Enum}. When that concatenation equals a real top-level type name, the generated file contains two declarations with the same name — a Dart compile error.Concrete case (reduced from a real crate):
The
Fieldvariant ofConditionbecomesclass FieldCondition extends Condition, clashing with theFieldConditionrecord.dart analyzethen reportsduplicate_definition(and the FfiConverter'sreaddispatch calls.readon the wrong class). The same happens for aNamedvariant over aNamedVectortype, etc.Fix
Collect the top-level type names (records / enums / objects) and, only on collision, suffix the variant class with
Variant(FieldConditionVariant). Non-colliding variants are unchanged, so existing generated APIs are untouched.The name is produced by a single shared closure used both where the variant class is defined and where the enum's FfiConverter
readdispatches to it — the two computed the name independently before, so fixing only the definition site left a dangling.readreference.Test
Adds an
enum_variant_collisionfixture reproducing the clash (Condition::Field+ recordFieldCondition). It fails to compile without the fix (duplicate class) and round-trips with it.cargo test -p enum_variant_collisionis green.Note
Includes a small
chorecommit dropping a redundant&in a fewformat!/println!args — stable clippy 1.97+ addedredundant reference in format! argument, which failscargo clippy -- -D warningson the currentTestingCI for pre-existing code. Needed for green CI here; harmless to drop on rebase if it lands elsewhere first.