From aa718f6a100569cd6c51179ed679a985b5c78b6f Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 2 Jul 2025 15:03:06 +0100 Subject: [PATCH 1/4] Create ParsedAttribute struct and module, and move attributes into there --- crates/cxx-qt-gen/src/parser/attribute.rs | 78 +++++++++++++++++++ crates/cxx-qt-gen/src/parser/externcxxqt.rs | 10 +-- crates/cxx-qt-gen/src/parser/externqobject.rs | 7 +- crates/cxx-qt-gen/src/parser/externrustqt.rs | 7 +- crates/cxx-qt-gen/src/parser/inherit.rs | 7 +- crates/cxx-qt-gen/src/parser/method.rs | 8 +- crates/cxx-qt-gen/src/parser/mod.rs | 55 ++----------- crates/cxx-qt-gen/src/parser/qenum.rs | 7 +- crates/cxx-qt-gen/src/parser/qnamespace.rs | 4 +- crates/cxx-qt-gen/src/parser/qobject.rs | 10 ++- crates/cxx-qt-gen/src/parser/signals.rs | 9 +-- 11 files changed, 119 insertions(+), 83 deletions(-) create mode 100644 crates/cxx-qt-gen/src/parser/attribute.rs diff --git a/crates/cxx-qt-gen/src/parser/attribute.rs b/crates/cxx-qt-gen/src/parser/attribute.rs new file mode 100644 index 000000000..80634d01b --- /dev/null +++ b/crates/cxx-qt-gen/src/parser/attribute.rs @@ -0,0 +1,78 @@ +// SPDX-FileCopyrightText: 2025 Klarälvdalens Datakonsult AB, a KDAB Group company +// SPDX-FileContributor: Ben Ford +// +// SPDX-License-Identifier: MIT OR Apache-2.0 + +use crate::{parser, syntax::path::path_compare_str}; +use std::collections::BTreeMap; +use syn::{spanned::Spanned, Attribute, Error, Result}; + +pub struct ParsedAttribute<'a> { + pub cxx_qt_attrs: BTreeMap<&'a str, &'a Attribute>, + pub passthrough_attrs: BTreeMap<&'a str, &'a Attribute>, +} + +/// Iterate the attributes of the method to extract cfg attributes +pub fn extract_cfgs(attrs: &[Attribute]) -> Vec { + attrs + .iter() + .filter(|attr| path_compare_str(attr.meta.path(), &["cfg"])) + .cloned() + .collect() +} + +/// Iterate the attributes of the method to extract Doc attributes (doc comments are parsed as this) +pub fn extract_docs(attrs: &[Attribute]) -> Vec { + attrs + .iter() + .filter(|attr| path_compare_str(attr.meta.path(), &["doc"])) + .cloned() + .collect() +} + +impl<'a> ParsedAttribute<'a> { + /// Collects a Map of all attributes found from the allowed list + /// Will error if an attribute which is not in the allowed list is found + pub fn require_attributes( + attrs: &'a [Attribute], + allowed: &'a [&str], + ) -> Result> { + let mut output = BTreeMap::default(); + for attr in attrs { + let index = allowed + .iter() + .position(|string| path_compare_str(attr.meta.path(), &parser::split_path(string))); + if let Some(index) = index { + output.insert(allowed[index], attr); // TODO: Doesn't error on duplicates + } else { + return Err(Error::new( + attr.span(), + format!( + "Unsupported attribute! The only attributes allowed on this item are\n{}", + allowed.join(", ") + ), + )); + } + } + Ok(Self { + cxx_qt_attrs: output, + passthrough_attrs: Default::default(), + }) + } + + // Wrapper methods for the internal BTreeMaps + // TODO: Refactor usage to use more specialised methods / rename + + /// Search in first the CXX-Qt, and then passthrough attributes by key + pub fn get(&self, key: &str) -> Option<&Attribute> { + self.cxx_qt_attrs + .get(key) + .or(self.passthrough_attrs.get(key)) + .map(|attr| &**attr) + } + + /// 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) || self.passthrough_attrs.contains_key(key) + } +} diff --git a/crates/cxx-qt-gen/src/parser/externcxxqt.rs b/crates/cxx-qt-gen/src/parser/externcxxqt.rs index 4364c9027..36f9c4607 100644 --- a/crates/cxx-qt-gen/src/parser/externcxxqt.rs +++ b/crates/cxx-qt-gen/src/parser/externcxxqt.rs @@ -3,11 +3,9 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::parser::attribute::ParsedAttribute; use crate::{ - parser::{ - externqobject::ParsedExternQObject, require_attributes, signals::ParsedSignal, - CaseConversion, - }, + parser::{externqobject::ParsedExternQObject, signals::ParsedSignal, CaseConversion}, syntax::{attribute::attribute_get_path, expr::expr_to_string}, }; use syn::{ @@ -36,12 +34,12 @@ impl ParsedExternCxxQt { parent_namespace: Option<&str>, ) -> Result { // TODO: support cfg on foreign mod blocks - let attrs = require_attributes( + let attrs = ParsedAttribute::require_attributes( &foreign_mod.attrs, &["namespace", "auto_cxx_name", "auto_rust_name"], )?; - let auto_case = CaseConversion::from_attrs(&attrs)?; + let auto_case = CaseConversion::from_attrs(&attrs.cxx_qt_attrs)?; let namespace = attrs .get("namespace") diff --git a/crates/cxx-qt-gen/src/parser/externqobject.rs b/crates/cxx-qt-gen/src/parser/externqobject.rs index b7ea93faa..5d1218a1a 100644 --- a/crates/cxx-qt-gen/src/parser/externqobject.rs +++ b/crates/cxx-qt-gen/src/parser/externqobject.rs @@ -3,7 +3,8 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::naming::Name; -use crate::parser::{parse_base_type, require_attributes, CaseConversion}; +use crate::parser::attribute::ParsedAttribute; +use crate::parser::{parse_base_type, CaseConversion}; use syn::{ForeignItemType, Ident, Result}; /// A representation of a QObject to be generated in an extern C++ block @@ -32,9 +33,9 @@ impl ParsedExternQObject { module_ident: &Ident, parent_namespace: Option<&str>, ) -> Result { - let attributes = require_attributes(&ty.attrs, &Self::ALLOWED_ATTRS)?; + let attrs = ParsedAttribute::require_attributes(&ty.attrs, &Self::ALLOWED_ATTRS)?; - let base_class = parse_base_type(&attributes)?; + let base_class = parse_base_type(&attrs.cxx_qt_attrs)?; Ok(Self { name: Name::from_ident_and_attrs( diff --git a/crates/cxx-qt-gen/src/parser/externrustqt.rs b/crates/cxx-qt-gen/src/parser/externrustqt.rs index 6da520bdf..19e8ba52b 100644 --- a/crates/cxx-qt-gen/src/parser/externrustqt.rs +++ b/crates/cxx-qt-gen/src/parser/externrustqt.rs @@ -4,11 +4,12 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::naming::cpp::err_unsupported_item; +use crate::parser::attribute::ParsedAttribute; use crate::parser::inherit::ParsedInheritedMethod; use crate::parser::method::ParsedMethod; use crate::parser::qobject::ParsedQObject; use crate::parser::signals::ParsedSignal; -use crate::parser::{require_attributes, CaseConversion}; +use crate::parser::CaseConversion; use crate::syntax::attribute::attribute_get_path; use crate::syntax::expr::expr_to_string; use crate::syntax::foreignmod::ForeignTypeIdentAlias; @@ -38,12 +39,12 @@ impl ParsedExternRustQt { parent_namespace: Option<&str>, ) -> Result { // TODO: support cfg on foreign mod blocks - let attrs = require_attributes( + let attrs = ParsedAttribute::require_attributes( &foreign_mod.attrs, &["namespace", "auto_cxx_name", "auto_rust_name"], )?; - let auto_case = CaseConversion::from_attrs(&attrs)?; + let auto_case = CaseConversion::from_attrs(&attrs.cxx_qt_attrs)?; let mut extern_rustqt_block = Self { unsafety: foreign_mod.unsafety, diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 9ebb5af17..93f9d9e05 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -3,9 +3,8 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::{ - extract_cfgs, extract_docs, method::MethodFields, require_attributes, CaseConversion, -}; +use crate::parser::attribute::{extract_cfgs, extract_docs, ParsedAttribute}; +use crate::parser::{method::MethodFields, CaseConversion}; use core::ops::Deref; use quote::format_ident; use std::ops::DerefMut; @@ -32,7 +31,7 @@ impl ParsedInheritedMethod { ]; pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result { - require_attributes(&method.attrs, &Self::ALLOWED_ATTRS)?; + ParsedAttribute::require_attributes(&method.attrs, &Self::ALLOWED_ATTRS)?; let docs = extract_docs(&method.attrs); let cfgs = extract_cfgs(&method.attrs); diff --git a/crates/cxx-qt-gen/src/parser/method.rs b/crates/cxx-qt-gen/src/parser/method.rs index 89c708da5..b45da80ba 100644 --- a/crates/cxx-qt-gen/src/parser/method.rs +++ b/crates/cxx-qt-gen/src/parser/method.rs @@ -2,10 +2,11 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::parser::attribute::{extract_cfgs, ParsedAttribute}; use crate::parser::CaseConversion; use crate::{ naming::Name, - parser::{extract_cfgs, parameter::ParsedFunctionParameter, require_attributes}, + parser::parameter::ParsedFunctionParameter, syntax::{foreignmod, types}, }; use core::ops::Deref; @@ -121,13 +122,14 @@ impl ParsedMethod { unsafe_block: bool, ) -> Result { let fields = MethodFields::parse(method, auto_case)?; - let attrs = require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?; + let attrs = + ParsedAttribute::require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?; let cfgs = extract_cfgs(&fields.method.attrs); // Determine if the method is invokable let is_qinvokable = attrs.contains_key("qinvokable"); let is_pure = attrs.contains_key("cxx_pure"); - let specifiers = ParsedQInvokableSpecifiers::from_attrs(attrs); + let specifiers = ParsedQInvokableSpecifiers::from_attrs(attrs.cxx_qt_attrs); Ok(Self { method_fields: fields, diff --git a/crates/cxx-qt-gen/src/parser/mod.rs b/crates/cxx-qt-gen/src/parser/mod.rs index 97b11ec1f..07b07886c 100644 --- a/crates/cxx-qt-gen/src/parser/mod.rs +++ b/crates/cxx-qt-gen/src/parser/mod.rs @@ -3,6 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 +pub mod attribute; pub mod constructor; pub mod cxxqtdata; pub mod externcxxqt; @@ -18,10 +19,8 @@ pub mod qobject; pub mod signals; pub mod trait_impl; -use crate::{ - naming::TypeNames, - syntax::{expr::expr_to_string, path::path_compare_str}, -}; +use crate::parser::attribute::ParsedAttribute; +use crate::{naming::TypeNames, syntax::expr::expr_to_string}; use convert_case::Case; use cxxqtdata::ParsedCxxQtData; use std::collections::BTreeMap; @@ -87,24 +86,6 @@ impl CaseConversion { } } -/// Iterate the attributes of the method to extract cfg attributes -pub fn extract_cfgs(attrs: &[Attribute]) -> Vec { - attrs - .iter() - .filter(|attr| path_compare_str(attr.meta.path(), &["cfg"])) - .cloned() - .collect() -} - -/// Iterate the attributes of the method to extract Doc attributes (doc comments are parsed as this) -pub fn extract_docs(attrs: &[Attribute]) -> Vec { - attrs - .iter() - .filter(|attr| path_compare_str(attr.meta.path(), &["doc"])) - .cloned() - .collect() -} - /// Splits a path by :: separators e.g. "cxx_qt::bridge" becomes ["cxx_qt", "bridge"] fn split_path(path_str: &str) -> Vec<&str> { let path = if path_str.contains("::") { @@ -115,32 +96,6 @@ fn split_path(path_str: &str) -> Vec<&str> { path } -/// Collects a Map of all attributes found from the allowed list -/// Will error if an attribute which is not in the allowed list is found -pub fn require_attributes<'a>( - attrs: &'a [Attribute], - allowed: &'a [&str], -) -> Result> { - let mut output = BTreeMap::default(); - for attr in attrs { - let index = allowed - .iter() - .position(|string| path_compare_str(attr.meta.path(), &split_path(string))); - if let Some(index) = index { - output.insert(allowed[index], attr); // Doesn't error on duplicates - } else { - return Err(Error::new( - attr.span(), - format!( - "Unsupported attribute! The only attributes allowed on this item are\n{}", - allowed.join(", ") - ), - )); - } - } - Ok(output) -} - // Extract base identifier from attribute pub fn parse_base_type(attributes: &BTreeMap<&str, &Attribute>) -> Result> { attributes @@ -174,7 +129,7 @@ impl PassthroughMod { Self { items, - docs: extract_docs(&module.attrs), + docs: attribute::extract_docs(&module.attrs), module_ident: module.ident, vis: module.vis, } @@ -196,7 +151,7 @@ pub struct Parser { impl Parser { fn parse_mod_attributes(module: &mut ItemMod) -> Result> { - let attrs = require_attributes(&module.attrs, &["doc", "cxx_qt::bridge"])?; + let attrs = ParsedAttribute::require_attributes(&module.attrs, &["doc", "cxx_qt::bridge"])?; let mut namespace = None; // Check for the cxx_qt::bridge attribute diff --git a/crates/cxx-qt-gen/src/parser/qenum.rs b/crates/cxx-qt-gen/src/parser/qenum.rs index 069831a1b..8af7db9bc 100644 --- a/crates/cxx-qt-gen/src/parser/qenum.rs +++ b/crates/cxx-qt-gen/src/parser/qenum.rs @@ -3,8 +3,9 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::{extract_cfgs, extract_docs, CaseConversion}; -use crate::{naming::Name, parser::require_attributes, syntax::path::path_compare_str}; +use crate::parser::attribute::{extract_cfgs, extract_docs, ParsedAttribute}; +use crate::parser::CaseConversion; +use crate::{naming::Name, syntax::path::path_compare_str}; use quote::ToTokens; use syn::{Attribute, Ident, ItemEnum, Result, Variant}; @@ -60,7 +61,7 @@ impl ParsedQEnum { parent_namespace: Option<&str>, module: &Ident, ) -> Result { - require_attributes(&qenum.attrs, &Self::ALLOWED_ATTRS)?; + ParsedAttribute::require_attributes(&qenum.attrs, &Self::ALLOWED_ATTRS)?; let cfgs = extract_cfgs(&qenum.attrs); let docs = extract_docs(&qenum.attrs); diff --git a/crates/cxx-qt-gen/src/parser/qnamespace.rs b/crates/cxx-qt-gen/src/parser/qnamespace.rs index ab483ec20..4eddf6cef 100644 --- a/crates/cxx-qt-gen/src/parser/qnamespace.rs +++ b/crates/cxx-qt-gen/src/parser/qnamespace.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::require_attributes; +use crate::parser::attribute::ParsedAttribute; use syn::{ItemMacro, LitStr, Result}; pub struct ParsedQNamespace { @@ -15,7 +15,7 @@ pub struct ParsedQNamespace { impl ParsedQNamespace { pub fn parse(mac: ItemMacro) -> Result { - let attrs = require_attributes(&mac.attrs, &["qml_element"])?; + let attrs = ParsedAttribute::require_attributes(&mac.attrs, &["qml_element"])?; let namespace_literal: LitStr = syn::parse2(mac.mac.tokens)?; let namespace = namespace_literal.value(); if namespace.contains(char::is_whitespace) { diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 30e1ed051..49623b3cc 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -5,12 +5,13 @@ use crate::{ naming::Name, - parser::{extract_cfgs, property::ParsedQProperty, require_attributes}, + parser::property::ParsedQProperty, syntax::{expr::expr_to_string, foreignmod::ForeignTypeIdentAlias, path::path_compare_str}, }; #[cfg(test)] use quote::format_ident; +use crate::parser::attribute::{extract_cfgs, ParsedAttribute}; use crate::parser::{parse_base_type, CaseConversion}; use syn::{Attribute, Error, Ident, Meta, Result}; @@ -85,13 +86,14 @@ impl ParsedQObject { module: &Ident, auto_case: CaseConversion, ) -> Result { - let attributes = require_attributes(&declaration.attrs, &Self::ALLOWED_ATTRS)?; + let attributes = + ParsedAttribute::require_attributes(&declaration.attrs, &Self::ALLOWED_ATTRS)?; // TODO: handle docs through to generation let cfgs = extract_cfgs(&declaration.attrs); let has_qobject_macro = attributes.contains_key("qobject"); - let base_class = parse_base_type(&attributes)?; + let base_class = parse_base_type(&attributes.cxx_qt_attrs)?; // Ensure that if there is no qobject macro that a base class is specificed if !has_qobject_macro && base_class.is_none() { @@ -130,7 +132,7 @@ impl ParsedQObject { } fn parse_qml_metadata(name: &Name, attrs: &[Attribute]) -> Result> { - let attributes = require_attributes(attrs, &Self::ALLOWED_ATTRS)?; + let attributes = ParsedAttribute::require_attributes(attrs, &Self::ALLOWED_ATTRS)?; if let Some(attr) = attributes.get("qml_element") { // Extract the name of the qml_element from macro, else use the c++ name // This will use the name provided by cxx_name if that attr was present diff --git a/crates/cxx-qt-gen/src/parser/signals.rs b/crates/cxx-qt-gen/src/parser/signals.rs index 81e264ebb..c8b83ba49 100644 --- a/crates/cxx-qt-gen/src/parser/signals.rs +++ b/crates/cxx-qt-gen/src/parser/signals.rs @@ -2,11 +2,9 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::parser::attribute::{extract_cfgs, extract_docs, ParsedAttribute}; use crate::parser::CaseConversion; -use crate::{ - parser::{extract_cfgs, extract_docs, method::MethodFields, require_attributes}, - syntax::path::path_compare_str, -}; +use crate::{parser::method::MethodFields, syntax::path::path_compare_str}; use core::ops::Deref; use std::ops::DerefMut; use syn::{spanned::Spanned, Attribute, Error, ForeignItemFn, Result, Visibility}; @@ -40,7 +38,8 @@ impl ParsedSignal { let docs = extract_docs(&method.attrs); let cfgs = extract_cfgs(&method.attrs); let fields = MethodFields::parse(method, auto_case)?; - let attrs = require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?; + let attrs = + ParsedAttribute::require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?; if !fields.mutable { return Err(Error::new( From b5e2b50f318935459c15a1e8554466b8c0fda8f8 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 2 Jul 2025 17:04:11 +0100 Subject: [PATCH 2/4] Integrate ParsedAttrbiutes struct, containing a map of all attributes - Add get_one method to ignore duplicates and just get the first --- crates/cxx-qt-gen/src/parser/attribute.rs | 69 ++++++++++++++----- crates/cxx-qt-gen/src/parser/externcxxqt.rs | 10 +-- crates/cxx-qt-gen/src/parser/externqobject.rs | 9 +-- crates/cxx-qt-gen/src/parser/externrustqt.rs | 10 +-- crates/cxx-qt-gen/src/parser/inherit.rs | 9 ++- crates/cxx-qt-gen/src/parser/method.rs | 34 ++++++--- crates/cxx-qt-gen/src/parser/mod.rs | 20 +++--- crates/cxx-qt-gen/src/parser/qenum.rs | 12 ++-- crates/cxx-qt-gen/src/parser/qnamespace.rs | 4 +- crates/cxx-qt-gen/src/parser/qobject.rs | 29 ++++---- crates/cxx-qt-gen/src/parser/signals.rs | 11 ++- 11 files changed, 136 insertions(+), 81 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/attribute.rs b/crates/cxx-qt-gen/src/parser/attribute.rs index 80634d01b..35aebdf2f 100644 --- a/crates/cxx-qt-gen/src/parser/attribute.rs +++ b/crates/cxx-qt-gen/src/parser/attribute.rs @@ -4,12 +4,14 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::{parser, syntax::path::path_compare_str}; +use std::collections::btree_map::Entry; use std::collections::BTreeMap; use syn::{spanned::Spanned, Attribute, Error, Result}; -pub struct ParsedAttribute<'a> { - pub cxx_qt_attrs: BTreeMap<&'a str, &'a Attribute>, - pub passthrough_attrs: BTreeMap<&'a str, &'a Attribute>, +#[derive(Clone)] +pub struct ParsedAttributes { + pub cxx_qt_attrs: BTreeMap>, + pub passthrough_attrs: Vec, } /// Iterate the attributes of the method to extract cfg attributes @@ -30,20 +32,28 @@ pub fn extract_docs(attrs: &[Attribute]) -> Vec { .collect() } -impl<'a> ParsedAttribute<'a> { +impl<'a> ParsedAttributes { /// Collects a Map of all attributes found from the allowed list /// Will error if an attribute which is not in the allowed list is found pub fn require_attributes( - attrs: &'a [Attribute], + mut attrs: Vec, allowed: &'a [&str], - ) -> Result> { - let mut output = BTreeMap::default(); - for attr in attrs { + ) -> Result { + let mut output = BTreeMap::>::default(); + for attr in attrs.drain(..) { let index = allowed .iter() .position(|string| path_compare_str(attr.meta.path(), &parser::split_path(string))); if let Some(index) = index { - output.insert(allowed[index], attr); // TODO: Doesn't error on duplicates + // TODO: ATTR Doesn't error on duplicates / distinguish allowed and disallowed duplicates + match output.entry(allowed[index].into()) { + Entry::Occupied(mut entry) => { + entry.get_mut().push(attr); + } + Entry::Vacant(entry) => { + entry.insert(vec![attr]); + } + } } else { return Err(Error::new( attr.span(), @@ -56,23 +66,50 @@ impl<'a> ParsedAttribute<'a> { } Ok(Self { cxx_qt_attrs: output, - passthrough_attrs: Default::default(), + passthrough_attrs: Default::default(), // TODO: ATTR Pass the actual docs, cfgs, etc... here }) } + // TODO: ATTR Can this return references instead? + pub fn extract_docs(&self) -> Vec { + self.cxx_qt_attrs + .values() + .flatten() + .filter(|attr| path_compare_str(attr.meta.path(), &["doc"])) + .map(|attr| (*attr).clone()) + .collect() + } + + // TODO: ATTR Can this return references instead + pub fn extract_cfgs(&self) -> Vec { + self.cxx_qt_attrs + .values() + .flatten() + .filter(|attr| path_compare_str(attr.meta.path(), &["cfg"])) + .map(|attr| (*attr).clone()) + .collect() + } + + /// Returns all the attributes stored within the struct + /// TODO: ATTR Can we use this without clone + pub fn clone_attrs(&self) -> Vec { + self.cxx_qt_attrs + .values() + .flatten() + .cloned() + .collect::>() + } + // Wrapper methods for the internal BTreeMaps // TODO: Refactor usage to use more specialised methods / rename /// Search in first the CXX-Qt, and then passthrough attributes by key - pub fn get(&self, key: &str) -> Option<&Attribute> { - self.cxx_qt_attrs - .get(key) - .or(self.passthrough_attrs.get(key)) - .map(|attr| &**attr) + pub fn get_one(&self, key: &str) -> Option<&Attribute> { + self.cxx_qt_attrs.get(key)?.first() } /// 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) || self.passthrough_attrs.contains_key(key) + 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 index 36f9c4607..459b11a99 100644 --- a/crates/cxx-qt-gen/src/parser/externcxxqt.rs +++ b/crates/cxx-qt-gen/src/parser/externcxxqt.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::ParsedAttribute; +use crate::parser::attribute::ParsedAttributes; use crate::{ parser::{externqobject::ParsedExternQObject, signals::ParsedSignal, CaseConversion}, syntax::{attribute::attribute_get_path, expr::expr_to_string}, @@ -34,15 +34,15 @@ impl ParsedExternCxxQt { parent_namespace: Option<&str>, ) -> Result { // TODO: support cfg on foreign mod blocks - let attrs = ParsedAttribute::require_attributes( - &foreign_mod.attrs, + let attrs = ParsedAttributes::require_attributes( + foreign_mod.attrs, &["namespace", "auto_cxx_name", "auto_rust_name"], )?; - let auto_case = CaseConversion::from_attrs(&attrs.cxx_qt_attrs)?; + let auto_case = CaseConversion::from_attrs(&attrs)?; let namespace = attrs - .get("namespace") + .get_one("namespace") .map(|attr| -> Result { expr_to_string(&attr.meta.require_name_value()?.value) }) diff --git a/crates/cxx-qt-gen/src/parser/externqobject.rs b/crates/cxx-qt-gen/src/parser/externqobject.rs index 5d1218a1a..7a335c9f6 100644 --- a/crates/cxx-qt-gen/src/parser/externqobject.rs +++ b/crates/cxx-qt-gen/src/parser/externqobject.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::naming::Name; -use crate::parser::attribute::ParsedAttribute; +use crate::parser::attribute::ParsedAttributes; use crate::parser::{parse_base_type, CaseConversion}; use syn::{ForeignItemType, Ident, Result}; @@ -33,14 +33,15 @@ impl ParsedExternQObject { module_ident: &Ident, parent_namespace: Option<&str>, ) -> Result { - let attrs = ParsedAttribute::require_attributes(&ty.attrs, &Self::ALLOWED_ATTRS)?; + // TODO: ATTR Can this be done without clone + let attrs = ParsedAttributes::require_attributes(ty.attrs.clone(), &Self::ALLOWED_ATTRS)?; - let base_class = parse_base_type(&attrs.cxx_qt_attrs)?; + let base_class = parse_base_type(&attrs)?; Ok(Self { name: Name::from_ident_and_attrs( &ty.ident, - &ty.attrs, + &attrs.clone_attrs(), parent_namespace, Some(module_ident), CaseConversion::none(), diff --git a/crates/cxx-qt-gen/src/parser/externrustqt.rs b/crates/cxx-qt-gen/src/parser/externrustqt.rs index 19e8ba52b..ff6d7d429 100644 --- 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::ParsedAttribute; +use crate::parser::attribute::ParsedAttributes; use crate::parser::inherit::ParsedInheritedMethod; use crate::parser::method::ParsedMethod; use crate::parser::qobject::ParsedQObject; @@ -39,12 +39,12 @@ impl ParsedExternRustQt { parent_namespace: Option<&str>, ) -> Result { // TODO: support cfg on foreign mod blocks - let attrs = ParsedAttribute::require_attributes( - &foreign_mod.attrs, + let attrs = ParsedAttributes::require_attributes( + foreign_mod.attrs, &["namespace", "auto_cxx_name", "auto_rust_name"], )?; - let auto_case = CaseConversion::from_attrs(&attrs.cxx_qt_attrs)?; + let auto_case = CaseConversion::from_attrs(&attrs)?; let mut extern_rustqt_block = Self { unsafety: foreign_mod.unsafety, @@ -52,7 +52,7 @@ impl ParsedExternRustQt { }; let namespace = attrs - .get("namespace") + .get_one("namespace") .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value)) .transpose()? .or_else(|| parent_namespace.map(String::from)); diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 93f9d9e05..7a2245289 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -3,7 +3,6 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::{extract_cfgs, extract_docs, ParsedAttribute}; use crate::parser::{method::MethodFields, CaseConversion}; use core::ops::Deref; use quote::format_ident; @@ -31,12 +30,12 @@ impl ParsedInheritedMethod { ]; pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result { - ParsedAttribute::require_attributes(&method.attrs, &Self::ALLOWED_ATTRS)?; - let docs = extract_docs(&method.attrs); - let cfgs = extract_cfgs(&method.attrs); + let method_fields = MethodFields::parse(method, auto_case, &Self::ALLOWED_ATTRS)?; + let docs = method_fields.attrs.extract_docs(); + let cfgs = method_fields.attrs.extract_cfgs(); Ok(Self { - method_fields: MethodFields::parse(method, auto_case)?, + method_fields, docs, cfgs, }) diff --git a/crates/cxx-qt-gen/src/parser/method.rs b/crates/cxx-qt-gen/src/parser/method.rs index b45da80ba..238982d08 100644 --- a/crates/cxx-qt-gen/src/parser/method.rs +++ b/crates/cxx-qt-gen/src/parser/method.rs @@ -2,7 +2,7 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::{extract_cfgs, ParsedAttribute}; +use crate::parser::attribute::{extract_cfgs, ParsedAttributes}; use crate::parser::CaseConversion; use crate::{ naming::Name, @@ -34,7 +34,7 @@ impl ParsedQInvokableSpecifiers { } } - fn from_attrs(attrs: BTreeMap<&str, &Attribute>) -> HashSet { + fn from_attrs(attrs: BTreeMap>) -> HashSet { let mut output = HashSet::new(); for specifier in [ ParsedQInvokableSpecifiers::Final, @@ -121,15 +121,14 @@ impl ParsedMethod { auto_case: CaseConversion, unsafe_block: bool, ) -> Result { - let fields = MethodFields::parse(method, auto_case)?; - let attrs = - ParsedAttribute::require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?; + let fields = MethodFields::parse(method, auto_case, &Self::ALLOWED_ATTRS)?; let cfgs = extract_cfgs(&fields.method.attrs); // Determine if the method is invokable - let is_qinvokable = attrs.contains_key("qinvokable"); - let is_pure = attrs.contains_key("cxx_pure"); - let specifiers = ParsedQInvokableSpecifiers::from_attrs(attrs.cxx_qt_attrs); + let is_qinvokable = fields.attrs.contains_key("qinvokable"); + let is_pure = fields.attrs.contains_key("cxx_pure"); + // TODO: Can this be done without clone + let specifiers = ParsedQInvokableSpecifiers::from_attrs(fields.attrs.cxx_qt_attrs.clone()); Ok(Self { method_fields: fields, @@ -166,18 +165,30 @@ pub struct MethodFields { pub parameters: Vec, pub safe: bool, pub name: Name, + pub attrs: ParsedAttributes, } impl MethodFields { - pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result { + pub fn parse( + method: ForeignItemFn, + auto_case: CaseConversion, + allowed: &[&str], + ) -> Result { + // TODO: Remove this clone? + let attrs = ParsedAttributes::require_attributes(method.attrs.clone(), allowed)?; 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_ident_and_attrs(&method.sig.ident, &method.attrs, None, None, auto_case)?; + let name = Name::from_ident_and_attrs( + &method.sig.ident, + &attrs.clone_attrs(), + None, + None, + auto_case, + )?; Ok(MethodFields { method, @@ -186,6 +197,7 @@ impl MethodFields { parameters, safe, name, + attrs, }) } diff --git a/crates/cxx-qt-gen/src/parser/mod.rs b/crates/cxx-qt-gen/src/parser/mod.rs index 07b07886c..c292763d4 100644 --- a/crates/cxx-qt-gen/src/parser/mod.rs +++ b/crates/cxx-qt-gen/src/parser/mod.rs @@ -19,11 +19,10 @@ pub mod qobject; pub mod signals; pub mod trait_impl; -use crate::parser::attribute::ParsedAttribute; +use crate::parser::attribute::ParsedAttributes; use crate::{naming::TypeNames, syntax::expr::expr_to_string}; use convert_case::Case; use cxxqtdata::ParsedCxxQtData; -use std::collections::BTreeMap; use syn::{ punctuated::Punctuated, spanned::Spanned, @@ -72,13 +71,14 @@ impl CaseConversion { /// Create a CaseConversion object from a Map of attributes, collected using `require_attributes` /// Parses both `auto_cxx_name` and `auto_cxx_name = Camel` - pub fn from_attrs(attrs: &BTreeMap<&str, &Attribute>) -> Result { + pub fn from_attrs(attrs: &ParsedAttributes) -> Result { + // TODO: ATTR Won't error on duplicates, and needs error handling let rust = attrs - .get("auto_rust_name") + .get_one("auto_rust_name") .map(|attr| meta_to_case(attr, Case::Snake)) .transpose()?; let cxx = attrs - .get("auto_cxx_name") + .get_one("auto_cxx_name") .map(|attr| meta_to_case(attr, Case::Camel)) .transpose()?; @@ -97,9 +97,9 @@ fn split_path(path_str: &str) -> Vec<&str> { } // Extract base identifier from attribute -pub fn parse_base_type(attributes: &BTreeMap<&str, &Attribute>) -> Result> { +pub fn parse_base_type(attributes: &ParsedAttributes) -> Result> { attributes - .get("base") + .get_one("base") // TODO: ATTR Check for duplicates .map(|attr| -> Result { let expr = &attr.meta.require_name_value()?.value; if let Expr::Path(path_expr) = expr { @@ -151,11 +151,13 @@ pub struct Parser { impl Parser { fn parse_mod_attributes(module: &mut ItemMod) -> Result> { - let attrs = ParsedAttribute::require_attributes(&module.attrs, &["doc", "cxx_qt::bridge"])?; + // TODO: ATTR Can this be done without clone + let attrs = + ParsedAttributes::require_attributes(module.attrs.clone(), &["doc", "cxx_qt::bridge"])?; let mut namespace = None; // Check for the cxx_qt::bridge attribute - if let Some(attr) = attrs.get("cxx_qt::bridge") { + if let Some(attr) = attrs.get_one("cxx_qt::bridge") { // If we are not #[cxx_qt::bridge] but #[cxx_qt::bridge(A = B)] then process if !matches!(attr.meta, Meta::Path(_)) { let nested = diff --git a/crates/cxx-qt-gen/src/parser/qenum.rs b/crates/cxx-qt-gen/src/parser/qenum.rs index 8af7db9bc..4d0e3aa67 100644 --- a/crates/cxx-qt-gen/src/parser/qenum.rs +++ b/crates/cxx-qt-gen/src/parser/qenum.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::{extract_cfgs, extract_docs, ParsedAttribute}; +use crate::parser::attribute::ParsedAttributes; use crate::parser::CaseConversion; use crate::{naming::Name, syntax::path::path_compare_str}; use quote::ToTokens; @@ -61,9 +61,11 @@ impl ParsedQEnum { parent_namespace: Option<&str>, module: &Ident, ) -> Result { - ParsedAttribute::require_attributes(&qenum.attrs, &Self::ALLOWED_ATTRS)?; - let cfgs = extract_cfgs(&qenum.attrs); - let docs = extract_docs(&qenum.attrs); + // TODO: ATTR Can this be done without clone? + let attrs = + ParsedAttributes::require_attributes(qenum.attrs.clone(), &Self::ALLOWED_ATTRS)?; + let cfgs = attrs.extract_cfgs(); + let docs = attrs.extract_docs(); if qenum.variants.is_empty() { return Err(syn::Error::new_spanned( @@ -74,7 +76,7 @@ impl ParsedQEnum { let name = Name::from_ident_and_attrs( &qenum.ident, - &qenum.attrs, + &attrs.clone_attrs(), parent_namespace, Some(module), CaseConversion::none(), diff --git a/crates/cxx-qt-gen/src/parser/qnamespace.rs b/crates/cxx-qt-gen/src/parser/qnamespace.rs index 4eddf6cef..62d9ba12a 100644 --- a/crates/cxx-qt-gen/src/parser/qnamespace.rs +++ b/crates/cxx-qt-gen/src/parser/qnamespace.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::ParsedAttribute; +use crate::parser::attribute::ParsedAttributes; use syn::{ItemMacro, LitStr, Result}; pub struct ParsedQNamespace { @@ -15,7 +15,7 @@ pub struct ParsedQNamespace { impl ParsedQNamespace { pub fn parse(mac: ItemMacro) -> Result { - let attrs = ParsedAttribute::require_attributes(&mac.attrs, &["qml_element"])?; + let attrs = ParsedAttributes::require_attributes(mac.attrs, &["qml_element"])?; let namespace_literal: LitStr = syn::parse2(mac.mac.tokens)?; let namespace = namespace_literal.value(); if namespace.contains(char::is_whitespace) { diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 49623b3cc..9267dc2ef 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -11,7 +11,7 @@ use crate::{ #[cfg(test)] use quote::format_ident; -use crate::parser::attribute::{extract_cfgs, ParsedAttribute}; +use crate::parser::attribute::ParsedAttributes; use crate::parser::{parse_base_type, CaseConversion}; use syn::{Attribute, Error, Ident, Meta, Result}; @@ -86,14 +86,16 @@ impl ParsedQObject { module: &Ident, auto_case: CaseConversion, ) -> Result { - let attributes = - ParsedAttribute::require_attributes(&declaration.attrs, &Self::ALLOWED_ATTRS)?; + // TODO: ATTR Can this be done without clone + let attrs = + ParsedAttributes::require_attributes(declaration.attrs.clone(), &Self::ALLOWED_ATTRS)?; + // TODO: handle docs through to generation - let cfgs = extract_cfgs(&declaration.attrs); + let cfgs = attrs.extract_cfgs(); - let has_qobject_macro = attributes.contains_key("qobject"); + let has_qobject_macro = attrs.contains_key("qobject"); - let base_class = parse_base_type(&attributes.cxx_qt_attrs)?; + let base_class = parse_base_type(&attrs)?; // Ensure that if there is no qobject macro that a base class is specificed if !has_qobject_macro && base_class.is_none() { @@ -105,18 +107,18 @@ impl ParsedQObject { let name = Name::from_ident_and_attrs( &declaration.ident_left, - &declaration.attrs, + &attrs.clone_attrs(), namespace, Some(module), CaseConversion::none(), )?; // Find any QML metadata - let qml_metadata = Self::parse_qml_metadata(&name, &declaration.attrs)?; + let qml_metadata = Self::parse_qml_metadata(&name, attrs.clone_attrs())?; // Parse any properties in the type // and remove the #[qproperty] attribute - let properties = Self::parse_property_attributes(&declaration.attrs, auto_case)?; + let properties = Self::parse_property_attributes(&attrs.clone_attrs(), auto_case)?; let inner = declaration.ident_right.clone(); Ok(Self { @@ -131,9 +133,12 @@ impl ParsedQObject { }) } - fn parse_qml_metadata(name: &Name, attrs: &[Attribute]) -> Result> { - let attributes = ParsedAttribute::require_attributes(attrs, &Self::ALLOWED_ATTRS)?; - if let Some(attr) = attributes.get("qml_element") { + fn parse_qml_metadata( + name: &Name, + attrs: Vec, + ) -> Result> { + let attributes = ParsedAttributes::require_attributes(attrs, &Self::ALLOWED_ATTRS)?; + if let Some(attr) = attributes.get_one("qml_element") { // Extract the name of the qml_element from macro, else use the c++ name // This will use the name provided by cxx_name if that attr was present let name = match &attr.meta { diff --git a/crates/cxx-qt-gen/src/parser/signals.rs b/crates/cxx-qt-gen/src/parser/signals.rs index c8b83ba49..1fe83b40b 100644 --- a/crates/cxx-qt-gen/src/parser/signals.rs +++ b/crates/cxx-qt-gen/src/parser/signals.rs @@ -2,7 +2,6 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::{extract_cfgs, extract_docs, ParsedAttribute}; use crate::parser::CaseConversion; use crate::{parser::method::MethodFields, syntax::path::path_compare_str}; use core::ops::Deref; @@ -35,11 +34,9 @@ impl ParsedSignal { } pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result { - let docs = extract_docs(&method.attrs); - let cfgs = extract_cfgs(&method.attrs); - let fields = MethodFields::parse(method, auto_case)?; - let attrs = - ParsedAttribute::require_attributes(&fields.method.attrs, &Self::ALLOWED_ATTRS)?; + let fields = MethodFields::parse(method, auto_case, &Self::ALLOWED_ATTRS)?; + let cfgs = fields.attrs.extract_cfgs(); + let docs = fields.attrs.extract_docs(); if !fields.mutable { return Err(Error::new( @@ -48,7 +45,7 @@ impl ParsedSignal { )); } - let inherit = attrs.contains_key("inherit"); + let inherit = fields.attrs.contains_key("inherit"); let private = if let Visibility::Restricted(vis_restricted) = &fields.method.vis { path_compare_str(&vis_restricted.path, &["self"]) From 3100295c27a1a53d33e5c11b4f6c47113262faae Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Fri, 18 Jul 2025 12:20:46 +0100 Subject: [PATCH 3/4] WIP add attribute parsing enum (check coverage) --- crates/cxx-qt-gen/src/parser/attribute.rs | 26 ++++++ crates/cxx-qt-gen/src/parser/externcxxqt.rs | 48 +++++++++-- crates/cxx-qt-gen/src/parser/externrustqt.rs | 65 +++++++++++++-- crates/cxx-qt-gen/src/parser/mod.rs | 84 ++++++++++++++++---- 4 files changed, 193 insertions(+), 30 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/attribute.rs b/crates/cxx-qt-gen/src/parser/attribute.rs index 35aebdf2f..168e314d1 100644 --- 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, } +// TODO: ATTR could this instead be used as Result 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 { 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 index 459b11a99..c5604b01d 100644 --- 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 { - 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 index ff6d7d429..9e1298954 100644 --- 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 index c292763d4..03a4b53a8 100644 --- 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 { // 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> { - attributes - .get_one("base") // TODO: ATTR Check for duplicates - .map(|attr| -> Result { + 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 From 166a64c27818e4825b28e72d00ef204868b0e23a Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Fri, 18 Jul 2025 14:57:42 +0100 Subject: [PATCH 4/4] WIP Try new parsing system --- crates/cxx-qt-gen/src/parser/attribute.rs | 74 +++++++-------- crates/cxx-qt-gen/src/parser/externcxxqt.rs | 32 ++----- crates/cxx-qt-gen/src/parser/externqobject.rs | 18 ++-- crates/cxx-qt-gen/src/parser/externrustqt.rs | 40 +++----- crates/cxx-qt-gen/src/parser/inherit.rs | 15 +-- crates/cxx-qt-gen/src/parser/method.rs | 24 ++--- crates/cxx-qt-gen/src/parser/mod.rs | 92 +++++-------------- crates/cxx-qt-gen/src/parser/qenum.rs | 12 ++- crates/cxx-qt-gen/src/parser/qnamespace.rs | 7 +- crates/cxx-qt-gen/src/parser/qobject.rs | 34 ++++--- crates/cxx-qt-gen/src/parser/signals.rs | 11 ++- 11 files changed, 154 insertions(+), 205 deletions(-) diff --git a/crates/cxx-qt-gen/src/parser/attribute.rs b/crates/cxx-qt-gen/src/parser/attribute.rs index 168e314d1..77c5c791c 100644 --- a/crates/cxx-qt-gen/src/parser/attribute.rs +++ b/crates/cxx-qt-gen/src/parser/attribute.rs @@ -14,18 +14,13 @@ pub struct ParsedAttributes { pub passthrough_attrs: Vec, } -// TODO: ATTR could this instead be used as Result 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>), +pub enum AttributeConstraint { + /// Indicates that there must be only one of this attribute + Unique, + /// Indicates there can be multiple of this attribute + Duplicate, + /// Indicates that this attribute is required + Required, } /// Iterate the attributes of the method to extract cfg attributes @@ -48,32 +43,49 @@ pub fn extract_docs(attrs: &[Attribute]) -> Vec { impl<'a> ParsedAttributes { /// Collects a Map of all attributes found from the allowed list - /// Will error if an attribute which is not in the allowed list is found + /// Will error if an attribute which is not in the allowed list is found, or attribute is used incorrectly pub fn require_attributes( mut attrs: Vec, - allowed: &'a [&str], + allowed: &'a [(AttributeConstraint, &str)], ) -> Result { let mut output = BTreeMap::>::default(); for attr in attrs.drain(..) { - let index = allowed - .iter() - .position(|string| path_compare_str(attr.meta.path(), &parser::split_path(string))); + let index = allowed.iter().position(|(_, string)| { + path_compare_str(attr.meta.path(), &parser::split_path(string)) + }); if let Some(index) = index { - // TODO: ATTR Doesn't error on duplicates / distinguish allowed and disallowed duplicates - match output.entry(allowed[index].into()) { - Entry::Occupied(mut entry) => { - entry.get_mut().push(attr); - } - Entry::Vacant(entry) => { - entry.insert(vec![attr]); + match allowed[index].0 { + AttributeConstraint::Unique => { + match output.entry(allowed[index].1.into()) { + Entry::Occupied(_) => return Err(Error::new_spanned( + attr, + "There must be at most one of this attribute on this given item", + )), + Entry::Vacant(entry) => { + entry.insert(vec![attr]); + } + } } + AttributeConstraint::Duplicate => match output.entry(allowed[index].1.into()) { + Entry::Occupied(mut entry) => { + entry.get_mut().push(attr); + } + Entry::Vacant(entry) => { + entry.insert(vec![attr]); + } + }, + AttributeConstraint::Required => {} } } else { return Err(Error::new( attr.span(), format!( "Unsupported attribute! The only attributes allowed on this item are\n{}", - allowed.join(", ") + allowed + .iter() + .map(|(_, string)| *string) + .collect::>() + .join(", ") ), )); } @@ -122,18 +134,6 @@ 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 index c5604b01d..774d791af 100644 --- a/crates/cxx-qt-gen/src/parser/externcxxqt.rs +++ b/crates/cxx-qt-gen/src/parser/externcxxqt.rs @@ -3,12 +3,11 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::{ParsedAttribute, ParsedAttributes}; +use crate::parser::attribute::{AttributeConstraint, 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, }; @@ -37,30 +36,19 @@ impl ParsedExternCxxQt { // TODO: support cfg on foreign mod blocks let attrs = ParsedAttributes::require_attributes( foreign_mod.attrs, - &["namespace", "auto_cxx_name", "auto_rust_name"], + &[ + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Unique, "auto_cxx_name"), + (AttributeConstraint::Unique, "auto_rust_name"), + ], )?; let auto_case = CaseConversion::from_attrs(&attrs)?; - 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 namespace = attrs + .get_one("namespace") + .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value)) + .transpose()?; let mut extern_cxx_block = ParsedExternCxxQt { namespace, diff --git a/crates/cxx-qt-gen/src/parser/externqobject.rs b/crates/cxx-qt-gen/src/parser/externqobject.rs index 7a335c9f6..b2a212720 100644 --- a/crates/cxx-qt-gen/src/parser/externqobject.rs +++ b/crates/cxx-qt-gen/src/parser/externqobject.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::naming::Name; -use crate::parser::attribute::ParsedAttributes; +use crate::parser::attribute::{AttributeConstraint, ParsedAttributes}; use crate::parser::{parse_base_type, CaseConversion}; use syn::{ForeignItemType, Ident, Result}; @@ -18,14 +18,14 @@ pub struct ParsedExternQObject { } impl ParsedExternQObject { - const ALLOWED_ATTRS: [&'static str; 7] = [ - "cxx_name", - "rust_name", - "namespace", - "cfg", - "doc", - "qobject", - "base", + const ALLOWED_ATTRS: [(AttributeConstraint, &'static str); 7] = [ + (AttributeConstraint::Unique, "cxx_name"), + (AttributeConstraint::Unique, "rust_name"), + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Duplicate, "cfg"), + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Unique, "qobject"), + (AttributeConstraint::Unique, "base"), ]; pub fn parse( diff --git a/crates/cxx-qt-gen/src/parser/externrustqt.rs b/crates/cxx-qt-gen/src/parser/externrustqt.rs index 9e1298954..7683c41bb 100644 --- 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::{ParsedAttribute, ParsedAttributes}; +use crate::parser::attribute::{AttributeConstraint, 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, Span}; +use proc_macro2::Ident; use syn::spanned::Spanned; use syn::{Error, ForeignItem, ForeignItemFn, ItemForeignMod, Result, Token}; @@ -41,7 +41,11 @@ impl ParsedExternRustQt { // TODO: support cfg on foreign mod blocks let attrs = ParsedAttributes::require_attributes( foreign_mod.attrs, - &["namespace", "auto_cxx_name", "auto_rust_name"], + &[ + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Unique, "auto_cxx_name"), + (AttributeConstraint::Unique, "auto_rust_name"), + ], )?; let auto_case = CaseConversion::from_attrs(&attrs)?; @@ -51,31 +55,11 @@ 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 = 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)); + 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)); for item in foreign_mod.items.drain(..) { match item { diff --git a/crates/cxx-qt-gen/src/parser/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 7a2245289..3c398a1c5 100644 --- a/crates/cxx-qt-gen/src/parser/inherit.rs +++ b/crates/cxx-qt-gen/src/parser/inherit.rs @@ -3,6 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::parser::attribute::AttributeConstraint; use crate::parser::{method::MethodFields, CaseConversion}; use core::ops::Deref; use quote::format_ident; @@ -20,13 +21,13 @@ pub struct ParsedInheritedMethod { } impl ParsedInheritedMethod { - const ALLOWED_ATTRS: [&'static str; 6] = [ - "cxx_name", - "rust_name", - "qinvokable", - "doc", - "inherit", - "cfg", + const ALLOWED_ATTRS: [(AttributeConstraint, &'static str); 6] = [ + (AttributeConstraint::Unique, "cxx_name"), + (AttributeConstraint::Unique, "rust_name"), + (AttributeConstraint::Unique, "qinvokable"), + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Unique, "inherit"), + (AttributeConstraint::Duplicate, "cfg"), ]; pub fn parse(method: ForeignItemFn, auto_case: CaseConversion) -> Result { diff --git a/crates/cxx-qt-gen/src/parser/method.rs b/crates/cxx-qt-gen/src/parser/method.rs index 238982d08..021611900 100644 --- a/crates/cxx-qt-gen/src/parser/method.rs +++ b/crates/cxx-qt-gen/src/parser/method.rs @@ -2,7 +2,7 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::{extract_cfgs, ParsedAttributes}; +use crate::parser::attribute::{extract_cfgs, AttributeConstraint, ParsedAttributes}; use crate::parser::CaseConversion; use crate::{ naming::Name, @@ -69,16 +69,16 @@ pub struct ParsedMethod { } impl ParsedMethod { - const ALLOWED_ATTRS: [&'static str; 9] = [ - "cxx_name", - "rust_name", - "qinvokable", - "cxx_final", - "cxx_override", - "cxx_virtual", - "cxx_pure", - "doc", - "cfg", + const ALLOWED_ATTRS: [(AttributeConstraint, &'static str); 9] = [ + (AttributeConstraint::Unique, "cxx_name"), + (AttributeConstraint::Unique, "rust_name"), + (AttributeConstraint::Unique, "qinvokable"), + (AttributeConstraint::Unique, "cxx_final"), + (AttributeConstraint::Unique, "cxx_override"), + (AttributeConstraint::Unique, "cxx_virtual"), + (AttributeConstraint::Unique, "cxx_pure"), + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Duplicate, "cfg"), ]; #[cfg(test)] @@ -172,7 +172,7 @@ impl MethodFields { pub fn parse( method: ForeignItemFn, auto_case: CaseConversion, - allowed: &[&str], + allowed: &[(AttributeConstraint, &str)], ) -> Result { // TODO: Remove this clone? let attrs = ParsedAttributes::require_attributes(method.attrs.clone(), allowed)?; diff --git a/crates/cxx-qt-gen/src/parser/mod.rs b/crates/cxx-qt-gen/src/parser/mod.rs index 03a4b53a8..766e32c8e 100644 --- a/crates/cxx-qt-gen/src/parser/mod.rs +++ b/crates/cxx-qt-gen/src/parser/mod.rs @@ -19,11 +19,10 @@ pub mod qobject; pub mod signals; pub mod trait_impl; -use crate::parser::attribute::{ParsedAttribute, ParsedAttributes}; +use crate::parser::attribute::{AttributeConstraint, 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,51 +72,14 @@ impl CaseConversion { /// Create a CaseConversion object from a Map of attributes, collected using `require_attributes` /// Parses both `auto_cxx_name` and `auto_cxx_name = Camel` pub fn from_attrs(attrs: &ParsedAttributes) -> Result { - // 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 = 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 - } - }; + 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()?; Ok(Self { rust, cxx }) } @@ -135,35 +97,20 @@ fn split_path(path_str: &str) -> Vec<&str> { // Extract base identifier from attribute pub fn parse_base_type(attributes: &ParsedAttributes) -> Result> { - match attributes.require_one("base") { - ParsedAttribute::Single(attr) => { + attributes + .get_one("base") + .map(|attr| -> Result { let expr = &attr.meta.require_name_value()?.value; if let Expr::Path(path_expr) = expr { - Ok(Some(path_expr.path.require_ident()?.clone())) + Ok(path_expr.path.require_ident()?.clone()) } else { Err(Error::new_spanned( expr, "There must be a single base identifier, not string, and cannot be empty!", )) } - } - 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 - } - } + }) + .transpose() } /// Struct representing the necessary components of a cxx mod to be passed through to generation @@ -204,8 +151,13 @@ pub struct Parser { impl Parser { fn parse_mod_attributes(module: &mut ItemMod) -> Result> { // TODO: ATTR Can this be done without clone - let attrs = - ParsedAttributes::require_attributes(module.attrs.clone(), &["doc", "cxx_qt::bridge"])?; + let attrs = ParsedAttributes::require_attributes( + module.attrs.clone(), + &[ + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Unique, "cxx_qt::bridge"), + ], + )?; let mut namespace = None; // Check for the cxx_qt::bridge attribute diff --git a/crates/cxx-qt-gen/src/parser/qenum.rs b/crates/cxx-qt-gen/src/parser/qenum.rs index 4d0e3aa67..2b79ef425 100644 --- a/crates/cxx-qt-gen/src/parser/qenum.rs +++ b/crates/cxx-qt-gen/src/parser/qenum.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::ParsedAttributes; +use crate::parser::attribute::{AttributeConstraint, ParsedAttributes}; use crate::parser::CaseConversion; use crate::{naming::Name, syntax::path::path_compare_str}; use quote::ToTokens; @@ -25,8 +25,14 @@ pub struct ParsedQEnum { } impl ParsedQEnum { - const ALLOWED_ATTRS: [&'static str; 6] = - ["cfg", "doc", "cxx_name", "rust_name", "namespace", "qenum"]; + const ALLOWED_ATTRS: [(AttributeConstraint, &'static str); 6] = [ + (AttributeConstraint::Duplicate, "cfg"), + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Unique, "cxx_name"), + (AttributeConstraint::Unique, "rust_name"), + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Unique, "qenum"), + ]; fn parse_variant(variant: &Variant) -> Result { fn err(spanned: &impl ToTokens, message: &str) -> Result { Err(syn::Error::new_spanned(spanned, message)) diff --git a/crates/cxx-qt-gen/src/parser/qnamespace.rs b/crates/cxx-qt-gen/src/parser/qnamespace.rs index 62d9ba12a..4ca8e9e40 100644 --- a/crates/cxx-qt-gen/src/parser/qnamespace.rs +++ b/crates/cxx-qt-gen/src/parser/qnamespace.rs @@ -3,7 +3,7 @@ // // SPDX-License-Identifier: MIT OR Apache-2.0 -use crate::parser::attribute::ParsedAttributes; +use crate::parser::attribute::{AttributeConstraint, ParsedAttributes}; use syn::{ItemMacro, LitStr, Result}; pub struct ParsedQNamespace { @@ -15,7 +15,10 @@ pub struct ParsedQNamespace { impl ParsedQNamespace { pub fn parse(mac: ItemMacro) -> Result { - let attrs = ParsedAttributes::require_attributes(mac.attrs, &["qml_element"])?; + let attrs = ParsedAttributes::require_attributes( + mac.attrs, + &[(AttributeConstraint::Unique, "qml_element")], + )?; let namespace_literal: LitStr = syn::parse2(mac.mac.tokens)?; let namespace = namespace_literal.value(); if namespace.contains(char::is_whitespace) { diff --git a/crates/cxx-qt-gen/src/parser/qobject.rs b/crates/cxx-qt-gen/src/parser/qobject.rs index 9267dc2ef..b33158b5c 100644 --- a/crates/cxx-qt-gen/src/parser/qobject.rs +++ b/crates/cxx-qt-gen/src/parser/qobject.rs @@ -11,7 +11,7 @@ use crate::{ #[cfg(test)] use quote::format_ident; -use crate::parser::attribute::ParsedAttributes; +use crate::parser::attribute::{AttributeConstraint, ParsedAttributes}; use crate::parser::{parse_base_type, CaseConversion}; use syn::{Attribute, Error, Ident, Meta, Result}; @@ -48,19 +48,20 @@ pub struct ParsedQObject { } impl ParsedQObject { - const ALLOWED_ATTRS: [&'static str; 11] = [ - "cxx_name", - "rust_name", - "namespace", - "cfg", - "doc", - "qobject", - "base", - "qml_element", - "qml_uncreatable", - "qml_singleton", - "qproperty", + const ALLOWED_ATTRS: [(AttributeConstraint, &'static str); 11] = [ + (AttributeConstraint::Unique, "cxx_name"), + (AttributeConstraint::Unique, "rust_name"), + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Duplicate, "cfg"), + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Unique, "qobject"), + (AttributeConstraint::Unique, "base"), + (AttributeConstraint::Unique, "qml_element"), + (AttributeConstraint::Unique, "qml_uncreatable"), + (AttributeConstraint::Unique, "qml_singleton"), + (AttributeConstraint::Duplicate, "qproperty"), ]; + #[cfg(test)] pub fn mock() -> Self { ParsedQObject { @@ -360,6 +361,13 @@ pub mod tests { #[base = ""] type MyObject = super::T; } + // Duplicate attribute should fail + { + #[qobject] + #[qobject] + #[base = ""] + type MyObject = super::T; + } { type MyObject = super::T; } } } diff --git a/crates/cxx-qt-gen/src/parser/signals.rs b/crates/cxx-qt-gen/src/parser/signals.rs index 1fe83b40b..0e9c9367f 100644 --- a/crates/cxx-qt-gen/src/parser/signals.rs +++ b/crates/cxx-qt-gen/src/parser/signals.rs @@ -2,6 +2,7 @@ // SPDX-FileContributor: Andrew Hayzen // // SPDX-License-Identifier: MIT OR Apache-2.0 +use crate::parser::attribute::AttributeConstraint; use crate::parser::CaseConversion; use crate::{parser::method::MethodFields, syntax::path::path_compare_str}; use core::ops::Deref; @@ -24,8 +25,14 @@ pub struct ParsedSignal { } impl ParsedSignal { - const ALLOWED_ATTRS: [&'static str; 6] = - ["cfg", "cxx_name", "rust_name", "inherit", "doc", "qsignal"]; + const ALLOWED_ATTRS: [(AttributeConstraint, &'static str); 6] = [ + (AttributeConstraint::Duplicate, "cfg"), + (AttributeConstraint::Unique, "cxx_name"), + (AttributeConstraint::Unique, "rust_name"), + (AttributeConstraint::Unique, "inherit"), + (AttributeConstraint::Duplicate, "doc"), + (AttributeConstraint::Unique, "qsignal"), + ]; #[cfg(test)] /// Test fn for creating a mocked signal from a method body