Feature naming on properties (KDAB#1072)

* Switch ParsedQProperty to use Name instead of Ident, and allow cxx_name and rust_name in flags - Add closure to group behaviour of flags requiring a value passed * Add parsing checks for name flags - using cxx_name or rust_name is allowed without READ flag - Maintains error for using property flags e.g. WRITE without including READ - cxx_name and rust_name must have strings as values, whereas other flags still need idents * Add naming to a property in test_inputs - Also refactor value assignment in parse_meta_name_value - Refactor cxx_name so that having no cxx_name requires no rust_name to do camelcase conversion * Add new Name method to combine common logic - Applies options with the correct renaming logic, and this is used in property renaming - Also switch from format_ident to a propagating syn::parse_string instead * Update changelog and book * Add to example and refactor auto_camel in name function
LeonMatthesKDAB · Sep 19, 2024 · 4b23a7b · 4b23a7b
1 parent 4da68f5
commit 4b23a7b
Show file tree

Hide file tree

Showing 19 changed files with 614 additions and 195 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add support for the constant, required, reset and final flags in the qproperty macro
 - QObject subclasses can now inherit from other CXX-Qt generated QObject classes
 - `BUILD_WASM` CMake option to support WebAssembly builds and a book page for building for WASM
+- Add support for cxx_name and rust_name on qproperty attributes which applies to the QProperty generated as well as functions
 
 ### Changed
 

diff --git a/book/src/bridge/extern_rustqt.md b/book/src/bridge/extern_rustqt.md
@@ -141,6 +141,8 @@ Using the read flag will cause CXX-Qt to generate a getter function with an auto
 
 If a custom function is specified, an implementation both in qobject::MyObject and and export in the bridge is expected.
 
+Additionally, usng cxx_name and rust_name is possible similarly to the attributes avilable on other items. e.g. `#[qproperty(i32, num, cxx_name = "numberProp")]`
+
 ### Examples
 
 - `#[qproperty(TYPE, NAME, READ)]` A read only property
@@ -165,6 +167,10 @@ If a custom function is specified, an implementation both in qobject::MyObject a
   - Specifies that the property will not be overriden by a derived class
 - `RESET = my_reset`
   - Specifies a function to reset the property to a default value, user function __must__ be provided or it will not compile
+- `cxx_name = "myCxxName`
+  - Specifies an alternative name to use on the C++ side, applying to the property name as well as autogenerated functions
+- `rust_name = "my_rust_name"`
+  - Specifies an alternative name to use on the rust side, applying to the property name as well as autogenerated functions
 
 ## Methods
 

diff --git a/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs b/crates/cxx-qt-gen/src/generator/cpp/property/mod.rs
@@ -62,6 +62,7 @@ pub fn generate_cpp_properties(
 pub mod tests {
     use super::*;
 
+    use crate::generator::naming::property::property_name_from_rust_name;
     use crate::generator::naming::qobject::tests::create_qobjectname;
     use crate::generator::structuring::Structures;
     use crate::parser::property::QPropertyFlags;
@@ -471,7 +472,7 @@ pub mod tests {
     #[test]
     fn test_generate_cpp_properties_mapped_cxx_name() {
         let properties = vec![ParsedQProperty {
-            ident: format_ident!("mapped_property"),
+            name: property_name_from_rust_name(format_ident!("mapped_property")),
             ty: parse_quote! { A },
             flags: QPropertyFlags::default(),
         }];

diff --git a/crates/cxx-qt-gen/src/generator/naming/property.rs b/crates/cxx-qt-gen/src/generator/naming/property.rs
@@ -63,14 +63,14 @@ impl QPropertyNames {
         property: &ParsedQProperty,
         structured_qobject: &StructuredQObject,
     ) -> Result<Self> {
-        let property_name = property_name_from_rust_name(property.ident.clone());
+        let property_name = &property.name;
 
         // Cache flags as they are accessed multiple times
         let flags = &property.flags;
 
         let getter = NameState::from_flag_with_auto_fn(
             &flags.read,
-            || getter_name_from_property(&property_name),
+            || getter_name_from_property(property_name),
             structured_qobject,
             false,
         )?;
@@ -81,7 +81,7 @@ impl QPropertyNames {
             .map(|setter| {
                 NameState::from_flag_with_auto_fn(
                     &setter,
-                    || setter_name_from_property(&property_name),
+                    || setter_name_from_property(property_name),
                     structured_qobject,
                     false,
                 )
@@ -94,7 +94,7 @@ impl QPropertyNames {
             .map(|notify| {
                 NameState::from_flag_with_auto_fn(
                     &notify,
-                    || notify_name_from_property(&property_name),
+                    || notify_name_from_property(property_name),
                     structured_qobject,
                     true,
                 )
@@ -112,14 +112,15 @@ impl QPropertyNames {
             setter,
             notify,
             reset,
-            name: property_name,
+            name: property_name.clone(),
         })
     }
 }
 
-fn property_name_from_rust_name(ident: Ident) -> Name {
+pub fn property_name_from_rust_name(ident: Ident) -> Name {
     // TODO: ParsedQProperty should probably take care of this already and allow the user to set
     // their own name for C++ if they want to.
+    // REMOVE THIS FN
     let cxx_name = ident.to_string().to_case(Case::Camel);
     Name::new(ident).with_cxx_name(cxx_name)
 }
@@ -159,7 +160,7 @@ pub mod tests {
 
     pub fn create_i32_qpropertyname() -> QPropertyNames {
         let property = ParsedQProperty {
-            ident: format_ident!("my_property"),
+            name: property_name_from_rust_name(format_ident!("my_property")),
             ty: parse_quote! { i32 },
             flags: QPropertyFlags::default(),
         };
@@ -175,27 +176,24 @@ pub mod tests {
     fn test_parsed_property() {
         let names = create_i32_qpropertyname();
         assert_eq!(names.name.cxx_unqualified(), "myProperty");
-        assert_eq!(names.name.rust_unqualified(), &format_ident!("my_property"));
+        assert_eq!(names.name.rust_unqualified(), "my_property");
         assert_eq!(names.getter.cxx_unqualified(), "getMyProperty");
-        assert_eq!(
-            names.getter.rust_unqualified(),
-            &format_ident!("my_property")
-        );
+        assert_eq!(names.getter.rust_unqualified(), "my_property");
         assert_eq!(
             names.setter.as_ref().unwrap().cxx_unqualified(),
             "setMyProperty"
         );
         assert_eq!(
             names.setter.as_ref().unwrap().rust_unqualified(),
-            &format_ident!("set_my_property")
+            "set_my_property"
         );
         assert_eq!(
             names.notify.as_ref().unwrap().cxx_unqualified(),
             "myPropertyChanged"
         );
         assert_eq!(
             names.notify.as_ref().unwrap().rust_unqualified(),
-            &format_ident!("my_property_changed")
+            "my_property_changed"
         );
     }
 }
diff --git a/crates/cxx-qt-gen/src/generator/naming/qobject.rs b/crates/cxx-qt-gen/src/generator/naming/qobject.rs
@@ -109,19 +109,13 @@ pub mod tests {
         let names =
             QObjectNames::from_qobject(&create_parsed_qobject(), &TypeNames::mock()).unwrap();
         assert_eq!(names.name.cxx_unqualified(), "MyObject");
-        assert_eq!(names.name.rust_unqualified(), &format_ident!("MyObject"));
+        assert_eq!(names.name.rust_unqualified(), "MyObject");
         assert_eq!(names.rust_struct.cxx_unqualified(), "MyObjectRust");
-        assert_eq!(
-            names.rust_struct.rust_unqualified(),
-            &format_ident!("MyObjectRust")
-        );
-        assert_eq!(
-            names.cxx_qt_thread_class,
-            format_ident!("MyObjectCxxQtThread")
-        );
+        assert_eq!(names.rust_struct.rust_unqualified(), "MyObjectRust");
+        assert_eq!(names.cxx_qt_thread_class, "MyObjectCxxQtThread");
         assert_eq!(
             names.cxx_qt_thread_queued_fn_struct,
-            format_ident!("MyObjectCxxQtThreadQueuedFn")
+            "MyObjectCxxQtThreadQueuedFn"
         );
 
         assert_eq!(

diff --git a/crates/cxx-qt-gen/src/generator/rust/constructor.rs b/crates/cxx-qt-gen/src/generator/rust/constructor.rs
@@ -794,16 +794,15 @@ mod tests {
 
     #[test]
     fn constructor_impl_with_unused_lifetime() {
-        let result = super::generate(
+        assert!(generate(
             &[&Constructor {
                 lifetime: Some(parse_quote! { 'a }),
                 ..mock_constructor()
             }],
             &mock_name(),
             &mock_namespace(),
             &TypeNames::mock(),
-        );
-
-        assert!(result.is_err());
+        )
+        .is_err());
     }
 }
diff --git a/crates/cxx-qt-gen/src/generator/rust/property/mod.rs b/crates/cxx-qt-gen/src/generator/rust/property/mod.rs
@@ -67,6 +67,7 @@ pub fn generate_rust_properties(
 mod tests {
     use super::*;
 
+    use crate::generator::naming::property::property_name_from_rust_name;
     use crate::parser::property::QPropertyFlags;
     use crate::parser::qobject::ParsedQObject;
     use crate::{generator::naming::qobject::tests::create_qobjectname, tests::assert_tokens_eq};
@@ -77,17 +78,17 @@ mod tests {
     fn test_generate_rust_properties() {
         let properties = vec![
             ParsedQProperty {
-                ident: format_ident!("trivial_property"),
+                name: property_name_from_rust_name(format_ident!("trivial_property")),
                 ty: parse_quote! { i32 },
                 flags: QPropertyFlags::default(),
             },
             ParsedQProperty {
-                ident: format_ident!("opaque_property"),
+                name: property_name_from_rust_name(format_ident!("opaque_property")),
                 ty: parse_quote! { UniquePtr<QColor> },
                 flags: QPropertyFlags::default(),
             },
             ParsedQProperty {
-                ident: format_ident!("unsafe_property"),
+                name: property_name_from_rust_name(format_ident!("unsafe_property")),
                 ty: parse_quote! { *mut T },
                 flags: QPropertyFlags::default(),
             },

diff --git a/crates/cxx-qt-gen/src/generator/structuring/mod.rs b/crates/cxx-qt-gen/src/generator/structuring/mod.rs
@@ -123,92 +123,6 @@ mod tests {
     use quote::format_ident;
     use syn::{parse_quote, ItemMod};
 
-    #[test]
-    fn test_structuring_unknown_qobject() {
-        let module = parse_quote! {
-            #[cxx_qt::bridge]
-            mod ffi {
-                extern "RustQt" {
-                    #[qobject]
-                    type MyObject = super::MyObjectRust;
-                }
-
-                unsafe extern "RustQt" {
-                    #[qsignal]
-                    fn ready(self: Pin<&mut UnknownObject>);
-                }
-            }
-        };
-        let parser = Parser::from(module).unwrap();
-        let structures = Structures::new(&parser.cxx_qt_data);
-
-        assert!(structures.is_err());
-    }
-
-    #[test]
-    fn test_module_invalid_qobject_qenum() {
-        let module = parse_quote! {
-            #[cxx_qt::bridge]
-            mod ffi {
-                #[qenum(MyObject)]
-                enum MyEnum {
-                    A,
-                }
-            }
-        };
-
-        let parser = Parser::from(module).unwrap();
-        assert!(Structures::new(&parser.cxx_qt_data).is_err());
-    }
-
-    #[test]
-    fn test_module_invalid_qobject_method() {
-        let module = parse_quote! {
-            #[cxx_qt::bridge]
-            mod ffi {
-                unsafe extern "RustQt" {
-                    #[qinvokable]
-                    fn test_fn(self: Pin<&mut MyObject>);
-                }
-            }
-        };
-
-        let parser = Parser::from(module).unwrap();
-        assert!(Structures::new(&parser.cxx_qt_data).is_err());
-    }
-
-    #[test]
-    fn test_module_invalid_qobject_signal() {
-        let module = parse_quote! {
-            #[cxx_qt::bridge]
-            mod ffi {
-                unsafe extern "RustQt" {
-                    #[qsignal]
-                    fn test_fn(self: Pin<&mut MyObject>);
-                }
-            }
-        };
-
-        let parser = Parser::from(module).unwrap();
-        assert!(Structures::new(&parser.cxx_qt_data).is_err());
-    }
-
-    #[test]
-    fn test_module_invalid_qobject_inherited() {
-        let module = parse_quote! {
-            #[cxx_qt::bridge]
-            mod ffi {
-                unsafe extern "RustQt" {
-                    #[inherit]
-                    fn test_fn(self: Pin<&mut MyObject>);
-                }
-            }
-        };
-
-        let parser = Parser::from(module).unwrap();
-        assert!(Structures::new(&parser.cxx_qt_data).is_err());
-    }
-
     #[test]
     fn test_invalid_lookup() {
         let module = parse_quote! {
@@ -338,20 +252,15 @@ mod tests {
         }
     }
 
-    fn assert_structuring_error(additional_items: impl IntoIterator<Item = syn::Item>) {
-        let mut bridge = mock_bridge();
-        bridge.content.as_mut().unwrap().1.extend(additional_items);
-        let parser = Parser::from(bridge).unwrap();
-        assert!(Structures::new(&parser.cxx_qt_data).is_err());
-    }
-
     #[test]
     fn test_incompatible_trait_impl() {
-        // TODO: Possible to use assert_parse_errors?
-        assert_structuring_error([
+        let mut bridge = mock_bridge();
+        bridge.content.as_mut().unwrap().1.extend([
             parse_quote! {impl cxx_qt::Threading for MyObject {}},
             parse_quote! {impl cxx_qt::Threading for MyObject {}},
         ]);
+        let parser = Parser::from(bridge).unwrap();
+        assert!(Structures::new(&parser.cxx_qt_data).is_err());
     }
 
     #[test]

diff --git a/crates/cxx-qt-gen/src/naming/mod.rs b/crates/cxx-qt-gen/src/naming/mod.rs
@@ -16,5 +16,5 @@ mod name;
 pub(crate) mod rust;
 mod type_names;
 
-pub use name::Name;
+pub use name::{AutoCamel, Name};
 pub use type_names::TypeNames;