Skip to content

Commit

Permalink
Python: Fix class hierarchy generated for traits. Fixes mozilla#2264.
Browse files Browse the repository at this point in the history
  • Loading branch information
mhammond committed Oct 11, 2024
1 parent 69ecfbd commit b5a44de
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

- `uniffi.toml` of crates without a `lib` type where ignored in 0.28.1
- Python: Fixed a bug when enum/error names were not proper camel case (HTMLError instead of HtmlError).
- Python: Fixed the class hierarcy generated for traits ((#2264)[https://github.com/mozilla/uniffi-rs/issues/2264])

[All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.28.1...HEAD).

Expand Down
4 changes: 4 additions & 0 deletions fixtures/proc-macro/tests/bindings/test_proc_macro.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

from proc_macro import *
from proc_macro import TraitWithForeignImpl

one = make_one(123)
assert one.inner == 123
Expand All @@ -25,11 +26,14 @@
assert(rename_test())

trait_impl = obj.get_trait(None)
assert isinstance(trait_impl, Trait)
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"

trait_impl2 = obj.get_trait_with_foreign(None)
# This is an instance of `TraitWithForeignImpl` - `TraitWithForeign` is used primarily for subclassing.
assert isinstance(trait_impl2, TraitWithForeignImpl)
assert trait_impl2.name() == "RustTraitImpl"
assert obj.get_trait_with_foreign(trait_impl2).name() == "RustTraitImpl"

Expand Down
23 changes: 0 additions & 23 deletions uniffi_bindgen/src/bindings/python/gen_python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,24 +423,6 @@ impl PythonCodeOracle {
None => "0".to_owned(),
}
}

/// Get the name of the protocol and class name for an object.
///
/// If we support callback interfaces, the protocol name is the object name, and the class name is derived from that.
/// Otherwise, the class name is the object name and the protocol name is derived from that.
///
/// This split determines what types `FfiConverter.lower()` inputs. If we support callback
/// interfaces, `lower` must lower anything that implements the protocol. If not, then lower
/// only lowers the concrete class.
fn object_names(&self, obj: &Object) -> (String, String) {
let class_name = self.class_name(obj.name());
if obj.has_callback_interface() {
let impl_name = format!("{class_name}Impl");
(class_name, impl_name)
} else {
(format!("{class_name}Protocol"), class_name)
}
}
}

impl VisitMut for PythonCodeOracle {
Expand Down Expand Up @@ -659,11 +641,6 @@ pub mod filters {
Ok(PythonCodeOracle.ffi_struct_name(nm))
}

/// Get the idiomatic Python rendering of an individual enum variant.
pub fn object_names(obj: &Object) -> Result<(String, String), askama::Error> {
Ok(PythonCodeOracle.object_names(obj))
}

/// Get the idiomatic Python rendering of docstring
pub fn docstring(docstring: &str, spaces: &i32) -> Result<String, askama::Error> {
let docstring = textwrap::dedent(docstring);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{%- let cbi = ci|get_callback_interface_definition(name) %}
{%- let ffi_init_callback = cbi.ffi_init_callback() %}
{%- let protocol_name = type_name.clone() %}
{%- let protocol_base_class = "typing.Protocol" %}
{%- let protocol_docstring = cbi.docstring() %}
{%- let vtable = cbi.vtable() %}
{%- let methods = cbi.methods() %}
Expand Down
23 changes: 20 additions & 3 deletions uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
{%- let obj = ci|get_object_definition(name) %}
{%- let (protocol_name, impl_name) = obj|object_names %}
{%- let methods = obj.methods() %}
{%- let protocol_name = format!("{type_name}Protocol") %}
{%- let protocol_docstring = obj.docstring() %}
{%- let protocol_base_class = "typing.Protocol" %}
{%- include "Protocol.py" %}

{% include "Protocol.py" %}
{%- let impl_name %}
{%- if obj.has_callback_interface() %}
# {{ type_name }} is a foreign trait so treated like a callback interface, where the
# primary use-case is the trait being implemented locally.
# It is a base-class local implementations might subclass.
{# We reuse "Protocol.py" for this, even though here we are not generating a protocol #}
{%- let protocol_name = format!("{type_name}") %}
{%- let protocol_base_class = "" %}
{% include "Protocol.py" %}

{%- let impl_name = format!("{type_name}Impl") %}
# `{{ impl_name }}` is the implementation for a Rust implemented version.
{%- else %}
# {{ type_name }} is a Rust-only trait - it's a wrapper around a Rust implementation.
{%- let impl_name = type_name.clone() %}
{%- endif %}

{% if ci.is_name_used_as_error(name) %}
{%- if ci.is_name_used_as_error(name) %}
class {{ impl_name }}(Exception):
{%- else %}
class {{ impl_name }}:
Expand Down
3 changes: 2 additions & 1 deletion uniffi_bindgen/src/bindings/python/templates/Protocol.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class {{ protocol_name }}(typing.Protocol):
{# misnamed - a generic "abstract base class". Used as both a protocol and an ABC for traits. #}
class {{ protocol_name }}({{ protocol_base_class }}):
{%- call py::docstring_value(protocol_docstring, 4) %}
{%- for meth in methods.iter() %}
def {{ meth.name() }}(self, {% call py::arg_list_decl(meth) %}):
Expand Down

0 comments on commit b5a44de

Please sign in to comment.