WIP add attribute parsing enum (check coverage)

BenFordTytherington · BenFordTytherington · commit 3100295c27a1 · 2025-07-18T13:21:34.000+01:00
diff --git a/crates/cxx-qt-gen/src/parser/attribute.rs b/crates/cxx-qt-gen/src/parser/attribute.rs
@@ -14,6 +14,20 @@ pub struct ParsedAttributes {
     pub passthrough_attrs: Vec<Attribute>,
 }
 
+// TODO: ATTR could this instead be used as Result<ParsedAttribute> to encapsulate error states
+pub enum ParsedAttribute<'a> {
+    /// A single attribute was found
+    Single(&'a Attribute),
+    /// An attribute was not found, but this is ok
+    Absent,
+    /// An attribute was not found, and this is an error
+    AbsentRequired,
+    /// Multiple attributes were found, but this is ok
+    Multiple(Vec<&'a Attribute>),
+    /// Multiple attributes were found, but this is an error
+    MultipleDisallowed(Vec<&'a Attribute>),
+}
+
 /// Iterate the attributes of the method to extract cfg attributes
 pub fn extract_cfgs(attrs: &[Attribute]) -> Vec<Attribute> {
     attrs
@@ -108,6 +122,18 @@ impl<'a> ParsedAttributes {
         self.cxx_qt_attrs.get(key)?.first()
     }
 
+    pub fn require_one(&self, key: &str) -> ParsedAttribute {
+        if let Some(attrs) = self.cxx_qt_attrs.get(key) {
+            if attrs.len() != 1 {
+                ParsedAttribute::MultipleDisallowed(attrs.iter().by_ref().collect())
+            } else {
+                ParsedAttribute::Single(attrs.first().expect("Expected at least one attribute"))
+            }
+        } else {
+            ParsedAttribute::Absent
+        }
+    }
+
     /// Check if CXX-Qt or passthrough attributes contains a particular key
     pub fn contains_key(&self, key: &str) -> bool {
         self.cxx_qt_attrs.contains_key(key) // TODO: Check in passthrough too
diff --git a/crates/cxx-qt-gen/src/parser/externcxxqt.rs b/crates/cxx-qt-gen/src/parser/externcxxqt.rs
@@ -3,11 +3,12 @@
 //
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use crate::parser::attribute::ParsedAttributes;
+use crate::parser::attribute::{ParsedAttribute, ParsedAttributes};
 use crate::{
     parser::{externqobject::ParsedExternQObject, signals::ParsedSignal, CaseConversion},
     syntax::{attribute::attribute_get_path, expr::expr_to_string},
 };
+use proc_macro2::Span;
 use syn::{
     spanned::Spanned, Error, ForeignItem, ForeignItemFn, Ident, ItemForeignMod, Result, Token,
 };
@@ -41,12 +42,25 @@ impl ParsedExternCxxQt {
 
         let auto_case = CaseConversion::from_attrs(&attrs)?;
 
-        let namespace = attrs
-            .get_one("namespace")
-            .map(|attr| -> Result<String> {
-                expr_to_string(&attr.meta.require_name_value()?.value)
-            })
-            .transpose()?;
+        let namespace = match attrs.require_one("namespace") {
+            ParsedAttribute::Single(attr) => {
+                expr_to_string(&attr.meta.require_name_value()?.value).ok()
+            }
+            ParsedAttribute::Absent => None,
+            ParsedAttribute::MultipleDisallowed(_) => {
+                Err(Error::new(
+                    Span::call_site(),
+                    "There must be at most one namespace attribute",
+                ))? // TODO: ATTR use real span
+            }
+            _ => {
+                // CODECOV_EXCLUDE_START
+                unreachable!(
+                    "Namepsace is not an allowed duplicate, nor required so this block should be unreachable"
+                )
+                // CODECOV_EXCLUDE_STOP
+            }
+        };
 
         let mut extern_cxx_block = ParsedExternCxxQt {
             namespace,
@@ -219,6 +233,26 @@ mod tests {
                     type QPushButton;
                 }
             }
+
+            // Duplicate base attr is an error
+            {
+                extern "C++Qt" {
+                    #[base = QPushButton]
+                    #[base = QPushButton]
+                    #[qobject]
+                    type QPushButtonChild;
+                }
+            }
+
+            // All types in "C++Qt" blocks must be marked as QObjects
+            {
+                #[namespace = "My namespace"]
+                #[namespace = "My other namespace"]
+                unsafe extern "C++Qt" {
+                    #[qobject]
+                    type QPushButton;
+                }
+            }
         );
     }
 }
diff --git a/crates/cxx-qt-gen/src/parser/externrustqt.rs b/crates/cxx-qt-gen/src/parser/externrustqt.rs
@@ -4,7 +4,7 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::naming::cpp::err_unsupported_item;
-use crate::parser::attribute::ParsedAttributes;
+use crate::parser::attribute::{ParsedAttribute, ParsedAttributes};
 use crate::parser::inherit::ParsedInheritedMethod;
 use crate::parser::method::ParsedMethod;
 use crate::parser::qobject::ParsedQObject;
@@ -13,7 +13,7 @@ use crate::parser::CaseConversion;
 use crate::syntax::attribute::attribute_get_path;
 use crate::syntax::expr::expr_to_string;
 use crate::syntax::foreignmod::ForeignTypeIdentAlias;
-use proc_macro2::Ident;
+use proc_macro2::{Ident, Span};
 use syn::spanned::Spanned;
 use syn::{Error, ForeignItem, ForeignItemFn, ItemForeignMod, Result, Token};
 
@@ -51,11 +51,31 @@ impl ParsedExternRustQt {
             ..Default::default()
         };
 
-        let namespace = attrs
-            .get_one("namespace")
-            .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value))
-            .transpose()?
-            .or_else(|| parent_namespace.map(String::from));
+        // let namespace = attrs
+        //     .get_one("namespace")
+        //     .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value))
+        //     .transpose()?
+        //     .or_else(|| parent_namespace.map(String::from));
+
+        let namespace = match attrs.require_one("namespace") {
+            ParsedAttribute::Single(attr) => {
+                expr_to_string(&attr.meta.require_name_value()?.value).ok()
+            }
+            ParsedAttribute::Absent => None,
+            ParsedAttribute::MultipleDisallowed(_) => {
+                Err(Error::new(
+                    Span::call_site(),
+                    "There must be at most one namespace attribute",
+                ))? // TODO: ATTR use real span
+            }
+            _ => {
+                // CODECOV_EXCLUDE_START
+                unreachable!(
+                    "Namepsace is not an allowed duplicate, nor required so this block should be unreachable"
+                )
+                // CODECOV_EXCLUDE_STOP
+            }
+        }.or_else(|| parent_namespace.map(String::from));
 
         for item in foreign_mod.items.drain(..) {
             match item {
@@ -224,6 +244,37 @@ mod tests {
                 }
             }
 
+            // Duplicate namespace not allowed
+            {
+                #[namespace = "My Namespace"]
+                #[namespace = "My Other Namespace"]
+                unsafe extern "RustQt" {
+                    #[qinvokable]
+                    fn invokable(self: &MyObject);
+                }
+            }
+
+            // Duplicate auto_cxx_name not allowed
+            {
+
+                #[auto_cxx_name]
+                #[auto_cxx_name]
+                unsafe extern "RustQt" {
+                    #[qobject]
+                    type MyObject = super::MyObjectRust;
+                }
+            }
+
+            // Duplicate auto_rust_name not allowed
+            {
+                #[auto_rust_name]
+                #[auto_rust_name]
+                unsafe extern "RustQt" {
+                    #[qobject]
+                    type MyObject = super::MyObjectRust;
+                }
+            }
+
             // Namespaces aren't allowed on qinvokables
             {
                 unsafe extern "RustQt" {
diff --git a/crates/cxx-qt-gen/src/parser/mod.rs b/crates/cxx-qt-gen/src/parser/mod.rs
@@ -19,10 +19,11 @@ pub mod qobject;
 pub mod signals;
 pub mod trait_impl;
 
-use crate::parser::attribute::ParsedAttributes;
+use crate::parser::attribute::{ParsedAttribute, ParsedAttributes};
 use crate::{naming::TypeNames, syntax::expr::expr_to_string};
 use convert_case::Case;
 use cxxqtdata::ParsedCxxQtData;
+use proc_macro2::Span;
 use syn::{
     punctuated::Punctuated,
     spanned::Spanned,
@@ -73,14 +74,50 @@ impl CaseConversion {
     /// Parses both `auto_cxx_name` and `auto_cxx_name = Camel`
     pub fn from_attrs(attrs: &ParsedAttributes) -> Result<Self> {
         // TODO: ATTR Won't error on duplicates, and needs error handling
-        let rust = attrs
-            .get_one("auto_rust_name")
-            .map(|attr| meta_to_case(attr, Case::Snake))
-            .transpose()?;
-        let cxx = attrs
-            .get_one("auto_cxx_name")
-            .map(|attr| meta_to_case(attr, Case::Camel))
-            .transpose()?;
+        // let rust = attrs
+        //     .get_one("auto_rust_name")
+        //     .map(|attr| meta_to_case(attr, Case::Snake))
+        //     .transpose()?;
+        // let cxx = attrs
+        //     .get_one("auto_cxx_name")
+        //     .map(|attr| meta_to_case(attr, Case::Camel))
+        //     .transpose()?;
+
+        let rust = match attrs.require_one("auto_rust_name") {
+            ParsedAttribute::Single(attr) => Some(meta_to_case(attr, Case::Snake)?),
+            ParsedAttribute::Absent => None,
+            ParsedAttribute::MultipleDisallowed(_) => {
+                Err(Error::new(
+                    Span::call_site(),
+                    "There must be at most one auto_rust_name attribute",
+                ))? // TODO: ATTR use real span
+            }
+            _ => {
+                // CODECOV_EXCLUDE_START
+                unreachable!(
+                    "Auto_rust_name is not an allowed duplicate, nor required so this block should be unreachable"
+                )
+                // CODECOV_EXCLUDE_STOP
+            }
+        };
+
+        let cxx = match attrs.require_one("auto_cxx_name") {
+            ParsedAttribute::Single(attr) => Some(meta_to_case(attr, Case::Camel)?),
+            ParsedAttribute::Absent => None,
+            ParsedAttribute::MultipleDisallowed(_) => {
+                Err(Error::new(
+                    Span::call_site(),
+                    "There must be at most one auto_cxx_name attribute",
+                ))? // TODO: ATTR use real span
+            }
+            _ => {
+                // CODECOV_EXCLUDE_START
+                unreachable!(
+                    "Auto_cxx_name is not an allowed duplicate, nor required so this block should be unreachable"
+                )
+                // CODECOV_EXCLUDE_STOP
+            }
+        };
 
         Ok(Self { rust, cxx })
     }
@@ -98,20 +135,35 @@ fn split_path(path_str: &str) -> Vec<&str> {
 
 // Extract base identifier from attribute
 pub fn parse_base_type(attributes: &ParsedAttributes) -> Result<Option<Ident>> {
-    attributes
-        .get_one("base") // TODO: ATTR Check for duplicates
-        .map(|attr| -> Result<Ident> {
+    match attributes.require_one("base") {
+        ParsedAttribute::Single(attr) => {
             let expr = &attr.meta.require_name_value()?.value;
             if let Expr::Path(path_expr) = expr {
-                Ok(path_expr.path.require_ident()?.clone())
+                Ok(Some(path_expr.path.require_ident()?.clone()))
             } else {
                 Err(Error::new_spanned(
                     expr,
-                    "Base must be a identifier and cannot be empty!",
+                    "There must be a single base identifier, not string, and cannot be empty!",
                 ))
             }
-        })
-        .transpose()
+        }
+        ParsedAttribute::Absent => Ok(None),
+        ParsedAttribute::MultipleDisallowed(attrs) => {
+            let attr = attrs.first().expect("Expected at least one attribute");
+            let expr = &attr.meta.require_name_value()?.value;
+            Err(Error::new_spanned(
+                expr,
+                "There must be a single base identifier, not string, and cannot be empty!",
+            ))
+        }
+        _ => {
+            // CODECOV_EXCLUDE_START
+            unreachable!(
+                "base is not an allowed duplicate, nor required so this block should be unreachable"
+            )
+            // CODECOV_EXCLUDE_STOP
+        }
+    }
 }
 
 /// Struct representing the necessary components of a cxx mod to be passed through to generation
