Skip to content

Wiebren fix/rust discriminated union child fields - #25133

Closed
wing328 wants to merge 11 commits into
masterfrom
wiebren-fix/rust-discriminated-union-child-fields
Closed

wing328 wants to merge 11 commits into
masterfrom
wiebren-fix/rust-discriminated-union-child-fields

Conversation

@wing328

@wing328 wing328 commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Summary by cubic

Fixes the Rust generator silently dropping child-specific fields in discriminated unions. Mapped children now generate boxed newtype variants like ObjectExists(Box<models::ObjectExists>) instead of inline structs built from the parent's vars.

Behavior

  • The child's discriminator property defaults and skips serialization while unset, so serde no longer fails on the consumed tag key or writes the tag twice.
  • Standalone use of child models is unchanged: constructors and field access keep working as before.
  • Unions whose mapping names the base itself or uses an enum-typed discriminator keep the old inline variants, since there is no struct to wrap or the tag would be duplicated.
  • Adds a rust reqwest sample and test spec covering nullability, enum-typed tags, and self-mapping.

Written for commit 41564e5. Summary will update on new commits.

Review in cubic

wiebren and others added 11 commits September 8, 2026 10:36
A schema with a discriminator and mapped children generated a serde
internally-tagged enum whose variants were inline structs built from the
parent's vars - every child-specific field was silently dropped, and with
duplicate mappings the variants even mixed vars across models. Wrap the
mapped model in a newtype variant instead (boxed, like the oneOf variants),
named by the uniquified modelName while wrapping the model's real classname.

serde's internally-tagged deserialization consumes the tag key, so a
wrapped child's own required discriminator property would fail with
"missing field": RustClientCodegen now marks mapped children's
discriminator properties (the rust var context never set isDiscriminator)
and the template defaults them, skipping them back out while empty so the
tag stays the only occurrence on the wire. Optional discriminator
properties already tolerate absence through Option and are untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz
Review found the serde-attribute approach broke on a required nullable
discriminator (String::is_empty on an Option<String> does not compile)
and could duplicate the tag when a caller populated the child's field.
Remove the property from mapped children instead, exactly as
postProcessModels already removes it from the discriminating parent: the
variant name carries the type information, deserialization never misses a
consumed tag key, and the tag is structurally the only occurrence on the
wire.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz
…de tag

Removing the property from mapped children went too far: getMappedModels()
covers every allOf descendant, and those models are also returned and
accepted standalone (create_bar returns Bar, create_foo takes Foo), so
they lost a required field and their new() signature changed.

Keep the property declared and mark it instead, so the template defaults
it - the internally-tagged union consumes the key before the child
deserializes - and skips it back out while unset, keeping the tag the only
occurrence on the wire. The skip predicate follows the type: Option::is_none
for a nullable discriminator, String::is_empty for a non-nullable string,
which is what the first attempt got wrong. Standalone use is unchanged:
the caller sets and reads the discriminator as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz
serde's internally tagged enum consumes the tag before the wrapped child
deserializes and writes it again next to the child's own discriminator
field. The previous attempt defaulted and skipped that field on the
child, which broke enum-typed discriminators (E0308 on
`skip_serializing_if = "String::is_empty"`) and still wrote the tag
twice for a child built with `new()`.

The newtype unions now get a generated Serialize/Deserialize that goes
through serde_json::Value: the child reads its own discriminator from
the payload, and the variant's tag is written exactly once. Children are
back to master (no attribute changes, no Java marking).

A union whose mapping names the base itself has no struct to wrap and
keeps master's inline variants.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…child"

This reverts commit 881bd64. The
hand-written Serialize/Deserialize through serde_json::Value tied the
unions to JSON, doubled the (de)serialization work and did not round-trip
nested unions. Unions derive #[serde(tag)] again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An enum-typed discriminator on a mapped child generated
`skip_serializing_if = "String::is_empty"` on an enum field (E0308): it
now gets `default` only. A union whose mapping names the base itself
would wrap itself (E0275 once it is serialized): it keeps master's
inline variants and its children are left untouched.

Also mark only `vars` (the only list the template reads) and drop the
unreachable classname fallback; shorter comments.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With an enum-typed discriminator (inline or a $ref to an enum schema) the
newtype variant writes the tag twice on the way out, and the child's own
copy keeps the enum's default: {"petType":"Dog","bark":true} came back as
{"petType":"Dog","petType":"Cat","bark":true}. Flag those unions like the
self-mapped ones, so they keep master's inline variants and their children
are left unmarked, identical to master.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ted-union-child-fields

# Conflicts:
#	modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustClientCodegenTest.java
@wing328 wing328 closed this Oct 5, 2026
@wing328
wing328 deleted the wiebren-fix/rust-discriminated-union-child-fields branch October 5, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants