Skip to content

Commit 657a43c

Browse files
committed
fix(swift): keep unit enums as raw value, JSON only for tagged enums
The prior fix serialized every enum-typed struct field getter with serde_json, which broke unit enums: it emitted a JSON-quoted string that Swift's Type(rawValue:) initializer cannot parse. Branch on enum kind instead — unit enums (all variants fieldless) keep returning their bare serde raw value via the bridge wrapper's to_string(), while tagged enums (a variant carries data) serialize the source value with serde_json::to_string for JSONDecoder. Threads unit_enum_names through the type-wrapper and getter emitters and covers both kinds with struct-field getter tests.
1 parent 81b04ec commit 657a43c

5 files changed

Lines changed: 155 additions & 34 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
register/unregister/clear lifecycle calls. The call sites now narrow via `(int) (long)`, matching
2020
the canonical pattern already used for `last_error_code`.
2121

22-
- **swift**: serialize enum-typed struct field getters to JSON instead of emitting a bare variant
23-
name. The discriminant-only bridge wrapper's `.to_string()` dropped the variant payload and
24-
returned an unquoted name (e.g. `Text`), which Swift's `JSONDecoder` rejected with "The given data
25-
was not valid JSON." Getters now emit `serde_json::to_string` of the source enum value, matching
26-
the bidirectional `*_from_json` representation the Swift `Codable` types expect.
22+
- **swift**: encode enum-typed struct field getters to match how the Swift side decodes each enum
23+
kind. Tagged enums (some variant carries data, e.g. `AssistantContent`) are serialized with
24+
`serde_json::to_string` of the source value and decoded via `JSONDecoder` — the discriminant-only
25+
bridge wrapper's `.to_string()` previously dropped the payload and returned an unquoted name (e.g.
26+
`Text`), which `JSONDecoder` rejected with "The given data was not valid JSON." Unit enums (all
27+
variants fieldless, e.g. `FinishReason`) keep returning their bare serde raw value via the wrapper's
28+
`.to_string()`, which Swift reconstructs with `Type(rawValue:)`; serializing those to JSON would
29+
emit a quoted string the rawValue init cannot parse.
2730

2831
- **elixir**: keep async NIF symbols suffixed internally while exposing async free functions under
2932
their original public names in the high-level Elixir facade. Generated modules now expose

