From ab738e26806349105afc815f7ed83e637d30bb36 Mon Sep 17 00:00:00 2001 From: Chris Riccomini Date: Fri, 3 Apr 2026 17:29:45 -0700 Subject: [PATCH 1/5] fix(java): resolve local trait impl interfaces in submodules Java trait implementation rendering was assuming that a trait type's full module_path could always be resolved directly via ComponentInterface::find_component_interface. That is not true for local callback interfaces defined in a submodule of the current crate, such as slatedb_uniffi::metrics::MetricsRecorder. In that case the metadata still belongs to the current component interface, but trait_interface_name looked up the full module path and failed with: "no interface with module_path: slatedb_uniffi::metrics". This broke Java code generation for local object trait impls and forced downstream crates to keep callback traits at the crate root as a workaround. Fix the lookup by resolving the component interface from the trait type's crate name first. If the crate name matches the current component interface, use the current interface directly. Otherwise keep the existing external-interface lookup behavior, falling back from the full module path to the crate name. Add a regression test covering a local object that implements a callback interface declared in test::metrics. The test verifies both the filter result and the generated Java class declaration so the original failure mode is exercised directly. --- src/gen_java/mod.rs | 80 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/src/gen_java/mod.rs b/src/gen_java/mod.rs index 8398c93..29426c5 100644 --- a/src/gen_java/mod.rs +++ b/src/gen_java/mod.rs @@ -862,6 +862,19 @@ mod filters { } } + fn component_interface_for_module_path<'a>( + ci: &'a ComponentInterface, + module_path: &str, + ) -> Option<&'a ComponentInterface> { + let crate_name = module_path.split("::").next().unwrap_or(module_path); + if crate_name == ci.crate_name() { + Some(ci) + } else { + ci.find_component_interface(module_path) + .or_else(|| ci.find_component_interface(crate_name)) + } + } + pub(super) fn canonical_name( as_ct: &impl AsCodeType, _v: &dyn askama::Values, @@ -1139,7 +1152,7 @@ mod filters { "Invalid trait_type: {trait_ty:?}" ))); }; - let Some(ci_look) = ci.find_component_interface(module_path) else { + let Some(ci_look) = component_interface_for_module_path(ci, module_path) else { return Err(to_askama_error(&format!( "no interface with module_path: {}", module_path @@ -1432,8 +1445,9 @@ mod tests { use super::*; use uniffi_bindgen::interface::ComponentInterface; use uniffi_meta::{ - EnumMetadata, EnumShape, FnMetadata, FnParamMetadata, Metadata, MetadataGroup, - NamespaceMetadata, Type, VariantMetadata, + CallbackInterfaceMetadata, EnumMetadata, EnumShape, FnMetadata, FnParamMetadata, Metadata, + MetadataGroup, NamespaceMetadata, ObjectImpl, ObjectMetadata, ObjectTraitImplMetadata, + Type, VariantMetadata, }; #[test] @@ -1757,4 +1771,64 @@ mod tests { "android bindings should preserve java.lang.Exception" ); } + + #[test] + fn local_trait_impls_accept_submodule_module_paths() { + let mut group = MetadataGroup { + namespace: NamespaceMetadata { + crate_name: "test".to_string(), + name: "test".to_string(), + }, + namespace_docstring: None, + items: Default::default(), + }; + group.add_item(Metadata::Object(ObjectMetadata { + module_path: "test".to_string(), + name: "DefaultMetricsRecorder".to_string(), + remote: false, + imp: ObjectImpl::Struct, + docstring: None, + })); + group.add_item(Metadata::CallbackInterface(CallbackInterfaceMetadata { + module_path: "test::metrics".to_string(), + name: "MetricsRecorder".to_string(), + docstring: None, + })); + group.add_item(Metadata::ObjectTraitImpl(ObjectTraitImplMetadata { + ty: Type::Object { + module_path: "test".to_string(), + name: "DefaultMetricsRecorder".to_string(), + imp: ObjectImpl::Struct, + }, + trait_ty: Type::CallbackInterface { + module_path: "test::metrics".to_string(), + name: "MetricsRecorder".to_string(), + }, + })); + + let mut ci = ComponentInterface::from_metadata(group).unwrap(); + ci.derive_ffi_funcs().unwrap(); + + let interface_name = super::filters::trait_interface_name( + &Type::CallbackInterface { + module_path: "test::metrics".to_string(), + name: "MetricsRecorder".to_string(), + }, + &(), + &ci, + ) + .unwrap(); + assert_eq!(interface_name, "MetricsRecorder"); + + let bindings = generate_bindings(&Config::default(), &ci).unwrap(); + let relevant_lines = bindings + .lines() + .filter(|line| line.contains("class DefaultMetricsRecorder")) + .collect::>() + .join("\n"); + assert!( + bindings.contains("DefaultMetricsRecorderInterface, MetricsRecorder"), + "expected local callback trait impls with submodule paths to render successfully:\n{relevant_lines}" + ); + } } From 4196d27c71c02a8d09afabba30a907a24426ea08 Mon Sep 17 00:00:00 2001 From: Chris Riccomini Date: Fri, 3 Apr 2026 18:45:41 -0700 Subject: [PATCH 2/5] fix: avoid Java keyword collisions in callback helper classes Generate callback helper implementation classes with a class-style name derived from the trait method plus a Callback suffix instead of reusing the method identifier directly. This prevents invalid nested types such as "record" from being emitted when a callback method name collides with a Java keyword, while keeping the user-facing callback method name unchanged. Add a regression test for a "record" callback interface method. --- src/gen_java/mod.rs | 60 +++++++++++++++++++++++- src/templates/CallbackInterfaceImpl.java | 9 ++-- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/gen_java/mod.rs b/src/gen_java/mod.rs index 29426c5..d4b76f0 100644 --- a/src/gen_java/mod.rs +++ b/src/gen_java/mod.rs @@ -1447,7 +1447,7 @@ mod tests { use uniffi_meta::{ CallbackInterfaceMetadata, EnumMetadata, EnumShape, FnMetadata, FnParamMetadata, Metadata, MetadataGroup, NamespaceMetadata, ObjectImpl, ObjectMetadata, ObjectTraitImplMetadata, - Type, VariantMetadata, + TraitMethodMetadata, Type, VariantMetadata, }; #[test] @@ -1831,4 +1831,62 @@ mod tests { "expected local callback trait impls with submodule paths to render successfully:\n{relevant_lines}" ); } + + #[test] + fn callback_interface_helpers_use_class_style_names() { + let mut group = MetadataGroup { + namespace: NamespaceMetadata { + crate_name: "test".to_string(), + name: "test".to_string(), + }, + namespace_docstring: None, + items: Default::default(), + }; + group.add_item(Metadata::CallbackInterface(CallbackInterfaceMetadata { + module_path: "test".to_string(), + name: "Histogram".to_string(), + docstring: None, + })); + group.add_item(Metadata::TraitMethod(TraitMethodMetadata { + module_path: "test".to_string(), + trait_name: "Histogram".to_string(), + index: 0, + name: "record".to_string(), + is_async: false, + inputs: vec![FnParamMetadata { + name: "value".to_string(), + ty: Type::Float64, + by_ref: false, + optional: false, + default: None, + }], + return_type: None, + throws: None, + takes_self_by_arc: false, + checksum: None, + docstring: None, + })); + + let mut ci = ComponentInterface::from_metadata(group).unwrap(); + ci.derive_ffi_funcs().unwrap(); + + let bindings = generate_bindings(&Config::default(), &ci).unwrap(); + + assert!( + bindings.contains("public void record(double value);"), + "expected callback interface API to preserve the Rust method name:\n{bindings}" + ); + assert!( + bindings.contains("public static final class RecordCallback implements UniffiCallbackInterfaceHistogramMethod0.Fn"), + "expected callback helper class to use a Java class-style name:\n{bindings}" + ); + assert!( + bindings.contains("RecordCallback.INSTANCE"), + "expected generated callback helper references to use the renamed helper class:\n{bindings}" + ); + assert!( + !bindings.contains("public static class record implements"), + "unexpected lowercase helper class leaked into generated bindings:\n{bindings}" + ); + } } diff --git a/src/templates/CallbackInterfaceImpl.java b/src/templates/CallbackInterfaceImpl.java index a228fb2..53b1fe5 100644 --- a/src/templates/CallbackInterfaceImpl.java +++ b/src/templates/CallbackInterfaceImpl.java @@ -17,7 +17,7 @@ public class {{ trait_impl }} { {{ vtable|ffi_struct_type_name }}.setuniffiFree(vtable, {{ "CallbackInterfaceFree"|ffi_callback_name }}.toUpcallStub(UniffiFree.INSTANCE, java.lang.foreign.Arena.global())); {{ vtable|ffi_struct_type_name }}.setuniffiClone(vtable, {{ "CallbackInterfaceClone"|ffi_callback_name }}.toUpcallStub(UniffiClone.INSTANCE, java.lang.foreign.Arena.global())); {%- for (ffi_callback, meth) in vtable_methods.iter() %} - {{ vtable|ffi_struct_type_name }}.set{{ meth.name()|var_name_raw }}(vtable, {{ ffi_callback.name()|ffi_callback_name }}.toUpcallStub({{ meth.name()|var_name }}.INSTANCE, java.lang.foreign.Arena.global())); + {{ vtable|ffi_struct_type_name }}.set{{ meth.name()|var_name_raw }}(vtable, {{ ffi_callback.name()|ffi_callback_name }}.toUpcallStub({{ meth.name()|class_name(ci) }}Callback.INSTANCE, java.lang.foreign.Arena.global())); {%- endfor %} } @@ -27,10 +27,9 @@ void register() { } {%- for (ffi_callback, meth) in vtable_methods.iter() %} - {% let inner_method_class = meth.name()|var_name %} - public static class {{ inner_method_class }} implements {{ ffi_callback.name()|ffi_callback_name }}.Fn { - public static final {{ inner_method_class }} INSTANCE = new {{ inner_method_class }}(); - private {{ inner_method_class }}() {} + public static final class {{ meth.name()|class_name(ci) }}Callback implements {{ ffi_callback.name()|ffi_callback_name }}.Fn { + public static final {{ meth.name()|class_name(ci) }}Callback INSTANCE = new {{ meth.name()|class_name(ci) }}Callback(); + private {{ meth.name()|class_name(ci) }}Callback() {} @Override public {% match ffi_callback.return_type() %}{% when Some(return_type) %}{{ return_type|ffi_type_name(config, ci) }}{% when None %}void{% endmatch %} callback( From eda747856900a60bea24a229f1581880a2c5cf68 Mon Sep 17 00:00:00 2001 From: Chris Riccomini Date: Fri, 3 Apr 2026 19:30:36 -0700 Subject: [PATCH 3/5] refactor(template): extract callback class name local --- src/templates/CallbackInterfaceImpl.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/templates/CallbackInterfaceImpl.java b/src/templates/CallbackInterfaceImpl.java index 53b1fe5..66c8bd1 100644 --- a/src/templates/CallbackInterfaceImpl.java +++ b/src/templates/CallbackInterfaceImpl.java @@ -27,9 +27,10 @@ void register() { } {%- for (ffi_callback, meth) in vtable_methods.iter() %} - public static final class {{ meth.name()|class_name(ci) }}Callback implements {{ ffi_callback.name()|ffi_callback_name }}.Fn { - public static final {{ meth.name()|class_name(ci) }}Callback INSTANCE = new {{ meth.name()|class_name(ci) }}Callback(); - private {{ meth.name()|class_name(ci) }}Callback() {} + {% let callback_class = meth.name()|class_name(ci) %} + public static final class {{ callback_class }}Callback implements {{ ffi_callback.name()|ffi_callback_name }}.Fn { + public static final {{ callback_class }}Callback INSTANCE = new {{ callback_class }}Callback(); + private {{ callback_class }}Callback() {} @Override public {% match ffi_callback.return_type() %}{% when Some(return_type) %}{{ return_type|ffi_type_name(config, ci) }}{% when None %}void{% endmatch %} callback( From ff335beec31809cdc86877308b966572f5627462 Mon Sep 17 00:00:00 2001 From: Chris Riccomini Date: Fri, 3 Apr 2026 19:35:49 -0700 Subject: [PATCH 4/5] docs(gen_java): document component interface lookup helper --- src/gen_java/mod.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/gen_java/mod.rs b/src/gen_java/mod.rs index d4b76f0..2d7c83b 100644 --- a/src/gen_java/mod.rs +++ b/src/gen_java/mod.rs @@ -862,6 +862,19 @@ mod filters { } } + /// Returns the [`ComponentInterface`] that owns `module_path`. + /// + /// `ci` is the local component interface for the crate currently being generated. + /// `module_path` is the UniFFI module path recorded on a type or trait, such as + /// `crate_name::nested_module`. + /// + /// If `module_path` belongs to the current crate, this returns `ci` directly. + /// Otherwise it looks up an external component interface first by the full + /// module path and then by the crate name derived from the first `::` segment. + /// That fallback handles metadata that may identify external items by either + /// their full module path or just their crate. + /// + /// Returns `None` if no loaded component interface matches either lookup. fn component_interface_for_module_path<'a>( ci: &'a ComponentInterface, module_path: &str, From af3e58871550a0006ed07742f04a62071570501e Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Mon, 6 Apr 2026 11:21:17 -0600 Subject: [PATCH 5/5] Small cleanups, better test output --- src/gen_java/mod.rs | 117 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 21 deletions(-) diff --git a/src/gen_java/mod.rs b/src/gen_java/mod.rs index 2d7c83b..1db1616 100644 --- a/src/gen_java/mod.rs +++ b/src/gen_java/mod.rs @@ -864,17 +864,9 @@ mod filters { /// Returns the [`ComponentInterface`] that owns `module_path`. /// - /// `ci` is the local component interface for the crate currently being generated. - /// `module_path` is the UniFFI module path recorded on a type or trait, such as - /// `crate_name::nested_module`. - /// - /// If `module_path` belongs to the current crate, this returns `ci` directly. - /// Otherwise it looks up an external component interface first by the full - /// module path and then by the crate name derived from the first `::` segment. - /// That fallback handles metadata that may identify external items by either - /// their full module path or just their crate. - /// - /// Returns `None` if no loaded component interface matches either lookup. + /// If `module_path` belongs to the current crate, returns `ci` directly. + /// Otherwise looks up an external component interface by the full module + /// path first, then by the crate name (first `::` segment). fn component_interface_for_module_path<'a>( ci: &'a ComponentInterface, module_path: &str, @@ -1785,6 +1777,69 @@ mod tests { ); } + #[test] + fn trait_impl_with_submodule_path() { + // Regression test: when a crate has multiple modules, module_path is + // "crate_name::submodule". component_interface_for_module_path() handles + // this by checking the local CI's crate_name first before falling back + // to find_component_interface() for external crates. + let submodule_path = "mycrate::inner"; + let mut group = MetadataGroup { + namespace: NamespaceMetadata { + crate_name: "mycrate".to_string(), + name: "mycrate".to_string(), + }, + namespace_docstring: None, + items: Default::default(), + }; + + // A trait object defined in a submodule + group.add_item(Metadata::Object(ObjectMetadata { + module_path: submodule_path.to_string(), + name: "MyTrait".to_string(), + remote: false, + imp: ObjectImpl::CallbackTrait, + docstring: None, + })); + + // A concrete object that implements the trait, also in the submodule + group.add_item(Metadata::Object(ObjectMetadata { + module_path: submodule_path.to_string(), + name: "MyObj".to_string(), + remote: false, + imp: ObjectImpl::Struct, + docstring: None, + })); + + group.add_item(Metadata::ObjectTraitImpl(ObjectTraitImplMetadata { + ty: Type::Object { + module_path: submodule_path.to_string(), + name: "MyObj".to_string(), + imp: ObjectImpl::Struct, + }, + trait_ty: Type::Object { + module_path: submodule_path.to_string(), + name: "MyTrait".to_string(), + imp: ObjectImpl::CallbackTrait, + }, + })); + + let mut ci = ComponentInterface::from_metadata(group).unwrap(); + ci.derive_ffi_funcs().unwrap(); + let bindings = generate_bindings(&Config::default(), &ci).unwrap(); + + // The object should implement the trait interface + assert!( + bindings.contains("implements AutoCloseable, MyObjInterface, MyTrait"), + "MyObj should implement MyTrait via trait_interface_name even with submodule path:\n{}", + bindings + .lines() + .filter(|l| l.contains("MyObj") || l.contains("MyTrait")) + .collect::>() + .join("\n") + ); + } + #[test] fn local_trait_impls_accept_submodule_module_paths() { let mut group = MetadataGroup { @@ -1834,14 +1889,14 @@ mod tests { assert_eq!(interface_name, "MetricsRecorder"); let bindings = generate_bindings(&Config::default(), &ci).unwrap(); - let relevant_lines = bindings - .lines() - .filter(|line| line.contains("class DefaultMetricsRecorder")) - .collect::>() - .join("\n"); assert!( bindings.contains("DefaultMetricsRecorderInterface, MetricsRecorder"), - "expected local callback trait impls with submodule paths to render successfully:\n{relevant_lines}" + "expected local callback trait impls with submodule paths to render successfully:\n{}", + bindings + .lines() + .filter(|line| line.contains("class DefaultMetricsRecorder")) + .collect::>() + .join("\n") ); } @@ -1887,19 +1942,39 @@ mod tests { assert!( bindings.contains("public void record(double value);"), - "expected callback interface API to preserve the Rust method name:\n{bindings}" + "expected callback interface API to preserve the Rust method name:\n{}", + bindings + .lines() + .filter(|l| l.contains("double value")) + .collect::>() + .join("\n") ); assert!( bindings.contains("public static final class RecordCallback implements UniffiCallbackInterfaceHistogramMethod0.Fn"), - "expected callback helper class to use a Java class-style name:\n{bindings}" + "expected callback helper class to use a Java class-style name:\n{}", + bindings + .lines() + .filter(|l| l.contains("implements UniffiCallbackInterfaceHistogramMethod0.Fn")) + .collect::>() + .join("\n") ); assert!( bindings.contains("RecordCallback.INSTANCE"), - "expected generated callback helper references to use the renamed helper class:\n{bindings}" + "expected generated callback helper references to use the renamed helper class:\n{}", + bindings + .lines() + .filter(|l| l.contains(".INSTANCE")) + .collect::>() + .join("\n") ); assert!( !bindings.contains("public static class record implements"), - "unexpected lowercase helper class leaked into generated bindings:\n{bindings}" + "unexpected lowercase helper class leaked into generated bindings:\n{}", + bindings + .lines() + .filter(|l| l.contains("class record")) + .collect::>() + .join("\n") ); } }