WIP Try new parsing system

Author: BenFordTytherington

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

@@ -14,18 +14,11 @@ 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>),
+pub enum AttributeConstraint {
+    /// Indicates that there must be only one of this attribute
+    Unique,
+    /// Indicates there can be multiple of this attribute
+    Duplicate,
 }
 
 /// Iterate the attributes of the method to extract cfg attributes
@@ -48,32 +41,49 @@ pub fn extract_docs(attrs: &[Attribute]) -> Vec<Attribute> {
 
 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<Attribute>,
-        allowed: &'a [&str],
+        allowed: &'a [(AttributeConstraint, &str)],
     ) -> Result<ParsedAttributes> {
         let mut output = BTreeMap::<String, Vec<Attribute>>::default();
+        // Iterate all attributes found
         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]);
+                        }
+                    },
                 }
             } 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::<Vec<_>>()
+                            .join(", ")
                     ),
                 ));
             }
@@ -122,18 +132,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

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,

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(

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 {

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<Self> {

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

@@ -2,7 +2,7 @@
 // SPDX-FileContributor: Andrew Hayzen <andrew.hayzen@kdab.com>
 //
 // 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<Self> {
         // TODO: Remove this clone?
         let attrs = ParsedAttributes::require_attributes(method.attrs.clone(), allowed)?;