src/backends/swift/gen_rust_crate/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,7 @@ fn emit_lib_rs(
759759
&source_crate,
760760
&type_paths,
761761
&enum_names,
762+
&unit_enum_names,
762763
&no_serde_names,
763764
exclude_fields,
764765
configured_features,

src/backends/swift/gen_rust_crate/wrappers/constructors.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ use std::collections::{HashMap, HashSet};
2020

2121
use super::getters::emit_getters;
2222

23+
#[allow(clippy::too_many_arguments)]
2324
pub(crate) fn emit_type_wrapper(
2425
ty: &TypeDef,
2526
source_crate: &str,
2627
type_paths: &HashMap<String, String>,
2728
enum_names: &HashSet<&str>,
29+
unit_enum_names: &HashSet<&str>,
2830
no_serde_names: &HashSet<&str>,
2931
exclude_fields: &HashSet<String>,
3032
configured_features: &HashSet<&str>,
@@ -153,6 +155,7 @@ pub(crate) fn emit_type_wrapper(
153155
ty,
154156
type_paths,
155157
enum_names,
158+
unit_enum_names,
156159
no_serde_names,
157160
exclude_fields,
158161
configured_features,
@@ -302,6 +305,7 @@ mod tests {
302305
&std::collections::HashMap::new(),
303306
&std::collections::HashSet::new(),
304307
&std::collections::HashSet::new(),
308+
&std::collections::HashSet::new(),
305309
&exclude_fields,
306310
&configured_features,
307311
);

src/backends/swift/gen_rust_crate/wrappers/getters.rs

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,12 @@ struct GetterCtx {
9393
}
9494

9595
/// Emit getter methods for all fields of a type wrapper.
96+
#[allow(clippy::too_many_arguments)]
9697
pub(super) fn emit_getters(
9798
ty: &TypeDef,
9899
type_paths: &HashMap<String, String>,
99100
enum_names: &HashSet<&str>,
101+
unit_enum_names: &HashSet<&str>,
100102
no_serde_names: &HashSet<&str>,
101103
exclude_fields: &HashSet<String>,
102104
configured_features: &std::collections::HashSet<&str>,
@@ -162,16 +164,19 @@ pub(super) fn emit_getters(
162164
},
163165
));
164166
} else if is_enum_named(&field.ty, enum_names) {
165-
// Enum-typed Named field: return the serde-JSON encoding of the source enum
166-
// value as a String. The opaque enum type is NOT declared in the extern block
167-
// (see extern_block.rs), so we must not return the wrapper type here; and the
168-
// discriminant-only wrapper would lose the variant payload, so serialize the
169-
// original value instead. The Swift side JSON-decodes this back into its Codable
170-
// enum (matching the bidirectional `*_from_json` representation).
171-
emit_enum_string_getter(field, &ctx, enum_names, out);
167+
// Enum-typed Named field: return the enum as a String. The opaque enum type is
168+
// NOT declared in the extern block (see extern_block.rs), so we must not return
169+
// the wrapper type here. Unit enums (all variants fieldless) serialize to their
170+
// bare serde raw value via the wrapper's `to_string()`, which Swift reconstructs
171+
// with `Type(rawValue:)`. Tagged enums (variants carry data) cannot use the
172+
// discriminant-only wrapper — it drops the payload — so the source value is
173+
// serialized with `serde_json::to_string`, which Swift decodes via `JSONDecoder`
174+
// (matching the bidirectional `*_from_json` representation).
175+
emit_enum_string_getter(field, &ctx, enum_names, unit_enum_names, out);
172176
} else if is_vec_of_enum(&field.ty, enum_names) {
173-
// Vec<Named(enum)>: map each element to String via to_string().
174-
emit_vec_enum_string_getter(field, &ctx, enum_names, out);
177+
// Vec<Named(enum)>: map each element to a String (raw value for unit enums,
178+
// serde-JSON for tagged enums — see emit_enum_string_getter).
179+
emit_vec_enum_string_getter(field, &ctx, enum_names, unit_enum_names, out);
175180
} else if let TypeRef::Named(wrapper) = &field.ty {
176181
emit_named_getter(field, wrapper, &ctx, enum_names, out);
177182
} else if let TypeRef::Vec(inner) = &field.ty {
@@ -257,29 +262,42 @@ pub(super) fn emit_getters(
257262

258263
/// Emit a `String`-returning getter for an enum-typed `Named` field.
259264
///
260-
/// Instead of returning the opaque enum wrapper (which would trigger swift-bridge's
261-
/// `Vec<EnumType> Vectorizable` generation), this serializes the source enum value to
262-
/// JSON via `serde_json::to_string`. The discriminant-only bridge wrapper drops the
263-
/// variant payload and its `.to_string()` emits a bare variant name (invalid JSON), so
264-
/// the original value must be serialized for the Swift `JSONDecoder` to reconstruct it.
265+
/// Returns the opaque enum as a `String` (avoids swift-bridge's `Vec<EnumType>
266+
/// Vectorizable` generation). The encoding depends on the enum kind:
267+
/// - Unit enums (all variants fieldless): the bridge wrapper's `to_string()` yields the
268+
/// bare serde raw value (e.g. `stop`), which Swift reconstructs via `Type(rawValue:)`.
269+
/// - Tagged enums (some variant carries data): the discriminant-only wrapper drops the
270+
/// payload, so the source value is serialized with `serde_json::to_string` and Swift
271+
/// decodes it via `JSONDecoder` (matching the bidirectional `*_from_json` representation).
265272
fn emit_enum_string_getter(
266273
field: &crate::core::ir::FieldDef,
267274
ctx: &GetterCtx,
268275
enum_names: &HashSet<&str>,
276+
unit_enum_names: &HashSet<&str>,
269277
out: &mut String,
270278
) {
271279
let TypeRef::Named(wrapper) = &field.ty else {
272280
return;
273281
};
274282
let is_enum = enum_names.contains(wrapper.as_str());
275283
debug_assert!(is_enum, "emit_enum_string_getter called with non-enum Named type");
284+
let is_unit = unit_enum_names.contains(wrapper.as_str());
276285

277286
let name = &ctx.name;
278287
let getter_name = &ctx.getter_name;
279288

280289
if field.optional {
281-
// Option<EnumType> → Option<String>. Boxed and Arc fields both deref to the enum via `&*w`.
282-
let map_expr = if field.is_boxed || matches!(field.core_wrapper, CoreWrapper::Arc) {
290+
// Option<EnumType> → Option<String>.
291+
let map_expr = if is_unit {
292+
if field.is_boxed {
293+
format!("self.0.{name}.clone().map(|w| {wrapper}::from(*w).to_string())")
294+
} else if matches!(field.core_wrapper, CoreWrapper::Arc) {
295+
format!("self.0.{name}.clone().map(|w| {wrapper}::from((*w).clone()).to_string())")
296+
} else {
297+
format!("self.0.{name}.clone().map(|w| {wrapper}::from(w).to_string())")
298+
}
299+
} else if field.is_boxed || matches!(field.core_wrapper, CoreWrapper::Arc) {
300+
// Tagged enum: serialize the source value. Boxed and Arc both deref via `&*w`.
283301
format!(
284302
"self.0.{name}.clone().map(|w| serde_json::to_string(&*w).unwrap_or_else(|_| \"null\".to_string()))"
285303
)
@@ -294,8 +312,17 @@ fn emit_enum_string_getter(
294312
},
295313
));
296314
} else {
297-
// EnumType → String. Boxed and Arc fields both deref to the enum via `&*self.0.{name}`.
298-
let expr = if field.is_boxed || matches!(field.core_wrapper, CoreWrapper::Arc) {
315+
// EnumType → String.
316+
let expr = if is_unit {
317+
if field.is_boxed {
318+
format!("{wrapper}::from(*self.0.{name}.clone()).to_string()")
319+
} else if matches!(field.core_wrapper, CoreWrapper::Arc) {
320+
format!("{wrapper}::from((*self.0.{name}).clone()).to_string()")
321+
} else {
322+
format!("{wrapper}::from(self.0.{name}.clone()).to_string()")
323+
}
324+
} else if field.is_boxed || matches!(field.core_wrapper, CoreWrapper::Arc) {
325+
// Tagged enum: serialize the source value. Boxed and Arc both deref via `&*self.0.{name}`.
299326
format!("serde_json::to_string(&*self.0.{name}).unwrap_or_else(|_| \"null\".to_string())")
300327
} else {
301328
format!("serde_json::to_string(&self.0.{name}).unwrap_or_else(|_| \"null\".to_string())")
@@ -312,11 +339,14 @@ fn emit_enum_string_getter(
312339

313340
/// Emit a `Vec<String>`-returning getter for a `Vec<Named(enum)>` field.
314341
///
315-
/// Maps each enum element to its serde-JSON encoding via `serde_json::to_string`.
342+
/// Maps each enum element to a `String`: the bridge wrapper's `to_string()` raw value for
343+
/// unit enums, or `serde_json::to_string` of the source value for tagged enums (see
344+
/// `emit_enum_string_getter` for the encoding rationale).
316345
fn emit_vec_enum_string_getter(
317346
field: &crate::core::ir::FieldDef,
318347
ctx: &GetterCtx,
319348
enum_names: &HashSet<&str>,
349+
unit_enum_names: &HashSet<&str>,
320350
out: &mut String,
321351
) {
322352
let TypeRef::Vec(inner) = &field.ty else {
@@ -327,14 +357,22 @@ fn emit_vec_enum_string_getter(
327357
};
328358
let is_enum = enum_names.contains(wrapper.as_str());
329359
debug_assert!(is_enum, "emit_vec_enum_string_getter called with non-enum Vec<Named>");
360+
let is_unit = unit_enum_names.contains(wrapper.as_str());
330361

331362
let name = &ctx.name;
332363
let getter_name = &ctx.getter_name;
333364

334-
// Build the per-element mapping expression based on wrapping strategy.
335-
let elem_expr = match field.vec_inner_core_wrapper {
336-
CoreWrapper::Arc => "serde_json::to_string(&**elem).unwrap_or_else(|_| \"null\".to_string())".to_string(),
337-
_ => "serde_json::to_string(elem).unwrap_or_else(|_| \"null\".to_string())".to_string(),
365+
// Build the per-element mapping expression based on enum kind and wrapping strategy.
366+
let elem_expr = if is_unit {
367+
match field.vec_inner_core_wrapper {
368+
CoreWrapper::Arc => format!("{wrapper}::from((**elem).clone()).to_string()"),
369+
_ => format!("{wrapper}::from(elem.clone()).to_string()"),
370+
}
371+
} else {
372+
match field.vec_inner_core_wrapper {
373+
CoreWrapper::Arc => "serde_json::to_string(&**elem).unwrap_or_else(|_| \"null\".to_string())".to_string(),
374+
_ => "serde_json::to_string(elem).unwrap_or_else(|_| \"null\".to_string())".to_string(),
375+
}
338376
};
339377

340378
if field.optional {

tests/backends_swift_gen_rust_crate_test.rs

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -750,10 +750,10 @@ fn lib_rs_enum_extern_block_and_wrapper() {
750750

751751
#[test]
752752
fn lib_rs_struct_with_enum_field_returns_string() {
753-
// A struct with an enum-typed field must have that getter return `String`,
753+
// A struct with a UNIT enum-typed field must have that getter return `String`,
754754
// not the opaque enum wrapper type. The extern block must declare `fn foo(&self) -> String`
755-
// (not `fn foo(&self) -> Status`), and the wrapper impl must serialize the enum to JSON via
756-
// serde_json::to_string so the Swift side can decode the variant payload.
755+
// (not `fn foo(&self) -> Status`), and the wrapper impl must emit the bare serde raw value
756+
// via `Status::from(...).to_string()` (Swift reconstructs it with `Type(rawValue:)`).
757757
let api = ApiSurface {
758758
crate_name: "demo".into(),
759759
version: "0.1.0".into(),
@@ -781,10 +781,10 @@ fn lib_rs_struct_with_enum_field_returns_string() {
781781
"extern block must declare status() -> String, not the opaque enum type: {}",
782782
lib.content
783783
);
784-
// Wrapper impl must serialize the enum to JSON instead of emitting a bare variant name.
784+
// Unit-enum wrapper impl must convert to String via Status::from(...).to_string().
785785
assert!(
786-
lib.content.contains("serde_json::to_string(&self.0.status)"),
787-
"wrapper impl must serialize the enum field via serde_json::to_string: {}",
786+
lib.content.contains("Status::from(") && lib.content.contains(".to_string()"),
787+
"wrapper impl must call Status::from(...).to_string(): {}",
788788
lib.content
789789
);
790790
// Opaque enum type IS declared in its own extern block (required for parameter usage).
@@ -796,6 +796,81 @@ fn lib_rs_struct_with_enum_field_returns_string() {
796796
);
797797
}
798798

799+
#[test]
800+
fn lib_rs_struct_with_tagged_enum_field_serializes_to_json() {
801+
// A struct with a TAGGED enum field (at least one variant carries data) must serialize the
802+
// getter to JSON via serde_json::to_string. The discriminant-only bridge wrapper drops the
803+
// variant payload, so its to_string() would emit an invalid bare name; Swift decodes the
804+
// JSON back via JSONDecoder (matching the bidirectional `*_from_json` representation).
805+
let payload = EnumDef {
806+
name: "Payload".to_string(),
807+
rust_path: "demo::Payload".to_string(),
808+
original_rust_path: String::new(),
809+
variants: vec![EnumVariant {
810+
name: "Text".to_string(),
811+
fields: vec![make_field("value", TypeRef::String)],
812+
doc: String::new(),
813+
is_default: false,
814+
serde_rename: None,
815+
binding_excluded: false,
816+
binding_exclusion_reason: None,
817+
is_tuple: false,
818+
originally_had_data_fields: true,
819+
cfg: None,
820+
version: Default::default(),
821+
}],
822+
methods: vec![],
823+
doc: String::new(),
824+
cfg: None,
825+
serde_tag: Some("type".to_string()),
826+
serde_untagged: false,
827+
serde_rename_all: None,
828+
is_copy: false,
829+
has_serde: true,
830+
binding_excluded: false,
831+
binding_exclusion_reason: None,
832+
excluded_variants: vec![],
833+
version: Default::default(),
834+
has_default: false,
835+
};
836+
let api = ApiSurface {
837+
crate_name: "demo".into(),
838+
version: "0.1.0".into(),
839+
types: vec![{
840+
let mut t = make_type(
841+
"Item",
842+
vec![make_field("payload", TypeRef::Named("Payload".to_string()))],
843+
);
844+
t.has_serde = true;
845+
t
846+
}],
847+
functions: vec![],
848+
enums: vec![payload],
849+
errors: vec![],
850+
excluded_type_paths: ::std::collections::HashMap::new(),
851+
excluded_trait_names: ::std::collections::HashSet::new(),
852+
services: vec![],
853+
handler_contracts: vec![],
854+
unsupported_public_items: Vec::new(),
855+
};
856+
857+
let files = gen_rust_crate::emit(&api, &make_config()).unwrap();
858+
let lib = files.iter().find(|f| f.path.ends_with("lib.rs")).unwrap();
859+
860+
// Getter must be declared as String in the extern block.
861+
assert!(
862+
lib.content.contains("fn payload(&self) -> String;"),
863+
"extern block must declare payload() -> String: {}",
864+
lib.content
865+
);
866+
// Tagged enum must serialize via serde_json::to_string, NOT the discriminant wrapper to_string().
867+
assert!(
868+
lib.content.contains("serde_json::to_string(&self.0.payload)"),
869+
"tagged-enum getter must serialize via serde_json::to_string: {}",
870+
lib.content
871+
);
872+
}
873+
799874
// ── trait bridge tests ────────────────────────────────────────────────────────
800875

801876
fn make_method(

0 commit comments

Comments
 (0)