Allow unrestricted property names in zbus-xmlgen

This fixes #550 by adding a struct PropertyName to zbus-names. It's essentially a copy of the already existing struct MemberName but only checks for the name length instead of enforcing all name restrictions.
Also added/changed the appropriate tests.

zbus_names/src/error.rs

@@ -20,6 +20,8 @@ pub enum Error {
     InvalidInterfaceName(String),
     /// Invalid member (method or signal) name.
     InvalidMemberName(String),
+    /// Invalid property name.
+    InvalidPropertyName(String),
     /// Invalid error name.
     InvalidErrorName(String),
 }
@@ -34,6 +36,7 @@ impl PartialEq for Error {
             (Self::InvalidUniqueName(_), Self::InvalidUniqueName(_)) => true,
             (Self::InvalidInterfaceName(_), Self::InvalidInterfaceName(_)) => true,
             (Self::InvalidMemberName(_), Self::InvalidMemberName(_)) => true,
+            (Self::InvalidPropertyName(_), Self::InvalidPropertyName(_)) => true,
             (Self::InvalidErrorName(_), Self::InvalidErrorName(_)) => true,
             (Self::Variant(s), Self::Variant(o)) => s == o,
             (_, _) => false,
@@ -50,6 +53,7 @@ impl error::Error for Error {
             Error::InvalidInterfaceName(_) => None,
             Error::InvalidErrorName(_) => None,
             Error::InvalidMemberName(_) => None,
+            Error::InvalidPropertyName(_) => None,
             Error::Variant(e) => Some(e),
         }
     }
@@ -70,6 +74,7 @@ impl fmt::Display for Error {
             Error::InvalidInterfaceName(s) => write!(f, "Invalid interface or error name: {s}"),
             Error::InvalidErrorName(s) => write!(f, "Invalid interface or error name: {s}"),
             Error::InvalidMemberName(s) => write!(f, "Invalid method or signal name: {s}"),
+            Error::InvalidPropertyName(s) => write!(f, "Invalid property name: {s}"),
         }
     }
 }

zbus_names/src/lib.rs

@@ -25,6 +25,9 @@ pub use interface_name::*;
 mod member_name;
 pub use member_name::*;
 
+mod property_name;
+pub use property_name::*;
+
 mod error;
 pub use error::*;
 

zbus_names/src/property_name.rs

