From 23696ea6814b1e1d618ffcef15457fe5f2aa2147 Mon Sep 17 00:00:00 2001 From: Kristupas Antanavicius Date: Fri, 9 Feb 2024 16:14:04 +0200 Subject: [PATCH] Implement docstrings (#41) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kristupas Antanavičius --- Cargo.lock | 14 +++ bindgen/Cargo.toml | 1 + bindgen/src/gen_go/mod.rs | 8 ++ .../templates/CallbackInterfaceTemplate.go | 3 +- bindgen/templates/EnumTemplate.go | 4 + bindgen/templates/ErrorTemplate.go | 3 + bindgen/templates/ObjectTemplate.go | 4 + bindgen/templates/RecordTemplate.go | 2 + bindgen/templates/TopLevelFunctionTemplate.go | 7 +- bindgen/templates/macros.go | 8 +- bindgen/templates/wrapper.go | 1 + binding_tests/callbacks_test.go | 6 +- binding_tests/custom_types_test.go | 4 +- binding_tests/docstring_test.go | 93 +++++++++++++++++++ binding_tests/fixture_callbacks_test.go | 6 +- fixtures/Cargo.toml | 3 +- fixtures/src/lib.rs | 1 + 17 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 binding_tests/docstring_test.go diff --git a/Cargo.lock b/Cargo.lock index 6092ece..1524b06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1183,6 +1183,7 @@ dependencies = [ "paste", "serde", "serde_json", + "textwrap", "toml", "uniffi_bindgen", "uniffi_meta", @@ -1203,6 +1204,7 @@ dependencies = [ "uniffi-example-todolist", "uniffi-fixture-callbacks", "uniffi-fixture-coverall", + "uniffi-fixture-docstring", "uniffi-fixture-ext-types", "uniffi-fixture-ext-types-guid", "uniffi-fixture-ext-types-lib-one", @@ -1294,6 +1296,18 @@ dependencies = [ "uniffi", ] +[[package]] +name = "uniffi-fixture-docstring" +version = "0.22.0" +dependencies = [ + "camino", + "glob", + "thiserror", + "uniffi", + "uniffi_bindgen", + "uniffi_testing", +] + [[package]] name = "uniffi-fixture-ext-types" version = "0.22.0" diff --git a/bindgen/Cargo.toml b/bindgen/Cargo.toml index d49e7b1..b8984f7 100644 --- a/bindgen/Cargo.toml +++ b/bindgen/Cargo.toml @@ -27,3 +27,4 @@ camino = "1.0.8" fs-err = "2.7.0" paste = "1.0" serde_json = "1.0.0" +textwrap = "0.16" diff --git a/bindgen/src/gen_go/mod.rs b/bindgen/src/gen_go/mod.rs index f52a6ad..e6b78b7 100644 --- a/bindgen/src/gen_go/mod.rs +++ b/bindgen/src/gen_go/mod.rs @@ -497,6 +497,14 @@ pub mod filters { pub fn enum_variant_name(nm: &str) -> Result { Ok(oracle().enum_variant_name(nm)) } + + /// Get the idiomatic Go rendering of docstring + pub fn docstring(docstring: &str, tabs: &i32) -> Result { + let docstring = textwrap::indent(&textwrap::dedent(docstring), "// "); + + let tabs = usize::try_from(*tabs).unwrap_or_default(); + Ok(textwrap::indent(&docstring, &"\t".repeat(tabs))) + } } /// Renders Go helper code for all types diff --git a/bindgen/templates/CallbackInterfaceTemplate.go b/bindgen/templates/CallbackInterfaceTemplate.go index a7493e6..6bc4a79 100644 --- a/bindgen/templates/CallbackInterfaceTemplate.go +++ b/bindgen/templates/CallbackInterfaceTemplate.go @@ -5,9 +5,10 @@ {% if self.include_once_check("CallbackInterfaceRuntime.go") %}{% include "CallbackInterfaceRuntime.go" %}{% endif %} {{- self.add_import("sync") }} -// Declaration and FfiConverters for {{ type_name }} Callback Interface +{%- call go::docstring(cbi, 0) %} type {{ type_name }} interface { {% for meth in cbi.methods() -%} + {%- call go::docstring(meth, 1) %} {{ meth.name()|fn_name }}({% call go::arg_list_decl(meth) %}) {% call go::return_type_decl_cb(meth) %} {% endfor %} } diff --git a/bindgen/templates/EnumTemplate.go b/bindgen/templates/EnumTemplate.go index de49a28..dd83ede 100644 --- a/bindgen/templates/EnumTemplate.go +++ b/bindgen/templates/EnumTemplate.go @@ -5,20 +5,24 @@ {% let e = ci.get_enum_definition(name).expect("missing enum") -%} {%- if e.is_flat() -%} +{%- call go::docstring(e, 0) %} type {{ type_name }} uint const ( {%- for variant in e.variants() %} + {%- call go::docstring(variant, 1) %} {{ type_name }}{{ variant.name()|enum_variant_name }} {{ type_name }} = {{ loop.index }} {%- endfor %} ) {%- else %} +{%- call go::docstring(e, 0) %} type {{ type_name }} interface { Destroy() } {%- for variant in e.variants() %} +{%- call go::docstring(variant, 0) %} type {{ type_name }}{{ variant.name()|class_name }} struct { {%- for field in variant.fields() %} {{ field.name()|field_name }} {{ field|type_name}} diff --git a/bindgen/templates/ErrorTemplate.go b/bindgen/templates/ErrorTemplate.go index 9f68bef..3a58c13 100644 --- a/bindgen/templates/ErrorTemplate.go +++ b/bindgen/templates/ErrorTemplate.go @@ -2,6 +2,7 @@ * 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/. */#} +{%- call go::docstring(e, 0) %} type {{ type_name|class_name }} struct { err error } @@ -23,6 +24,7 @@ var Err{{ variant_class_name }} = fmt.Errorf("{{ variant_class_name }}") // Variant structs {%- for variant in e.variants() %} {%- let variant_class_name = (type_name.clone() + variant.name())|class_name %} +{%- call go::docstring(variant, 0) %} type {{ variant_class_name }} struct { {%- if e.is_flat() %} message string @@ -33,6 +35,7 @@ type {{ variant_class_name }} struct { {%- endif %} } +{%- call go::docstring(variant, 0) %} func New{{ variant_class_name }}( {%- if !e.is_flat() %} {%- for field in variant.fields() %} diff --git a/bindgen/templates/ObjectTemplate.go b/bindgen/templates/ObjectTemplate.go index 687f11c..ccfd831 100644 --- a/bindgen/templates/ObjectTemplate.go +++ b/bindgen/templates/ObjectTemplate.go @@ -8,12 +8,14 @@ {%- let obj_name = obj.name()|class_name %} {%- if self.include_once_check("ObjectRuntime.go") %}{% include "ObjectRuntime.go" %}{% endif %} +{%- call go::docstring(obj, 0) %} type {{ obj_name }} struct { ffiObject FfiObject } {%- match obj.primary_constructor() %} {%- when Some with (cons) %} +{%- call go::docstring(cons, 0) %} func New{{ obj_name }}({% call go::arg_list_decl(cons) -%}) {% call go::return_type_decl(cons) %} { {% call go::ffi_call_binding(func, "") %} } @@ -21,12 +23,14 @@ func New{{ obj_name }}({% call go::arg_list_decl(cons) -%}) {% call go::return_t {%- endmatch %} {% for cons in obj.alternate_constructors() -%} +{%- call go::docstring(cons, 0) %} func {{ obj_name }}{{ cons.name()|fn_name }}({% call go::arg_list_decl(cons) %}) {% call go::return_type_decl(cons) %} { {% call go::ffi_call_binding(func, "") %} } {% endfor %} {% for func in obj.methods() -%} +{%- call go::docstring(func, 0) %} func (_self {{ type_name }}){{ func.name()|fn_name }}({%- call go::arg_list_decl(func) -%}) {% call go::return_type_decl(func) %} { _pointer := _self.ffiObject.incrementPointer("{{ type_name }}") defer _self.ffiObject.decrementPointer() diff --git a/bindgen/templates/RecordTemplate.go b/bindgen/templates/RecordTemplate.go index df34078..61610b1 100644 --- a/bindgen/templates/RecordTemplate.go +++ b/bindgen/templates/RecordTemplate.go @@ -4,8 +4,10 @@ {%- let rec = ci.get_record_definition(name).expect("missing record") %} +{%- call go::docstring(rec, 0) %} type {{ type_name }} struct { {%- for field in rec.fields() %} + {%- call go::docstring(field, 1) %} {{ field.name()|field_name }} {{ field|type_name -}} {%- endfor %} } diff --git a/bindgen/templates/TopLevelFunctionTemplate.go b/bindgen/templates/TopLevelFunctionTemplate.go index b819cdf..411627e 100644 --- a/bindgen/templates/TopLevelFunctionTemplate.go +++ b/bindgen/templates/TopLevelFunctionTemplate.go @@ -2,12 +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/. */#} -{%- if func.is_async() %} +{%- call go::docstring(func, 0) %} func {{ func.name()|fn_name}}({%- call go::arg_list_decl(func) -%}) {% call go::return_type_decl(func) %} { +{%- if func.is_async() %} {% call go::async_ffi_call_binding(func, "") %} -} {%- else %} -func {{ func.name()|fn_name}}({%- call go::arg_list_decl(func) -%}) {% call go::return_type_decl(func) %} { {% call go::ffi_call_binding(func, "") %} +{%- endif %} } -{% endif %} diff --git a/bindgen/templates/macros.go b/bindgen/templates/macros.go index 8c410d8..0fbb555 100644 --- a/bindgen/templates/macros.go +++ b/bindgen/templates/macros.go @@ -204,4 +204,10 @@ RustBufferFromExternal({{ arg|lower_fn }}({{ arg.name()|var_name }})) {%- endmatch -%} {%- endmacro -%} - +{%- macro docstring(defn, indent_tabs) %} +{%- match defn.docstring() %} +{%- when Some(docstring) %} +{{ docstring|docstring(indent_tabs) }} +{%- else %} +{%- endmatch %} +{%- endmacro %} diff --git a/bindgen/templates/wrapper.go b/bindgen/templates/wrapper.go index ee65f49..bf920e6 100644 --- a/bindgen/templates/wrapper.go +++ b/bindgen/templates/wrapper.go @@ -2,6 +2,7 @@ * 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/. */#} +{%- call go::docstring(ci.namespace_definition(), 0) %} package {{ ci.namespace() }} // #include <{{ config.header_filename() }}> diff --git a/binding_tests/callbacks_test.go b/binding_tests/callbacks_test.go index 348b19e..6a02cde 100644 --- a/binding_tests/callbacks_test.go +++ b/binding_tests/callbacks_test.go @@ -59,7 +59,7 @@ func TestCallbackRegistrationIsNotAffectedByGC(t *testing.T) { func TestCallbackReferenceIsDropped(t *testing.T) { telephone := callbacks.NewTelephone() dropped := false - done := make(chan struct{}) + done := make(chan struct{}) func() { callback := &OnCallAnswerImpl{} runtime.SetFinalizer(callback, func(cb *OnCallAnswerImpl) { @@ -71,9 +71,9 @@ func TestCallbackReferenceIsDropped(t *testing.T) { runtime.GC() // runtime.GC() is not a fully blocking call select { - case <- time.After(time.Millisecond * 100): + case <-time.After(time.Millisecond * 100): panic("timed out") - case <- done: + case <-done: assert.Equal(t, true, dropped) } } diff --git a/binding_tests/custom_types_test.go b/binding_tests/custom_types_test.go index 88de8d9..c80b679 100644 --- a/binding_tests/custom_types_test.go +++ b/binding_tests/custom_types_test.go @@ -8,9 +8,9 @@ import ( "net/url" "testing" - "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/custom_types" + "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/custom_types" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" ) func unwrap[T any](value T, err error) T { diff --git a/binding_tests/docstring_test.go b/binding_tests/docstring_test.go new file mode 100644 index 0000000..16e5e44 --- /dev/null +++ b/binding_tests/docstring_test.go @@ -0,0 +1,93 @@ +package binding_tests + +import ( + "testing" + + docstrings "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/uniffi_docstring" + + "fmt" + "github.com/stretchr/testify/assert" + "io" + "os" + "regexp" + "strings" +) + +type ExampleCallback struct{} + +func (_ ExampleCallback) Test() { +} + +// Make sure symbols are not accidentally commented out by docstrings +func TestSymbolsWithDocstringsExist(t *testing.T) { + docstrings.Test() + + _ = docstrings.EnumTestOne + _ = docstrings.EnumTestTwo + + _ = docstrings.AssociatedEnumTestTest{0} + _ = docstrings.AssociatedEnumTestTest2{0} + + _ = docstrings.ErrErrorTestOne + _ = docstrings.ErrErrorTestTwo + + _ = docstrings.NewAssociatedErrorTestTest(0) + _ = docstrings.NewAssociatedErrorTestTest2(0) + + obj := docstrings.NewObjectTest() + obj = docstrings.ObjectTestNewAlternate() + obj.Test() + + record := docstrings.RecordTest{0} + _ = record.Test + + var callback docstrings.CallbackTest + callback = ExampleCallback{} + callback.Test() +} + +func TestDocstringsAppearInBindings(t *testing.T) { + bindingsContent := readDocstringBindingsFile(t) + expectedDocstrings := getExpectedDocstrings(t) + + var missingDocstrings []string + + for _, docstring := range expectedDocstrings { + if !strings.Contains(bindingsContent, docstring) { + missingDocstrings = append(missingDocstrings, docstring) + } + } + + assert.Empty(t, missingDocstrings) +} + +func readDocstringBindingsFile(t *testing.T) string { + cwd, err := os.Getwd() + assert.NoError(t, err) + + file, err := os.Open(fmt.Sprintf("%s/generated/uniffi_docstring/uniffi_docstring.go", cwd)) + assert.NoError(t, err) + defer file.Close() + + bytes, err := io.ReadAll(file) + assert.NoError(t, err) + + return string(bytes) +} + +func getExpectedDocstrings(t *testing.T) []string { + cwd, err := os.Getwd() + assert.NoError(t, err) + + file, err := os.Open(fmt.Sprintf("%s/../3rd-party/uniffi-rs/fixtures/docstring/tests/test_generated_bindings.rs", cwd)) + assert.NoError(t, err) + defer file.Close() + + bytes, err := io.ReadAll(file) + assert.NoError(t, err) + + re, err := regexp.Compile(``) + assert.NoError(t, err) + + return re.FindAllString(string(bytes), -1) +} diff --git a/binding_tests/fixture_callbacks_test.go b/binding_tests/fixture_callbacks_test.go index b062f25..c2ebcdc 100644 --- a/binding_tests/fixture_callbacks_test.go +++ b/binding_tests/fixture_callbacks_test.go @@ -44,10 +44,10 @@ func (getters) GetList(v []int32, arg2 bool) ([]int32, *fixture_callbacks.Simple func (getters) GetNothing(v string) *fixture_callbacks.SimpleError { if v == "bad-argument" { return fixture_callbacks.NewSimpleErrorBadArgument() - } - if v == "unexpected-error" { + } + if v == "unexpected-error" { return fixture_callbacks.NewSimpleErrorUnexpectedError() - } + } return nil } diff --git a/fixtures/Cargo.toml b/fixtures/Cargo.toml index 92fa2a8..737263e 100644 --- a/fixtures/Cargo.toml +++ b/fixtures/Cargo.toml @@ -21,12 +21,11 @@ uniffi-example-todolist = { path = "../3rd-party/uniffi-rs/examples/todolist" } # Fixtures uniffi-fixture-callbacks = { path = "../3rd-party/uniffi-rs/fixtures/callbacks" } uniffi-fixture-coverall = { path = "../3rd-party/uniffi-rs/fixtures/coverall" } -# ext types +uniffi-fixture-docstring = { path = "../3rd-party/uniffi-rs/fixtures/docstring" } uniffi-fixture-ext-types = { path = "../3rd-party/uniffi-rs/fixtures/ext-types/lib" } uniffi-fixture-ext-types-proc-macro = { path = "../3rd-party/uniffi-rs/fixtures/ext-types/proc-macro-lib" } uniffi-fixture-ext-types-lib-one = { path = "../3rd-party/uniffi-rs/fixtures/ext-types/uniffi-one" } uniffi-fixture-ext-types-guid = { path = "../3rd-party/uniffi-rs/fixtures/ext-types/guid" } - uniffi-fixture-foreign-executor = { path = "../3rd-party/uniffi-rs/fixtures/foreign-executor" } uniffi-fixture-futures = { path = "../3rd-party/uniffi-rs/fixtures/futures" } uniffi-fixture-large-enum = { package = "large-enum", path = "../3rd-party/uniffi-rs/fixtures/large-enum" } diff --git a/fixtures/src/lib.rs b/fixtures/src/lib.rs index c6172b6..9ef99dc 100644 --- a/fixtures/src/lib.rs +++ b/fixtures/src/lib.rs @@ -15,6 +15,7 @@ mod uniffi_fixtures { // Fixtures + uniffi_fixture_docstring::uniffi_reexport_scaffolding!(); uniffi_fixture_callbacks::uniffi_reexport_scaffolding!(); uniffi_chronological::uniffi_reexport_scaffolding!(); uniffi_coverall::uniffi_reexport_scaffolding!();