Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions generators/rust/model/src/__test__/generateModels.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,23 @@ describe("generateModels type-specific tests", () => {
expect(hasTaggedUnion).toBeTruthy();
});

it("should keep the flattened wrapper for union variants that inherit properties", async () => {
const context = await createSampleGeneratorContext("union-types");
const files = generateModels({ context });

// SproutedEvent has no properties of its own and WateredEvent has one, but both
// inherit `occurred_at` from PlantEventBase. Inlining copies own properties only,
// so these variants must keep the `#[serde(flatten)]` wrapper or the inherited
// fields would be silently dropped from the wire payload.
const plantEvent = files.find((file) => file.fileContents.includes("pub enum PlantEvent"));
expect(plantEvent?.fileContents).toContain("data: SproutedEvent,");
expect(plantEvent?.fileContents).toContain("data: WateredEvent,");

// The wrapper structs must still be generated, with their inherited fields.
const sproutedEvent = files.find((file) => file.fileContents.includes("pub struct SproutedEvent"));
expect(sproutedEvent?.fileContents).toContain("plant_event_base_fields: PlantEventBase");
Comment on lines +118 to +124

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 suggestion

files.find(...) can return undefined, and the ?. then hands undefined to toContain, producing a confusing matcher error rather than "file not generated". Assert the files exist first, and while here consider asserting the inherited field on WateredEvent too (that's the variant most likely to regress silently since it has an own property).

Suggested change
const plantEvent = files.find((file) => file.fileContents.includes("pub enum PlantEvent"));
expect(plantEvent?.fileContents).toContain("data: SproutedEvent,");
expect(plantEvent?.fileContents).toContain("data: WateredEvent,");
// The wrapper structs must still be generated, with their inherited fields.
const sproutedEvent = files.find((file) => file.fileContents.includes("pub struct SproutedEvent"));
expect(sproutedEvent?.fileContents).toContain("plant_event_base_fields: PlantEventBase");
const plantEvent = files.find((file) => file.fileContents.includes("pub enum PlantEvent"));
expect(plantEvent).toBeDefined();
expect(plantEvent?.fileContents).toContain("data: SproutedEvent,");
expect(plantEvent?.fileContents).toContain("data: WateredEvent,");
// The wrapper structs must still be generated, with their inherited fields.
const sproutedEvent = files.find((file) => file.fileContents.includes("pub struct SproutedEvent"));
expect(sproutedEvent).toBeDefined();
expect(sproutedEvent?.fileContents).toContain("plant_event_base_fields: PlantEventBase");
const wateredEvent = files.find((file) => file.fileContents.includes("pub struct WateredEvent"));
expect(wateredEvent).toBeDefined();
expect(wateredEvent?.fileContents).toContain("plant_event_base_fields: PlantEventBase");

});

it("should generate unions for union types (all are discriminated in Fern)", async () => {
const context = await createSampleGeneratorContext("undiscriminated-union-types");
const files = generateModels({ context });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
pub use crate::prelude::*;

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(tag = "event")]
#[non_exhaustive]
pub enum PlantEvent {
#[serde(rename = "sprouted")]
#[non_exhaustive]
Sprouted {
#[serde(flatten)]
data: SproutedEvent,
},

#[serde(rename = "watered")]
#[non_exhaustive]
Watered {
#[serde(flatten)]
data: WateredEvent,
},

/// Catch-all variant for unrecognized discriminant values.
/// If the server sends a discriminant not recognized by the current SDK
/// version, the raw payload is captured here so callers can still inspect it.
#[serde(untagged)]
__Unknown(serde_json::Value),
}

impl PlantEvent {
pub fn sprouted(data: SproutedEvent) -> Self {
Self::Sprouted { data }
}

pub fn watered(data: WateredEvent) -> Self {
Self::Watered { data }
}

pub fn unknown(value: serde_json::Value) -> Self {
Self::__Unknown(value)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
pub use crate::prelude::*;

#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq, Hash)]
pub struct PlantEventBase {
#[serde(default)]
pub occurred_at: String,
}

impl PlantEventBase {
pub fn builder() -> PlantEventBaseBuilder {
<PlantEventBaseBuilder as Default>::default()
}
}

#[derive(Clone, PartialEq, Default, Debug)]
#[non_exhaustive]
pub struct PlantEventBaseBuilder {
occurred_at: Option<String>,
}

impl PlantEventBaseBuilder {
pub fn occurred_at(mut self, value: impl Into<String>) -> Self {
self.occurred_at = Some(value.into());
self
}

/// Consumes the builder and constructs a [`PlantEventBase`].
/// This method will fail if any of the following fields are not set:
/// - [`occurred_at`](PlantEventBaseBuilder::occurred_at)
pub fn build(self) -> Result<PlantEventBase, BuildError> {
Ok(PlantEventBase {
occurred_at: self.occurred_at.ok_or_else(|| BuildError::missing_field("occurred_at"))?,
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
pub use crate::prelude::*;

#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq, Hash)]
pub struct SproutedEvent {
#[serde(flatten)]
pub plant_event_base_fields: PlantEventBase,
}

impl SproutedEvent {
pub fn builder() -> SproutedEventBuilder {
<SproutedEventBuilder as Default>::default()
}
}

#[derive(Clone, PartialEq, Default, Debug)]
#[non_exhaustive]
pub struct SproutedEventBuilder {
plant_event_base_fields: Option<PlantEventBase>,
}

impl SproutedEventBuilder {
pub fn plant_event_base_fields(mut self, value: PlantEventBase) -> Self {
self.plant_event_base_fields = Some(value);
self
}

/// Consumes the builder and constructs a [`SproutedEvent`].
/// This method will fail if any of the following fields are not set:
/// - [`plant_event_base_fields`](SproutedEventBuilder::plant_event_base_fields)
pub fn build(self) -> Result<SproutedEvent, BuildError> {
Ok(SproutedEvent {
plant_event_base_fields: self.plant_event_base_fields.ok_or_else(|| BuildError::missing_field("plant_event_base_fields"))?,
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
pub use crate::prelude::*;

#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq)]
pub struct WateredEvent {
#[serde(flatten)]
pub plant_event_base_fields: PlantEventBase,
#[serde(default)]
#[serde(with = "crate::core::number_serializers")]
pub liters: f64,
}

impl WateredEvent {
pub fn builder() -> WateredEventBuilder {
<WateredEventBuilder as Default>::default()
}
}

#[derive(Clone, PartialEq, Default, Debug)]
#[non_exhaustive]
pub struct WateredEventBuilder {
plant_event_base_fields: Option<PlantEventBase>,
liters: Option<f64>,
}

impl WateredEventBuilder {
pub fn plant_event_base_fields(mut self, value: PlantEventBase) -> Self {
self.plant_event_base_fields = Some(value);
self
}

pub fn liters(mut self, value: f64) -> Self {
self.liters = Some(value);
self
}

/// Consumes the builder and constructs a [`WateredEvent`].
/// This method will fail if any of the following fields are not set:
/// - [`plant_event_base_fields`](WateredEventBuilder::plant_event_base_fields)
/// - [`liters`](WateredEventBuilder::liters)
pub fn build(self) -> Result<WateredEvent, BuildError> {
Ok(WateredEvent {
plant_event_base_fields: self.plant_event_base_fields.ok_or_else(|| BuildError::missing_field("plant_event_base_fields"))?,
liters: self.liters.ok_or_else(|| BuildError::missing_field("liters"))?,
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,28 @@ types:
- CSV
- XML
- YAML

# Union whose variants inherit their fields via extends. The variant types are each
# referenced once, so they are inlining candidates, but inlining would drop everything
# they inherit.
PlantEvent:
discriminant: event
union:
sprouted:
type: SproutedEvent
watered:
type: WateredEvent

PlantEventBase:
properties:
occurred_at: string

SproutedEvent:
extends:
- PlantEventBase

WateredEvent:
extends:
- PlantEventBase
properties:
liters: double
6 changes: 5 additions & 1 deletion generators/rust/model/src/generateModels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ export function generateModels({ context }: { context: ModelGeneratorContext }):
* 2. It is not referenced anywhere else in the IR (object fields, other unions,
* service endpoints, aliases, containers, etc.)
* 3. It is an object type (not an enum, alias, or another union)
* 4. It does not inherit properties via `extends`. Inlining copies the object's own
* properties only, so inheriting types would lose every inherited field; they keep
* the `#[serde(flatten)]` wrapper struct, which preserves the full shape.
*/
function computeInlinedUnionVariantTypeIds(context: ModelGeneratorContext): void {
const ir = context.ir;
Expand Down Expand Up @@ -287,11 +290,12 @@ function computeInlinedUnionVariantTypeIds(context: ModelGeneratorContext): void
// - Referenced exactly once (the samePropertiesAsObject reference)
// - That one reference is as samePropertiesAsObject
// - The type is an object (not enum, alias, or union)
// - The object declares no `extends` (inherited properties are not inlined)
for (const typeId of samePropertiesRefs) {
const count = referenceCount.get(typeId) ?? 0;
if (count === 1) {
const typeDecl = ir.types[typeId];
if (typeDecl?.shape.type === "object") {
if (typeDecl?.shape.type === "object" && typeDecl.shape.extends.length === 0) {
context.inlinedUnionVariantTypeIds.add(typeId);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# yaml-language-server: $schema=../../../../../fern-changes-yml.schema.json

- summary: |
Fix discriminated union variants silently dropping inherited properties. When a variant's
referenced object type got all of its properties from `extends`, the variant was inlined to
an empty struct, so the payload deserialized successfully but every field was discarded.
Such types are no longer inlined and keep their `#[serde(flatten)]` wrapper.
type: fix

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 suggestion

Two things:

  1. The seed diff shows this changes generated public API for affected unions (FooExtended { age }FooExtended { data: FooExtended }, and foo_extended(age: i64)foo_extended(data: FooExtended)). Users on 0.24.x with extends-based variants will get compile errors on upgrade — worth calling out explicitly in the summary even if the type stays fix.
  2. The behavior change originates in generators/rust/model. If that generator has its own changes/unreleased directory and is published separately, it needs an entry there as well.

2 changes: 2 additions & 0 deletions seed/rust-sdk/unions-with-local-date/src/api/types/mod.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions seed/rust-sdk/unions/src/api/types/mod.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 47 additions & 0 deletions seed/rust-sdk/unions/src/api/types/types_foo_extended.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading