Skip to content

Commit 9d8b9d6

Browse files
committed
fix(backend-php): resolve compile errors from options_field bridge
- Pass opaque_type_names to gen_php_struct so serde(skip) is emitted on opaque handle fields (fixes 8 E0277 serde trait errors). - Emit `let mut {name}_core` for optional params in gen_bridge_field_function so options_core.as_mut() compiles in the visitor_attach branch. - Set param_is_optional: is_optional || param.optional in find_bridge_field (alef-codegen) to cover Named+optional=true IR pattern, fixing the wrong visitor_attach branch selection (E0609 no field 'visitor' on Option<...>). - Emit (*v.inner).clone() for optional opaque params in gen_php_call_args, fixing the Option<&Arc<...>> vs Option<Rc<...>> type mismatch (E0308).
1 parent 115732b commit 9d8b9d6

6 files changed

Lines changed: 297 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- fix(backend-php): resolve compile errors when `bind_via = "options_field"` is used
13+
in `[[trait_bridges]]` with an options type that has an opaque handle field:
14+
- `gen_php_struct` now receives `opaque_type_names` from `mod.rs` so
15+
`gen_struct_with_per_field_attrs` can emit `#[serde(skip)]` on fields whose type
16+
is an opaque handle (e.g. `VisitorHandle`), fixing 8 `E0277` serde trait errors.
17+
- `gen_bridge_field_function` now emits `let mut {name}_core` for optional params,
18+
enabling `options_core.as_mut()` in the generated `visitor_attach` call.
19+
- `find_bridge_field` (alef-codegen) now sets
20+
`param_is_optional: is_optional || param.optional`, covering the IR pattern where
21+
the options param is stored as `ty: Named("T") + optional: true` rather than
22+
`ty: Optional(Named("T"))`. This caused the wrong `visitor_attach` branch to be
23+
selected, producing a `no field 'visitor' on type Option<...>` compile error.
24+
- `gen_php_call_args` now emits `visitor.map(|v| (*v.inner).clone())` for optional
25+
opaque params, replacing the former `visitor.as_ref().map(|v| &v.inner)` which
26+
produced a type mismatch (`Option<&Arc<...>>` vs `Option<Rc<...>>`).
27+
1228
- fix(backend-wasm): resolve four compile errors when `bind_via = "options_field"` is
1329
used in `[[trait_bridges]]` with an options type that has a handle field:
1430
- `gen_new_method` now appends `..Default::default()` to the struct literal when

