Skip to content

Commit

Permalink
Update the BindingGenerator trait and remove BindingsConfig.
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
mhammond committed May 5, 2024
1 parent 139a024 commit 06e58c7
Show file tree
Hide file tree
Showing 12 changed files with 241 additions and 285 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

### What's changed?
- The internal bindings generation has changed to make it friendlier for external language bindings.
However, this is likely to be a small **breaking change** for these bindings.
However, this a **breaking change** for these bindings.
No consumers of any languages are impacted, only the maintainers of these language bindings.
([#2066](https://github.com/mozilla/uniffi-rs/issues/2066))
([#2066](https://github.com/mozilla/uniffi-rs/issues/2066)), ([#2094](https://github.com/mozilla/uniffi-rs/pull/2094))

- The async runtime can be specified for constructors/methods, this will override the runtime specified at the impl block level.

Expand Down
30 changes: 3 additions & 27 deletions uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use serde::{Deserialize, Serialize};
use crate::backend::TemplateExpression;

use crate::interface::*;
use crate::BindingsConfig;

mod callback_interface;
mod compounds;
Expand Down Expand Up @@ -72,13 +71,13 @@ trait CodeType: Debug {
// config options to customize the generated Kotlin.
#[derive(Debug, Default, Clone, Serialize, Deserialize)]
pub struct Config {
package_name: Option<String>,
cdylib_name: Option<String>,
pub(super) package_name: Option<String>,
pub(super) cdylib_name: Option<String>,
generate_immutable_records: Option<bool>,
#[serde(default)]
custom_types: HashMap<String, CustomTypeConfig>,
#[serde(default)]
external_packages: HashMap<String, String>,
pub(super) external_packages: HashMap<String, String>,
#[serde(default)]
android: bool,
#[serde(default)]
Expand Down Expand Up @@ -122,29 +121,6 @@ impl Config {
}
}

impl BindingsConfig for Config {
fn update_from_ci(&mut self, ci: &ComponentInterface) {
self.package_name
.get_or_insert_with(|| format!("uniffi.{}", ci.namespace()));
self.cdylib_name
.get_or_insert_with(|| format!("uniffi_{}", ci.namespace()));
}

fn update_from_cdylib_name(&mut self, cdylib_name: &str) {
self.cdylib_name
.get_or_insert_with(|| cdylib_name.to_string());
}

fn update_from_dependency_configs(&mut self, config_map: HashMap<&str, &Self>) {
for (crate_name, config) in config_map {
if !self.external_packages.contains_key(crate_name) {
self.external_packages
.insert(crate_name.to_string(), config.package_name());
}
}
}
}

// Generate kotlin bindings for the given ComponentInterface, as a string.
pub fn generate_bindings(config: &Config, ci: &ComponentInterface) -> Result<String> {
KotlinWrapper::new(config.clone(), ci)
Expand Down
76 changes: 55 additions & 21 deletions uniffi_bindgen/src/bindings/kotlin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use crate::{BindingGenerator, ComponentInterface};
use crate::{BindingGenerator, Component};
use anyhow::Result;
use camino::{Utf8Path, Utf8PathBuf};
use fs_err as fs;
use std::collections::HashMap;
use std::process::Command;

mod gen_kotlin;
Expand All @@ -26,33 +27,66 @@ impl BindingGenerator for KotlinBindingGenerator {
)
}

fn write_bindings(
fn update_component_configs(
&self,
ci: &ComponentInterface,
config: &Config,
out_dir: &Utf8Path,
try_format_code: bool,
cdylib: &Option<&str>,
library_path: Option<&Utf8Path>,
components: &mut Vec<Component<Self::Config>>,
) -> Result<()> {
let mut kt_file = full_bindings_path(config, out_dir);
fs::create_dir_all(&kt_file)?;
kt_file.push(format!("{}.kt", ci.namespace()));
fs::write(&kt_file, generate_bindings(config, ci)?)?;
if try_format_code {
if let Err(e) = Command::new("ktlint").arg("-F").arg(&kt_file).output() {
println!(
"Warning: Unable to auto-format {} using ktlint: {e:?}",
kt_file.file_name().unwrap(),
);
// I think this is just trying to insist on a cdylib being able to be obtained from the lib_path?
if let (None, Some(l)) = (cdylib, library_path) {
anyhow::bail!("Generate bindings for Kotlin requires a cdylib, but {l} was given");
}

for c in &mut *components {
c.config
.package_name
.get_or_insert_with(|| format!("uniffi.{}", c.ci.namespace()));
c.config.cdylib_name.get_or_insert_with(|| {
cdylib
.map(Into::into)
.unwrap_or_else(|| format!("uniffi_{}", c.ci.namespace()))
});
}
// We need to update package names
let packages = HashMap::<String, String>::from_iter(
components
.iter()
.map(|c| (c.ci.crate_name().to_string(), c.config.package_name())),
);
for c in components {
for (ext_crate, ext_package) in &packages {
if ext_crate != c.ci.crate_name()
&& !c.config.external_packages.contains_key(ext_crate)
{
c.config
.external_packages
.insert(ext_crate.to_string(), ext_package.clone());
}
}
}
Ok(())
}

fn check_library_path(&self, library_path: &Utf8Path, cdylib_name: Option<&str>) -> Result<()> {
if cdylib_name.is_none() {
anyhow::bail!(
"Generate bindings for Kotlin requires a cdylib, but {library_path} was given"
);
fn write_bindings(
&self,
components: &[Component<Self::Config>],
out_dir: &Utf8Path,
try_format_code: bool,
) -> Result<()> {
for Component { ci, config, .. } in components {
let mut kt_file = full_bindings_path(config, out_dir);
fs::create_dir_all(&kt_file)?;
kt_file.push(format!("{}.kt", ci.namespace()));
fs::write(&kt_file, generate_bindings(config, ci)?)?;
if try_format_code {
if let Err(e) = Command::new("ktlint").arg("-F").arg(&kt_file).output() {
println!(
"Warning: Unable to auto-format {} using ktlint: {e:?}",
kt_file.file_name().unwrap(),
);
}
}
}
Ok(())
}
Expand Down
15 changes: 1 addition & 14 deletions uniffi_bindgen/src/bindings/python/gen_python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use std::fmt::Debug;
use crate::backend::TemplateExpression;

use crate::interface::*;
use crate::BindingsConfig;

mod callback_interface;
mod compounds;
Expand Down Expand Up @@ -112,7 +111,7 @@ static KEYWORDS: Lazy<HashSet<String>> = Lazy::new(|| {
// Config options to customize the generated python.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct Config {
cdylib_name: Option<String>,
pub(super) cdylib_name: Option<String>,
#[serde(default)]
custom_types: HashMap<String, CustomTypeConfig>,
#[serde(default)]
Expand Down Expand Up @@ -148,18 +147,6 @@ impl Config {
}
}

impl BindingsConfig for Config {
fn update_from_ci(&mut self, ci: &ComponentInterface) {
self.cdylib_name
.get_or_insert_with(|| format!("uniffi_{}", ci.namespace()));
}

fn update_from_cdylib_name(&mut self, cdylib_name: &str) {
self.cdylib_name
.get_or_insert_with(|| cdylib_name.to_string());
}
}

// Generate python bindings for the given ComponentInterface, as a string.
pub fn generate_python_bindings(config: &Config, ci: &ComponentInterface) -> Result<String> {
PythonWrapper::new(config.clone(), ci)
Expand Down
55 changes: 34 additions & 21 deletions uniffi_bindgen/src/bindings/python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use fs_err as fs;

mod gen_python;
mod test;
use super::super::interface::ComponentInterface;
use crate::Component;
use gen_python::{generate_python_bindings, Config};
pub use test::{run_script, run_test};

Expand All @@ -28,34 +28,47 @@ impl crate::BindingGenerator for PythonBindingGenerator {
)
}

fn write_bindings(
fn update_component_configs(
&self,
ci: &ComponentInterface,
config: &Config,
out_dir: &Utf8Path,
try_format_code: bool,
cdylib: &Option<&str>,
library_path: Option<&Utf8Path>,
components: &mut Vec<Component<Self::Config>>,
) -> Result<()> {
let py_file = out_dir.join(format!("{}.py", ci.namespace()));
fs::write(&py_file, generate_python_bindings(config, ci)?)?;

if try_format_code {
if let Err(e) = Command::new("yapf").arg(&py_file).output() {
println!(
"Warning: Unable to auto-format {} using yapf: {e:?}",
py_file.file_name().unwrap(),
)
}
// I think this is just trying to insist on a cdylib being able to be obtained from the lib_path?
if let (None, Some(l)) = (cdylib, library_path) {
anyhow::bail!("Generate bindings for Python requires a cdylib, but {l} was given");
}

for c in &mut *components {
c.config.cdylib_name.get_or_insert_with(|| {
cdylib
.map(Into::into)
.unwrap_or_else(|| format!("uniffi_{}", c.ci.namespace()))
});
}
Ok(())
}

fn check_library_path(&self, library_path: &Utf8Path, cdylib_name: Option<&str>) -> Result<()> {
if cdylib_name.is_none() {
anyhow::bail!(
"Generate bindings for Python requires a cdylib, but {library_path} was given"
);
fn write_bindings(
&self,
components: &[Component<Self::Config>],
out_dir: &Utf8Path,
try_format_code: bool,
) -> Result<()> {
for Component { ci, config, .. } in components {
let py_file = out_dir.join(format!("{}.py", ci.namespace()));
fs::write(&py_file, generate_python_bindings(config, ci)?)?;

if try_format_code {
if let Err(e) = Command::new("yapf").arg(&py_file).output() {
println!(
"Warning: Unable to auto-format {} using yapf: {e:?}",
py_file.file_name().unwrap(),
)
}
}
}

Ok(())
}
}
15 changes: 1 addition & 14 deletions uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use serde::{Deserialize, Serialize};
use std::borrow::Borrow;

use crate::interface::*;
use crate::BindingsConfig;

const RESERVED_WORDS: &[&str] = &[
"alias", "and", "BEGIN", "begin", "break", "case", "class", "def", "defined?", "do", "else",
Expand Down Expand Up @@ -82,7 +81,7 @@ pub fn canonical_name(t: &Type) -> String {
// since the details of the underlying component are entirely determined by the `ComponentInterface`.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct Config {
cdylib_name: Option<String>,
pub(super) cdylib_name: Option<String>,
cdylib_path: Option<String>,
}

Expand All @@ -102,18 +101,6 @@ impl Config {
}
}

impl BindingsConfig for Config {
fn update_from_ci(&mut self, ci: &ComponentInterface) {
self.cdylib_name
.get_or_insert_with(|| format!("uniffi_{}", ci.namespace()));
}

fn update_from_cdylib_name(&mut self, cdylib_name: &str) {
self.cdylib_name
.get_or_insert_with(|| cdylib_name.to_string());
}
}

#[derive(Template)]
#[template(syntax = "rb", escape = "none", path = "wrapper.rb")]
pub struct RubyWrapper<'a> {
Expand Down
54 changes: 33 additions & 21 deletions uniffi_bindgen/src/bindings/ruby/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use std::process::Command;

use crate::{BindingGenerator, ComponentInterface};
use crate::{BindingGenerator, Component, ComponentInterface};
use anyhow::{Context, Result};
use camino::Utf8Path;
use fs_err as fs;
Expand All @@ -27,33 +27,45 @@ impl BindingGenerator for RubyBindingGenerator {
)
}

fn write_bindings(
fn update_component_configs(
&self,
ci: &ComponentInterface,
config: &Config,
out_dir: &Utf8Path,
try_format_code: bool,
cdylib: &Option<&str>,
library_path: Option<&Utf8Path>,
components: &mut Vec<Component<Self::Config>>,
) -> Result<()> {
let rb_file = out_dir.join(format!("{}.rb", ci.namespace()));
fs::write(&rb_file, generate_ruby_bindings(config, ci)?)?;

if try_format_code {
if let Err(e) = Command::new("rubocop").arg("-A").arg(&rb_file).output() {
println!(
"Warning: Unable to auto-format {} using rubocop: {e:?}",
rb_file.file_name().unwrap(),
)
}
// I think this is just trying to insist on a cdylib being able to be obtained from the lib_path?
if let (None, Some(l)) = (cdylib, library_path) {
anyhow::bail!("Generate bindings for Ruby requires a cdylib, but {l} was given");
}

for c in &mut *components {
c.config.cdylib_name.get_or_insert_with(|| {
cdylib
.map(Into::into)
.unwrap_or_else(|| format!("uniffi_{}", c.ci.namespace()))
});
}
Ok(())
}

fn check_library_path(&self, library_path: &Utf8Path, cdylib_name: Option<&str>) -> Result<()> {
if cdylib_name.is_none() {
anyhow::bail!(
"Generate bindings for Ruby requires a cdylib, but {library_path} was given"
);
fn write_bindings(
&self,
components: &[Component<Self::Config>],
out_dir: &Utf8Path,
try_format_code: bool,
) -> Result<()> {
for Component { ci, config, .. } in components {
let rb_file = out_dir.join(format!("{}.rb", ci.namespace()));
fs::write(&rb_file, generate_ruby_bindings(config, ci)?)?;

if try_format_code {
if let Err(e) = Command::new("rubocop").arg("-A").arg(&rb_file).output() {
println!(
"Warning: Unable to auto-format {} using rubocop: {e:?}",
rb_file.file_name().unwrap(),
)
}
}
}
Ok(())
}
Expand Down
Loading

0 comments on commit 06e58c7

Please sign in to comment.