[rust] fix: discriminated-union variants drop every child-specific field by wiebren · Pull Request #24897 · OpenAPITools/openapi-generator

wiebren · 2026-09-08T08:36:47Z

A schema with a discriminator and mapped children generates an internally tagged enum whose variants are inline structs built from the parent's vars, so every child-specific field is dropped (ObjectExists { message } loses identifier). The test-duplicates sample even mixed vars across models.

Fix

Newtype variants wrapping the mapped models, boxed like the oneOf variants, with the derived #[serde(tag)] kept:

ObjectExists(Box<models::ObjectExists>),

The variant is named by the uniquified modelName and wraps the model's real classname.

serde consumes the tag before the child deserializes, so a mapped child's discriminator field is marked and defaulted, and skipped while unset: Option::is_none if nullable, String::is_empty if a string. Standalone use of the child is unchanged.

Two kinds of union keep master's inline variants, children untouched:

a mapping that names the base itself would wrap itself (E0275);
an enum-typed discriminator (inline or $ref) would be written twice, the child's copy holding the enum's default ("petType":"Dog","petType":"Cat").

Verification

RustClientCodegenTest#testDiscriminatedUnionKeepsChildFields: string, nullable, inline-enum, enum-ref and self-mapped discriminators.
cargo check of the fixture; cargo run wire → union → wire keeps child fields and writes one tag. Enum-discriminated output is byte-identical to master.
Samples: oneOf and test-duplicates (hyper + reqwest) unions become newtypes; oneOf children gain the @type attribute. All build.

Known gaps

A child built with new() and wrapped in the union writes the tag twice ({"type":"ObjectExists","type":"ObjectExists",...}).
Self-mapped and enum-discriminated unions still drop their children's fields, as on master.

PR checklist

Read the contribution guidelines.
Built the project and updated samples (./bin/generate-samples.sh ./bin/configs/rust-*).
Technical committee (Rust): @frol @farcaller @richardwhiuk @paladinzh @jacob-pro @dsteeley

Summary by cubic

Fixes Rust client generation for schemas with a discriminator and mapped children so enum variants keep their child-specific fields. Variants were inline structs built from the parent's vars, silently dropping every field the child added; they now wrap the mapped model in a boxed newtype variant.

Behavior

Duplicate mapping entries sharing one schema still get distinct variants, each wrapping the real model.
The child's discriminator property stays declared but is marked as a discriminator: the consumed serde tag key defaults instead of erroring and is skipped back out while unset (Option::is_none for nullable, String::is_empty for non-nullable strings), keeping the tag the only occurrence on the wire.
Self-mapped and enum-typed discriminators keep the inline variants, so those unions still drop their children's fields; a child built with new() still writes the tag twice through the union.

Testing

Adds the discriminated-union.yaml fixture and a regression test pinning the derived #[serde(tag)].

^{Written for commit daa9c0d. Summary will update on new commits.}

Generated with Claude Code

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

cubic-dev-ai

All reported issues were addressed across 26 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

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

wiebren · 2026-09-08T09:36:15Z

cubic's findings reshaped this for the better. The serde-attribute approach is gone: mapped children now have their discriminator property removed, exactly as postProcessModels already removes it from the discriminating parent. That resolves all three remarks at once:

the required-nullable discriminator P1 cannot arise (there is no field to attach a predicate to),
the duplicate-tag P2 cannot arise (the tag is structurally the only occurrence on serialization),
and the untagged-union remark reduces to the pre-existing semantics: an untagged oneOf variant matches by shape, and a discriminator property whose only role is the tag no longer participates - the same trade the parent models made when their property was removed. BarRef in the oneOf sample keeps matching everything its (all-optional) shape implies, which is the nature of untagged unions rather than something this PR changes.

