diff --git a/parsers/src/util/macro_tests.rs b/parsers/src/util/macro_tests.rs index 918f00b4b1d411955d55ace8da5130919a822681..5f5c667f7f567eaad41a63407ce524cd08d48e07 100644 --- a/parsers/src/util/macro_tests.rs +++ b/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::( + "", + ) +} + +#[test] +fn element_catch_one_negative_none() { + #[allow(unused_imports)] + use core::{ + option::Option::{None, Some}, + result::Result::{Err, Ok}, + }; + match parse_str::("") { + 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::("") { + 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::( + "", + ) +} + +#[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::( + "", + ) +} + +#[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::( + "", + ) { + 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 { diff --git a/xso-proc/src/field/child.rs b/xso-proc/src/field/child.rs index 0710f3d22c9775b414ab50249b3bd51c89940246..268a59f901f2409a306377224b146f6071222d46 100644 --- a/xso-proc/src/field/child.rs +++ b/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(), diff --git a/xso-proc/src/field/element.rs b/xso-proc/src/field/element.rs index c4e376cc57148ac4658c4930fa1cb738112e1530..2df6f4070f7cd7bbaa73f294c15b9294de2b1adf 100644 --- a/xso-proc/src/field/element.rs +++ b/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 { + 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 { 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) - } - } - }, - }) + }, + }), + } } } diff --git a/xso-proc/src/field/mod.rs b/xso-proc/src/field/mod.rs index c14adf475eeff386e3e4591838ea937852b7782f..1ef52b8e165a4639cf39b7b0399e5153254e997a 100644 --- a/xso-proc/src/field/mod.rs +++ b/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 } => { diff --git a/xso-proc/src/meta.rs b/xso-proc/src/meta.rs index 8018d15a4e8d15fa8db141469d92c7aec7611f96..751940b515c04d86398a7605c44728525ee3a0b1 100644 --- a/xso-proc/src/meta.rs +++ b/xso-proc/src/meta.rs @@ -1035,17 +1035,19 @@ impl XmlFieldMeta { /// Parse a `#[xml(element)]` meta. fn element_from_meta(meta: ParseNestedMeta<'_>) -> Result { 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, diff --git a/xso/ChangeLog b/xso/ChangeLog index b9a527fa67e1f0f9a42d0c63a2d2db4f272e18df..376af4f0d8f7f7be1769772fdba6d551935a45d3 100644 --- a/xso/ChangeLog +++ b/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) diff --git a/xso/src/from_xml_doc.md b/xso/src/from_xml_doc.md index 4dbd1800288d67027d8edd2854351a6dc3cc1283..720f99fb800e2bbc0cd33c8541b20d4626263e4b 100644 --- a/xso/src/from_xml_doc.md +++ b/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`][`core::iter::IntoIterator`]. In addition, the field's type must implement [`Extend`][`core::iter::Extend`] to derive `FromXml` and the