cxx-qt-gen: introduce attribute_take_path for common pattern

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

@@ -3,7 +3,7 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use crate::syntax::attribute::attribute_find_path;
+use crate::syntax::attribute::{attribute_find_path, attribute_take_path};
 use crate::syntax::foreignmod::{foreign_mod_to_foreign_item_types, ForeignTypeIdentAlias};
 use crate::syntax::path::path_from_idents;
 use crate::syntax::safety::Safety;
@@ -94,11 +94,9 @@ impl ParsedCxxQtData {
                                     syn::parse2(tokens.clone())?;
 
                                 // Check this type is tagged with a #[qobject]
-                                if let Some(index) =
-                                    attribute_find_path(&foreign_alias.attrs, &["qobject"])
+                                if attribute_take_path(&mut foreign_alias.attrs, &["qobject"])
+                                    .is_some()
                                 {
-                                    foreign_alias.attrs.remove(index);
-
                                     // Load the QObject
                                     let mut qobject = ParsedQObject::try_from(&foreign_alias)?;
 
@@ -260,10 +258,7 @@ impl ParsedCxxQtData {
         for item in foreign_mod.items.drain(..) {
             if let ForeignItem::Fn(mut foreign_fn) = item {
                 // Test if the function is a signal
-                if let Some(index) = attribute_find_path(&foreign_fn.attrs, &["qsignal"]) {
-                    // Remove the signals attribute
-                    foreign_fn.attrs.remove(index);
-
+                if attribute_take_path(&mut foreign_fn.attrs, &["qsignal"]).is_some() {
                     let parsed_signal_method = ParsedSignal::parse(foreign_fn, safe_call)?;
 
                     self.with_qobject(&parsed_signal_method.qobject_ident)?
@@ -272,10 +267,7 @@ impl ParsedCxxQtData {
                 // Test if the function is an inheritance method
                 //
                 // Note that we need to test for qsignal first as qsignals have their own inherit meaning
-                } else if let Some(index) = attribute_find_path(&foreign_fn.attrs, &["inherit"]) {
-                    // Remove the inherit attribute
-                    foreign_fn.attrs.remove(index);
-
+                } else if attribute_take_path(&mut foreign_fn.attrs, &["inherit"]).is_some() {
                     let parsed_inherited_method =
                         ParsedInheritedMethod::parse(foreign_fn, safe_call)?;
 

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

@@ -7,7 +7,7 @@ use crate::{
     generator::naming::CombinedIdent,
     parser::parameter::ParsedFunctionParameter,
     syntax::{
-        attribute::attribute_find_path, expr::expr_to_string, foreignmod, safety::Safety, types,
+        attribute::attribute_take_path, expr::expr_to_string, foreignmod, safety::Safety, types,
     },
 };
 use quote::format_ident;
@@ -46,13 +46,11 @@ impl ParsedInheritedMethod {
 
         let mut ident = CombinedIdent::from_rust_function(method.sig.ident.clone());
 
-        if let Some(index) = attribute_find_path(&method.attrs, &["cxx_name"]) {
+        if let Some(attr) = attribute_take_path(&mut method.attrs, &["cxx_name"]) {
             ident.cpp = format_ident!(
                 "{}",
-                expr_to_string(&method.attrs[index].meta.require_name_value()?.value)?
+                expr_to_string(&attr.meta.require_name_value()?.value)?
             );
-
-            method.attrs.remove(index);
         }
 
         let safe = method.sig.unsafety.is_none();

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

@@ -5,7 +5,7 @@
 
 use crate::{
     parser::parameter::ParsedFunctionParameter,
-    syntax::{attribute::*, foreignmod, safety::Safety, types},
+    syntax::{attribute::attribute_take_path, foreignmod, safety::Safety, types},
 };
 use std::collections::HashSet;
 use syn::{spanned::Spanned, Error, ForeignItemFn, Ident, Result};
@@ -56,13 +56,7 @@ impl ParsedMethod {
         }
 
         // Determine if the method is invokable
-        let is_qinvokable = if let Some(index) = attribute_find_path(&method.attrs, &["qinvokable"])
-        {
-            method.attrs.remove(index);
-            true
-        } else {
-            false
-        };
+        let is_qinvokable = attribute_take_path(&mut method.attrs, &["qinvokable"]).is_some();
 
         // Parse any C++ specifiers
         let mut specifiers = HashSet::new();
@@ -71,8 +65,7 @@ impl ParsedMethod {
             ParsedQInvokableSpecifiers::Override,
             ParsedQInvokableSpecifiers::Virtual,
         ] {
-            if let Some(index) = attribute_find_path(&method.attrs, specifier.as_str_slice()) {
-                method.attrs.remove(index);
+            if attribute_take_path(&mut method.attrs, specifier.as_str_slice()).is_some() {
                 specifiers.insert(specifier);
             }
         }

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

@@ -12,7 +12,7 @@ pub mod property;
 pub mod qobject;
 pub mod signals;
 
-use crate::syntax::{attribute::attribute_find_path, expr::expr_to_string};
+use crate::syntax::{attribute::attribute_take_path, expr::expr_to_string};
 use cxxqtdata::ParsedCxxQtData;
 use syn::{
     punctuated::Punctuated, spanned::Spanned, token::Brace, Error, ItemMod, Meta, Result, Token,
@@ -39,9 +39,7 @@ impl Parser {
         let mut cxx_file_stem = module.ident.to_string();
 
         // Remove the cxx_qt::bridge attribute
-        if let Some(index) = attribute_find_path(&module.attrs, &["cxx_qt", "bridge"]) {
-            let attr = module.attrs.remove(index);
-
+        if let Some(attr) = attribute_take_path(&mut module.attrs, &["cxx_qt", "bridge"]) {
             // If we are no #[cxx_qt::bridge] but #[cxx_qt::bridge(A = B)] then process
             if !matches!(attr.meta, Meta::Path(_)) {
                 let nested =

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

@@ -9,7 +9,7 @@ use crate::{
         property::ParsedQProperty, signals::ParsedSignal,
     },
     syntax::{
-        attribute::attribute_find_path, expr::expr_to_string, foreignmod::ForeignTypeIdentAlias,
+        attribute::attribute_take_path, expr::expr_to_string, foreignmod::ForeignTypeIdentAlias,
         path::path_compare_str,
     },
 };
@@ -75,19 +75,13 @@ impl TryFrom<&ForeignTypeIdentAlias> for ParsedQObject {
         let qml_metadata = Self::parse_qml_metadata(&qobject_ty.ident_left, &mut qobject_ty.attrs)?;
 
         // Find if there is any base class
-        let base_class = attribute_find_path(&qobject_ty.attrs, &["base"])
-            .map(|index| {
-                let attr = qobject_ty.attrs.remove(index);
-                expr_to_string(&attr.meta.require_name_value()?.value)
-            })
+        let base_class = attribute_take_path(&mut qobject_ty.attrs, &["base"])
+            .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value))
             .transpose()?;
 
         // Load the namespace, if it is empty then the ParsedCxxQtData will inject any global namespace
-        let namespace = attribute_find_path(&qobject_ty.attrs, &["namespace"])
-            .map(|index| {
-                let attr = qobject_ty.attrs.remove(index);
-                expr_to_string(&attr.meta.require_name_value()?.value)
-            })
+        let namespace = attribute_take_path(&mut qobject_ty.attrs, &["namespace"])
+            .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value))
             .transpose()?
             .unwrap_or_else(|| "".to_owned());
 
@@ -119,22 +113,18 @@ impl ParsedQObject {
         attrs: &mut Vec<Attribute>,
     ) -> Result<Option<QmlElementMetadata>> {
         // Find if there is a qml_element attribute
-        if let Some(index) = attribute_find_path(attrs, &["qml_element"]) {
+        if let Some(attr) = attribute_take_path(attrs, &["qml_element"]) {
             // Extract the name of the qml_element
-            let name = match attrs.remove(index).meta {
+            let name = match attr.meta {
                 Meta::NameValue(name_value) => expr_to_string(&name_value.value)?,
                 _ => qobject_ident.to_string(),
             };
 
             // Determine if this element is uncreatable
-            let uncreatable = attribute_find_path(attrs, &["qml_uncreatable"])
-                .map(|index| attrs.remove(index))
-                .is_some();
+            let uncreatable = attribute_take_path(attrs, &["qml_uncreatable"]).is_some();
 
             // Determine if this element is a singleton
-            let singleton = attribute_find_path(attrs, &["qml_singleton"])
-                .map(|index| attrs.remove(index))
-                .is_some();
+            let singleton = attribute_take_path(attrs, &["qml_singleton"]).is_some();
 
             return Ok(Some(QmlElementMetadata {
                 name,
@@ -221,8 +211,8 @@ impl ParsedQObject {
         // elements once using path_compare_str and then building ParsedQProperty
         // from the extracted elements.
         // https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.extract_if
-        while let Some(index) = attribute_find_path(attrs, &["qproperty"]) {
-            properties.push(ParsedQProperty::parse(attrs.remove(index))?);
+        while let Some(attr) = attribute_take_path(attrs, &["qproperty"]) {
+            properties.push(ParsedQProperty::parse(attr)?);
         }
 
         Ok(properties)

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

@@ -4,7 +4,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::parser::parameter::ParsedFunctionParameter;
-use crate::syntax::attribute::attribute_find_path;
+use crate::syntax::attribute::attribute_take_path;
 use crate::syntax::expr::expr_to_string;
 use crate::syntax::foreignmod;
 use crate::syntax::safety::Safety;
@@ -56,8 +56,6 @@ impl ParsedSignal {
             ));
         }
 
-        let mut inherit = false;
-
         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();
@@ -72,21 +70,14 @@ impl ParsedSignal {
 
         let mut ident = CombinedIdent::from_rust_function(method.sig.ident.clone());
 
-        if let Some(index) = attribute_find_path(&method.attrs, &["cxx_name"]) {
+        if let Some(attr) = attribute_take_path(&mut method.attrs, &["cxx_name"]) {
             ident.cpp = format_ident!(
                 "{}",
-                expr_to_string(&method.attrs[index].meta.require_name_value()?.value)?
+                expr_to_string(&attr.meta.require_name_value()?.value)?
             );
-
-            method.attrs.remove(index);
-        }
-
-        if let Some(index) = attribute_find_path(&method.attrs, &["inherit"]) {
-            inherit = true;
-
-            method.attrs.remove(index);
         }
 
+        let inherit = attribute_take_path(&mut method.attrs, &["inherit"]).is_some();
         let safe = method.sig.unsafety.is_none();
 
         Ok(Self {

crates/cxx-qt-gen/src/syntax/attribute.rs

@@ -17,6 +17,11 @@ pub fn attribute_find_path(attrs: &[Attribute], path: &[&str]) -> Option<usize>
     None
 }
 
+/// Takes and returns the first [syn::Attribute] that matches a given path
+pub fn attribute_take_path(attrs: &mut Vec<Attribute>, path: &[&str]) -> Option<Attribute> {
+    attribute_find_path(attrs, path).map(|index| attrs.remove(index))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -38,4 +43,19 @@ mod tests {
         assert!(attribute_find_path(&module.attrs, &["cxx_qt", "object"]).is_some());
         assert!(attribute_find_path(&module.attrs, &["cxx_qt", "missing"]).is_none());
     }
+
+    #[test]
+    fn test_attribute_take_path() {
+        let mut module: ItemMod = parse_quote! {
+            #[qinvokable]
+            #[cxx_qt::bridge]
+            #[cxx_qt::object(MyObject)]
+            #[cxx_qt::bridge(namespace = "my::namespace")]
+            mod module;
+        };
+
+        assert_eq!(module.attrs.len(), 4);
+        assert!(attribute_take_path(&mut module.attrs, &["qinvokable"]).is_some());
+        assert_eq!(module.attrs.len(), 3);
+    }
 }