Reimplement Python bindings using the pipeline code

docs/manual/src/internals/bindings_ir_pipeline.md

@@ -65,6 +65,33 @@ pub struct Node {
 }
 ```
 
+### Field Conversion order
+
+Fields are converted in declaration order.
+This matters when a field is listed twice with one of them using `#[node(from(<name_of_the_another_field>)]`.
+In this case the first field declared will be initialized with the data from the previous pass and the second field will be initialized to the default value.
+
+You can take advantage of this to convert an optional value into a non-optional value.
+For example:
+
+```rust
+#[derive(Node)]
+pub struct SourceNode {
+    foo: Option<String>
+}
+
+#[derive(Node)]
+pub struct DestNode {
+    /// This field will be set to `SourceNode::foo`
+    #[node(from(foo))]
+    maybe_foo: Option<String>
+    /// This field will be a non-optional version of `SourceNode::foo`.
+    /// It will be initialized to an empty string, then a pipeline pass will populate it using
+    // `maybe_foo` combined with logic to handle the `None` case.
+    foo: String,
+}
+```
+
 ## Module structure
 
 Each IRs will typically have a module dedicated to them with the following structure:

examples/custom-types/uniffi.toml

@@ -50,9 +50,10 @@ lower = "{}.toString()"
 
 [bindings.python.custom_types.Url]
 # We're going to be the urllib.parse.ParseResult class, which is the closest
-# thing Python has to a Url class.  No need to specify `type_name` though,
-# since Python is loosely typed.
-# modules to import
+# thing Python has to a Url class. 
+
+# `type_name` is optional, but it improves Python's optional type-checking
+type_name = "urllib.parse.ParseResult"
 imports = ["urllib.parse"]
 # Function to lift a string from Rust into a Python URL.
 lift = "urllib.parse.urlparse({})"

fixtures/docstring-proc-macro/tests/test_generated_bindings.rs

@@ -99,11 +99,6 @@ mod tests {
         test_docstring(KotlinBindingGenerator, "kt");
     }
 