crates/alef-backend-php/src/gen_bindings/helpers.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,9 @@ pub(crate) fn gen_php_call_args(params: &[alef_core::ir::ParamDef], opaque_types
129129
}
130130
TypeRef::Named(name) if opaque_types.contains(name.as_str()) => {
131131
if p.optional {
132-
format!("{}.as_ref().map(|v| &v.inner)", p.name)
132+
// Param is Option<&OpaqueType>; dereference the Arc<inner> to obtain
133+
// an owned core value (Rc<RefCell<dyn Trait>>) that the builder expects.
134+
format!("{}.map(|v| (*v.inner).clone())", p.name)
133135
} else {
134136
format!("&{}.inner", p.name)
135137
}

crates/alef-backend-php/src/gen_bindings/mod.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,14 @@ impl Backend for PhpBackend {
159159
if !opaque_types.is_empty() {
160160
builder.add_import("std::sync::Arc");
161161
}
162+
// Sorted vec so gen_struct_with_per_field_attrs can emit #[serde(skip)] on
163+
// fields whose type is an opaque handle (e.g. VisitorHandle), preventing compile
164+
// errors when the struct derives serde::Serialize/Deserialize.
165+
let opaque_type_names_vec: Vec<String> = {
166+
let mut v: Vec<String> = opaque_types.iter().cloned().collect();
167+
v.sort();
168+
v
169+
};
162170

163171
// Compute the PHP namespace for namespaced class registration.
164172
// Delegates to config so [php].namespace overrides are respected.
@@ -218,7 +226,13 @@ impl Backend for PhpBackend {
218226
} else {
219227
// gen_struct adds #[derive(Default)] when typ.has_default is true,
220228
// so no separate Default impl is needed.
221-
builder.add_item(&gen_php_struct(typ, &mapper, &cfg, Some(&php_namespace), &enum_names));
229+
// Pass opaque_type_names so gen_struct_with_per_field_attrs can add
230+
// #[serde(skip)] to fields whose type is an opaque handle (e.g. VisitorHandle).
231+
let struct_cfg = RustBindingConfig {
232+
opaque_type_names: &opaque_type_names_vec,
233+
..cfg
234+
};
235+
builder.add_item(&gen_php_struct(typ, &mapper, &struct_cfg, Some(&php_namespace), &enum_names));
222236
builder.add_item(&types::gen_struct_methods_with_exclude(
223237
typ,
224238
&mapper,

crates/alef-backend-php/src/trait_bridge.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,8 +822,9 @@ pub fn gen_bridge_field_function(
822822
};
823823
let core_path = format!("{core_import}::{core_type_name}");
824824
if p.optional || matches!(&p.ty, TypeRef::Optional(_)) {
825+
// Use `let mut` so visitor_attach can call `.as_mut()` on the Option.
825826
format!(
826-
"let {name}_core: Option<{core_path}> = {name}.map(|v| {{\n \
827+
"let mut {name}_core: Option<{core_path}> = {name}.map(|v| {{\n \
827828
let json = serde_json::to_string(&v){err_conv}?;\n \
828829
serde_json::from_str(&json){err_conv}\n \
829830
}}).transpose()?;\n "

crates/alef-backend-php/tests/gen_bindings_test.rs

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,3 +2308,261 @@ fn test_options_field_bridge_public_api_passes_visitor_from_options() {
23082308
"PHP facade must pass $options->visitor to the native extension; content:\n{content}"
23092309
);
23102310
}
2311+
2312+
// --- Tests for Named+optional=true param_is_optional and opaque field serde(skip) fixes ---
2313+
2314+
/// Helper: VisitorHandle opaque type (simulates the real IR where VisitorHandle is opaque).
2315+
fn make_visitor_handle_opaque_type() -> TypeDef {
2316+
TypeDef {
2317+
name: "VisitorHandle".to_string(),
2318+
rust_path: "my_lib::visitor::VisitorHandle".to_string(),
2319+
original_rust_path: String::new(),
2320+
fields: vec![],
2321+
methods: vec![],
2322+
is_opaque: true,
2323+
is_clone: false,
2324+
is_copy: false,
2325+
is_trait: false,
2326+
has_default: false,
2327+
has_stripped_cfg_fields: false,
2328+
is_return_type: false,
2329+
serde_rename_all: None,
2330+
has_serde: false,
2331+
super_traits: vec![],
2332+
doc: String::new(),
2333+
cfg: None,
2334+
}
2335+
}
2336+
2337+
/// Helper: ConversionOptions with opaque VisitorHandle field (real IR representation).
2338+
/// In the real IR, the visitor field has ty=Named("VisitorHandle") and optional=true,
2339+
/// unlike the sanitized representation used by make_conversion_options_type().
2340+
fn make_conversion_options_with_opaque_visitor() -> TypeDef {
2341+
TypeDef {
2342+
name: "ConversionOptions".to_string(),
2343+
rust_path: "my_lib::ConversionOptions".to_string(),
2344+
original_rust_path: String::new(),
2345+
fields: vec![
2346+
make_field("parser", TypeRef::String, true),
2347+
make_field("visitor", TypeRef::Named("VisitorHandle".to_string()), true),
2348+
],
2349+
methods: vec![],
2350+
is_opaque: false,
2351+
is_clone: true,
2352+
is_copy: false,
2353+
is_trait: false,
2354+
has_default: true,
2355+
has_stripped_cfg_fields: false,
2356+
is_return_type: false,
2357+
serde_rename_all: None,
2358+
has_serde: true,
2359+
super_traits: vec![],
2360+
doc: "HTML to Markdown conversion options".to_string(),
2361+
cfg: None,
2362+
}
2363+
}
2364+
2365+
/// Helper: convert function where options param uses Named+optional=true (real IR pattern).
2366+
/// Distinct from make_convert_function() which wraps the type in Optional<Named>.
2367+
fn make_convert_function_named_optional() -> FunctionDef {
2368+
FunctionDef {
2369+
name: "convert".to_string(),
2370+
rust_path: "my_lib::convert".to_string(),
2371+
original_rust_path: String::new(),
2372+
params: vec![
2373+
ParamDef {
2374+
name: "html".to_string(),
2375+
ty: TypeRef::String,
2376+
optional: false,
2377+
default: None,
2378+
sanitized: false,
2379+
typed_default: None,
2380+
is_ref: true,
2381+
is_mut: false,
2382+
newtype_wrapper: None,
2383+
original_type: None,
2384+
},
2385+
ParamDef {
2386+
name: "options".to_string(),
2387+
// Real IR representation: Named with optional=true (not Optional<Named>)
2388+
ty: TypeRef::Named("ConversionOptions".to_string()),
2389+
optional: true,
2390+
default: None,
2391+
sanitized: false,
2392+
typed_default: None,
2393+
is_ref: false,
2394+
is_mut: false,
2395+
newtype_wrapper: None,
2396+
original_type: None,
2397+
},
2398+
],
2399+
return_type: TypeRef::Named("ConversionResult".to_string()),
2400+
is_async: false,
2401+
error_type: Some("ConversionError".to_string()),
2402+
doc: "Convert HTML to Markdown".to_string(),
2403+
cfg: None,
2404+
sanitized: false,
2405+
return_sanitized: false,
2406+
returns_ref: false,
2407+
returns_cow: false,
2408+
return_newtype_wrapper: None,
2409+
}
2410+
}
2411+
2412+
#[test]
2413+
fn test_opaque_visitor_field_gets_serde_skip() {
2414+
// When ConversionOptions has a visitor field of type Named("VisitorHandle") where
2415+
// VisitorHandle is an opaque type, the generated struct must add #[serde(skip)]
2416+
// to prevent compile errors (VisitorHandle does not implement serde).
2417+
let backend = PhpBackend;
2418+
let api = ApiSurface {
2419+
crate_name: "my-lib".to_string(),
2420+
version: "1.0.0".to_string(),
2421+
types: vec![
2422+
make_conversion_options_with_opaque_visitor(),
2423+
make_visitor_handle_opaque_type(),
2424+
make_visitor_trait_def(),
2425+
],
2426+
functions: vec![make_convert_function_named_optional()],
2427+
enums: vec![],
2428+
errors: vec![],
2429+
};
2430+
let mut config = make_config();
2431+
config.trait_bridges = vec![make_options_field_bridge_cfg()];
2432+
2433+
let files = backend.generate_bindings(&api, &config).unwrap();
2434+
let lib_rs = files
2435+
.iter()
2436+
.find(|f| f.path.to_string_lossy().contains("lib.rs"))
2437+
.unwrap();
2438+
let content = &lib_rs.content;
2439+
2440+
// The visitor field in ConversionOptions must be annotated with #[serde(skip)]
2441+
assert!(
2442+
content.contains("#[serde(skip)]"),
2443+
"opaque VisitorHandle field must get #[serde(skip)] to avoid serde compile errors; content:\n{content}"
2444+
);
2445+
}
2446+
2447+
#[test]
2448+
fn test_named_optional_param_generates_let_mut_options_core() {
2449+
// When options param is Named+optional=true (the real IR pattern), the generated
2450+
// serde binding must declare `let mut options_core` so visitor_attach can call
2451+
// options_core.as_mut() without a borrow error.
2452+
let backend = PhpBackend;
2453+
let api = ApiSurface {
2454+
crate_name: "my-lib".to_string(),
2455+
version: "1.0.0".to_string(),
2456+
types: vec![
2457+
make_conversion_options_with_opaque_visitor(),
2458+
make_visitor_handle_opaque_type(),
2459+
make_visitor_trait_def(),
2460+
],
2461+
functions: vec![make_convert_function_named_optional()],
2462+
enums: vec![],
2463+
errors: vec![],
2464+
};
2465+
let mut config = make_config();
2466+
config.trait_bridges = vec![make_options_field_bridge_cfg()];
2467+
2468+
let files = backend.generate_bindings(&api, &config).unwrap();
2469+
let lib_rs = files
2470+
.iter()
2471+
.find(|f| f.path.to_string_lossy().contains("lib.rs"))
2472+
.unwrap();
2473+
let content = &lib_rs.content;
2474+
2475+
// Must use `let mut` so .as_mut() can be called to attach the visitor.
2476+
assert!(
2477+
content.contains("let mut options_core"),
2478+
"options serde binding must declare `let mut options_core`; content:\n{content}"
2479+
);
2480+
2481+
// Must use the correct optional visitor_attach pattern.
2482+
assert!(
2483+
content.contains("options_core.as_mut()"),
2484+
"visitor_attach must call options_core.as_mut() for optional options; content:\n{content}"
2485+
);
2486+
}
2487+
2488+
#[test]
2489+
fn test_opaque_optional_param_in_builder_uses_deref_clone() {
2490+
// When an opaque type is passed as an optional param to a builder method,
2491+
// gen_php_call_args must emit `v.map(|v| (*v.inner).clone())` to unwrap the Arc
2492+
// and obtain the owned core value, rather than `v.as_ref().map(|v| &v.inner)`.
2493+
let visitor_builder_method = MethodDef {
2494+
name: "visitor".to_string(),
2495+
params: vec![ParamDef {
2496+
name: "visitor".to_string(),
2497+
ty: TypeRef::Named("VisitorHandle".to_string()),
2498+
optional: true,
2499+
default: None,
2500+
sanitized: false,
2501+
typed_default: None,
2502+
is_ref: false,
2503+
is_mut: false,
2504+
newtype_wrapper: None,
2505+
original_type: None,
2506+
}],
2507+
return_type: TypeRef::Named("ConversionOptionsBuilder".to_string()),
2508+
is_async: false,
2509+
is_static: false,
2510+
error_type: None,
2511+
doc: "Set the visitor used during conversion.".to_string(),
2512+
receiver: Some(ReceiverKind::Owned),
2513+
sanitized: false,
2514+
trait_source: None,
2515+
returns_ref: false,
2516+
returns_cow: false,
2517+
return_newtype_wrapper: None,
2518+
has_default_impl: false,
2519+
};
2520+
let builder_type = TypeDef {
2521+
name: "ConversionOptionsBuilder".to_string(),
2522+
rust_path: "my_lib::ConversionOptionsBuilder".to_string(),
2523+
original_rust_path: String::new(),
2524+
fields: vec![],
2525+
methods: vec![visitor_builder_method],
2526+
is_opaque: true,
2527+
is_clone: true,
2528+
is_copy: false,
2529+
is_trait: false,
2530+
has_default: false,
2531+
has_stripped_cfg_fields: false,
2532+
is_return_type: false,
2533+
serde_rename_all: None,
2534+
has_serde: false,
2535+
super_traits: vec![],
2536+
doc: String::new(),
2537+
cfg: None,
2538+
};
2539+
let backend = PhpBackend;
2540+
let api = ApiSurface {
2541+
crate_name: "my-lib".to_string(),
2542+
version: "1.0.0".to_string(),
2543+
types: vec![builder_type, make_visitor_handle_opaque_type(), make_visitor_trait_def()],
2544+
functions: vec![],
2545+
enums: vec![],
2546+
errors: vec![],
2547+
};
2548+
let config = make_config();
2549+
2550+
let files = backend.generate_bindings(&api, &config).unwrap();
2551+
let lib_rs = files
2552+
.iter()
2553+
.find(|f| f.path.to_string_lossy().contains("lib.rs"))
2554+
.unwrap();
2555+
let content = &lib_rs.content;
2556+
2557+
// Must deref the Arc to get owned core value — not pass a reference.
2558+
assert!(
2559+
content.contains("(*v.inner).clone()"),
2560+
"optional opaque param in builder must use (*v.inner).clone() not &v.inner; content:\n{content}"
2561+
);
2562+
2563+
// Must NOT use the wrong reference pattern.
2564+
assert!(
2565+
!content.contains("as_ref().map(|v| &v.inner)"),
2566+
"optional opaque param must not use as_ref().map(|v| &v.inner); content:\n{content}"
2567+
);
2568+
}

crates/alef-codegen/src/generators/trait_bridge.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,9 @@ pub fn find_bridge_field<'a>(
721721
param_index: idx,
722722
param_name: param.name.clone(),
723723
options_type: type_name.to_string(),
724-
param_is_optional: is_optional,
724+
// param.optional covers the case where the IR stores the type as
725+
// Named("T") with optional=true, rather than Optional(Named("T")).
726+
param_is_optional: is_optional || param.optional,
725727
field_name: field.name.clone(),
726728
field,
727729
bridge,

0 commit comments

Comments
 (0)