Add HashSet of flags to ParsedQPoperty for optional getters and setters

CHANGELOG.md

@@ -15,7 +15,8 @@
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased](https://github.com/KDAB/cxx-qt/compare/v0.6.1...HEAD)
+- Add support for specifying read write and notify in qproperty macro, including support for custom user defined functions
 
 ### Added
 

book/src/bridge/extern_rustqt.md

@@ -131,8 +131,20 @@
 
 These setters and getters assure that the changed signal is emitted every time the property is edited.
 
-> Note that in the future it will be possible to specify custom getters and setters
+It is also possible to specify custom getters, setters and on-changed functions, using flags passed like so:
+`#[qproperty(TYPE, NAME, read = myGetter, write = mySetter, notify = myOnChanged)]`
+
+> Note: currently TypeName lookups are not currently supported, so either the name must be camel case or specified like `#[cxx_name = "my_getter"]`
+
+It is also possible to use any combination of custom functions or omitting entirely, but if flags are specified, read must be one of them as all properties need to be able to be read.
+
+### Examples:
+- `#[qproperty(TYPE, NAME, read)]` A read only prop
+- `#[qproperty(TYPE, NAME, read = myGetter, write, notify)]` custom getter provided but will generate setter and on-changed
+- `#[qproperty(TYPE, NAME)]` is syntactic sugar for `#[qproperty(TYPE, NAME, read, write, notify)]`
+- `#[qproperty(TYPE, NAME, write)]` is an error as read was not passed
+
 
 ## Methods
 
 Any signature with a `self` parameter is interpreted as a Rust method and exposed to C++ method for the given type.

crates/cxx-qt-gen/src/generator/cpp/property/getter.rs

@@ -14,7 +14,8 @@ pub fn generate(
     qobject_ident: &str,
     return_cxx_ty: &str,
 ) -> Option<CppFragment> {
-    if let NameState::Auto(name) = &idents.getter {
+    if let (NameState::Auto(name), Some(getter_wrapper)) = (&idents.getter, &idents.getter_wrapper)
+    {
         Some(CppFragment::Pair {
             header: format!(
                 "{return_cxx_ty} const& {ident_getter}() const;",
@@ -30,17 +31,19 @@ pub fn generate(
                     }}
                     "#,
                 ident_getter = name.cxx_unqualified(),
-                ident_getter_wrapper = idents.getter_wrapper.cxx_unqualified(),
+                ident_getter_wrapper = getter_wrapper.cxx_unqualified(),
             ),
         })
     } else {
         None
     }
 }
 
-pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> CppFragment {
-    CppFragment::Header(format!(
-        "{cxx_ty} const& {ident_getter_wrapper}() const noexcept;",
-        ident_getter_wrapper = idents.getter_wrapper.cxx_unqualified()
-    ))
+pub fn generate_wrapper(idents: &QPropertyNames, cxx_ty: &str) -> Option<CppFragment> {
+    idents.getter_wrapper.as_ref().map(|name| {
+        CppFragment::Header(format!(
+            "{cxx_ty} const& {ident_getter_wrapper}() const noexcept;",
+            ident_getter_wrapper = name.cxx_unqualified()
+        ))
+    })
 }

crates/cxx-qt-gen/src/generator/cpp/property/mod.rs

@@ -35,12 +35,11 @@ pub fn generate_cpp_properties(
 
         if let Some(getter) = getter::generate(&idents, &qobject_ident, &cxx_ty) {
             generated.methods.push(getter);
-            generated
-                .private_methods
-                .push(getter::generate_wrapper(&idents, &cxx_ty));
         }
 
-        // Optional generations, returning None if flags specified not to generate anything
+        if let Some(getter_wrapper) = getter::generate_wrapper(&idents, &cxx_ty) {
+            generated.private_methods.push(getter_wrapper);
+        }
 
         if let Some(setter) = setter::generate(&idents, &qobject_ident, &cxx_ty) {
             generated.methods.push(setter)

crates/cxx-qt-gen/src/generator/cpp/property/signal.rs

@@ -16,8 +16,8 @@ use crate::{
 pub fn generate(idents: &QPropertyNames, qobject_idents: &QObjectNames) -> Option<ParsedSignal> {
     // We build our signal in the generation phase as we need to use the naming
     // structs to build the signal name
-    let cpp_class_rust = &qobject_idents.name.rust_unqualified();
     if let Some(NameState::Auto(notify)) = &idents.notify {
+        let cpp_class_rust = &qobject_idents.name.rust_unqualified();
         let notify_cpp = notify.cxx_unqualified();
         let notify_rust = notify.rust_unqualified();
         let method: ForeignItemFn = syn::parse_quote! {

crates/cxx-qt-gen/src/generator/naming/property.rs

@@ -42,7 +42,7 @@ impl NameState {
 pub struct QPropertyNames {
     pub name: Name,
     pub getter: NameState,
-    pub getter_wrapper: Name,
+    pub getter_wrapper: Option<Name>,
     pub setter: Option<NameState>,
     pub setter_wrapper: Option<Name>,
     pub notify: Option<NameState>,
@@ -73,8 +73,14 @@ impl From<&ParsedQProperty> for QPropertyNames {
             None
         };
 
+        let getter_wrapper = if let NameState::Auto(ref getter) = getter {
+            Some(wrapper_name_from_function_name(getter))
+        } else {
+            None
+        };
+
         Self {
-            getter_wrapper: wrapper_name_from_function_name(&getter),
+            getter_wrapper,
             getter,
             setter_wrapper,
             setter,

crates/cxx-qt-gen/src/generator/rust/property/getter.rs

@@ -23,9 +23,11 @@ pub fn generate(
     cxx_ty: &Type,
     type_names: &TypeNames,
 ) -> Result<Option<RustFragmentPair>> {
-    if let NameState::Auto(getter) = &idents.getter {
+    if let (NameState::Auto(getter), Some(getter_wrapper)) =
+        (&idents.getter, &idents.getter_wrapper)
+    {
         let cpp_class_name_rust = &qobject_idents.name.rust_unqualified();
-        let getter_wrapper_cpp = idents.getter_wrapper.cxx_unqualified();
+        let getter_wrapper_cpp = getter_wrapper.cxx_unqualified();
         let getter_rust = getter.rust_unqualified();
         let ident = idents.name.rust_unqualified();
         let ident_str = ident.to_string();

crates/cxx-qt-gen/src/generator/rust/property/setter.rs

@@ -67,7 +67,6 @@ pub fn generate(
                     #[doc = #ident_str]
                     pub fn #setter_rust(mut self: core::pin::Pin<&mut Self>, value: #qualified_ty) {
                         use cxx_qt::CxxQtType;
-
                         if self.#ident == value {
                             // don't want to set the value again and reemit the signal,
                             // as this can cause binding loops

crates/cxx-qt-gen/src/parser/property.rs

@@ -10,13 +10,14 @@ use syn::{
     Attribute, Expr, Ident, Meta, MetaNameValue, Result, Token, Type,
 };
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum FlagState {
     Auto, // Might need to refactor to also store the generated ident here
     Custom(Ident),
 }
 
 impl FlagState {
+    #[cfg(test)]
     pub fn is_auto(&self) -> bool {
         matches!(self, Self::Auto)
     }
@@ -41,17 +42,6 @@ impl Default for QPropertyFlags {
     }
 }
 
-impl QPropertyFlags {
-    // Doesn't really represent a realistic state, just used for collecting in the parser
-    fn new() -> Self {
-        Self {
-            read: FlagState::Auto,
-            write: None,
-            notify: None,
-        }
-    }
-}
-
 /// Describes a single Q_PROPERTY for a struct
 pub struct ParsedQProperty {
     /// The [syn::Ident] of the property
@@ -62,7 +52,6 @@ pub struct ParsedQProperty {
     pub flags: QPropertyFlags,
 }
 
-// TODO: Returning struct instead of tuple might be more descriptive
 fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> {
     let ident = name_value.path.require_ident()?.clone();
     let expr = &name_value.value;
@@ -80,6 +69,20 @@ fn parse_meta_name_value(name_value: &MetaNameValue) -> Result<(Ident, Ident)> {
     Ok((ident, func_signature))
 }
 
+fn parse_meta(meta: Meta) -> Result<(Ident, Option<Ident>)> {
+    match meta {
+        Meta::Path(path) => Ok((path.require_ident()?.clone(), None)),
+        Meta::NameValue(name_value) => {
+            let (field, ident) = parse_meta_name_value(&name_value)?;
+            Ok((field, Some(ident)))
+        }
+        _ => Err(Error::new(
+            meta.span(),
+            "Invalid syntax, flags must be specified as either `read` or `read = my_getter`",
+        )),
+    }
+}
+
 impl ParsedQProperty {
     pub fn parse(attr: Attribute) -> Result<Self> {
         attr.parse_args_with(|input: ParseStream| -> Result<Self> {
@@ -88,7 +91,7 @@ impl ParsedQProperty {
             let ident = input.parse()?;
 
             if input.is_empty() {
-                // No flags passed so desugar: ```#[qproperty(T, ident)]``` -> ```#[qproperty(T, ident, read, write, notify)]```
+                // No flags passed so desugar: #[qproperty(T, ident)] -> #[qproperty(T, ident, read, write, notify)]
                 Ok(Self {
                     ident,
                     ty,
@@ -126,25 +129,8 @@ impl ParsedQProperty {
                 };
 
                 for flag in flags {
-                    // Could maybe refactor parse_meta_name_value to parse_meta,
-                    // which would return an (Ident, Option<Ident>) and extract the logic below to a new fn
-                    match flag {
-                        Meta::Path(path) => {
-                            // Set flag as auto
-                            update_fields(path.require_ident()?, None)?;
-                        },
-                        Meta::NameValue(name_value) => {
-                            let kv_pair = parse_meta_name_value(&name_value)?;
-                            let value = Some(kv_pair.1);
-
-                            // Set Flags with Custom value
-                            update_fields(&kv_pair.0, value)?;
-                        },
-                        _ => return Err(Error::new(
-                            flag.span(),
-                            "Invalid syntax, flags must be specified as either `read` or `read = my_getter`",
-                        )),
-                    };
+                    let (field, maybe_value) = parse_meta(flag)?;
+                    update_fields(&field, maybe_value)?;
                 }
 
                 if let Some(read) = read {
@@ -157,8 +143,7 @@ impl ParsedQProperty {
                             notify,
                         },
                     })
-                }
-                else {
+                } else {
                     Err(Error::new(
                         punctuated_flags.span(),
                         "If flags are passed, read must be explicitly specified",
@@ -228,22 +213,29 @@ mod tests {
         assert_eq!(property.ty, parse_quote! { T });
 
         // Can use  assert_matches! when https://github.com/rust-lang/rust/issues/82775 gets stabilised
-        let expected_read = format_ident!("my_getter");
-        assert!(matches!(
+        // let expected_read = format_ident!("my_getter");
+        // assert!(matches!(
+        //     property.flags.read,
+        //     FlagState::Custom(ident) if ident == expected_read
+        // ));
+        assert_eq!(
             property.flags.read,
-            FlagState::Custom(ident) if ident == expected_read
-        ));
+            FlagState::Custom(format_ident!("my_getter"))
+        );
 
-        assert!(property.flags.write.is_some());
-        assert!(property.flags.write.unwrap().is_auto());
+        assert_eq!(property.flags.write, Some(FlagState::Auto));
 
         assert!(property.flags.notify.is_some());
 
-        let expected_notify = format_ident!("my_notifier");
-        assert!(matches!(
+        // let expected_notify = format_ident!("my_notifier");
+        // assert!(matches!(
+        //     property.flags.notify,
+        //     Some(FlagState::Custom(ident)) if ident == expected_notify
+        // ));
+        assert_eq!(
             property.flags.notify,
-            Some(FlagState::Custom(ident)) if ident == expected_notify
-        ));
+            Some(FlagState::Custom(format_ident!("my_notifier")))
+        );
     }
 
     #[test]