From 9b6b6b544bdea0ec386829ff518fbc2e967585e1 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Mon, 9 Oct 2023 16:36:37 -0600 Subject: [PATCH 01/10] Add doc generation to library mode bindings --- uniffi_bindgen/src/library_mode.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/uniffi_bindgen/src/library_mode.rs b/uniffi_bindgen/src/library_mode.rs index a60909c07f..8f67e10ed6 100644 --- a/uniffi_bindgen/src/library_mode.rs +++ b/uniffi_bindgen/src/library_mode.rs @@ -76,7 +76,12 @@ pub fn generate_bindings( } } - for source in sources.iter() { + for source in sources.iter_mut() { + if source.config.bindings.doc_comments.unwrap_or_default() { + let path = &source.package.manifest_path.with_file_name("src/lib.rs"); + let documentation = uniffi_docs::extract_documentation_from_path(path)?; + source.ci.attach_documentation(documentation); + } for &language in target_languages { if cdylib_name.is_none() && language != TargetLanguage::Swift { bail!("Generate bindings for {language} requires a cdylib, but {library_path} was given"); From c1b8ce4614b5ad4ecbc4863196377b92bdf49e26 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Mon, 9 Oct 2023 17:00:54 -0600 Subject: [PATCH 02/10] Fix the merge --- uniffi_bindgen/src/lib.rs | 8 ++++++++ uniffi_bindgen/src/library_mode.rs | 13 ++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/uniffi_bindgen/src/lib.rs b/uniffi_bindgen/src/lib.rs index 7604d2e9f7..6cc1e2583c 100644 --- a/uniffi_bindgen/src/lib.rs +++ b/uniffi_bindgen/src/lib.rs @@ -129,6 +129,10 @@ pub trait BindingsConfig: DeserializeOwned { /// config_map maps crate names to config instances. This is mostly used to set up external /// types. fn update_from_dependency_configs(&mut self, config_map: HashMap<&str, &Self>); + + fn doc_generation_enabled(&self) -> bool { + false + } } /// Binding generator config with no members @@ -497,6 +501,10 @@ impl BindingsConfig for Config { .collect(), ); } + + fn doc_generation_enabled(&self) -> bool { + self.bindings.doc_comments.unwrap_or_default() + } } // FIXME(HACK): diff --git a/uniffi_bindgen/src/library_mode.rs b/uniffi_bindgen/src/library_mode.rs index ed270acb82..42b7f1c3eb 100644 --- a/uniffi_bindgen/src/library_mode.rs +++ b/uniffi_bindgen/src/library_mode.rs @@ -100,13 +100,12 @@ pub fn generate_external_bindings( } } - if source.config.bindings.doc_comments.unwrap_or_default() { - let path = &source.package.manifest_path.with_file_name("src/lib.rs"); - let documentation = uniffi_docs::extract_documentation_from_path(path)?; - source.ci.attach_documentation(documentation); - } - - for source in sources.iter() { + for source in sources.iter_mut() { + if source.config.doc_generation_enabled() { + let path = &source.package.manifest_path.with_file_name("src/lib.rs"); + let documentation = uniffi_docs::extract_documentation_from_path(path)?; + source.ci.attach_documentation(documentation); + } binding_generator.write_bindings(&source.ci, &source.config, out_dir)?; } From 1e0a97ef1b7d80d006fba5edfcb683af8cb29a1e Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Tue, 10 Oct 2023 10:39:54 -0600 Subject: [PATCH 03/10] Add guard to not chase missing files --- uniffi_docs/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/uniffi_docs/src/lib.rs b/uniffi_docs/src/lib.rs index b4f3fa7266..92c6f704c3 100644 --- a/uniffi_docs/src/lib.rs +++ b/uniffi_docs/src/lib.rs @@ -178,7 +178,9 @@ fn traverse_module_tree>(path: P) -> Result { path.as_ref().with_file_name(format!("{name}/mod.rs")) }; - source_code_buff.push_str(&traverse_module_tree(to_traverse_further)?) + if to_traverse_further.exists() { + source_code_buff.push_str(&traverse_module_tree(to_traverse_further)?) + } } } From c422e8714ba43db8b892df9f554c0e4c899165db Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Tue, 10 Oct 2023 15:46:30 -0600 Subject: [PATCH 04/10] Error and error variant docs only added to python template right now, can test kotlin but not easily ruby/swift --- .../bindings/python/templates/EnumVariantDocsTemplate.py | 6 +++--- .../src/bindings/python/templates/ErrorTemplate.py | 5 ++++- .../bindings/python/templates/ErrorVariantDocsTemplate.py | 6 ++++++ 3 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 uniffi_bindgen/src/bindings/python/templates/ErrorVariantDocsTemplate.py diff --git a/uniffi_bindgen/src/bindings/python/templates/EnumVariantDocsTemplate.py b/uniffi_bindgen/src/bindings/python/templates/EnumVariantDocsTemplate.py index 9e26b5e7e9..52d52bcc6d 100644 --- a/uniffi_bindgen/src/bindings/python/templates/EnumVariantDocsTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/EnumVariantDocsTemplate.py @@ -1,6 +1,6 @@ {% match variant.documentation() -%} -{% when Some with (docs) %}""" + {% when Some with (docs) %}""" {% for line in docs.lines() %} {{ line }} {% endfor %} """ -{%- when None %} -{%- endmatch %} \ No newline at end of file + {%- when None %} +{%- endmatch %} diff --git a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py index 26a1e6452a..b8ec041675 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py @@ -9,15 +9,18 @@ class {{ type_name }}(Exception): _UniffiTemp{{ type_name }} = {{ type_name }} -class {{ type_name }}: # type: ignore +# TODO(murph): do the same for swift, kotlin, ruby +class {{ type_name }}: # type: ignore{% let struct = e %}{% include "StructureDocsTemplate.py" %} {%- for variant in e.variants() -%} {%- let variant_type_name = variant.name()|class_name -%} {%- if e.is_flat() %} class {{ variant_type_name }}(_UniffiTemp{{ type_name }}): + {% include "ErrorVariantDocsTemplate.py" %} def __repr__(self): return "{{ type_name }}.{{ variant_type_name }}({})".format(repr(str(self))) {%- else %} class {{ variant_type_name }}(_UniffiTemp{{ type_name }}): + {% include "ErrorVariantDocsTemplate.py" %} def __init__(self{% for field in variant.fields() %}, {{ field.name()|var_name }}{% endfor %}): {%- if variant.has_fields() %} super().__init__(", ".join([ diff --git a/uniffi_bindgen/src/bindings/python/templates/ErrorVariantDocsTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ErrorVariantDocsTemplate.py new file mode 100644 index 0000000000..aeadbe1431 --- /dev/null +++ b/uniffi_bindgen/src/bindings/python/templates/ErrorVariantDocsTemplate.py @@ -0,0 +1,6 @@ +{% match variant.documentation() -%} + {% when Some with (docs) %}""" + {% for line in docs.lines() %} {{ line }} + {% endfor %} """ + {%- when None %} +{%- endmatch %} From e37c08713d6486692a6e67876f390c6a602b2864 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Thu, 12 Oct 2023 13:08:58 -0600 Subject: [PATCH 05/10] Checkpointing working trait method comment in tree Python is consuming them, nothing else is yet. --- .gitignore | 1 + Cargo.lock | 10 - .../python/templates/ObjectTemplate.py | 2 +- .../src/bindings/python/templates/macros.py | 6 + uniffi_bindgen/src/interface/mod.rs | 11 ++ uniffi_docs/Cargo.toml | 3 +- uniffi_docs/src/lib.rs | 172 ++++++++++++++++-- 7 files changed, 173 insertions(+), 32 deletions(-) diff --git a/.gitignore b/.gitignore index 791d974674..4bd7c4df25 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ xcuserdata docs/manual/src/internals/api examples/app/ios/Generated examples/app/ios/IOSApp.xcodeproj/project.xcworkspace/xcshareddata/ +.direnv diff --git a/Cargo.lock b/Cargo.lock index b1c1e3d1dc..8097cdefcc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2051,16 +2051,6 @@ dependencies = [ "uniffi_meta", ] -[[package]] -name = "uniffi_fixture_metadata" -version = "0.1.0" -dependencies = [ - "thiserror", - "uniffi", - "uniffi_core", - "uniffi_meta", -] - [[package]] name = "uniffi_macros" version = "0.24.3" diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index 037b7b401b..568ef8569b 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -89,4 +89,4 @@ def lift(value): @staticmethod def lower(value): - return value._pointer \ No newline at end of file + return value._pointer diff --git a/uniffi_bindgen/src/bindings/python/templates/macros.py b/uniffi_bindgen/src/bindings/python/templates/macros.py index ef3b1bb94d..e5d95daaab 100644 --- a/uniffi_bindgen/src/bindings/python/templates/macros.py +++ b/uniffi_bindgen/src/bindings/python/templates/macros.py @@ -101,6 +101,8 @@ {% if meth.is_async() %} def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}): + {%- let func = meth -%} + {% include "MethodDocsTemplate.py" %} {%- call setup_args_extra_indent(meth) %} return _uniffi_rust_call_async( _UniffiLib.{{ meth.ffi_func().name() }}( @@ -131,6 +133,8 @@ def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}): {%- when Some with (return_type) %} def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}) -> "{{ return_type|type_name }}": + {%- let func = meth -%} + {% include "MethodDocsTemplate.py" %} {%- call setup_args_extra_indent(meth) %} return {{ return_type|lift_fn }}( {% call to_ffi_call_with_prefix("self._pointer", meth) %} @@ -139,6 +143,8 @@ def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}) -> "{{ return_typ {%- when None %} def {{ py_method_name }}(self, {% call arg_list_decl(meth) %}): + {%- let func = meth -%} + {% include "MethodDocsTemplate.py" %} {%- call setup_args_extra_indent(meth) %} {% call to_ffi_call_with_prefix("self._pointer", meth) %} {% endmatch %} diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 2343b1a944..a9fefffcf5 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -982,6 +982,17 @@ impl ComponentInterface { function.documentation = Some(doc); } } + + // TODO(murph): this has a comment, but that comment isn't being attached in places where I'd expect it to + // primary current test case is on the produced Kotlin interface of the same name. The trait comment _is_ + // showing up on the class object of the same name next to the interface. Was this class object being produced + // before my code? + dbg!(&self + .objects + .iter() + .filter(|o| o.name == "CloakedAiDecryptOps".to_string()) + .next() + .unwrap()); } } diff --git a/uniffi_docs/Cargo.toml b/uniffi_docs/Cargo.toml index dcdf418f5d..0f7dba7fb7 100644 --- a/uniffi_docs/Cargo.toml +++ b/uniffi_docs/Cargo.toml @@ -10,7 +10,8 @@ keywords = ["ffi", "bindgen", "docs"] [dependencies] anyhow = "1" -syn = { version = "1.0", features = ["full"] } +# remove extra-traits when done debugging +syn = { version = "1.0", features = ["full", "extra-traits"] } pulldown-cmark = { version = "0.9.2"} uniffi_meta = { path = "../uniffi_meta", version = "=0.24.3" } diff --git a/uniffi_docs/src/lib.rs b/uniffi_docs/src/lib.rs index 92c6f704c3..389af13780 100644 --- a/uniffi_docs/src/lib.rs +++ b/uniffi_docs/src/lib.rs @@ -7,7 +7,7 @@ use std::{collections::HashMap, fs::read_to_string, path::Path, str::FromStr}; use anyhow::Result; use pulldown_cmark::{Event, HeadingLevel::H1, Parser, Tag}; use syn::Attribute; -use uniffi_meta::Checksum; +use uniffi_meta::{AsType, Checksum}; /// Function documentation. #[derive(Debug, Clone, PartialEq, Eq, Checksum)] @@ -120,6 +120,26 @@ struct Impl { methods: HashMap, } +#[derive(Debug, PartialEq, Eq)] +struct Trait { + /// The docs on the trait itself + description: String, + /// Methods documentation + methods: HashMap, +} + +// TODO(murph): is this even necessary? Is there overlap with normal structures +// or should I be creating a structure for the trait from the start +impl Into for Trait { + fn into(self) -> Structure { + Structure { + description: self.description, + members: HashMap::default(), + methods: self.methods, + } + } +} + #[derive(Debug, PartialEq, Eq)] pub struct Documentation { pub functions: HashMap, @@ -195,6 +215,44 @@ pub fn extract_documentation(source_code: &str) -> Result { let mut structures = HashMap::new(); let mut impls = HashMap::new(); + // we build traits up first so we know they're all there to be used when encountering impls later + let mut traits: HashMap = HashMap::new(); + + // first pass to get trait documentation only + for item in file.items.iter() { + match item { + syn::Item::Trait(item) => { + if let Some(description) = extract_doc_comment(&item.attrs) { + let name = item.ident.to_string(); + let methods = item + .items + .iter() + .filter_map(|item| { + if let syn::TraitItem::Method(method) = item { + let name = method.sig.ident.to_string(); + extract_doc_comment(&method.attrs).map(|doc| (name, doc)) + } else { + None + } + }) + .map(|(name, description)| { + (name, Function::from_str(&description).unwrap()) + }) + .collect(); + + traits.insert( + name, + Trait { + description, + methods, + }, + ); + } + } + _ => (), // other item types are ignored, + } + } + for item in file.items.into_iter() { match item { syn::Item::Enum(item) => { @@ -248,28 +306,50 @@ pub fn extract_documentation(source_code: &str) -> Result { } } syn::Item::Impl(item) => { - if item.trait_.is_none() { - if let syn::Type::Path(path) = *item.self_ty { - let name = path.path.segments[0].ident.to_string(); - - let methods = item - .items - .into_iter() - .filter_map(|item| { - if let syn::ImplItem::Method(method) = item { + if let syn::Type::Path(path) = *item.self_ty { + let name = path.path.segments[0].ident.to_string(); + let maybe_trait_name = item.trait_.and_then(|t| match t { + (None, syn::Path { segments, .. }, _) => { + segments.first().map(|segment| segment.ident.to_string()) + } + _ => None, + }); + let methods: HashMap = item + .items + .into_iter() + .filter_map(|inner_item| { + if let syn::ImplItem::Method(method) = inner_item { + // if this is a trait impl, pull the doc from the trait for this method + // TODO(murph): right now the trait method comment shows up on CloakedAiInterface in Kotlin and nowhere in Python + // comments made directly on the impl for methods don't show up either + if let Some(trait_name) = &maybe_trait_name { + let method_name = method.sig.ident.to_string(); + traits + .get(trait_name) + .and_then(|trait_doc| trait_doc.methods.get(&method_name)) + .map(|method_doc| { + (method_name, method_doc.description.clone()) + }) + } else { + // if this isn't a trait impl (or there wasn't a doc for the trait method), get the + // doc directly on the method let name = method.sig.ident.to_string(); extract_doc_comment(&method.attrs).map(|doc| (name, doc)) - } else { - None } - }) - .map(|(name, description)| { - (name, Function::from_str(&description).unwrap()) - }) - .collect(); - - impls.insert(name, Impl { methods }); - } + } else { + None + } + }) + .map(|(name, description)| { + (name, Function::from_str(&description).unwrap()) + }) + .collect(); + impls + .entry(name) + .and_modify(|i: &mut Impl| + // this is safe because impls can't have conflicting names for the same struct + i.methods.extend(methods.clone())) + .or_insert(Impl { methods }); } } syn::Item::Fn(item) => { @@ -288,6 +368,11 @@ pub fn extract_documentation(source_code: &str) -> Result { } } + // TODO(murph): this isn't being consumed how I thought it would. Check trait output in attached AST + for (name, trait_) in traits { + structures.insert(name, trait_.into()); + } + Ok(Documentation { functions, structures, @@ -404,6 +489,12 @@ mod tests { } } + impl Animal for Person { + fn eat(&self, food: String) -> String { + format!("{} ate {food}.", self.get_name()) + } + } + /// Create hello message to a pet. /// /// # Arguments @@ -428,6 +519,12 @@ mod tests { /// A letter 'C'. C, } + + /// Functionality common to animals. + pub trait Animal { + /// Get a message about the Animal eating. + fn eat(&self, food: String) -> String; + } } .to_string(); @@ -471,6 +568,18 @@ mod tests { return_description: None, }, ); + methods.insert( + "eat".to_string(), + Function { + description: indoc! {" + Get a message about the Animal eating. + "} + .trim() + .to_string(), + arguments_descriptions: HashMap::new(), + return_description: None, + }, + ); structures.insert( "Person".to_string(), @@ -508,6 +617,29 @@ mod tests { }, ); + let mut methods = HashMap::new(); + methods.insert( + "eat".to_string(), + Function { + description: indoc! {" + Get a message about the Animal eating. + "} + .trim() + .to_string(), + arguments_descriptions: HashMap::new(), + return_description: None, + }, + ); + + structures.insert( + "Animal".to_string(), + Structure { + description: "Functionality common to animals.".to_string(), + members: HashMap::new(), + methods, + }, + ); + let expected = Documentation { functions, structures, From 83c9eb1eebf64698182f4ca36772c087a24a27a9 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Thu, 12 Oct 2023 15:50:44 -0600 Subject: [PATCH 06/10] Trait methods and comments added to all languages Tested in Kotlin and Python. Ruby already had doc behaviour that makes this work as is once the doc definitions showed up in the tree. Swift is untested. --- .../src/bindings/kotlin/templates/ObjectTemplate.kt | 3 +++ .../src/bindings/swift/templates/ObjectTemplate.swift | 3 +++ uniffi_bindgen/src/interface/mod.rs | 11 ----------- uniffi_docs/src/lib.rs | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index 3b6f0a46c9..40304d287e 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -3,6 +3,7 @@ {{- self.add_import("java.util.concurrent.atomic.AtomicLong") }} {{- self.add_import("java.util.concurrent.atomic.AtomicBoolean") }} +{% let struct = obj %}{% include "StructureDocsTemplate.kt" %} public interface {{ type_name }}Interface { {% for meth in obj.methods() -%} {%- let func = meth -%} @@ -55,6 +56,8 @@ class {{ type_name }}( } {% for meth in obj.methods() -%} + {%- let func = meth -%} + {%- include "FunctionDocsTemplate.kt" -%} {%- match meth.throws_type() -%} {%- when Some with (throwable) %} @Throws({{ throwable|error_type_name }}::class) diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 6c39693498..49e3eff64a 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -1,4 +1,5 @@ {%- let obj = ci|get_object_definition(name) %} +{% let struct = obj %}{% include "StructureDocsTemplate.swift" %} public protocol {{ obj.name() }}Protocol { {% for meth in obj.methods() -%} {%- let func = meth -%} @@ -50,6 +51,8 @@ public class {{ type_name }}: {{ obj.name() }}Protocol { {% for meth in obj.methods() -%} {%- if meth.is_async() %} + {%- let func = meth -%} + {%- include "FunctionDocsTemplate.swift" %} public func {{ meth.name()|fn_name }}({%- call swift::arg_list_decl(meth) -%}) async {% call swift::throws(meth) %}{% match meth.return_type() %}{% when Some with (return_type) %} -> {{ return_type|type_name }}{% when None %}{% endmatch %} { return {% call swift::try(meth) %} await uniffiRustCallAsync( rustFutureFunc: { diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index a9fefffcf5..2343b1a944 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -982,17 +982,6 @@ impl ComponentInterface { function.documentation = Some(doc); } } - - // TODO(murph): this has a comment, but that comment isn't being attached in places where I'd expect it to - // primary current test case is on the produced Kotlin interface of the same name. The trait comment _is_ - // showing up on the class object of the same name next to the interface. Was this class object being produced - // before my code? - dbg!(&self - .objects - .iter() - .filter(|o| o.name == "CloakedAiDecryptOps".to_string()) - .next() - .unwrap()); } } diff --git a/uniffi_docs/src/lib.rs b/uniffi_docs/src/lib.rs index 389af13780..b8a5f9241c 100644 --- a/uniffi_docs/src/lib.rs +++ b/uniffi_docs/src/lib.rs @@ -7,7 +7,7 @@ use std::{collections::HashMap, fs::read_to_string, path::Path, str::FromStr}; use anyhow::Result; use pulldown_cmark::{Event, HeadingLevel::H1, Parser, Tag}; use syn::Attribute; -use uniffi_meta::{AsType, Checksum}; +use uniffi_meta::Checksum; /// Function documentation. #[derive(Debug, Clone, PartialEq, Eq, Checksum)] From ddd6861280860f65169503e02b298b045b18c8f3 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Thu, 12 Oct 2023 17:01:42 -0600 Subject: [PATCH 07/10] Add Error variant doc gen to remaining lang --- docker/cargo-docker.sh | 2 +- .../src/bindings/kotlin/templates/ErrorTemplate.kt | 3 +++ .../src/bindings/python/templates/ErrorTemplate.py | 1 - uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb | 3 +++ .../src/bindings/swift/templates/ErrorTemplate.swift | 3 +++ uniffi_docs/src/lib.rs | 5 ----- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/docker/cargo-docker.sh b/docker/cargo-docker.sh index 1ed35f3063..2621e54871 100755 --- a/docker/cargo-docker.sh +++ b/docker/cargo-docker.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Key points for these cmd-line args: # * run a transient image that deletes itself on successful completion diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt index 56f685225e..677c43c961 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt @@ -3,6 +3,7 @@ {%- let canonical_type_name = type_|error_canonical_name %} {% if e.is_flat() %} +{% let struct = e %}{% include "StructureDocsTemplate.kt" %} sealed class {{ type_name }}(message: String): Exception(message){% if contains_object_references %}, Disposable {% endif %} { // Each variant is a nested class // Flat enums carries a string error message, so no special implementation is necessary. @@ -15,6 +16,7 @@ sealed class {{ type_name }}(message: String): Exception(message){% if contains_ } } {%- else %} +{% let struct = e %}{% include "StructureDocsTemplate.kt" %} sealed class {{ type_name }}: Exception(){% if contains_object_references %}, Disposable {% endif %} { // Each variant is a nested class {% for variant in e.variants() -%} @@ -38,6 +40,7 @@ sealed class {{ type_name }}: Exception(){% if contains_object_references %}, Di override fun destroy() { when(this) { {%- for variant in e.variants() %} + {% include "EnumVariantDocsTemplate.kt" %} is {{ type_name }}.{{ variant|error_variant|type_name }} -> { {%- if variant.has_fields() %} {% call kt::destroy_fields(variant) %} diff --git a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py index b8ec041675..1256f04a22 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py @@ -9,7 +9,6 @@ class {{ type_name }}(Exception): _UniffiTemp{{ type_name }} = {{ type_name }} -# TODO(murph): do the same for swift, kotlin, ruby class {{ type_name }}: # type: ignore{% let struct = e %}{% include "StructureDocsTemplate.py" %} {%- for variant in e.variants() -%} {%- let variant_type_name = variant.name()|class_name -%} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb index a7e26370c8..285b8988bf 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/ErrorTemplate.rb @@ -22,13 +22,16 @@ def to_s {%- for e in ci.enum_definitions() %} {% if ci.is_name_used_as_error(e.name()) %} {% if e.is_flat() %} +{% include "EnumDocsTemplate.rb" -%} class {{ e.name()|class_name_rb }} {%- for variant in e.variants() %} + {% include "EnumVariantDocsTemplate.rb" -%} {{ variant.name()|class_name_rb }} = Class.new StandardError {%- endfor %} {% else %} module {{ e.name()|class_name_rb }} {%- for variant in e.variants() %} + {% include "EnumVariantDocsTemplate.rb" -%} class {{ variant.name()|class_name_rb }} < StandardError def initialize({% for field in variant.fields() %}{{ field.name()|var_name_rb }}{% if !loop.last %}, {% endif %}{% endfor %}) {%- for field in variant.fields() %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift index 786091395b..1ea389de22 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift @@ -1,13 +1,16 @@ +{% let struct = e %}{% include "StructureDocsTemplate.swift" %} public enum {{ type_name }} { {% if e.is_flat() %} {% for variant in e.variants() %} + {% include "EnumVariantDocsTemplate.swift" %} // Simple error enums only carry a message case {{ variant.name()|class_name }}(message: String) {% endfor %} {%- else %} {% for variant in e.variants() %} + {% include "EnumVariantDocsTemplate.swift" %} case {{ variant.name()|class_name }}{% if variant.fields().len() > 0 %}({% call swift::field_list_decl(variant) %}){% endif -%} {% endfor %} diff --git a/uniffi_docs/src/lib.rs b/uniffi_docs/src/lib.rs index b8a5f9241c..253c0c736a 100644 --- a/uniffi_docs/src/lib.rs +++ b/uniffi_docs/src/lib.rs @@ -128,8 +128,6 @@ struct Trait { methods: HashMap, } -// TODO(murph): is this even necessary? Is there overlap with normal structures -// or should I be creating a structure for the trait from the start impl Into for Trait { fn into(self) -> Structure { Structure { @@ -320,8 +318,6 @@ pub fn extract_documentation(source_code: &str) -> Result { .filter_map(|inner_item| { if let syn::ImplItem::Method(method) = inner_item { // if this is a trait impl, pull the doc from the trait for this method - // TODO(murph): right now the trait method comment shows up on CloakedAiInterface in Kotlin and nowhere in Python - // comments made directly on the impl for methods don't show up either if let Some(trait_name) = &maybe_trait_name { let method_name = method.sig.ident.to_string(); traits @@ -368,7 +364,6 @@ pub fn extract_documentation(source_code: &str) -> Result { } } - // TODO(murph): this isn't being consumed how I thought it would. Check trait output in attached AST for (name, trait_) in traits { structures.insert(name, trait_.into()); } From 5c3aa4a9f7c603437c42c500833347e3d7dfe0c2 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Mon, 16 Oct 2023 10:07:28 -0600 Subject: [PATCH 08/10] Fix Kotlin error variant doc comments --- uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt index 677c43c961..72924d1bfa 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt @@ -8,6 +8,7 @@ sealed class {{ type_name }}(message: String): Exception(message){% if contains_ // Each variant is a nested class // Flat enums carries a string error message, so no special implementation is necessary. {% for variant in e.variants() -%} + {% include "EnumVariantDocsTemplate.kt" %} class {{ variant|error_variant|type_name }}(message: String) : {{ type_name }}(message) {% endfor %} @@ -20,6 +21,7 @@ sealed class {{ type_name }}(message: String): Exception(message){% if contains_ sealed class {{ type_name }}: Exception(){% if contains_object_references %}, Disposable {% endif %} { // Each variant is a nested class {% for variant in e.variants() -%} + {% include "EnumVariantDocsTemplate.kt" %} {%- let variant_name = variant|error_variant|type_name %} class {{ variant_name }}( {% for field in variant.fields() -%} @@ -40,7 +42,6 @@ sealed class {{ type_name }}: Exception(){% if contains_object_references %}, Di override fun destroy() { when(this) { {%- for variant in e.variants() %} - {% include "EnumVariantDocsTemplate.kt" %} is {{ type_name }}.{{ variant|error_variant|type_name }} -> { {%- if variant.has_fields() %} {% call kt::destroy_fields(variant) %} From b435392d288aca46122c50ac90df9f272e5220e2 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Fri, 27 Oct 2023 16:39:00 -0600 Subject: [PATCH 09/10] Remove syn debugging traits --- uniffi_docs/Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/uniffi_docs/Cargo.toml b/uniffi_docs/Cargo.toml index 67026651e8..62835c3604 100644 --- a/uniffi_docs/Cargo.toml +++ b/uniffi_docs/Cargo.toml @@ -10,8 +10,7 @@ keywords = ["ffi", "bindgen", "docs"] [dependencies] anyhow = "1" -# remove extra-traits when done debugging -syn = { version = "1.0", features = ["full", "extra-traits"] } +syn = { version = "1.0", features = ["full"] } pulldown-cmark = { version = "0.9.2"} uniffi_meta = { path = "../uniffi_meta", version = "=0.25.0" } From 9e7b7dd18fbbba2f6940f52a9f1c7a7ac8e05090 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Wed, 1 Nov 2023 11:18:43 -0600 Subject: [PATCH 10/10] Reuse new code, remove uneeded arg --- uniffi_bindgen/src/lib.rs | 8 -------- uniffi_bindgen/src/library_mode.rs | 10 ++++------ uniffi_docs/src/lib.rs | 2 +- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/uniffi_bindgen/src/lib.rs b/uniffi_bindgen/src/lib.rs index d3c770844b..0dd5fcc479 100644 --- a/uniffi_bindgen/src/lib.rs +++ b/uniffi_bindgen/src/lib.rs @@ -136,10 +136,6 @@ pub trait BindingsConfig: DeserializeOwned { /// config_map maps crate names to config instances. This is mostly used to set up external /// types. fn update_from_dependency_configs(&mut self, config_map: HashMap<&str, &Self>); - - fn doc_generation_enabled(&self) -> bool { - false - } } /// Binding generator config with no members @@ -516,10 +512,6 @@ impl BindingsConfig for Config { .collect(), ); } - - fn doc_generation_enabled(&self) -> bool { - self.bindings.doc_comments.unwrap_or_default() - } } // FIXME(HACK): diff --git a/uniffi_bindgen/src/library_mode.rs b/uniffi_bindgen/src/library_mode.rs index 42b7f1c3eb..ed69f2cb00 100644 --- a/uniffi_bindgen/src/library_mode.rs +++ b/uniffi_bindgen/src/library_mode.rs @@ -100,12 +100,10 @@ pub fn generate_external_bindings( } } - for source in sources.iter_mut() { - if source.config.doc_generation_enabled() { - let path = &source.package.manifest_path.with_file_name("src/lib.rs"); - let documentation = uniffi_docs::extract_documentation_from_path(path)?; - source.ci.attach_documentation(documentation); - } + for source in &mut sources { + // Assumes a lib.rs for the source package containing bindings + let path = &source.package.manifest_path.with_file_name("src/lib.rs"); + source.config.update_documentation(&mut source.ci, path)?; binding_generator.write_bindings(&source.ci, &source.config, out_dir)?; } diff --git a/uniffi_docs/src/lib.rs b/uniffi_docs/src/lib.rs index 3cf7ba4afc..63defb8f7d 100644 --- a/uniffi_docs/src/lib.rs +++ b/uniffi_docs/src/lib.rs @@ -343,7 +343,7 @@ pub fn extract_documentation(source_code: &str) -> Result { impls .entry(name) .and_modify(|i: &mut Impl| - // this is safe because impls can't have conflicting names for the same struct + // this is safe because impls can't have conflicting method names for the same struct i.methods.extend(methods.clone())) .or_insert(Impl { methods }); }