@@ -0,0 +1,301 @@
+use crate::{
+  utils::{impl_str_basic, impl_try_from},
+  Error, Result,
+};
+use serde::{de, Deserialize, Serialize};
+use static_assertions::assert_impl_all;
+use std::{
+  borrow::{Borrow, Cow},
+  fmt::{self, Debug, Display, Formatter},
+  ops::Deref,
+  sync::Arc,
+};
+use zvariant::{NoneValue, OwnedValue, Str, Type, Value};
+
+/// String that identifies a property name on the bus.
+///
+/// # Examples
+///
+/// ```
+/// use zbus_names::PropertyName;
+///
+/// // Valid property names.
+/// let name = PropertyName::try_from("Property_for_you").unwrap();
+/// assert_eq!(name, "Property_for_you");
+/// let name = PropertyName::try_from("CamelCase101").unwrap();
+/// assert_eq!(name, "CamelCase101");
+/// let name = PropertyName::try_from("a_very_loooooooooooooooooo_ooooooo_0000o0ngName").unwrap();
+/// assert_eq!(name, "a_very_loooooooooooooooooo_ooooooo_0000o0ngName");
+/// let name = PropertyName::try_from("Property_for_you-1").unwrap();
+/// assert_eq!(name, "Property_for_you-1");
+/// ```
+#[derive(
+  Clone, Debug, Hash, PartialEq, Eq, Serialize, Type, Value, PartialOrd, Ord, OwnedValue,
+)]
+pub struct PropertyName<'name>(Str<'name>);
+
+assert_impl_all!(PropertyName<'_>: Send, Sync, Unpin);
+
+impl_str_basic!(PropertyName<'_>);
+
+impl<'name> PropertyName<'name> {
+  /// This is faster than `Clone::clone` when `self` contains owned data.
+  pub fn as_ref(&self) -> PropertyName<'_> {
+      PropertyName(self.0.as_ref())
+  }
+
+  /// The property name as string.
+  pub fn as_str(&self) -> &str {
+      self.0.as_str()
+  }
+
+  /// Create a new `PropertyName` from the given string.
+  ///
+  /// Since the passed string is not checked for correctness, prefer using the
+  /// `TryFrom<&str>` implementation.
+  pub fn from_str_unchecked(name: &'name str) -> Self {
+      Self(Str::from(name))
+  }
+
+  /// Same as `try_from`, except it takes a `&'static str`.
+  pub fn from_static_str(name: &'static str) -> Result<Self> {
+      ensure_correct_property_name(name)?;
+      Ok(Self(Str::from_static(name)))
+  }
+
+  /// Same as `from_str_unchecked`, except it takes a `&'static str`.
+  pub const fn from_static_str_unchecked(name: &'static str) -> Self {
+      Self(Str::from_static(name))
+  }
+
+  /// Same as `from_str_unchecked`, except it takes an owned `String`.
+  ///
+  /// Since the passed string is not checked for correctness, prefer using the
+  /// `TryFrom<String>` implementation.
+  pub fn from_string_unchecked(name: String) -> Self {
+      Self(Str::from(name))
+  }
+
+  /// Creates an owned clone of `self`.
+  pub fn to_owned(&self) -> PropertyName<'static> {
+      PropertyName(self.0.to_owned())
+  }
+
+  /// Creates an owned clone of `self`.
+  pub fn into_owned(self) -> PropertyName<'static> {
+      PropertyName(self.0.into_owned())
+  }
+}
+
+impl Deref for PropertyName<'_> {
+  type Target = str;
+
+  fn deref(&self) -> &Self::Target {
+      self.as_str()
+  }
+}
+
+impl Borrow<str> for PropertyName<'_> {
+  fn borrow(&self) -> &str {
+      self.as_str()
+  }
+}
+
+impl Display for PropertyName<'_> {
+  fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+      Display::fmt(&self.as_str(), f)
+  }
+}
+
+impl PartialEq<str> for PropertyName<'_> {
+  fn eq(&self, other: &str) -> bool {
+      self.as_str() == other
+  }
+}
+
+impl PartialEq<&str> for PropertyName<'_> {
+  fn eq(&self, other: &&str) -> bool {
+      self.as_str() == *other
+  }
+}
+
+impl PartialEq<OwnedPropertyName> for PropertyName<'_> {
+  fn eq(&self, other: &OwnedPropertyName) -> bool {
+      *self == other.0
+  }
+}
+
+impl<'de: 'name, 'name> Deserialize<'de> for PropertyName<'name> {
+  fn deserialize<D>(deserializer: D) -> core::result::Result<Self, D::Error>
+  where
+      D: serde::Deserializer<'de>,
+  {
+      let name = <Cow<'name, str>>::deserialize(deserializer)?;
+
+      Self::try_from(name).map_err(|e| de::Error::custom(e.to_string()))
+  }
+}
+
+impl<'name> From<PropertyName<'name>> for Str<'name> {
+  fn from(value: PropertyName<'name>) -> Self {
+      value.0
+  }
+}
+
+impl_try_from! {
+  ty: PropertyName<'s>,
+  owned_ty: OwnedPropertyName,
+  validate_fn: ensure_correct_property_name,
+  try_from: [&'s str, String, Arc<str>, Cow<'s, str>, Str<'s>],
+}
+
+fn ensure_correct_property_name(name: &str) -> Result<()> {
+  // Rules
+  //
+  // * Only ASCII alphanumeric or `_`.
+  // * Must not begin with a digit.
+  // * Must contain at least 1 character.
+  // * <= 255 characters.
+  if name.is_empty() {
+      return Err(Error::InvalidPropertyName(format!(
+          "`{}` is {} characters long, which is smaller than minimum allowed (1)",
+          name,
+          name.len(),
+      )));
+  } else if name.len() > 255 {
+      return Err(Error::InvalidPropertyName(format!(
+          "`{}` is {} characters long, which is longer than maximum allowed (255)",
+          name,
+          name.len(),
+      )));
+  }
+
+  Ok(())
+}
+
+/// This never succeeds but is provided so it's easier to pass `Option::None` values for API
+/// requiring `Option<TryInto<impl BusName>>`, since type inference won't work here.
+impl TryFrom<()> for PropertyName<'_> {
+  type Error = Error;
+
+  fn try_from(_value: ()) -> Result<Self> {
+      unreachable!("Conversion from `()` is not meant to actually work");
+  }
+}
+
+impl<'name> From<&PropertyName<'name>> for PropertyName<'name> {
+  fn from(name: &PropertyName<'name>) -> Self {
+      name.clone()
+  }
+}
+
+impl<'name> NoneValue for PropertyName<'name> {
+  type NoneType = &'name str;
+
+  fn null_value() -> Self::NoneType {
+      <&str>::default()
+  }
+}
+
+/// Owned sibling of [`PropertyName`].
+#[derive(Clone, Hash, PartialEq, Eq, Serialize, Type, Value, PartialOrd, Ord, OwnedValue)]
+pub struct OwnedPropertyName(#[serde(borrow)] PropertyName<'static>);
+
+assert_impl_all!(OwnedPropertyName: Send, Sync, Unpin);
+
+impl_str_basic!(OwnedPropertyName);
+
+impl OwnedPropertyName {
+  /// Convert to the inner `PropertyName`, consuming `self`.
+  pub fn into_inner(self) -> PropertyName<'static> {
+      self.0
+  }
+
+  /// Get a reference to the inner `PropertyName`.
+  pub fn inner(&self) -> &PropertyName<'static> {
+      &self.0
+  }
+}
+
+impl Deref for OwnedPropertyName {
+  type Target = PropertyName<'static>;
+
+  fn deref(&self) -> &Self::Target {
+      &self.0
+  }
+}
+
+impl Borrow<str> for OwnedPropertyName {
+  fn borrow(&self) -> &str {
+      self.0.as_str()
+  }
+}
+
+impl From<OwnedPropertyName> for PropertyName<'static> {
+  fn from(o: OwnedPropertyName) -> Self {
+      o.into_inner()
+  }
+}
+
+impl<'unowned, 'owned: 'unowned> From<&'owned OwnedPropertyName> for PropertyName<'unowned> {
+  fn from(name: &'owned OwnedPropertyName) -> Self {
+      PropertyName::from_str_unchecked(name.as_str())
+  }
+}
+
+impl From<PropertyName<'_>> for OwnedPropertyName {
+  fn from(name: PropertyName<'_>) -> Self {
+      OwnedPropertyName(name.into_owned())
+  }
+}
+
+impl From<OwnedPropertyName> for Str<'static> {
+  fn from(value: OwnedPropertyName) -> Self {
+      value.into_inner().0
+  }
+}
+
+impl<'de> Deserialize<'de> for OwnedPropertyName {
+  fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
+  where
+      D: de::Deserializer<'de>,
+  {
+      String::deserialize(deserializer)
+          .and_then(|n| PropertyName::try_from(n).map_err(|e| de::Error::custom(e.to_string())))
+          .map(Self)
+  }
+}
+
+impl PartialEq<&str> for OwnedPropertyName {
+  fn eq(&self, other: &&str) -> bool {
+      self.as_str() == *other
+  }
+}
+
+impl PartialEq<PropertyName<'_>> for OwnedPropertyName {
+  fn eq(&self, other: &PropertyName<'_>) -> bool {
+      self.0 == *other
+  }
+}
+
+impl Debug for OwnedPropertyName {
+  fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+      f.debug_tuple("OwnedPropertyName")
+          .field(&self.as_str())
+          .finish()
+  }
+}
+
+impl Display for OwnedPropertyName {
+  fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+      Display::fmt(&PropertyName::from(self), f)
+  }
+}
+
+impl NoneValue for OwnedPropertyName {
+  type NoneType = <PropertyName<'static> as NoneValue>::NoneType;
+
+  fn null_value() -> Self::NoneType {
+      PropertyName::null_value()
+  }
+}

