Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to rename structs #2212

Open
pronebird opened this issue Aug 9, 2024 · 9 comments
Open

Make it possible to rename structs #2212

pronebird opened this issue Aug 9, 2024 · 9 comments

Comments

@pronebird
Copy link

pronebird commented Aug 9, 2024

Hi,

I'd like to be able to tweak the name of structs marked with uniffi::Record for Swift. I see that constructors and methods support custom names, but I can't find anything similar for structs.

@bendk
Copy link
Contributor

bendk commented Aug 14, 2024

This would be great and something that we've discussed. IMO the best way would be to do this in the uniffi.toml file, since you may want to rename things differently for different languages. #1426 covers that. Would that work for you or were you hoping for a different system?

@rafaelbeckel
Copy link
Contributor

It would be cool to support it through proc-macros just like it works for functions:

#[uniffi::export(name = "something")]
fn do_something() {
}

The same API for structs would be:

#[uniffi::export(name = "SomethingElse")]
impl Something {
}

A config file trying to change the same symbol would overwrite the custom name in the macro.

@pronebird
Copy link
Author

pronebird commented Aug 15, 2024

I think having everything in the source code would be the best. Since uniffi.toml supports mapping for individual languages, I think that the proc macro could account for that and provide additional attributes per language, i.e swift_name = .. which in absence of name could take precedence, although I am not sure if it's easy to support in macro rules. But even having a name attribute would be a great start!

#[uniffi::export(swift_name = "DNSSettings")]
struct DnsSettings {
  // fields
}

@bendk
Copy link
Contributor

bendk commented Aug 15, 2024

Can someone explain their use case a bit better? My first question is why not rename the struct in Rust to match what you want in the foreign language?

@rafaelbeckel
Copy link
Contributor

rafaelbeckel commented Aug 15, 2024

There's a simple use case of overriding the default CamelCase naming that uniffi exports. For example:

#[derive(uniffi::Object)]
pub struct ABCObject {}; // exported as AbcObject by default

I can think of some names that could benefit from customization: JSONSchema, DNSSetting (as @pronebird said), and DHLTrackingCode (some companies add prefixes to their internal libs).

Wasm-Bindgen has two keywords for that purpose, one for structs and one for impl blocks. I don't know exactly why they need two, but they're usually paired together:

#[wasm_bindgen(js_name = MyImage)] // avoids conflict with native JS Image
pub struct Image {};

#[wasm_bindgen(js_class = MyImage)]
impl Image {}

My specific use case is in a project that separates the FFI layer from the underlying Rust implementation so that they can evolve separately. The ffi module is responsible for wrapping some structs and reexporting them to other languages, using uniffi for mobile, wasm_bindgen for web, and extern functions for C.

It's convenient to do everything in one place without polluting the main implementation with annotations. For example:

use crate::Foo; // no annotations in Foo & allows extracting ffi module to another crate

#[repr(C)]
#[cfg_attr(wasm, wasm_bindgen(js_name = Foo))]
#[cfg_attr(mobile, derive(uniffi::Object))] // can't rename
pub struct FooWrapper {
    foo: Arc<Mutex<Foo>>,
}

#[cfg_attr(wasm, wasm_bindgen(js_class = Foo))]
#[cfg_attr(mobile, uniffi::export)] // can't rename
impl FooWrapper {
    pub fn some_foo_method(&mut self) {
        let mut foo = self.foo.lock().unwrap();
        foo.some_foo_method();
    }
}

#[no_mangle]
pub extern "C" fn foo_some_foo_method() {
    FooWrapper::some_foo_method()
}

I could rename the original on import, i.e., use crate::Foo as MyFoo and then name the wrapper Foo, but that messes with cbindgen (it ignores Rust namespacing and expects unique names for generating C headers).

@bendk
Copy link
Contributor

bendk commented Aug 15, 2024

Thanks for the example, that makes a lot of sense.

It seems like this is very similar to the uniffi.toml method, but for your use-case it's better to specify the configuration in code rather than in a separate TOML file.

I also feel like swift_name is better than name, since you're looking to set the final name without any more case transformations. If we just had a name field, then the naming would feel wrong if one of the foreign languages didn't use camel-case.

@mhammond
Copy link
Member

The 2 challenges I see here are:

  • it's perfectly fine to export multiple impl blocks, including the impl of traits etc. Names can also be used across crates, wrapped via custom types, etc. Covering all these edge-cases might be tricky.
  • I'm mildly against having binding specific attributes in the Rust source, mainly because I don't really understand how this might work for external bindings (and in particular, how the metadata system used by uniffi might support this capability)

I do understand that having it in the source seems more convenient, but I'm not sure this convenience factor outweighs the other concerns, nor that having 2 ways of specifying how bindings customize names etc is actually a win - eg, if we take this to the extreme, we could drop uniffi.toml entirely by having every possible binding customization be defined in the source code - but as above, how this might explode when trying to cover all possible bindings and all possible customizations isn't clear, and it's not clear that this specific feature request is more important to specify in the Rust source than any of the other customization options.

@rafaelbeckel
Copy link
Contributor

About impl blocks, I don't think they need to be renamed. Renaming the struct would suffice. I don't see the use case for renaming impl blocks differently of the struct it implements, being a trait impl or not.

Wasm Bindgen's design choice of renaming impl blocks (and having two different keywords at that) is a bit weird, and I assume it was due to compile-time technical constraints.

So, unless we face the same constraints, I'd prefer the annotation to happen at the struct level only. This would eliminate most of the challenges @mhammond mentioned.

For example:

#[derive(uniffi::Object(name="SomethingElse"))]
struct Something {
}

#[uniffi::export] // no need to rename it here, if technically possible
impl Something {
}

Another issue is that uniffi is fundamentally different than wasm-bindgen in the sense that we have multiple language targets, while they have JS only.

This imposes the challenge of potentially having different names for each target:

"python_name"
"kotlin_name"
"swift_name"
"ruby_name"

# 3rd-party
"golang_name"
"csharp_name"

Annotations could become not so pretty:

#[derive(uniffi::Object(python_name="SomethingElse", swift_name="SomethingElseEntirely", kotlin_name="SomethingElse", ruby_name="SomethingElse"))]
struct Something {
}

So, a default "name" could still be helpful:

#[derive(uniffi::Object(name="SomethingElse", swift_name="SomethingElseEntirely"))]
struct Something {
}

And, of course, one could scope the renaming to one target only:

#[derive(uniffi::Object(swift_name="SomethingElseEntirely"))]
struct Something {
}

@rafaelbeckel
Copy link
Contributor

To be clear, my suggestions are just brainstorming. They'd be convenient as an option, but it would be fine if this feature were available via config files only. I wouldn't mind setting the custom names in uniffi.toml.

We can continue the discussion at #1426.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants