Refactor ParsedMethod, ParsedSignal and ParsedInheritedMethod

crates/cxx-qt-gen/src/generator/cpp/inherit.rs

@@ -35,7 +35,7 @@ pub fn generate(
                   return {base_class}::{func_ident}(args...);
               }}"#,
         mutability = if method.mutable { "" } else { " const" },
-        func_ident = method.ident.cxx_unqualified(),
+        func_ident = method.name.cxx_unqualified(),
         wrapper_ident = method.wrapper_ident(),
         return_type = return_type.unwrap_or_else(|| "void".to_string()),
         base_class = base_class

crates/cxx-qt-gen/src/generator/rust/inherit.rs

@@ -30,6 +30,7 @@ pub fn generate(
                     quote! { #ident: #ty }
                 })
                 .collect::<Vec<TokenStream>>();
+
             let ident = &method.method.sig.ident;
             let cxx_name_string = &method.wrapper_ident().to_string();
             let self_param = if method.mutable {
@@ -44,12 +45,16 @@ pub fn generate(
             if method.safe {
                 std::mem::swap(&mut unsafe_call, &mut unsafe_block);
             }
+
             let attrs = &method.method.attrs;
+            let doc_comments = &method.docs;
+
             syn::parse2(quote_spanned! {
                 method.method.span() =>
                 #unsafe_block extern "C++" {
                     #(#attrs)*
                     #[cxx_name = #cxx_name_string]
+                    #(#doc_comments)*
                     #unsafe_call fn #ident(#self_param, #(#parameters),*) #return_type;
                 }
             })

crates/cxx-qt-gen/src/generator/rust/method.rs

@@ -37,10 +37,9 @@ pub fn generate_rust_methods(
 
         let return_type = &invokable.method.sig.output;
 
-        let mut unsafe_block = None;
         let mut unsafe_call = Some(quote! { unsafe });
         if invokable.safe {
-            std::mem::swap(&mut unsafe_call, &mut unsafe_block);
+            std::mem::swap(&mut unsafe_call, &mut None);
         }
 
         let fragment = RustFragmentPair {
@@ -51,8 +50,8 @@ pub fn generate_rust_methods(
                     // Note that we are exposing a Rust method on the C++ type to C++
                     //
                     // CXX ends up generating the source, then we generate the matching header.
-                    #[doc(hidden)]
                     #[cxx_name = #wrapper_ident_cpp]
+                    #[doc(hidden)]
                     // TODO: Add #[namespace] of the QObject
                     #unsafe_call fn #invokable_ident_rust(#parameter_signatures) #return_type;
                 }
@@ -179,7 +178,6 @@ mod tests {
             &generated.cxx_mod_contents[0],
             quote! {
                 extern "Rust" {
-                    #[doc(hidden)]
                     #[cxx_name = "voidInvokableWrapper"]
                     fn void_invokable(self: &MyObject);
                 }
@@ -191,7 +189,6 @@ mod tests {
             &generated.cxx_mod_contents[1],
             quote! {
                 extern "Rust" {
-                    #[doc(hidden)]
                     #[cxx_name = "trivialInvokableWrapper"]
                     fn trivial_invokable(self: &MyObject, param: i32) -> i32;
                 }
@@ -203,7 +200,6 @@ mod tests {
             &generated.cxx_mod_contents[2],
             quote! {
                 extern "Rust" {
-                    #[doc(hidden)]
                     #[cxx_name = "opaqueInvokableWrapper"]
                     fn opaque_invokable(self: Pin<&mut MyObject>, param: &QColor) -> UniquePtr<QColor>;
                 }
@@ -215,7 +211,6 @@ mod tests {
             &generated.cxx_mod_contents[3],
             quote! {
                 extern "Rust" {
-                    #[doc(hidden)]
                     #[cxx_name = "unsafeInvokableWrapper"]
                     unsafe fn unsafe_invokable(self:&MyObject, param: *mut T) -> *mut T;
                 }

crates/cxx-qt-gen/src/generator/structuring/mod.rs

@@ -210,6 +210,31 @@ mod tests {
             .is_err());
     }
 
+    #[test]
+    fn test_inherited_lookup() {
+        let module = parse_quote! {
+            #[cxx_qt::bridge]
+            mod ffi {
+                extern "RustQt" {
+                    #[qobject]
+                    type MyObject = super::MyObjectRust;
+                }
+
+                unsafe extern "RustQt" {
+                    #[qinvokable]
+                    #[inherit]
+                    fn test_fn(self: Pin<&mut MyObject>);
+                }
+            }
+        };
+
+        let parser = Parser::from(module).unwrap();
+        let structures = Structures::new(&parser.cxx_qt_data).unwrap();
+
+        let qobject = structures.qobjects.first().unwrap();
+        assert!(qobject.method_lookup(&format_ident!("test_fn")).is_ok());
+    }
+
     #[test]
     fn test_structures() {
         let module: ItemMod = parse_quote! {

crates/cxx-qt-gen/src/generator/structuring/qobject.rs

@@ -7,7 +7,7 @@ use crate::naming::Name;
 use crate::parser::inherit::ParsedInheritedMethod;
 use crate::parser::method::ParsedMethod;
 use crate::parser::signals::ParsedSignal;
-use crate::parser::{qenum::ParsedQEnum, qobject::ParsedQObject};
+use crate::parser::{qenum::ParsedQEnum, qobject::ParsedQObject, Invokable};
 use proc_macro2::Ident;
 use syn::{Error, Result};
 
@@ -21,6 +21,14 @@ pub struct StructuredQObject<'a> {
     pub signals: Vec<&'a ParsedSignal>,
 }
 
+fn lookup(invokables: &[impl Invokable], id: &Ident) -> Option<Name> {
+    invokables
+        .iter()
+        .map(|invokable| invokable.name())
+        .find(|name| name.rust_unqualified() == id)
+        .cloned()
+}
+
 impl<'a> StructuredQObject<'a> {
     pub fn has_qobject_name(&self, ident: &Ident) -> bool {
         self.declaration.name.rust_unqualified() == ident
@@ -37,22 +45,16 @@ impl<'a> StructuredQObject<'a> {
         }
     }
 
+    /// Returns the name of the method with the provided Rust ident if it exists, or an error
     pub fn method_lookup(&self, id: &Ident) -> Result<Name> {
-        // TODO account for inherited methods too since those are in a different vector
-        self.methods
-            .iter()
-            .map(|method| &method.name)
-            .find(|name| name.rust_unqualified() == id)
-            .cloned()
+        lookup(&self.methods, id)
+            .or_else(|| lookup(&self.inherited_methods, id)) // fallback to searching inherited methods
             .ok_or_else(|| Error::new_spanned(id, format!("Method with name '{id}' not found!")))
     }
 
+    /// Returns the name of the signal with the provided Rust ident if it exists, or an error
     pub fn signal_lookup(&self, id: &Ident) -> Result<Name> {
-        self.signals
-            .iter()
-            .map(|signal| &signal.name)
-            .find(|name| name.rust_unqualified() == id)
-            .cloned()
+        lookup(&self.signals, id)
             .ok_or_else(|| Error::new_spanned(id, format!("Signal with name '{id}' not found!")))
     }
 

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

@@ -3,15 +3,14 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
+use crate::parser::{check_safety, extract_common_fields, separate_docs, Invokable, MethodFields};
 use crate::{
     naming::Name,
     parser::parameter::ParsedFunctionParameter,
-    syntax::{
-        attribute::attribute_take_path, expr::expr_to_string, foreignmod, safety::Safety, types,
-    },
+    syntax::{attribute::attribute_take_path, safety::Safety},
 };
 use quote::format_ident;
-use syn::{spanned::Spanned, Error, ForeignItemFn, Ident, Result};
+use syn::{Attribute, ForeignItemFn, Ident, Result};
 
 /// Describes a method found in an extern "RustQt" with #[inherit]
 pub struct ParsedInheritedMethod {
@@ -26,46 +25,45 @@ pub struct ParsedInheritedMethod {
     /// the parameters of the method, without the `self` argument
     pub parameters: Vec<ParsedFunctionParameter>,
     /// the name of the function in Rust, as well as C++
-    pub ident: Name,
+    pub name: Name,
+    /// All the docs (each line) of the inherited method
+    pub docs: Vec<Attribute>,
+}
+
+impl Invokable for &ParsedInheritedMethod {
+    fn name(&self) -> &Name {
+        &self.name
+    }
 }
 
 impl ParsedInheritedMethod {
     pub fn parse(mut method: ForeignItemFn, safety: Safety) -> Result<Self> {
-        if safety == Safety::Unsafe && method.sig.unsafety.is_none() {
-            return Err(Error::new(
-                method.span(),
-                "Inherited methods must be marked as unsafe or wrapped in an `unsafe extern \"RustQt\"` block!",
-            ));
-        }
-
-        let self_receiver = foreignmod::self_type_from_foreign_fn(&method.sig)?;
-        let (qobject_ident, mutability) = types::extract_qobject_ident(&self_receiver.ty)?;
-        let mutable = mutability.is_some();
+        check_safety(&method, &safety)?;
 
-        let parameters = ParsedFunctionParameter::parse_all_ignoring_receiver(&method.sig)?;
+        let docs = separate_docs(&mut method);
+        let invokable_fields = extract_common_fields(&method, docs)?;
 
-        let mut ident =
-            Name::from_rust_ident_and_attrs(&method.sig.ident, &method.attrs, None, None)?;
+        // This block seems unnecessary but since attrs are passed through on generator/rust/inherit.rs a duplicate attr would occur without it
+        attribute_take_path(&mut method.attrs, &["cxx_name"]);
 
-        if let Some(attr) = attribute_take_path(&mut method.attrs, &["cxx_name"]) {
-            ident = ident.with_cxx_name(expr_to_string(&attr.meta.require_name_value()?.value)?);
-        }
-
-        let safe = method.sig.unsafety.is_none();
+        Ok(Self::from_invokable_fields(invokable_fields, method))
+    }
 
-        Ok(Self {
+    fn from_invokable_fields(fields: MethodFields, method: ForeignItemFn) -> Self {
+        Self {
             method,
-            qobject_ident,
-            mutable,
-            parameters,
-            ident,
-            safe,
-        })
+            qobject_ident: fields.qobject_ident,
+            mutable: fields.mutable,
+            safe: fields.safe,
+            parameters: fields.parameters,
+            name: fields.name,
+            docs: fields.docs,
+        }
     }
 
     /// the name of the wrapper function in C++
     pub fn wrapper_ident(&self) -> Ident {
-        format_ident!("{}CxxQtInherit", self.ident.cxx_unqualified())
+        format_ident!("{}CxxQtInherit", self.name.cxx_unqualified())
     }
 }
 
@@ -146,10 +144,10 @@ mod tests {
         assert_eq!(parsed.qobject_ident, format_ident!("T"));
         assert_eq!(parsed.parameters.len(), 2);
         assert_eq!(
-            parsed.ident.rust_unqualified().to_string(),
+            parsed.name.rust_unqualified().to_string(),
             String::from("test")
         );
-        assert_eq!(parsed.ident.cxx_unqualified(), String::from("testFunction"));
+        assert_eq!(parsed.name.cxx_unqualified(), String::from("testFunction"));
         assert_eq!(
             parsed.wrapper_ident(),
             format_ident!("testFunctionCxxQtInherit")

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

@@ -6,11 +6,12 @@
 use crate::{
     naming::Name,
     parser::parameter::ParsedFunctionParameter,
-    syntax::{attribute::attribute_take_path, foreignmod, safety::Safety, types},
+    syntax::{attribute::attribute_take_path, safety::Safety},
 };
 use std::collections::HashSet;
-use syn::{spanned::Spanned, Error, ForeignItemFn, Ident, Result};
+use syn::{Error, ForeignItemFn, Ident, Result};
 
+use crate::parser::{check_safety, extract_common_fields, separate_docs, Invokable, MethodFields};
 #[cfg(test)]
 use quote::format_ident;
 
@@ -52,12 +53,23 @@ pub struct ParsedMethod {
     pub name: Name,
 }
 
+impl Invokable for &ParsedMethod {
+    fn name(&self) -> &Name {
+        &self.name
+    }
+}
+
 impl ParsedMethod {
     pub fn parse(mut method: ForeignItemFn, safety: Safety) -> Result<Self> {
-        if safety == Safety::Unsafe && method.sig.unsafety.is_none() {
-            return Err(Error::new(
-                method.span(),
-                "Invokable methods must be marked as unsafe or wrapped in an `unsafe extern \"RustQt\"` block!",
+        check_safety(&method, &safety)?;
+
+        let docs = separate_docs(&mut method);
+        let invokable_fields = extract_common_fields(&method, docs)?;
+
+        if invokable_fields.name.namespace().is_some() {
+            return Err(Error::new_spanned(
+                method.sig.ident,
+                "Methods / QInvokables cannot have a namespace attribute",
             ));
         }
 
@@ -72,38 +84,34 @@ impl ParsedMethod {
             ParsedQInvokableSpecifiers::Virtual,
         ] {
             if attribute_take_path(&mut method.attrs, specifier.as_str_slice()).is_some() {
-                specifiers.insert(specifier);
+                specifiers.insert(specifier); // Should a fn be able to be Override AND Virtual?
             }
         }
 
-        // Determine if the invokable is mutable
-        let self_receiver = foreignmod::self_type_from_foreign_fn(&method.sig)?;
-        let (qobject_ident, mutability) = types::extract_qobject_ident(&self_receiver.ty)?;
-        let mutable = mutability.is_some();
-
-        let parameters = ParsedFunctionParameter::parse_all_ignoring_receiver(&method.sig)?;
-
-        let safe = method.sig.unsafety.is_none();
-
-        let name = Name::from_rust_ident_and_attrs(&method.sig.ident, &method.attrs, None, None)?;
-
-        if name.namespace().is_some() {
-            return Err(Error::new_spanned(
-                method.sig.ident,
-                "Methods / QInvokables cannot have a namespace attribute",
-            ));
-        }
+        Ok(ParsedMethod::from_invokable_fields(
+            invokable_fields,
+            method,
+            specifiers,
+            is_qinvokable,
+        ))
+    }
 
-        Ok(ParsedMethod {
+    fn from_invokable_fields(
+        fields: MethodFields,
+        method: ForeignItemFn,
+        specifiers: HashSet<ParsedQInvokableSpecifiers>,
+        is_qinvokable: bool,
+    ) -> Self {
+        Self {
             method,
-            qobject_ident,
-            mutable,
-            parameters,
+            qobject_ident: fields.qobject_ident,
+            mutable: fields.mutable,
+            safe: fields.safe,
+            parameters: fields.parameters,
             specifiers,
-            safe,
             is_qinvokable,
-            name,
-        })
+            name: fields.name,
+        }
     }
 
     #[cfg(test)]