diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b0d258f7c..da8fd9fbd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ * Methods now have typing annotations for return values ([#2625](https://github.com/mozilla/uniffi-rs/issues/2625)) * Fix relative imports ([#2657](https://github.com/mozilla/uniffi-rs/pull/2657)) * Fix shadowing param names with internal variables in Python (#2628/#2645) + * Don't allow objects to be passed as arguments when traits are expected (#2649) - Swift: * All object protocol conformances are public ([#2671](https://github.com/mozilla/uniffi-rs/pull/2671)) * Initialization functions now have a stable ordering when using external types. diff --git a/fixtures/coverall/src/traits.rs b/fixtures/coverall/src/traits.rs index da24e4bbc7..0426163f32 100644 --- a/fixtures/coverall/src/traits.rs +++ b/fixtures/coverall/src/traits.rs @@ -261,3 +261,33 @@ impl StringUtil for StringUtilImpl2 { format!("{a}{b}") } } + +/// Object that implements `StringUtil` +/// This lets us test what happens if foreign bindings pass in `StringUtilImpl` when `StringUtil` was expected. +/// Ideally, languages should coerce the object to it's trait, however that's not supported at this time. +/// The current test is simply that this doesn't cause a segfault (#2649) +#[derive(uniffi::Object)] +pub struct StringUtilObject { + separator: String, +} + +#[uniffi::export] +impl StringUtilObject { + #[uniffi::constructor] + pub fn new(separator: String) -> Self { + Self { separator } + } +} + +#[uniffi::export] +impl StringUtil for StringUtilObject { + fn concat(&self, a: &str, b: &str) -> String { + let separator = &self.separator; + format!("{a}{separator}{b}") + } +} + +#[uniffi::export] +pub fn concat_with_string_util(string_util: Arc, a: &str, b: &str) -> String { + string_util.concat(a, b) +} diff --git a/fixtures/coverall/tests/bindings/test_coverall.py b/fixtures/coverall/tests/bindings/test_coverall.py index 1c5d312232..9cddae5387 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.py +++ b/fixtures/coverall/tests/bindings/test_coverall.py @@ -502,6 +502,17 @@ def test_rust_only_traits(self): self.assertEqual(traits[0].concat("cow", "boy"), "cowboy") self.assertEqual(traits[1].concat("cow", "boy"), "cowboy") + def test_pass_object_to_function_that_input_trait(self): + string_util_obj = StringUtilObject("--") + + # StringUtilObject implements StringUtil. + # We currently don't support passing a `StringUtilObject` to a function that inputs a `dyn + # StringUtil`. + # This test checks that the result is a TypeError rather than a segfault (#2649) + + with self.assertRaises(TypeError): + concat_with_string_util(string_util_obj, "cow", "boy"), + def test_html_error(self): with self.assertRaises(HtmlError): validate_html("test") diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.py b/fixtures/proc-macro/tests/bindings/test_proc_macro.py index 82fe5c65ec..ecfd814c90 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.py +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.py @@ -30,7 +30,6 @@ assert trait_impl.concat_strings("foo", "bar") == "foobar" assert obj.get_trait(trait_impl).concat_strings("foo", "bar") == "foobar" assert concat_strings_by_ref(trait_impl, "foo", "bar") == "foobar" -assert issubclass(StructWithTrait, Trait) assert StructWithTrait("me").concat_strings("foo", "bar") == "me: foobar" trait_impl2 = obj.get_trait_with_foreign(None) diff --git a/uniffi_bindgen/src/bindings/python/pipeline/interfaces.rs b/uniffi_bindgen/src/bindings/python/pipeline/interfaces.rs index 18e31b7f83..449e03b670 100644 --- a/uniffi_bindgen/src/bindings/python/pipeline/interfaces.rs +++ b/uniffi_bindgen/src/bindings/python/pipeline/interfaces.rs @@ -43,13 +43,25 @@ pub fn pass(int: &mut Interface) -> Result<()> { Type::Interface { name, external_package_name, + imp, .. - } => (name, external_package_name), + } => { + // For trait interfaces implement in Rust-only, the protocol has `Protocol` appended. + // Trait interfaces with foreign implementations don't have that + match imp { + ObjectImpl::Trait => (format!("{name}Protocol"), external_package_name), + ObjectImpl::CallbackTrait => (name.to_string(), external_package_name), + ObjectImpl::Struct => { + bail!("Objects can only inherit from traits, not other objects") + } + } + } + Type::CallbackInterface { name, external_package_name, .. - } => (name, external_package_name), + } => (name.to_string(), external_package_name), _ => bail!("trait_ty {:?} isn't a trait", t), }; let fq = match external_package_name { diff --git a/uniffi_bindgen/src/bindings/python/pipeline/mod.rs b/uniffi_bindgen/src/bindings/python/pipeline/mod.rs index 351dce2e92..81b3f4f8f2 100644 --- a/uniffi_bindgen/src/bindings/python/pipeline/mod.rs +++ b/uniffi_bindgen/src/bindings/python/pipeline/mod.rs @@ -28,8 +28,8 @@ pub fn pipeline() -> Pipeline { .pass(config::pass) .pass(external_types::pass) .pass(names::pass) - .pass(modules::pass) .pass(interfaces::pass) + .pass(modules::pass) .pass(callback_interfaces::pass) .pass(types::pass) .pass(default::pass) diff --git a/uniffi_bindgen/src/bindings/python/pipeline/modules.rs b/uniffi_bindgen/src/bindings/python/pipeline/modules.rs index 9e30af2c2d..9498df69d6 100644 --- a/uniffi_bindgen/src/bindings/python/pipeline/modules.rs +++ b/uniffi_bindgen/src/bindings/python/pipeline/modules.rs @@ -33,7 +33,10 @@ pub fn pass(root: &mut Root) -> Result<()> { namespace.visit(|e: &Enum| exported_names.push(e.name.clone())); namespace.visit(|r: &Record| exported_names.push(r.name.clone())); namespace.visit(|f: &Function| exported_names.push(f.callable.name.clone())); - namespace.visit(|i: &Interface| exported_names.push(i.name.clone())); + namespace.visit(|i: &Interface| { + exported_names.push(i.name.clone()); + exported_names.push(i.protocol.name.clone()); + }); namespace.visit(|c: &CallbackInterface| exported_names.push(c.name.clone())); namespace.exported_names = exported_names;