-    #[test]
-    fn test_docstring_python() {
-        test_docstring(PythonBindingGenerator, "py");
-    }
-
     #[test]
     fn test_docstring_swift() {
         test_docstring(SwiftBindingGenerator, "swift");

fixtures/docstring/tests/test_generated_bindings.rs

@@ -89,11 +89,6 @@ mod tests {
         test_docstring(KotlinBindingGenerator, "kt");
     }
 
-    #[test]
-    fn test_docstring_python() {
-        test_docstring(PythonBindingGenerator, "py");
-    }
-
     #[test]
     fn test_docstring_swift() {
         test_docstring(SwiftBindingGenerator, "swift");

uniffi/src/cli/uniffi_bindgen.rs

@@ -234,16 +234,7 @@ fn gen_library_mode(
                 fmt,
             )?
             .len(),
-            TargetLanguage::Python => generate_bindings(
-                library_path,
-                crate_name.clone(),
-                &PythonBindingGenerator,
-                &config_supplier,
-                cfo,
-                out_dir,
-                fmt,
-            )?
-            .len(),
+            TargetLanguage::Python => anyhow::bail!("Python uses the new Bindings IR pipeline"),
             TargetLanguage::Ruby => generate_bindings(
                 library_path,
                 crate_name.clone(),
@@ -290,15 +281,7 @@ fn gen_bindings(
                 crate_name,
                 fmt,
             )?,
-            TargetLanguage::Python => generate_bindings(
-                udl_file,
-                cfo,
-                PythonBindingGenerator,
-                odo,
-                library_file,
-                crate_name,
-                fmt,
-            )?,
+            TargetLanguage::Python => anyhow::bail!("Python uses the new Bindings IR pipeline"),
             TargetLanguage::Ruby => generate_bindings(
                 udl_file,
                 cfo,
@@ -336,36 +319,61 @@ pub fn run_main() -> anyhow::Result<()> {
             library_mode,
             metadata_no_deps,
         } => {
-            if library_mode {
-                if lib_file.is_some() {
-                    panic!("--lib-file is not compatible with --library.")
-                }
-                let out_dir = out_dir.expect("--out-dir is required when using --library");
-                if language.is_empty() {
-                    panic!("please specify at least one language with --language")
-                }
-                gen_library_mode(
-                    &source,
-                    crate_name,
-                    language,
-                    config.as_deref(),
-                    &out_dir,
-                    !no_format,
-                    metadata_no_deps,
-                )?;
-            } else {
-                if metadata_no_deps {
-                    panic!("--metadata-no-deps makes no sense when not in library mode")
+            if language.is_empty() {
+                panic!("please specify at least one language with --language")
+            }
+            let (pipeline_languages, legacy_languages) = split_languages(language);
+
+            // Handle languages that use the bindings IR pipeline
+            for language in pipeline_languages {
+                let out_dir = out_dir
+                    .as_ref()
+                    .expect("--out-dir is required when generating {language} bindings");
+                let config_supplier = config_supplier(metadata_no_deps)?;
+                let initial_root = if library_mode {
+                    initial::Root::from_library(config_supplier, &source, crate_name.clone())?
+                } else {
+                    initial::Root::from_udl(config_supplier, &source, crate_name.clone())?
+                };
+
+                match language {
+                    TargetLanguage::Python => python::run_pipeline(initial_root, out_dir)?,
+                    language => {
+                        unimplemented!("{language} does not use the bindings IR pipeline yet")
+                    }
+                };
+            }
+
+            // Handle languages on the legacy system
+            if !legacy_languages.is_empty() {
+                if library_mode {
+                    if lib_file.is_some() {
+                        panic!("--lib-file is not compatible with --library.")
+                    }
+                    let out_dir = out_dir.expect("--out-dir is required when using --library");
+                    gen_library_mode(
+                        &source,
+                        crate_name,
+                        legacy_languages,
+                        config.as_deref(),
+                        &out_dir,
+                        !no_format,
+                        metadata_no_deps,
+                    )?;
+                } else {
+                    if metadata_no_deps {
+                        panic!("--metadata-no-deps makes no sense when not in library mode")
+                    }
+                    gen_bindings(
+                        &source,
+                        config.as_deref(),
+                        legacy_languages,
+                        out_dir.as_deref(),
+                        lib_file.as_deref(),
+                        crate_name.as_deref(),
+                        !no_format,
+                    )?;
                 }
-                gen_bindings(
-                    &source,
-                    config.as_deref(),
-                    language,
-                    out_dir.as_deref(),
-                    lib_file.as_deref(),
-                    crate_name.as_deref(),
-                    !no_format,
-                )?;
             }
         }
         Commands::Scaffolding {
@@ -401,3 +409,17 @@ pub fn run_main() -> anyhow::Result<()> {
     };
     Ok(())
 }
+
+/// Split the language vec into languages that have migrated to the pipeline and those that
+/// haven't.
+fn split_languages(languages: Vec<TargetLanguage>) -> (Vec<TargetLanguage>, Vec<TargetLanguage>) {
+    let mut pipeline = vec![];
+    let mut legacy = vec![];
+    for l in languages {
+        match l {
+            TargetLanguage::Python => pipeline.push(l),
+            _ => legacy.push(l),
+        }
+    }
+    (pipeline, legacy)
+}

uniffi/src/lib.rs

@@ -16,9 +16,7 @@ pub use uniffi_bindgen::cargo_metadata::CrateConfigSupplier as CargoMetadataConf
 pub use uniffi_bindgen::library_mode::generate_bindings as generate_bindings_library_mode;
 #[cfg(feature = "bindgen")]
 pub use uniffi_bindgen::{
-    bindings::{
-        KotlinBindingGenerator, PythonBindingGenerator, RubyBindingGenerator, SwiftBindingGenerator,
-    },
+    bindings::{KotlinBindingGenerator, RubyBindingGenerator, SwiftBindingGenerator},
     generate_bindings, print_repr,
 };
 #[cfg(feature = "build")]

uniffi_bindgen/Cargo.toml

@@ -25,7 +25,7 @@ fs-err = "2.7.0"
 glob = "0.3"
 goblin = "0.8"
 heck = "0.5"
-indexmap = { version = "2.2" }
+indexmap = { version = "2.2", features = ["serde"] }
 once_cell = "1.12"
 serde = { version = "1", features = ["derive"] }
 tempfile = "3"

uniffi_bindgen/src/bindings/mod.rs

@@ -10,7 +10,6 @@
 mod kotlin;
 pub use kotlin::KotlinBindingGenerator;
 pub mod python;
-pub use python::PythonBindingGenerator;
 mod ruby;
 pub use ruby::RubyBindingGenerator;
 mod swift;

uniffi_bindgen/src/bindings/python/filters.rs

@@ -0,0 +1,66 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+/// Template filters for Askama
+///
+/// In general, prefer adding fields using a pipeline pass to writing filters.
+/// That's allows devs to use the `pipeline` command to follow what's going on.
+///
+/// We currently only use filter functions for a couple reasons:
+///
+/// * When we want to leverage `AsRef` to implement something for any type that has an `AsRef`
+///   relationship.  This could be done using a pipeline pass, but it could be annoying/distracting
+///   to add fields like `type_name,` `lower_fn`, etc. to so many different Node structs.
+/// * When we want to implement somewhat complex display logic, like in the `docstring` filter.
+///   Implementing this as a pipeline pass means the pass would need to know how much each
+///   docstring gets indented, which doesn't seem right.
+use super::pipeline::nodes::*;
+use askama::Result;
+
+pub fn type_name(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(ty.as_ref().type_name.clone())
+}
+
+pub fn ffi_converter_name(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(ty.as_ref().ffi_converter_name.clone())
+}
+
+pub fn lower_fn(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(format!("{}.lower", ty.as_ref().ffi_converter_name))
+}
+
+pub fn check_lower_fn(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(format!("{}.check_lower", ty.as_ref().ffi_converter_name))
+}
+
+pub fn lift_fn(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(format!("{}.lift", ty.as_ref().ffi_converter_name))
+}
+
+pub fn write_fn(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(format!("{}.write", ty.as_ref().ffi_converter_name))
+}
+
+pub fn read_fn(ty: impl AsRef<TypeNode>) -> Result<String> {
+    Ok(format!("{}.read", ty.as_ref().ffi_converter_name))
+}
+
+/// Get the idiomatic Python rendering of docstring
+///
+/// If the docstring is set, this returns an indented Python docstring with
+/// a trailing newline. If not, it returns the empty string.
+///
+/// This makes it so the template code can use something like
+/// `{{ item.docstring|docstring(4) -}}` to render the correct docstring in both cases.
+pub fn docstring(docstring: &Option<String>, indent: usize) -> Result<String> {
+    let Some(docstring) = docstring.as_deref() else {
+        return Ok("".to_string());
+    };
+    let docstring = textwrap::dedent(docstring);
+    let indent = " ".repeat(indent);
+    // Escape triple quotes to avoid syntax error
+    let escaped = docstring.replace(r#"""""#, r#"\"\"\""#);
+    let indented = textwrap::indent(&escaped, &indent);
+    Ok(format!("\"\"\"\n{indented}\n\"\"\"\n{indent}"))
+}