From f327880b8202542f0770d1de6bbeb13a5f40d6b4 Mon Sep 17 00:00:00 2001 From: Kristupas Antanavicius Date: Tue, 24 Oct 2023 10:47:05 +0300 Subject: [PATCH] Normalize case handling for objects and callbacks Objects and callbacks were not handling case consistently in generated across code, leading to compilation errors in some cases (see #22). The main problem arises in capital acronym handling, i.e. `HTTPClient`. Such acronyms are converted using `heck::to_upper_camel_case`, which converts upper case acronym `HTTP` into upper camel case `Http`, i.e. `HttpClient`. --- Cargo.lock | 12 +++++ .../templates/CallbackInterfaceTemplate.go | 2 +- bindgen/templates/ObjectTemplate.go | 10 ++-- binding_tests/custom_types_test.go | 10 ++-- binding_tests/errors_test.go | 2 +- binding_tests/name_case_test.go | 30 +++++++++++ fixtures/Cargo.toml | 4 +- fixtures/name-case/Cargo.toml | 24 +++++++++ fixtures/name-case/README.md | 3 ++ fixtures/name-case/build.rs | 7 +++ fixtures/name-case/src/lib.rs | 53 +++++++++++++++++++ fixtures/name-case/src/name-case.udl | 35 ++++++++++++ fixtures/src/lib.rs | 3 +- test_bindings.sh | 1 + 14 files changed, 181 insertions(+), 15 deletions(-) create mode 100644 binding_tests/name_case_test.go create mode 100644 fixtures/name-case/Cargo.toml create mode 100644 fixtures/name-case/README.md create mode 100644 fixtures/name-case/build.rs create mode 100644 fixtures/name-case/src/lib.rs create mode 100644 fixtures/name-case/src/name-case.udl diff --git a/Cargo.lock b/Cargo.lock index 688090c..09dc643 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -851,6 +851,7 @@ dependencies = [ "uniffi-fixture-time", "uniffi-go-fixture-destroy", "uniffi-go-fixture-errors", + "uniffi-go-fixture-name-case", "uniffi-go-fixture-objects", ] @@ -969,6 +970,17 @@ dependencies = [ "uniffi_macros", ] +[[package]] +name = "uniffi-go-fixture-name-case" +version = "0.22.0" +dependencies = [ + "glob", + "thiserror", + "uniffi", + "uniffi_bindgen", + "uniffi_testing", +] + [[package]] name = "uniffi-go-fixture-objects" version = "1.0.0" diff --git a/bindgen/templates/CallbackInterfaceTemplate.go b/bindgen/templates/CallbackInterfaceTemplate.go index 211b5e5..170ce43 100644 --- a/bindgen/templates/CallbackInterfaceTemplate.go +++ b/bindgen/templates/CallbackInterfaceTemplate.go @@ -15,7 +15,7 @@ type {{ type_name }} interface { // {{ foreign_callback }} cannot be callable be a compiled function at a same time type {{ foreign_callback }} struct {} -{% let cgo_callback_fn = self.cgo_callback_fn(type_name) -%} +{% let cgo_callback_fn = self.cgo_callback_fn(cbi.name()) -%} //export {{ cgo_callback_fn }} func {{ cgo_callback_fn }}(handle C.uint64_t, method C.int32_t, args C.RustBuffer, outBuf *C.RustBuffer) C.int32_t { cb := {{ type_|lift_fn }}(uint64(handle)); diff --git a/bindgen/templates/ObjectTemplate.go b/bindgen/templates/ObjectTemplate.go index 9ed8130..57778e9 100644 --- a/bindgen/templates/ObjectTemplate.go +++ b/bindgen/templates/ObjectTemplate.go @@ -5,23 +5,23 @@ {{- self.add_import("runtime") }} {%- let obj = ci.get_object_definition(name).unwrap() %} -{%- let canonical_name = type_|canonical_name %} +{%- let obj_name = obj.name()|class_name %} {%- if self.include_once_check("ObjectRuntime.go") %}{% include "ObjectRuntime.go" %}{% endif %} -type {{ canonical_name }} struct { +type {{ obj_name }} struct { ffiObject FfiObject } {%- match obj.primary_constructor() %} {%- when Some with (cons) %} -func New{{ canonical_name }}({% call go::arg_list_decl(cons) -%}) {% call go::return_type_decl(cons) %} { +func New{{ obj_name }}({% call go::arg_list_decl(cons) -%}) {% call go::return_type_decl(cons) %} { {% call go::ffi_call_binding(func, "") %} } {%- when None %} {%- endmatch %} {% for cons in obj.alternate_constructors() -%} -func {{ canonical_name }}{{ cons.name()|fn_name }}({% call go::arg_list_decl(cons) %}) {% call go::return_type_decl(cons) %} { +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 %} @@ -44,7 +44,7 @@ type {{ obj|ffi_converter_name }} struct {} var {{ obj|ffi_converter_name }}INSTANCE = {{ obj|ffi_converter_name }}{} func (c {{ obj|ffi_converter_name }}) lift(pointer unsafe.Pointer) {{ type_name }} { - result := &{{ canonical_name }} { + result := &{{ obj_name }} { newFfiObject( pointer, func(pointer unsafe.Pointer, status *C.RustCallStatus) { diff --git a/binding_tests/custom_types_test.go b/binding_tests/custom_types_test.go index 99be872..79d8e2e 100644 --- a/binding_tests/custom_types_test.go +++ b/binding_tests/custom_types_test.go @@ -5,11 +5,11 @@ package binding_tests import ( - "testing" - "net/url" + "net/url" + "testing" - "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/uniffi/custom_types" - "github.com/stretchr/testify/assert" + "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/uniffi/custom_types" + "github.com/stretchr/testify/assert" ) func unwrap[T any](value T, err error) T { @@ -30,6 +30,6 @@ func TestCustomTypes(t *testing.T) { // Change some data and ensure that the round-trip works demo.Url = *unwrap(url.Parse("http://new.example.com/")) - demo.Handle = 456; + demo.Handle = 456 assert.Equal(t, demo, custom_types.GetCustomTypesDemo(&demo)) } diff --git a/binding_tests/errors_test.go b/binding_tests/errors_test.go index ce8e4dc..113004c 100644 --- a/binding_tests/errors_test.go +++ b/binding_tests/errors_test.go @@ -5,9 +5,9 @@ package binding_tests import ( - "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/uniffi/errors" goerrors "errors" "fmt" + "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/uniffi/errors" "github.com/stretchr/testify/assert" "testing" ) diff --git a/binding_tests/name_case_test.go b/binding_tests/name_case_test.go new file mode 100644 index 0000000..9c51872 --- /dev/null +++ b/binding_tests/name_case_test.go @@ -0,0 +1,30 @@ +/* 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/. */ + +package binding_tests + +import ( + "errors" + . "github.com/NordSecurity/uniffi-bindgen-go/binding_tests/generated/uniffi/name_case" + "testing" +) + +type MyNameCaseCallback struct{} + +func (*MyNameCaseCallback) Test() {} + +func TestNameCaseCompiles(t *testing.T) { + _ = EnumTestVariantOne + _ = AssociatedEnumTestVariantTest{0} + + var expectedError *ErrorTestVariantOne + errors.As(NewErrorTestVariantOne(), &expectedError) + + var expectedAssociatedError *AssociatedErrorTestVariantTest + errors.As(NewAssociatedErrorTestVariantTest(0), &expectedAssociatedError) + + var _ *ObjectTest = NewObjectTest() + _ = RecordTest{0} + var _ CallbackTest = &MyNameCaseCallback{} +} diff --git a/fixtures/Cargo.toml b/fixtures/Cargo.toml index 0179b01..09752b5 100644 --- a/fixtures/Cargo.toml +++ b/fixtures/Cargo.toml @@ -20,7 +20,7 @@ uniffi-fixture-callbacks = { path = "../3rd-party/uniffi-rs/fixtures/callbacks" uniffi-fixture-coverall = { path = "../3rd-party/uniffi-rs/fixtures/coverall" } uniffi-fixture-external-types = { path = "../3rd-party/uniffi-rs/fixtures/external-types/lib" } uniffi-fixture-time = { path = "../3rd-party/uniffi-rs/fixtures/uniffi-fixture-time" } -uniffi-go-fixture-errors = { path = "errors" } uniffi-go-fixture-destroy = { path = "destroy" } +uniffi-go-fixture-errors = { path = "errors" } +uniffi-go-fixture-name-case = { path = "name-case" } uniffi-go-fixture-objects = { path = "objects" } - diff --git a/fixtures/name-case/Cargo.toml b/fixtures/name-case/Cargo.toml new file mode 100644 index 0000000..98c5447 --- /dev/null +++ b/fixtures/name-case/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "uniffi-go-fixture-name-case" +version = "0.22.0" +authors = ["Firefox Sync Team "] +edition = "2021" +license = "MPL-2.0" +publish = false + +[lib] +name = "uniffi_go_name_case" +crate-type = ["lib", "cdylib"] + +[dependencies] +thiserror = "1.0" +uniffi = { path = "../../3rd-party/uniffi-rs/uniffi" } + +[build-dependencies] +uniffi = {path = "../../3rd-party/uniffi-rs/uniffi", features = ["build"] } + +[dev-dependencies] +glob = "0.3" +uniffi = {path = "../../3rd-party/uniffi-rs/uniffi", features = ["bindgen-tests"] } +uniffi_bindgen = { path = "../../3rd-party/uniffi-rs/uniffi_bindgen" } +uniffi_testing = { path = "../../3rd-party/uniffi-rs/uniffi_testing" } diff --git a/fixtures/name-case/README.md b/fixtures/name-case/README.md new file mode 100644 index 0000000..396ade9 --- /dev/null +++ b/fixtures/name-case/README.md @@ -0,0 +1,3 @@ +# A basic test for uniffi components + +This test covers case senstitivity for generate code. \ No newline at end of file diff --git a/fixtures/name-case/build.rs b/fixtures/name-case/build.rs new file mode 100644 index 0000000..ad20e98 --- /dev/null +++ b/fixtures/name-case/build.rs @@ -0,0 +1,7 @@ +/* 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/. */ + +fn main() { + uniffi::generate_scaffolding("./src/name-case.udl").unwrap(); +} diff --git a/fixtures/name-case/src/lib.rs b/fixtures/name-case/src/lib.rs new file mode 100644 index 0000000..40901e8 --- /dev/null +++ b/fixtures/name-case/src/lib.rs @@ -0,0 +1,53 @@ +/* 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/. */ + +include!(concat!(env!("OUT_DIR"), "/name-case.uniffi.rs")); + +pub enum ENUMTest { + VARIANTOne, +} + +pub enum AssociatedENUMTest { + VARIANTTest{ code: i16 }, +} + +#[derive(Debug, thiserror::Error)] +pub enum ERRORTest { + #[error("Test")] + VARIANTOne, +} + +#[derive(Debug, thiserror::Error)] +pub enum AssociatedERRORTest { + #[error("Test")] + VARIANTTest{ code: i16 }, +} + +pub struct OBJECTTest { +} + +impl OBJECTTest { + pub fn new() -> Self { + OBJECTTest{} + } + + pub fn new_alternate() -> Self { + OBJECTTest{} + } + + pub fn test(&self) { + } +} + +pub struct RECORDTest { + test: i32, +} + +pub fn test() { + let _ = ERRORTest::VARIANTOne; +} + +pub trait CALLBACKTest { + fn test(&self); +} diff --git a/fixtures/name-case/src/name-case.udl b/fixtures/name-case/src/name-case.udl new file mode 100644 index 0000000..d104df1 --- /dev/null +++ b/fixtures/name-case/src/name-case.udl @@ -0,0 +1,35 @@ +namespace name_case { + void test(); +}; + +enum ENUMTest { "VARIANTOne" }; + +[Enum] +interface AssociatedENUMTest { + VARIANTTest(i16 code); +}; + +[Error] +enum ERRORTest { "VARIANTOne" }; + +[Error] +interface AssociatedERRORTest { + VARIANTTest(i16 code); +}; + +interface OBJECTTest { + constructor(); + + [Name="new_alternate"] + constructor(); + + void test(); +}; + +dictionary RECORDTest { + i32 test; +}; + +callback interface CALLBACKTest { + void test(); +}; diff --git a/fixtures/src/lib.rs b/fixtures/src/lib.rs index 474cf3e..8de1309 100644 --- a/fixtures/src/lib.rs +++ b/fixtures/src/lib.rs @@ -16,7 +16,8 @@ mod uniffi_fixtures { uniffi_coverall::uniffi_reexport_scaffolding!(); uniffi_external_types_lib::uniffi_reexport_scaffolding!(); - uniffi_go_errors::uniffi_reexport_scaffolding!(); uniffi_go_destroy::uniffi_reexport_scaffolding!(); + uniffi_go_errors::uniffi_reexport_scaffolding!(); + uniffi_go_name_case::uniffi_reexport_scaffolding!(); uniffi_go_objects::uniffi_reexport_scaffolding!(); } diff --git a/test_bindings.sh b/test_bindings.sh index 13edee3..d0bdd18 100755 --- a/test_bindings.sh +++ b/test_bindings.sh @@ -25,6 +25,7 @@ bindings 3rd-party/uniffi-rs/fixtures/coverall/src/coverall.udl bindings 3rd-party/uniffi-rs/fixtures/uniffi-fixture-time/src/chronological.udl bindings fixtures/destroy/src/destroy.udl bindings fixtures/errors/src/errors.udl +bindings fixtures/name-case/src/name-case.udl bindings fixtures/objects/src/objects.udl pushd $BINDINGS_DIR/..