xso-proc: Add n = 1 flag to element meta

Emmanuel Gil Peyrot created

This allows exactly one arbitrary payload in any element, and is handled
after every other element with a more specific matcher has been parsed.

Both the #[xml(element(n = 1))] meta and its shortcut #[xml(element)]
are allowed and treated the exact same way.

Change summary

parsers/src/util/macro_tests.rs | 112 ++++++++++++++++++++++++
xso-proc/src/field/child.rs     |   3 
xso-proc/src/field/element.rs   | 160 +++++++++++++++++++++++++---------
xso-proc/src/field/mod.rs       |  19 ---
xso-proc/src/meta.rs            |  22 ++--
xso/ChangeLog                   |   3 
xso/src/from_xml_doc.md         |  26 ++++-
7 files changed, 265 insertions(+), 80 deletions(-)

Detailed changes

parsers/src/util/macro_tests.rs 🔗

@@ -1618,6 +1618,118 @@ fn optional_attribute_optional_extract_double_option_roundtrip_absent_child() {
     )
 }
 
+#[derive(FromXml, AsXml, PartialEq, Debug, Clone)]
+#[xml(namespace = NS1, name = "parent")]
+struct ElementCatchOne {
+    #[xml(element)]
+    child: ::minidom::Element,
+}
+
+#[test]
+fn element_catch_one_roundtrip() {
+    #[allow(unused_imports)]
+    use core::{
+        option::Option::{None, Some},
+        result::Result::{Err, Ok},
+    };
+    roundtrip_full::<ElementCatchOne>(
+        "<parent xmlns='urn:example:ns1'><child><deeper/></child></parent>",
+    )
+}
+
+#[test]
+fn element_catch_one_negative_none() {
+    #[allow(unused_imports)]
+    use core::{
+        option::Option::{None, Some},
+        result::Result::{Err, Ok},
+    };
+    match parse_str::<ElementCatchOne>("<parent xmlns='urn:example:ns1'/>") {
+        Err(::xso::error::FromElementError::Invalid(::xso::error::Error::Other(e)))
+            if e.contains("Missing child field") =>
+        {
+            ()
+        }
+        other => panic!("unexpected result: {:?}", other),
+    }
+}
+
+#[test]
+fn element_catch_one_negative_more_than_one_child() {
+    #[allow(unused_imports)]
+    use core::{
+        option::Option::{None, Some},
+        result::Result::{Err, Ok},
+    };
+    match parse_str::<ElementCatchOne>("<parent xmlns='urn:example:ns1'><child><deeper/></child><child xmlns='urn:example:ns2'/></parent>") {
+        Err(::xso::error::FromElementError::Invalid(::xso::error::Error::Other(e))) if e == "Unknown child in ElementCatchOne element." => (),
+        other => panic!("unexpected result: {:?}", other),
+    }
+}
+
+#[derive(FromXml, AsXml, PartialEq, Debug, Clone)]
+#[xml(namespace = NS1, name = "parent")]
+struct ElementCatchChildAndOne {
+    #[xml(child)]
+    child: Empty,
+
+    #[xml(element)]
+    element: ::minidom::Element,
+}
+
+#[test]
+fn element_catch_child_and_one_roundtrip() {
+    #[allow(unused_imports)]
+    use core::{
+        option::Option::{None, Some},
+        result::Result::{Err, Ok},
+    };
+    roundtrip_full::<ElementCatchChildAndOne>(
+        "<parent xmlns='urn:example:ns1'><foo/><element/></parent>",
+    )
+}
+
+#[derive(FromXml, AsXml, PartialEq, Debug, Clone)]
+#[xml(namespace = NS1, name = "parent")]
+struct ElementCatchOneAndMany {
+    #[xml(element)]
+    child: ::minidom::Element,
+
+    #[xml(element(n = ..))]
+    children: Vec<::minidom::Element>,
+}
+
+#[test]
+fn element_catch_one_and_many_roundtrip() {
+    #[allow(unused_imports)]
+    use core::{
+        option::Option::{None, Some},
+        result::Result::{Err, Ok},
+    };
+    roundtrip_full::<ElementCatchOneAndMany>(
+        "<parent xmlns='urn:example:ns1'><child num='0'><deeper/></child><child num='1'><deeper/></child></parent>",
+    )
+}
+
+#[test]
+fn element_catch_one_and_many_parse_in_order() {
+    #[allow(unused_imports)]
+    use core::{
+        option::Option::{None, Some},
+        result::Result::{Err, Ok},
+    };
+    match parse_str::<ElementCatchOneAndMany>(
+        "<parent xmlns='urn:example:ns1'><child num='0'/><child num='1'/></parent>",
+    ) {
+        Ok(ElementCatchOneAndMany { child, children }) => {
+            assert_eq!(child.attr("num"), Some("0"));
+            assert_eq!(children.len(), 1);
+            assert_eq!(children[0].attr("num"), Some("1"));
+        }
+        other => panic!("unexpected result: {:?}", other),
+    }
+}
+
 #[derive(FromXml, AsXml, PartialEq, Debug, Clone)]
 #[xml(namespace = NS1, name = "parent")]
 struct ElementCatchall {

xso-proc/src/field/child.rs 🔗

@@ -67,10 +67,9 @@ impl Field for ChildField {
                 } = scope;
 
                 let from_events = from_events_fn(element_ty.clone());
-                let from_xml_builder = from_xml_builder_ty(element_ty.clone());
 
                 let matcher = quote! { #from_events(name, attrs) };
-                let builder = from_xml_builder;
+                let builder = from_xml_builder_ty(element_ty.clone());
 
                 (
                     TokenStream::default(),

xso-proc/src/field/element.rs 🔗

@@ -13,52 +13,97 @@ use proc_macro2::{Span, TokenStream};
 use quote::quote;
 use syn::*;
 
-use crate::error_message::ParentRef;
+use crate::error_message::{self, ParentRef};
+use crate::meta::AmountConstraint;
 use crate::scope::{AsItemsScope, FromEventsScope};
 use crate::types::{
-    default_fn, element_ty, from_xml_builder_ty, into_iterator_into_iter_fn, into_iterator_iter_ty,
-    item_iter_ty, option_ty, ref_ty,
+    as_xml_iter_fn, default_fn, element_ty, from_events_fn, from_xml_builder_ty,
+    into_iterator_into_iter_fn, into_iterator_item_ty, into_iterator_iter_ty, item_iter_ty,
+    option_ty, ref_ty,
 };
 
 use super::{Field, FieldBuilderPart, FieldIteratorPart, FieldTempInit, NestedMatcher};
 
-pub(super) struct ElementField;
+pub(super) struct ElementField {
+    /// Number of child elements allowed.
+    pub(super) amount: AmountConstraint,
+}
 
 impl Field for ElementField {
     fn make_builder_part(
         &self,
         scope: &FromEventsScope,
-        _container_name: &ParentRef,
+        container_name: &ParentRef,
         member: &Member,
         ty: &Type,
     ) -> Result<FieldBuilderPart> {
+        let element_ty = match self.amount {
+            AmountConstraint::FixedSingle(_) => ty.clone(),
+            AmountConstraint::Any(_) => into_iterator_item_ty(ty.clone()),
+        };
+
         let FromEventsScope {
             ref substate_result,
             ..
         } = scope;
+
+        let from_events = from_events_fn(element_ty.clone());
+
+        let extra_defs = TokenStream::default();
         let field_access = scope.access_field(member);
 
-        let element_ty = element_ty(Span::call_site());
         let default_fn = default_fn(ty.clone());
         let builder = from_xml_builder_ty(element_ty.clone());
 
-        Ok(FieldBuilderPart::Nested {
-            extra_defs: TokenStream::default(),
-            value: FieldTempInit {
-                init: quote! { #default_fn() },
-                ty: ty.clone(),
-            },
-            matcher: NestedMatcher::Fallback(quote! {
-                #builder::new(name, attrs)
+        match self.amount {
+            AmountConstraint::FixedSingle(_) => {
+                let missing_msg = error_message::on_missing_child(container_name, member);
+                let on_absent = quote! {
+                    return ::core::result::Result::Err(::xso::error::Error::Other(#missing_msg).into())
+                };
+                Ok(FieldBuilderPart::Nested {
+                    extra_defs,
+                    value: FieldTempInit {
+                        init: quote! { ::core::option::Option::None },
+                        ty: option_ty(ty.clone()),
+                    },
+                    matcher: NestedMatcher::Selective(quote! {
+                        if #field_access.is_some() {
+                            ::core::result::Result::Err(::xso::error::FromEventsError::Mismatch { name, attrs })
+                        } else {
+                            #from_events(name, attrs)
+                        }
+                    }),
+                    builder,
+                    collect: quote! {
+                        #field_access = ::core::option::Option::Some(#substate_result);
+                    },
+                    finalize: quote! {
+                        match #field_access {
+                            ::core::option::Option::Some(value) => value,
+                            ::core::option::Option::None => #on_absent,
+                        }
+                    },
+                })
+            }
+            AmountConstraint::Any(_) => Ok(FieldBuilderPart::Nested {
+                extra_defs,
+                value: FieldTempInit {
+                    init: quote! { #default_fn() },
+                    ty: ty.clone(),
+                },
+                matcher: NestedMatcher::Fallback(quote! {
+                    #builder::new(name, attrs)
+                }),
+                builder,
+                collect: quote! {
+                    <#ty as ::core::iter::Extend::<#element_ty>>::extend(&mut #field_access, [#substate_result]);
+                },
+                finalize: quote! {
+                    #field_access
+                },
             }),
-            builder,
-            collect: quote! {
-                <#ty as ::core::iter::Extend::<#element_ty>>::extend(&mut #field_access, [#substate_result]);
-            },
-            finalize: quote! {
-                #field_access
-            },
-        })
+        }
     }
 
     fn make_iterator_part(
@@ -71,6 +116,15 @@ impl Field for ElementField {
     ) -> Result<FieldIteratorPart> {
         let AsItemsScope { ref lifetime, .. } = scope;
 
+        let item_ty = match self.amount {
+            AmountConstraint::FixedSingle(_) => ty.clone(),
+            AmountConstraint::Any(_) => {
+                // This should give us the type of element stored in the
+                // collection.
+                into_iterator_item_ty(ty.clone())
+            }
+        };
+
         let element_ty = element_ty(Span::call_site());
         let iter_ty = item_iter_ty(element_ty.clone(), lifetime.clone());
         let element_iter = into_iterator_iter_ty(ref_ty(ty.clone(), lifetime.clone()));
@@ -78,33 +132,49 @@ impl Field for ElementField {
 
         let state_ty = Type::Tuple(TypeTuple {
             paren_token: token::Paren::default(),
-            elems: [element_iter, option_ty(iter_ty)].into_iter().collect(),
+            elems: [element_iter, option_ty(iter_ty.clone())]
+                .into_iter()
+                .collect(),
         });
 
-        Ok(FieldIteratorPart::Content {
-            extra_defs: TokenStream::default(),
-            value: FieldTempInit {
-                init: quote! {
-                    (#into_iter(#bound_name), ::core::option::Option::None)
+        let extra_defs = TokenStream::default();
+        let as_xml_iter = as_xml_iter_fn(item_ty.clone());
+        let init = quote! { #as_xml_iter(#bound_name)? };
+        let iter_ty = item_iter_ty(item_ty.clone(), lifetime.clone());
+
+        match self.amount {
+            AmountConstraint::FixedSingle(_) => Ok(FieldIteratorPart::Content {
+                extra_defs,
+                value: FieldTempInit { init, ty: iter_ty },
+                generator: quote! {
+                    #bound_name.next().transpose()
+                },
+            }),
+            AmountConstraint::Any(_) => Ok(FieldIteratorPart::Content {
+                extra_defs,
+                value: FieldTempInit {
+                    init: quote! {
+                        (#into_iter(#bound_name), ::core::option::Option::None)
+                    },
+                    ty: state_ty,
                 },
-                ty: state_ty,
-            },
-            generator: quote! {
-                loop {
-                    if let ::core::option::Option::Some(current) = #bound_name.1.as_mut() {
-                        if let ::core::option::Option::Some(item) = current.next() {
-                            break ::core::option::Option::Some(item).transpose();
+                generator: quote! {
+                    loop {
+                        if let ::core::option::Option::Some(current) = #bound_name.1.as_mut() {
+                            if let ::core::option::Option::Some(item) = current.next() {
+                                break ::core::option::Option::Some(item).transpose();
+                            }
+                        }
+                        if let ::core::option::Option::Some(item) = #bound_name.0.next() {
+                            #bound_name.1 = ::core::option::Option::Some(
+                                <#element_ty as ::xso::AsXml>::as_xml_iter(item)?
+                            );
+                        } else {
+                            break ::core::result::Result::Ok(::core::option::Option::None)
                         }
                     }
-                    if let ::core::option::Option::Some(item) = #bound_name.0.next() {
-                        #bound_name.1 = ::core::option::Option::Some(
-                            <#element_ty as ::xso::AsXml>::as_xml_iter(item)?
-                        );
-                    } else {
-                        break ::core::result::Result::Ok(::core::option::Option::None)
-                    }
-                }
-            },
-        })
+                },
+            }),
+        }
     }
 }

xso-proc/src/field/mod.rs 🔗

@@ -358,7 +358,7 @@ fn new_field(
                         );
                         err.combine(Error::new(
                             *amount,
-                            "`n` was set to a non-1 value here, which enables connection logic",
+                            "`n` was set to a non-1 value here, which enables collection logic",
                         ));
                         return Err(err);
                     }
@@ -406,20 +406,9 @@ fn new_field(
         }
 
         #[cfg(feature = "minidom")]
-        XmlFieldMeta::Element { span, amount } => {
-            match amount {
-                Some(AmountConstraint::Any(_)) => (),
-                Some(AmountConstraint::FixedSingle(span)) => {
-                    return Err(Error::new(
-                        span,
-                        "only `n = ..` is supported for #[xml(element)]` currently",
-                    ))
-                }
-                None => return Err(Error::new(span, "`n` must be set to `..` currently")),
-            }
-
-            Ok(Box::new(ElementField))
-        }
+        XmlFieldMeta::Element { span, amount } => Ok(Box::new(ElementField {
+            amount: amount.unwrap_or(AmountConstraint::FixedSingle(span)),
+        })),
 
         #[cfg(not(feature = "minidom"))]
         XmlFieldMeta::Element { span, amount } => {

xso-proc/src/meta.rs 🔗

@@ -1035,17 +1035,19 @@ impl XmlFieldMeta {
     /// Parse a `#[xml(element)]` meta.
     fn element_from_meta(meta: ParseNestedMeta<'_>) -> Result<Self> {
         let mut amount = None;
-        meta.parse_nested_meta(|meta| {
-            if meta.path.is_ident("n") {
-                if amount.is_some() {
-                    return Err(Error::new_spanned(meta.path, "duplicate `n` key"));
+        if meta.input.peek(syn::token::Paren) {
+            meta.parse_nested_meta(|meta| {
+                if meta.path.is_ident("n") {
+                    if amount.is_some() {
+                        return Err(Error::new_spanned(meta.path, "duplicate `n` key"));
+                    }
+                    amount = Some(meta.value()?.parse()?);
+                    Ok(())
+                } else {
+                    Err(Error::new_spanned(meta.path, "unsupported key"))
                 }
-                amount = Some(meta.value()?.parse()?);
-                Ok(())
-            } else {
-                Err(Error::new_spanned(meta.path, "unsupported key"))
-            }
-        })?;
+            })?;
+        }
         Ok(Self::Element {
             span: meta.path.span(),
             amount,

xso/ChangeLog 🔗

@@ -21,7 +21,8 @@ Version NEXT:
       - Support for extracting data from child elements without intermediate
         structs.
       - Support for collecting all unknown children in a single field as
-        collection of `minidom::Element`.
+        collection of `minidom::Element`, or one unknown child as a
+        `minidom::Element`.
       - Support for "transparent" structs (newtype-like patterns for XSO).
       - FromXmlText and AsXmlText are now implemented for jid::NodePart,
         jid::DomainPart, and jid::ResourcePart (!485)

xso/src/from_xml_doc.md 🔗

@@ -249,6 +249,20 @@ The following mapping types are defined:
 | [`extract`](#extract-meta) | Map the field to contents of a child element of specified structure |
 | [`text`](#text-meta) | Map the field to the text content of the struct's element |
 
+#### Field order
+
+Field order **matters**. The fields are parsed in the order they are declared
+(for children, anyway). If multiple fields match a given child element, the
+first field which matches will be taken. The only exception is
+`#[xml(element(n = ..))]`, which is always processed last.
+
+When XML is generated from a struct, the child elements are also generated
+in the order of the fields. That means that passing an XML element through
+FromXml and IntoXml may re-order some child elements.
+
+Sorting order between elements which match the same field is generally
+preserved, if the container preserves sort order on insertion.
+
 ### `attribute` meta
 
 The `attribute` meta causes the field to be mapped to an XML attribute of the
@@ -419,15 +433,13 @@ The following keys can be used inside the `#[xml(extract(..))]` meta:
 
 | Key | Value type | Description |
 | --- | --- | --- |
-| `n` | `..` | Must be set to the value `..`. |
+| `n` | `1` or `..` | If `1`, a single element is parsed. If `..`, a collection is parsed. Defaults to `1`. |
 
-The `n` parameter will, in the future, support values other than `..`. In
-order to provide a non-breaking path into that future, it must be set to the
-value `..` right now, indicating that an arbitrary number of elements may be
-collected by this meta.
+When parsing a single child element (i.e. `n = 1` or no `n` value set at all),
+the field's type must be a `minidom::Element`.
 
-The field's type must be a collection of `minidom::Element`. It must thus
-implement
+When parsing a collection (with `n = ..`), the field's type must be a
+collection of `minidom::Element`. It must thus implement
 [`IntoIterator<Item = minidom::Element>`][`core::iter::IntoIterator`]. In
 addition, the field's type must implement
 [`Extend<minidom::Element>`][`core::iter::Extend`] to derive `FromXml` and the