Update the BindingGenerator trait and remove BindingsConfig.

[mhammond]

This is a breaking change for BindingGenerators, and follows up on other breaking changes made for this release (#2078). Between them, it is intended to offer a better framework for binding generators for future versions and break unintentional coupling between uniffi_bindgen and the builtin bindings.

This patch updates the BindingGenerator trait to give binding generators more control over the binding generation process and to simplify the interactions between the generator and uniffi_bindgen.

The trait BindingsConfig has been removed and replaced with a new method on BindingGenerator which passes the generator the entire list of all ComponentInterface and Config objects to be used in the generation, which the generator can modify as necessary. The binding generator is also passed the entire list of items to generate rather than called once per item - this gives the generator more flexibility in how the items are generated.

A new Component struct has been introduced which holds all necessary information for a single crate/namespace, including the ComponentInterface and Config. These structs are passed to the BindingGenerator

Calling everyone with an interest in external binding generators - cc @heinrich5991 @SalvatoreT @arg0d @skeet70 @Lipt0nas. All feedback welcome, including where I went too far, or where I didn't go far enough - eg

• I was thinking a new GenerationSettings struct which held things like out_dir, try_format_code, cdylib etc to make the function argument lists smaller and to make future additions less of a breaking change (eg, a new option would not change signatures, but instead just add a new item to a struct). etc.
• I'm still not that happy with how each config is created, but don't have any ideas how to fix that. Trying to remove Config as a type param and use toml::Value directly doesn't seem particularly viable. I'd welcome ideas here.

[heinrich5991]

I don't maintain external binding generators.

[arg0d]

Looks good, this is only a "mechanical" breaking change, shouldn't cause much difficulties migrating to this new vesrion.

But I wonder what would be some real use cases for accessing other component interfaces when generating code. I remember I wanted to access other component interfaces for some purpose when working with C# generator, but in the end it wasn't required. The only use case I can think of right now, is the ability to index external types from another component interface. For that purpose, it would be more useful to have components: HashMap instead of components: Vec. But like I said, this is not based on any present requirements.

[mhammond]

But I wonder what would be some real use cases for accessing other component interfaces when generating code.

Yeah, for now it's all going to be about external types - kotlin uses that to work out what "package" name to use to refer to external types in other crates.

[bendk]

This looks good to me, it's certainly an improvement. I think it maybe could be pushed further though. I love the idea of a GeneratorSetttings struct, I also posted some thoughts on simplifying the trait methods.

[bendk]

I think the purpose is to prevent static libraries being passed on the CLI when we're building for Kotlin. Static libraries can work for swift, but none of the other bindings. Maybe we could replace the 2 params with an enum?

enum Library {
   Shared(Utf8PathBuf),
   Static(Utf8PathBuf),
   None,
}

[mhammond]

Thanks for that - I think even better is to just have the CLI enforce that! My new version will do that.

[skeet70]

This looks fine to me, but even though I'm close to having Java binding generation working, I don't think I understand what's going on well enough to know if this is an improvement or not. For what I'm doing this mostly seems different, not necessarily good or bad.

[Lipt0nas]

Looks good, accessing other component interfaces should be useful for languages where import order matters

I am very interested in the GenerationSettings idea, would reduce a lot of friction when adding additional configuration options, and would work especially well for opt-in features like formatting

[mhammond]

Thanks for all the feedback - I incorporated much of it in the most recent version, including the new GenerationSettings.

I'm fairly happy with this so think it's ready for review, but still obviously open to all feedback.

[bendk]

I like this a lot. GenerationSettings seems like a nice step forward and in general this feels simpler.

[mhammond]

I meant to reply to:

The only use case I can think of right now, is the ability to index external types from another component interface. For that purpose, it would be more useful to have components: HashMap instead of components: Vec.

All the bindings do iterate over it - only kotlin builds its own map. Some bindings might need a map keyed by something over the crate name. And this is more convenient for the bindgen :) So I left the arrays. Welcome suggestions for further feedback in followups though.

Thanks everyone.