minidom: loosen type requirement on Element::attr{,_ns} getters

pep created

Signed-off-by: pep <pep@bouah.net>

Change summary

jid/src/lib.rs             | 39 +++++++--------------------------------
minidom/src/element.rs     | 34 ++++++++++++++++++++++------------
minidom/src/tests.rs       | 19 ++++++-------------
parsers/src/util/macros.rs |  6 +++---
parsers/src/xhtml.rs       | 39 ++++++++++++++-------------------------
5 files changed, 52 insertions(+), 85 deletions(-)

Detailed changes

jid/src/lib.rs 🔗

@@ -1126,35 +1126,19 @@ mod tests {
     #[test]
     fn minidom() {
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b/c'/>".parse().unwrap();
-        let to: Jid = elem
-            .attr("from".try_into().unwrap())
-            .unwrap()
-            .parse()
-            .unwrap();
+        let to: Jid = elem.attr("from").unwrap().parse().unwrap();
         assert_eq!(to, Jid::from(FullJid::new("a@b/c").unwrap()));
 
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b'/>".parse().unwrap();
-        let to: Jid = elem
-            .attr("from".try_into().unwrap())
-            .unwrap()
-            .parse()
-            .unwrap();
+        let to: Jid = elem.attr("from").unwrap().parse().unwrap();
         assert_eq!(to, Jid::from(BareJid::new("a@b").unwrap()));
 
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b/c'/>".parse().unwrap();
-        let to: FullJid = elem
-            .attr("from".try_into().unwrap())
-            .unwrap()
-            .parse()
-            .unwrap();
+        let to: FullJid = elem.attr("from").unwrap().parse().unwrap();
         assert_eq!(to, FullJid::new("a@b/c").unwrap());
 
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b'/>".parse().unwrap();
-        let to: BareJid = elem
-            .attr("from".try_into().unwrap())
-            .unwrap()
-            .parse()
-            .unwrap();
+        let to: BareJid = elem.attr("from").unwrap().parse().unwrap();
         assert_eq!(to, BareJid::new("a@b").unwrap());
     }
 
@@ -1165,28 +1149,19 @@ mod tests {
         let elem = minidom::Element::builder("message", "jabber:client")
             .attr("from".try_into().unwrap(), full.clone())
             .build();
-        assert_eq!(
-            elem.attr("from".try_into().unwrap()),
-            Some(full.to_string().as_str())
-        );
+        assert_eq!(elem.attr("from"), Some(full.to_string().as_str()));
 
         let bare = BareJid::new("a@b").unwrap();
         let elem = minidom::Element::builder("message", "jabber:client")
             .attr("from".try_into().unwrap(), bare.clone())
             .build();
-        assert_eq!(
-            elem.attr("from".try_into().unwrap()),
-            Some(bare.to_string().as_str())
-        );
+        assert_eq!(elem.attr("from"), Some(bare.to_string().as_str()));
 
         let jid = Jid::from(bare.clone());
         let _elem = minidom::Element::builder("message", "jabber:client")
             .attr("from".try_into().unwrap(), jid)
             .build();
-        assert_eq!(
-            elem.attr("from".try_into().unwrap()),
-            Some(bare.to_string().as_str())
-        );
+        assert_eq!(elem.attr("from"), Some(bare.to_string().as_str()));
     }
 
     #[test]

minidom/src/element.rs 🔗

@@ -26,10 +26,12 @@ use alloc::vec::Vec;
 use core::slice;
 use core::str::FromStr;
 
+use std::borrow::Borrow;
+use std::hash::Hash;
 use std::io;
 
 use rxml::writer::{Encoder, Item, TrackNamespace};
-use rxml::{AttrMap, Namespace as RxmlNamespace, NcName, NcNameStr, RawReader, XmlVersion};
+use rxml::{AttrMap, Namespace as RxmlNamespace, NcName, RawReader, XmlVersion};
 
 fn encode_and_write<W: io::Write, T: rxml::writer::TrackNamespace>(
     item: Item<'_>,
@@ -188,8 +190,8 @@ impl Element {
     ///
     /// assert_eq!(elem.name(), "name");
     /// assert_eq!(elem.ns(), "namespace".to_owned());
-    /// assert_eq!(elem.attr(xml_ncname!("name")), Some("value"));
-    /// assert_eq!(elem.attr(xml_ncname!("inexistent")), None);
+    /// assert_eq!(elem.attr("name"), Some("value"));
+    /// assert_eq!(elem.attr("inexistent"), None);
     /// assert_eq!(elem.text(), "inner");
     /// ```
     pub fn builder<S: AsRef<str>, NS: Into<String>>(name: S, namespace: NS) -> ElementBuilder {
@@ -210,13 +212,13 @@ impl Element {
     ///
     /// ```rust
     /// use minidom::Element;
-    /// use rxml::{Namespace, xml_ncname};
+    /// use rxml::Namespace;
     ///
     /// let bare = Element::bare("name", "namespace");
     ///
     /// assert_eq!(bare.name(), "name");
     /// assert_eq!(bare.ns(), "namespace");
-    /// assert_eq!(bare.attr(xml_ncname!("name")), None);
+    /// assert_eq!(bare.attr("name"), None);
     /// assert_eq!(bare.text(), "");
     /// ```
     pub fn bare<S: Into<String>, NS: Into<String>>(name: S, namespace: NS) -> Element {
@@ -243,7 +245,10 @@ impl Element {
 
     /// Returns a reference to the value of the given attribute, if it exists, else `None`.
     #[must_use]
-    pub fn attr<'a>(&'a self, name: &'a NcNameStr) -> Option<&'a str> {
+    pub fn attr<'a, N: Ord + Hash + Eq + ?Sized>(&'a self, name: &'a N) -> Option<&'a str>
+    where
+        NcName: Borrow<N>,
+    {
         if let Some(value) = self.attributes.get(&RxmlNamespace::NONE, name) {
             return Some(value);
         }
@@ -252,7 +257,15 @@ impl Element {
 
     /// Returns a reference to the value of the given namespaced attribute, if it exists, else `None`.
     #[must_use]
-    pub fn attr_ns<'a>(&'a self, ns: &'a RxmlNamespace, name: &'a NcNameStr) -> Option<&'a str> {
+    pub fn attr_ns<'a, NS: Ord + Hash + Eq + ?Sized, N: Ord + Hash + Eq + ?Sized>(
+        &'a self,
+        ns: &'a NS,
+        name: &'a N,
+    ) -> Option<&'a str>
+    where
+        RxmlNamespace: Borrow<NS>,
+        NcName: Borrow<N>,
+    {
         if let Some(value) = self.attributes.get(ns, name) {
             return Some(value);
         }
@@ -953,11 +966,8 @@ mod tests {
 
         assert_eq!(elem.name(), "name");
         assert_eq!(elem.ns(), "namespace".to_owned());
-        assert_eq!(
-            elem.attr_ns(&String::from("namespace").into(), xml_ncname!("name")),
-            Some("value")
-        );
-        assert_eq!(elem.attr(xml_ncname!("inexistent")), None);
+        assert_eq!(elem.attr_ns("namespace", "name"), Some("value"));
+        assert_eq!(elem.attr("inexistent"), None);
     }
 
     #[test]

minidom/src/tests.rs 🔗

@@ -281,8 +281,8 @@ fn builder_works() {
         .build();
     assert_eq!(elem.name(), "a");
     assert_eq!(elem.ns(), "b".to_owned());
-    assert_eq!(elem.attr(xml_ncname!("c")), Some("d"));
-    assert_eq!(elem.attr(xml_ncname!("x")), None);
+    assert_eq!(elem.attr("c"), Some("d"));
+    assert_eq!(elem.attr("x"), None);
     assert_eq!(elem.text(), "e");
     assert!(elem.has_child("child", "b"));
     assert!(elem.is("a", "b"));
@@ -311,15 +311,11 @@ fn get_child_works() {
         .unwrap()
         .is("child", "child_ns"));
     assert_eq!(
-        root.get_child("child", "root_ns")
-            .unwrap()
-            .attr(xml_ncname!("c")),
+        root.get_child("child", "root_ns").unwrap().attr("c"),
         Some("d")
     );
     assert_eq!(
-        root.get_child("child", "child_ns")
-            .unwrap()
-            .attr(xml_ncname!("d")),
+        root.get_child("child", "child_ns").unwrap().attr("d"),
         Some("e")
     );
 }
@@ -357,15 +353,12 @@ fn two_elements_with_same_arguments_different_order_are_equal() {
 #[test]
 fn namespace_attributes_works() {
     let root = Element::from_reader(TEST_STRING).unwrap();
-    assert_eq!(
-        Some("en"),
-        root.attr_ns(RxmlNamespace::xml(), xml_ncname!("lang"))
-    );
+    assert_eq!(Some("en"), root.attr_ns(RxmlNamespace::xml(), "lang"));
     assert_eq!(
         "fr",
         root.get_child("child", "child_ns")
             .unwrap()
-            .attr_ns(RxmlNamespace::xml(), xml_ncname!("lang"))
+            .attr_ns(RxmlNamespace::xml(), "lang")
             .unwrap()
     );
 }

parsers/src/util/macros.rs 🔗

@@ -15,13 +15,13 @@ macro_rules! get_attr {
         )
     };
     ($elem:ident, $attr:tt, Option, $value:ident, $func:expr) => {
-        match $elem.attr(::xso::exports::rxml::xml_ncname!($attr).into()) {
+        match $elem.attr($attr) {
             Some($value) => Some($func),
             None => None,
         }
     };
     ($elem:ident, $attr:tt, Required, $value:ident, $func:expr) => {
-        match $elem.attr(::xso::exports::rxml::xml_ncname!($attr).into()) {
+        match $elem.attr($attr) {
             Some($value) => $func,
             None => {
                 return Err(xso::error::Error::Other(
@@ -32,7 +32,7 @@ macro_rules! get_attr {
         }
     };
     ($elem:ident, $attr:tt, Default, $value:ident, $func:expr) => {
-        match $elem.attr(::xso::exports::rxml::xml_ncname!($attr).into()) {
+        match $elem.attr($attr) {
             Some($value) => $func,
             None => ::core::default::Default::default(),
         }

parsers/src/xhtml.rs 🔗

@@ -77,7 +77,7 @@ impl TryFrom<Element> for XhtmlIm {
             if child.is("body", ns::XHTML) {
                 let child = child.clone();
                 let lang = child
-                    .attr_ns(rxml::Namespace::xml(), rxml::xml_ncname!("lang").into())
+                    .attr_ns(rxml::Namespace::xml(), "lang")
                     .unwrap_or("")
                     .to_string();
                 let body = Body::try_from(child)?;
@@ -168,12 +168,9 @@ impl TryFrom<Element> for Body {
         }
 
         Ok(Body {
-            style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+            style: parse_css(elem.attr("style")),
             xml_lang: elem
-                .attr_ns(
-                    &Into::<Namespace>::into(String::from("xml")),
-                    rxml::xml_ncname!("lang").into(),
-                )
+                .attr_ns("xml", "lang")
                 .map(|xml_lang| xml_lang.to_string()),
             children,
         })
@@ -328,52 +325,44 @@ impl TryFrom<Element> for Tag {
 
         Ok(match elem.name() {
             "a" => Tag::A {
-                href: elem
-                    .attr(rxml::xml_ncname!("href"))
-                    .map(|href| href.to_string()),
+                href: elem.attr("href").map(|href| href.to_string()),
                 style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
-                type_: elem
-                    .attr(rxml::xml_ncname!("type"))
-                    .map(|type_| type_.to_string()),
+                type_: elem.attr("type").map(|type_| type_.to_string()),
                 children,
             },
             "blockquote" => Tag::Blockquote {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             "br" => Tag::Br,
             "cite" => Tag::Cite {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             "em" => Tag::Em { children },
             "img" => Tag::Img {
-                src: elem
-                    .attr(rxml::xml_ncname!("src"))
-                    .map(|src| src.to_string()),
-                alt: elem
-                    .attr(rxml::xml_ncname!("alt"))
-                    .map(|alt| alt.to_string()),
+                src: elem.attr("src").map(|src| src.to_string()),
+                alt: elem.attr("alt").map(|alt| alt.to_string()),
             },
             "li" => Tag::Li {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             "ol" => Tag::Ol {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             "p" => Tag::P {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             "span" => Tag::Span {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             "strong" => Tag::Strong { children },
             "ul" => Tag::Ul {
-                style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
+                style: parse_css(elem.attr("style")),
                 children,
             },
             _ => Tag::Unknown(children),