From a2c9acd83c8d78e5376ac0ec4a449a74d1f6b53a Mon Sep 17 00:00:00 2001 From: Martynas Gurskas Date: Tue, 28 Nov 2023 12:41:31 +0200 Subject: [PATCH 1/2] Add docstrings to generated code Signed-off-by: Martynas Gurskas --- bindgen/Cargo.toml | 1 + .../src/bindings/cpp/gen_cpp/filters/mod.rs | 7 +++ .../src/bindings/cpp/templates/callback.hpp | 2 + bindgen/src/bindings/cpp/templates/enum.hpp | 9 ++++ bindgen/src/bindings/cpp/templates/err.hpp | 5 ++ bindgen/src/bindings/cpp/templates/macros.cpp | 8 ++++ bindgen/src/bindings/cpp/templates/obj.hpp | 8 +++- bindgen/src/bindings/cpp/templates/rec.hpp | 2 + .../src/bindings/cpp/templates/wrapper.hpp | 2 + cpp-tests/CMakeLists.txt | 2 + cpp-tests/tests/uniffi_docstring/main.cpp | 48 +++++++++++++++++++ fixtures/src/lib.rs | 1 + 12 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 cpp-tests/tests/uniffi_docstring/main.cpp diff --git a/bindgen/Cargo.toml b/bindgen/Cargo.toml index 83bf549..56f92f0 100644 --- a/bindgen/Cargo.toml +++ b/bindgen/Cargo.toml @@ -12,4 +12,5 @@ heck = "0.4.1" paste = "1" serde = "1" topological-sort = "0.2.2" +textwrap = "0.16.0" uniffi_bindgen = { git = "https://github.com/NordSecurity/uniffi-rs.git", branch = "nordsec.0.25.0" } diff --git a/bindgen/src/bindings/cpp/gen_cpp/filters/mod.rs b/bindgen/src/bindings/cpp/gen_cpp/filters/mod.rs index e46cef7..0d27254 100644 --- a/bindgen/src/bindings/cpp/gen_cpp/filters/mod.rs +++ b/bindgen/src/bindings/cpp/gen_cpp/filters/mod.rs @@ -188,3 +188,10 @@ pub(crate) fn parameter(arg: &Argument) -> Result { t => format!("{} {}", type_name(&t)?, arg.name()), }) } + +pub(crate) fn docstring(docstring: &str, spaces: &i32) -> Result { + let middle = textwrap::indent(&textwrap::dedent(docstring), " * "); + let wrapped = format!("/**\n{middle}\n */"); + + Ok(textwrap::indent(&wrapped, &" ".repeat(*spaces as usize))) +} diff --git a/bindgen/src/bindings/cpp/templates/callback.hpp b/bindgen/src/bindings/cpp/templates/callback.hpp index fc124e3..6d72df9 100644 --- a/bindgen/src/bindings/cpp/templates/callback.hpp +++ b/bindgen/src/bindings/cpp/templates/callback.hpp @@ -3,8 +3,10 @@ {%- let class_name = type_name|class_name %} {%- let ffi_converter_name = typ|ffi_converter_name %} +{%- call macros::docstring(iface, 0) %} struct {{ class_name }} { {% for method in iface.methods() -%} + {%- call macros::docstring(method, 4) %} virtual {%- match method.return_type() -%} {% when Some with (return_type) %} {{ return_type|type_name }} {% else %} void {% endmatch -%} diff --git a/bindgen/src/bindings/cpp/templates/enum.hpp b/bindgen/src/bindings/cpp/templates/enum.hpp index dbf2afb..e32ce72 100644 --- a/bindgen/src/bindings/cpp/templates/enum.hpp +++ b/bindgen/src/bindings/cpp/templates/enum.hpp @@ -1,6 +1,8 @@ {%- if e.is_flat() %} +{%- call macros::docstring(e, 0) %} enum class {{ type_name }}: int32_t { {% for variant in e.variants() -%} + {%- call macros::docstring(variant, 4) %} {{ variant|variant_name }} = {{ loop.index }} {%- if !loop.last %}, {% endif -%} @@ -12,12 +14,16 @@ namespace uniffi { } {%- let ffi_converter_name = typ|ffi_converter_name|class_name %} + +{%- call macros::docstring(e, 0) %} struct {{ type_name }} { friend uniffi::{{ ffi_converter_name }}; {% for variant in e.variants() %} + {%- call macros::docstring(variant, 4) %} struct {{ variant|variant_name }} { {%- for field in variant.fields() %} + {%- call macros::docstring(field, 8) %} {{ field|type_name }} {{ field.name()|var_name }} {%- match field.default_value() %} {%- when Some with (literal) %} = {{ literal|literal_cpp(field) }};{%- else -%}; @@ -44,6 +50,9 @@ struct {{ type_name }} { return *this; } + /** + * Returns the variant of this enum + */ const std::variant<{% for variant in e.variants() -%} {{ variant|variant_name }} {%- if !loop.last %}, {% endif -%} {% endfor %}> &get_variant() const { return variant; } diff --git a/bindgen/src/bindings/cpp/templates/err.hpp b/bindgen/src/bindings/cpp/templates/err.hpp index 1545a73..e44a2ca 100644 --- a/bindgen/src/bindings/cpp/templates/err.hpp +++ b/bindgen/src/bindings/cpp/templates/err.hpp @@ -5,6 +5,7 @@ namespace uniffi { struct {{ ffi_converter_name }}; } +{% call macros::docstring(e, 0) %} struct {{ class_name }}: std::runtime_error { friend uniffi::{{ ffi_converter_name }}; @@ -21,8 +22,12 @@ struct {{ class_name }}: std::runtime_error { }; {% if e.variants().len() != 0 %} +/** + * Contains variants of {{ type_name }} + */ namespace {{ class_name|to_lower_snake_case }} { {% for variant in e.variants() %} + {%- call macros::docstring(variant, 4) %} struct {{ variant.name()|class_name }}: {{ class_name }} { {%- for field in variant.fields() %} {{ field|type_name }} {{ field.name()|var_name }} diff --git a/bindgen/src/bindings/cpp/templates/macros.cpp b/bindgen/src/bindings/cpp/templates/macros.cpp index 2e7bb51..192cf27 100644 --- a/bindgen/src/bindings/cpp/templates/macros.cpp +++ b/bindgen/src/bindings/cpp/templates/macros.cpp @@ -50,3 +50,11 @@ {{ namespace }}::uniffi::{{ arg|lower_fn }}({{ arg.name()|var_name }}){% if !loop.last %}, {% endif -%} {%- endfor %} {%- endmacro -%} + +{%- macro docstring(defn, indent_spaces) %} +{%- match defn.docstring() %} +{%- when Some(docstring) %} +{{ docstring|docstring(indent_spaces) }} +{%- else %} +{%- endmatch %} +{%- endmacro %} diff --git a/bindgen/src/bindings/cpp/templates/obj.hpp b/bindgen/src/bindings/cpp/templates/obj.hpp index 0844f93..e5e2aa0 100644 --- a/bindgen/src/bindings/cpp/templates/obj.hpp +++ b/bindgen/src/bindings/cpp/templates/obj.hpp @@ -5,6 +5,8 @@ {%- let type_name = typ|type_name %} namespace uniffi { struct {{ ffi_converter_name|class_name }}; } + +{%- call macros::docstring(obj, 0) %} struct {{ canonical_type_name }} { friend uniffi::{{ ffi_converter_name|class_name }}; @@ -16,18 +18,21 @@ struct {{ canonical_type_name }} { {% match obj.primary_constructor() %} {%- when Some with (ctor) -%} + {%- call macros::docstring(ctor, 4) %} static {{ type_name }} init({% call macros::param_list(ctor) %}); {%- else %} {%- endmatch -%} {% for ctor in obj.alternate_constructors() %} + {%- call macros::docstring(ctor, 4) %} static {{ type_name }} {{ ctor.name() }}({% call macros::param_list(ctor) %}); {%- endfor %} ~{{ canonical_type_name }}(); {% for method in obj.methods() %} - {%- match method.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }} {% else %}void {% endmatch %} + {%- call macros::docstring(method, 4) %} + {% match method.return_type() %}{% when Some with (return_type) %}{{ return_type|type_name }} {% else %}void {% endmatch %} {{- method.name()|fn_name }}({% call macros::param_list(method) %}); {% endfor %} @@ -35,6 +40,5 @@ struct {{ canonical_type_name }} { {{ canonical_type_name }}(void *); {{ canonical_type_name }}() = delete; - void *instance; }; diff --git a/bindgen/src/bindings/cpp/templates/rec.hpp b/bindgen/src/bindings/cpp/templates/rec.hpp index 669bc11..fad548c 100644 --- a/bindgen/src/bindings/cpp/templates/rec.hpp +++ b/bindgen/src/bindings/cpp/templates/rec.hpp @@ -2,8 +2,10 @@ {%- let type_name = typ|type_name %} {%- let class_name = type_name|class_name %} +{%- call macros::docstring(rec, 0) %} struct {{ type_name }} { {%- for field in rec.fields() %} + {%- call macros::docstring(field, 4) %} {{ field|type_name }} {{ field.name()|var_name }} {%- match field.default_value() %} {%- when Some with (literal) %} = {{ literal|literal_cpp(field) }};{%- else -%}; diff --git a/bindgen/src/bindings/cpp/templates/wrapper.hpp b/bindgen/src/bindings/cpp/templates/wrapper.hpp index 21a122e..bbd114e 100644 --- a/bindgen/src/bindings/cpp/templates/wrapper.hpp +++ b/bindgen/src/bindings/cpp/templates/wrapper.hpp @@ -24,6 +24,7 @@ {%- import "macros.cpp" as macros %} +{%- call macros::docstring(ci.namespace_definition(), 0) %} namespace {{ namespace }} { {%- for typ in ci.iter_types() %} {%- let type_name = typ|type_name %} @@ -221,6 +222,7 @@ namespace {{ namespace }} { } {% for func in ci.function_definitions() %} + {%- call macros::docstring(func, 4) %} {%- match func.return_type() %} {%- when Some with (return_type) %} {{ return_type|type_name }} {{ func.name()|fn_name }}({%- call macros::param_list(func) %}); diff --git a/cpp-tests/CMakeLists.txt b/cpp-tests/CMakeLists.txt index 439ea9b..8e7ccbc 100644 --- a/cpp-tests/CMakeLists.txt +++ b/cpp-tests/CMakeLists.txt @@ -16,6 +16,7 @@ add_executable(${TEST_NAME}-test tests/${TEST_NAME}/main.cpp ${BINDINGS_SRC_DIR} target_include_directories(${TEST_NAME}-test PRIVATE ${BINDINGS_SRC_DIR} tests/include) target_link_directories(${TEST_NAME}-test PRIVATE ${BINDINGS_BUILD_DIR}) target_link_libraries(${TEST_NAME}-test uniffi_bindgen_cpp_fixtures Threads::Threads) +target_compile_definitions(${TEST_NAME}-test PRIVATE UNIFFI_BINDING_DIR="${BINDINGS_SRC_DIR}") add_test(NAME ${TEST_NAME}-test COMMAND ${TEST_NAME}-test) @@ -35,6 +36,7 @@ test_case(sprites) test_case(todolist) test_case(traits) test_case(coverall) +test_case(uniffi_docstring) add_custom_target(libs ALL BYPRODUCTS ${BINDING_FILES} diff --git a/cpp-tests/tests/uniffi_docstring/main.cpp b/cpp-tests/tests/uniffi_docstring/main.cpp new file mode 100644 index 0000000..3bc4899 --- /dev/null +++ b/cpp-tests/tests/uniffi_docstring/main.cpp @@ -0,0 +1,48 @@ +#include "test_common.hpp" + +#include + +#include + +const std::vector expected_docstrings = { + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", +}; + +int main() { + auto stream = std::ifstream(UNIFFI_BINDING_DIR "/uniffi_docstring.hpp"); + ASSERT_TRUE(stream.is_open()); + + auto source = std::string(std::istreambuf_iterator(stream), std::istreambuf_iterator()); + ASSERT_FALSE(source.empty()); + + for (const auto& docstring : expected_docstrings) { + auto found = source.find(docstring); + if (found == std::string::npos) { + std::cerr << "Could not find docstring: " << docstring << std::endl; + ASSERT_TRUE(false); + } + } + + return 0; +} \ No newline at end of file diff --git a/fixtures/src/lib.rs b/fixtures/src/lib.rs index e87bc5b..98e448b 100644 --- a/fixtures/src/lib.rs +++ b/fixtures/src/lib.rs @@ -10,4 +10,5 @@ mod uniffi_fixtures { uniffi_chronological::uniffi_reexport_scaffolding!(); uniffi_trait_methods::uniffi_reexport_scaffolding!(); uniffi_coverall::uniffi_reexport_scaffolding!(); + uniffi_fixture_docstring::uniffi_reexport_scaffolding!(); } From 40fab00cd7c9124f4c9f9f17f79b67c11a272fa7 Mon Sep 17 00:00:00 2001 From: Martynas Gurskas Date: Wed, 29 Nov 2023 11:03:42 +0200 Subject: [PATCH 2/2] Use stricter docstring verification Instead of checking if a given set of docstrings exits, instead parse all present docstrings and verify that there is no more or no less than the expected entries Signed-off-by: Martynas Gurskas --- cpp-tests/tests/uniffi_docstring/main.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/cpp-tests/tests/uniffi_docstring/main.cpp b/cpp-tests/tests/uniffi_docstring/main.cpp index 3bc4899..80d5f07 100644 --- a/cpp-tests/tests/uniffi_docstring/main.cpp +++ b/cpp-tests/tests/uniffi_docstring/main.cpp @@ -1,6 +1,7 @@ #include "test_common.hpp" #include +#include #include @@ -36,10 +37,25 @@ int main() { auto source = std::string(std::istreambuf_iterator(stream), std::istreambuf_iterator()); ASSERT_FALSE(source.empty()); + std::regex docstring_reg("\\"); + std::smatch match; + std::vector extracted_docstrings; + + while (std::regex_search(source, match, docstring_reg)) { + extracted_docstrings.push_back(match[0].str()); + source = match.suffix().str(); + } + for (const auto& docstring : expected_docstrings) { - auto found = source.find(docstring); - if (found == std::string::npos) { - std::cerr << "Could not find docstring: " << docstring << std::endl; + if (std::find(extracted_docstrings.begin(), extracted_docstrings.end(), docstring) == extracted_docstrings.end()) { + std::cerr << "Could not find expected docstring: " << docstring << std::endl; + ASSERT_TRUE(false); + } + } + + for (const auto& docstring : extracted_docstrings) { + if (std::find(expected_docstrings.begin(), expected_docstrings.end(), docstring) == expected_docstrings.end()) { + std::cerr << "Unexpected docstring found: " << docstring << std::endl; ASSERT_TRUE(false); } }