zbus_xml/src/lib.rs

@@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize};
 use static_assertions::assert_impl_all;
 use std::io::{BufReader, Read, Write};
 
-use zbus_names::{InterfaceName, MemberName};
+use zbus_names::{InterfaceName, MemberName, PropertyName};
 use zvariant::CompleteType;
 
 /// Annotations are generic key/value pairs of metadata.
@@ -176,7 +176,7 @@ impl PropertyAccess {
 #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)]
 pub struct Property<'a> {
     #[serde(rename = "@name", borrow)]
-    name: MemberName<'a>,
+    name: PropertyName<'a>,
 
     #[serde(rename = "@type")]
     ty: CompleteType<'a>,
@@ -191,7 +191,7 @@ assert_impl_all!(Property<'_>: Send, Sync, Unpin);
 
 impl<'a> Property<'a> {
     /// Returns the property name.
-    pub fn name(&self) -> MemberName<'_> {
+    pub fn name(&self) -> PropertyName<'_> {
         self.name.as_ref()
     }
 

zbus_xml/tests/data/sample_object0.xml

@@ -24,4 +24,5 @@
     </interface>
     <node name="child_of_sample_object"/>
     <node name="another_child_of_sample_object"/>
+    <node name="another_child_of_sample_object-with-dashes"/>
 </node>

zbus_xml/tests/tests.rs

@@ -17,11 +17,11 @@ fn serde() -> Result<(), Box<dyn Error>> {
             .unwrap(),
         ArgDirection::In
     );
-    assert_eq!(node.nodes().len(), 3);
+    assert_eq!(node.nodes().len(), 4);
 
     let node_str: Node<'_> = example.try_into()?;
     assert_eq!(node_str.interfaces().len(), 1);
-    assert_eq!(node_str.nodes().len(), 3);
+    assert_eq!(node_str.nodes().len(), 4);
 
     let mut writer = Vec::with_capacity(128);
     node.to_writer(&mut writer).unwrap();