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..77c5c791c --- /dev/null +++ b/crates/cxx-qt-gen/src/parser/attribute.rs @@ -0,0 +1,141 @@ +// 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::btree_map::Entry; +use std::collections::BTreeMap; +use syn::{spanned::Spanned, Attribute, Error, Result}; + +#[derive(Clone)] +pub struct ParsedAttributes { + pub cxx_qt_attrs: BTreeMap>, + pub passthrough_attrs: Vec, +} + +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 +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> 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, or attribute is used incorrectly + pub fn require_attributes( + mut attrs: Vec, + 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)) + }); + if let Some(index) = index { + 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 + .iter() + .map(|(_, string)| *string) + .collect::>() + .join(", ") + ), + )); + } + } + Ok(Self { + cxx_qt_attrs: output, + 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_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) // 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 4364c9027..774d791af 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::{AttributeConstraint, ParsedAttributes}; 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,18 +34,20 @@ impl ParsedExternCxxQt { parent_namespace: Option<&str>, ) -> Result { // TODO: support cfg on foreign mod blocks - let attrs = require_attributes( - &foreign_mod.attrs, - &["namespace", "auto_cxx_name", "auto_rust_name"], + let attrs = ParsedAttributes::require_attributes( + foreign_mod.attrs, + &[ + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Unique, "auto_cxx_name"), + (AttributeConstraint::Unique, "auto_rust_name"), + ], )?; let auto_case = CaseConversion::from_attrs(&attrs)?; let namespace = attrs - .get("namespace") - .map(|attr| -> Result { - expr_to_string(&attr.meta.require_name_value()?.value) - }) + .get_one("namespace") + .map(|attr| expr_to_string(&attr.meta.require_name_value()?.value)) .transpose()?; let mut extern_cxx_block = ParsedExternCxxQt { @@ -221,6 +221,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/externqobject.rs b/crates/cxx-qt-gen/src/parser/externqobject.rs index b7ea93faa..b2a212720 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::{AttributeConstraint, ParsedAttributes}; +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 @@ -17,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( @@ -32,14 +33,15 @@ impl ParsedExternQObject { module_ident: &Ident, parent_namespace: Option<&str>, ) -> Result { - let attributes = 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(&attributes)?; + 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 6da520bdf..7683c41bb 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::{AttributeConstraint, ParsedAttributes}; 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,9 +39,13 @@ impl ParsedExternRustQt { parent_namespace: Option<&str>, ) -> Result { // TODO: support cfg on foreign mod blocks - let attrs = require_attributes( - &foreign_mod.attrs, - &["namespace", "auto_cxx_name", "auto_rust_name"], + let attrs = ParsedAttributes::require_attributes( + foreign_mod.attrs, + &[ + (AttributeConstraint::Unique, "namespace"), + (AttributeConstraint::Unique, "auto_cxx_name"), + (AttributeConstraint::Unique, "auto_rust_name"), + ], )?; let auto_case = CaseConversion::from_attrs(&attrs)?; @@ -51,7 +56,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)); @@ -223,6 +228,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/inherit.rs b/crates/cxx-qt-gen/src/parser/inherit.rs index 9ebb5af17..3c398a1c5 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::AttributeConstraint; +use crate::parser::{method::MethodFields, CaseConversion}; use core::ops::Deref; use quote::format_ident; use std::ops::DerefMut; @@ -22,22 +21,22 @@ 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 { - 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 89c708da5..021611900 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, AttributeConstraint, ParsedAttributes}; 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; @@ -33,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, @@ -68,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)] @@ -120,14 +121,14 @@ impl ParsedMethod { auto_case: CaseConversion, unsafe_block: bool, ) -> Result { - let fields = MethodFields::parse(method, auto_case)?; - let attrs = 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); + 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, @@ -164,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: &[(AttributeConstraint, &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, @@ -184,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 97b11ec1f..766e32c8e 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,13 +19,10 @@ 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::{AttributeConstraint, 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, @@ -73,13 +71,13 @@ 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 { 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()?; @@ -87,24 +85,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,36 +95,10 @@ 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> { +pub fn parse_base_type(attributes: &ParsedAttributes) -> Result> { attributes - .get("base") + .get_one("base") .map(|attr| -> Result { let expr = &attr.meta.require_name_value()?.value; if let Expr::Path(path_expr) = expr { @@ -152,7 +106,7 @@ pub fn parse_base_type(attributes: &BTreeMap<&str, &Attribute>) -> Result