Repository navigation
feat(renderer): add optional polymorphic From conversions - #323
Conversation
Bergmann89
left a comment
There was a problem hiding this comment.
Thanks, I like the feature!
I would prefer to implement it as its own render step instead of a renderer flag that is hooked into the four type renderers. The conversions do not depend on anything of these renderers, they only need the ComplexDataEnum. A separate step can match DataTypeVariant::Complex in render_type and walk ComplexData::Enum { type_, content_type } (including the content type for the unflattened case). It is then completely independent of the other steps, users add it with something like RenderStep::PolymorphicFrom to the pipeline, and RendererFlags::POLYMORPHIC_FROM is not needed anymore (which also avoids that RendererFlags::all() enables it).
Please also document in the step docs that payloads which only differ by an external type alias cannot be detected: the comparison uses the final identifiers, so a custom type Text with type Text = String next to a String variant gets two From impls for the same type and fails with E0119. The generator cannot see user aliases, so a note (use a newtype or do not use the step) is enough.
The remaining comments are about the tests.
dfe3186 to
403be06
Compare
Add an explicit PolymorphicFromRenderStep and expose it through RenderStep::PolymorphicFrom. Keep it independent of the type renderers so RendererFlags::all() does not opt in to conversions. Walk nested complex content to support flattened and content enums. Use the generated variant names, payload paths, boxing decisions and allocator configuration, skipping ambiguous payloads using conservative type keys. Document that external Rust aliases are opaque and may require a newtype or omitting the step to avoid conflicting From implementations. Cover explicit opt-in, independent step composition, allocator-aware boxing, aliases, recursive and nullable payloads, concrete bases, module layouts and Serde renderers. Keep fixtures focused on conversions and reuse alias and allocator coverage instead of duplicating snapshots.
403be06 to
6bc42eb
Compare
|
Thanks for the review, updated the PR and the implementation is cleaner now. |
Adds optional
From<Concrete>implementations for polymorphic choice enums so callers can construct them with.into(). SelectRenderStep::PolymorphicFromalongside dynamic-to-choice conversion. The step is absent from the default pipeline.Changes
PolymorphicFromRenderStep, including traversal of unflattened content enums. Type and serialization renderers need no conversion hooks, andRendererFlags::all()does not enable the feature.TextaliasingStringalongside aStringpayload can cause E0119. Use a newtype or omit the step in that case.Public API addition:
config::RenderStepgainsPolymorphicFrom; exhaustive matches must handle the new variant. The concretePolymorphicFromRenderStepis also exported. Default generated output is unchanged.Checklist
update-expectationsand reviewedcargo fmt,cargo clippyandcargo testpass (see CONTRIBUTING.md)