Verified again end to end: the serde round-trip on the generated crate still passes (child fields populated, exactly one type key out, re-parse equal), all four affected sample crates - composed-oneof included this time - build clean, and RustClientCodegenTest pins the removal (no r#type field, new() without the parameter).

cubic-dev-ai

All reported issues were addressed across 51 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

…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

cubic-dev-ai

All reported issues were addressed across 52 files (changes from recent commits).

_{Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

wiebren · 2026-09-10T18:22:34Z

cubic is right on both mechanisms, and measuring them makes the picture worse than the review states. Answering here rather than in three sample files, since all four violations are one design question.

Reproduced against the exact shape the oneOf sample generates, where Bar is a mapped child of the internally tagged Entity (#[serde(tag = "@type")]), and is returned standalone by create_bar, and is a member of the untagged BarRefOrValue:

Entity::Bar(Bar::new(id, "Bar"))  -> {"@type":"Bar","id":"1","@type":"Bar"}
  read back by the same client    -> ERROR duplicate field `@type` at line 1 column 31
Entity::Bar(Bar{ at_type: "" })   -> {"@type":"Bar","id":"1"}
  read back                       -> Bar { id: "1", at_type: "", bar_prop_a: None }
standalone Bar                    -> {"id":"1","@type":"Bar"}
untagged BarRefOrValue            -> {"id":"1","@type":"Bar"}

Three things follow:

The duplicate key is the default path, not a corner case. Bar::new(id, at_type) takes the discriminator as a required constructor argument, so a caller doing the obvious thing always has it set. skip_serializing_if = "String::is_empty" only fires for a deliberately empty value, which nothing in the generated API leads you to.
The client cannot deserialize its own output. It is not a cosmetic duplicate: serde raises duplicate field @type, so a value the client just serialized fails to parse back.
Reading through the union never populates the field, exactly as the ObjD remark says — the tag is consumed before the struct deserializes, so at_type comes back "", or None for a nullable discriminator.

The round-trips I reported on the previous push missed all three because they constructed children with the discriminator left unset, which is the one shape that behaves. My mistake, and worth saying plainly.

On the suggested remedy. "Remove the discriminator property from mapped child models" is the rework this PR already tried and withdrew, on cubic's own reading in #24897 (comment): getMappedModels() covers every allOf descendant, so removing the property strips @type from standalone Bar, Foo, Pasta and Pizza, and from the untagged BarRefOrValue where it is an ordinary required field, and changes every new() signature with it. Both constraints hold at once — the child must own the field for standalone and untagged use, and must not emit it inside an internally tagged enum, which assumes the inner type does not own the tag. No combination of serde attributes on the current shape satisfies both, so this needs a structural answer rather than another attribute.

Two that work:

A. Variant structs, populated from the child. Go back to an inline struct per variant, but build it from the child's own vars minus the discriminator instead of from the parent's vars. That fixes the original defect this PR opened for — child-specific fields silently dropped — and every point above, and leaves standalone models untouched. Cost: a variant is not the model type, so a Bar cannot be moved into an Entity or borrowed back out. That is the ergonomics master already has, so nothing regresses; it just does not improve.

B. Keep the newtype variants and generate Serialize for tagged unions. Deserialize stays derived; serialization emits the tag exactly once whatever the child holds. Correct in all four directions and keeps the API the newtype variants were for. Larger template change.

I lean to B because the newtype variants are the point of the PR, but A is the smaller change and I am happy to write either — a preference from a rust maintainer would settle it faster than me guessing.

Either way the fixture gains a round-trip that builds a variant through new() with the discriminator set, which is what would have caught this.

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>

cubic-dev-ai

2 issues found across 26 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustClientCodegen.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustClientCodegen.java:347">
P2: The all-or-nothing `x-rust-newtype-variants` flag falls back to inline struct variants for the entire union as soon as *any* mapping names the base itself, so non-self mapped children in the same union are silently stripped of their child-specific fields — the exact bug this PR fixes. In the new fixture's `Creature` union (`mapping: {Creature: ...Creature, Bird: ...Bird}`), `selfMapped` is true, so `Bird` (allOf Creature + `wingspan`) is generated as an inline variant built from the parent's `vars` (`{{#vars}}` resolves to the union model's own `vars`, which after removing `kind` only contains `name`), dropping `wingspan`. Deserializing `{"kind":"Bird","wingspan":5}` then silently discards `wingspan` (serde ignores unknown fields), and the test added for this case only asserts `Self::Bird {` exists, never the field. Generate newtype variants for every mapping entry that is not the base, and inline only the self-mapped entry (variants can mix tuple and struct forms in one enum).</violation>
</file>

<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustClientCodegenTest.java">

<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustClientCodegenTest.java:314">
P3: The comment claims the tag is written once, but nothing in this test executes the serde path: `testDiscriminatedUnionKeepsChildFields` contains only `assertFileContains`/`assertFileNotContains` source-text checks and never serializes or deserializes a value. The previous design's failure mode was a runtime duplicate-tag bug (a `new()`-constructed boxed variant serialized to two `@type` keys and failed to read back), so the source assertions can all pass while the wire format is broken.</violation>
</file>

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

cubic-dev-ai · 2026-09-23T07:20:49Z

+                // struct to wrap, so that union keeps the inline variants built from the base's own vars.
+                boolean selfMapped = cm.discriminator.getMappedModels() != null && cm.discriminator.getMappedModels().stream()
+                        .anyMatch(m -> cm.name.equals(m.getSchemaName()) || cm.classname.equals(m.getModelName()));
+                cm.discriminator.getVendorExtensions().put("x-rust-newtype-variants", cm.oneOf.isEmpty() && !selfMapped);


P2: The all-or-nothing x-rust-newtype-variants flag falls back to inline struct variants for the entire union as soon as any mapping names the base itself, so non-self mapped children in the same union are silently stripped of their child-specific fields — the exact bug this PR fixes. In the new fixture's Creature union (mapping: {Creature: ...Creature, Bird: ...Bird}), selfMapped is true, so Bird (allOf Creature + wingspan) is generated as an inline variant built from the parent's vars ({{#vars}} resolves to the union model's own vars, which after removing kind only contains name), dropping wingspan. Deserializing {"kind":"Bird","wingspan":5} then silently discards wingspan (serde ignores unknown fields), and the test added for this case only asserts Self::Bird { exists, never the field. Generate newtype variants for every mapping entry that is not the base, and inline only the self-mapped entry (variants can mix tuple and struct forms in one enum).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustClientCodegen.java, line 347: <comment>The all-or-nothing `x-rust-newtype-variants` flag falls back to inline struct variants for the entire union as soon as *any* mapping names the base itself, so non-self mapped children in the same union are silently stripped of their child-specific fields — the exact bug this PR fixes. In the new fixture's `Creature` union (`mapping: {Creature: ...Creature, Bird: ...Bird}`), `selfMapped` is true, so `Bird` (allOf Creature + `wingspan`) is generated as an inline variant built from the parent's `vars` (`{{#vars}}` resolves to the union model's own `vars`, which after removing `kind` only contains `name`), dropping `wingspan`. Deserializing `{"kind":"Bird","wingspan":5}` then silently discards `wingspan` (serde ignores unknown fields), and the test added for this case only asserts `Self::Bird {` exists, never the field. Generate newtype variants for every mapping entry that is not the base, and inline only the self-mapped entry (variants can mix tuple and struct forms in one enum).</comment> <file context> @@ -384,6 +340,12 @@ public ModelsMap postProcessModels(ModelsMap objs) { + // struct to wrap, so that union keeps the inline variants built from the base's own vars. + boolean selfMapped = cm.discriminator.getMappedModels() != null && cm.discriminator.getMappedModels().stream() + .anyMatch(m -> cm.name.equals(m.getSchemaName()) || cm.classname.equals(m.getModelName())); + cm.discriminator.getVendorExtensions().put("x-rust-newtype-variants", cm.oneOf.isEmpty() && !selfMapped); + for (CodegenProperty cp : cm.vars) { </file context>

The hand-written serde was withdrawn in 1c0da30; unions derive #[serde(tag)] again. Still applies, deliberately: a self-mapped union keeps master's inline variants whole (e9ad7d4), so its other children still lose their fields, as on master. Wrapping the base in its own union is E0275, and mixing variant shapes per entry is left out of this PR. It is listed under Known gaps in the description.

cubic-dev-ai · 2026-09-23T07:20:49Z

+        TestUtils.assertFileContains(unionPath, "Self::ObjectExists(Default::default())");
+        TestUtils.assertFileNotContains(unionPath, "ObjectExists {");
+
+        // not serde's internally tagged enum: the child reads its own discriminator, the tag is written once


P3: The comment claims the tag is written once, but nothing in this test executes the serde path: testDiscriminatedUnionKeepsChildFields contains only assertFileContains/assertFileNotContains source-text checks and never serializes or deserializes a value. The previous design's failure mode was a runtime duplicate-tag bug (a new()-constructed boxed variant serialized to two @type keys and failed to read back), so the source assertions can all pass while the wire format is broken.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustClientCodegenTest.java, line 314: <comment>The comment claims the tag is written once, but nothing in this test executes the serde path: `testDiscriminatedUnionKeepsChildFields` contains only `assertFileContains`/`assertFileNotContains` source-text checks and never serializes or deserializes a value. The previous design's failure mode was a runtime duplicate-tag bug (a `new()`-constructed boxed variant serialized to two `@type` keys and failed to read back), so the source assertions can all pass while the wire format is broken.</comment> <file context> @@ -304,31 +304,34 @@ public void testDiscriminatedUnionKeepsChildFields() throws IOException { - TestUtils.assertFileContains(childPath, "pub identifier: String,"); - TestUtils.assertFileContains(childPath, - "pub fn new(r#type: String, message: String, identifier: String) -> ObjectExists {"); + // not serde's internally tagged enum: the child reads its own discriminator, the tag is written once + TestUtils.assertFileNotContains(unionPath, "#[serde(tag"); + TestUtils.assertFileContains(unionPath, "Self::ObjectExists(inner) => (\"ObjectExists\", serde_json::to_value(inner)),"); </file context>

The hand-written serde was withdrawn in 1c0da30; unions derive #[serde(tag)] again. Partly moot: the claim the comment made is gone. Runtime checks are cargo run round-trips on the generated fixture, listed in the description. The codegen test stays a source-text test, like the rest of RustClientCodegenTest.

…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>

cubic-dev-ai

1 existing issue remains and no new issues found across 26 files (changes from recent commits).

_{Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

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 · 2026-10-05T18:51:59Z

rust tests passed via #25133

cubic-dev-ai Bot reviewed Sep 8, 2026

View reviewed changes

Comment thread modules/openapi-generator/src/main/resources/rust/model.mustache Resolved

Comment thread samples/client/others/rust/reqwest/oneOf/src/models/bar_ref.rs Resolved

Comment thread modules/openapi-generator/src/main/resources/rust/model.mustache Resolved

cubic-dev-ai Bot reviewed Sep 8, 2026

View reviewed changes

Comment thread ...es/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustClientCodegen.java Resolved

cubic-dev-ai Bot reviewed Sep 9, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Sep 23, 2026

View reviewed changes

wiebren and others added 2 commits September 23, 2026 11:38

cubic-dev-ai Bot reviewed Sep 23, 2026

View reviewed changes

Comment thread ...les/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustClientCodegenTest.java Resolved

wiebren and others added 3 commits September 23, 2026 12:44

test: [rust] pin the derived serde tag on the unions

37c3372

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

Merge remote-tracking branch 'origin/master' into fix/rust-discrimina…

daa9c0d

…ted-union-child-fields # Conflicts: # modules/openapi-generator/src/test/java/org/openapitools/codegen/rust/RustClientCodegenTest.java

wing328 added Client: Rust Issue: Bug labels Oct 5, 2026

wing328 merged commit 4d272cb into OpenAPITools:master Oct 5, 2026
17 checks passed

wing328 modified the milestones: 8.0.0, 7.26.0 Oct 5, 2026

Conversation

wiebren commented Sep 8, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Verification

Known gaps

PR checklist

Summary by cubic

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wiebren commented Sep 8, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wiebren commented Sep 10, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

wiebren Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

wiebren Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wing328 commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wiebren commented Sep 8, 2026 •

edited by cubic-dev-ai Bot

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading