Merge pull request #760 from Mingun/in-mind-of-write

Mingun · web-flow · commit 46742446fe6e · 2024-06-23T19:53:08.000+05:00
Remove requirement to have Reader when decoding attributes and write anything converted to Event
diff --git a/Changelog.md b/Changelog.md
@@ -19,6 +19,12 @@
 
 ### Misc Changes
 
+- [#760]: `Attribute::decode_and_unescape_value` and `Attribute::decode_and_unescape_value_with` now
+  accepts `Decoder` instead of `Reader`. Use `Reader::decoder()` to get it.
+- [#760]: `Writer::write_event` now consumes event. Use `Event::borrow()` if you want to keep ownership.
+
+[#760]: https://github.com/tafia/quick-xml/pull/760
+
 
 ## 0.33.0 -- 2024-06-21
 
diff --git a/benches/macrobenches.rs b/benches/macrobenches.rs
@@ -50,7 +50,7 @@ fn parse_document_from_str(doc: &str) -> XmlResult<()> {
         match criterion::black_box(r.read_event()?) {
             Event::Start(e) | Event::Empty(e) => {
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -75,7 +75,7 @@ fn parse_document_from_bytes(doc: &[u8]) -> XmlResult<()> {
         match criterion::black_box(r.read_event_into(&mut buf)?) {
             Event::Start(e) | Event::Empty(e) => {
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -101,7 +101,7 @@ fn parse_document_from_str_with_namespaces(doc: &str) -> XmlResult<()> {
             (resolved_ns, Event::Start(e) | Event::Empty(e)) => {
                 criterion::black_box(resolved_ns);
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {
@@ -129,7 +129,7 @@ fn parse_document_from_bytes_with_namespaces(doc: &[u8]) -> XmlResult<()> {
             (resolved_ns, Event::Start(e) | Event::Empty(e)) => {
                 criterion::black_box(resolved_ns);
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {
diff --git a/examples/custom_entities.rs b/examples/custom_entities.rs
@@ -47,7 +47,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                         .attributes()
                         .map(|a| {
                             a.unwrap()
-                                .decode_and_unescape_value_with(&reader, |ent| {
+                                .decode_and_unescape_value_with(reader.decoder(), |ent| {
                                     custom_entities.get(ent).map(|s| s.as_str())
                                 })
                                 .unwrap()
diff --git a/examples/read_nodes.rs b/examples/read_nodes.rs
@@ -70,8 +70,8 @@ impl Translation {
         for attr_result in element.attributes() {
             let a = attr_result?;
             match a.key.as_ref() {
-                b"Language" => lang = a.decode_and_unescape_value(reader)?,
-                b"Tag" => tag = a.decode_and_unescape_value(reader)?,
+                b"Language" => lang = a.decode_and_unescape_value(reader.decoder())?,
+                b"Tag" => tag = a.decode_and_unescape_value(reader.decoder())?,
                 _ => (),
             }
         }
@@ -138,7 +138,7 @@ fn main() -> Result<(), AppError> {
                                             Ok::<Cow<'_, str>, Infallible>(std::borrow::Cow::from(""))
                                         })
                                         .unwrap().to_string();
-                                    let value = a.decode_and_unescape_value(&reader).or_else(|err| {
+                                    let value = a.decode_and_unescape_value(reader.decoder()).or_else(|err| {
                                             dbg!("unable to read key in DefaultSettings attribute {:?}, utf8 error {:?}", &a, err);
                                             Ok::<Cow<'_, str>, Infallible>(std::borrow::Cow::from(""))
                                     }).unwrap().to_string();
diff --git a/fuzz/fuzz_targets/fuzz_target_1.rs b/fuzz/fuzz_targets/fuzz_target_1.rs
@@ -26,7 +26,7 @@ where
             let _event = black_box(event.borrow());
             let _event = black_box(event.as_ref());
             debug_format!(event);
-            debug_format!(writer.write_event(event));
+            debug_format!(writer.write_event(event.borrow()));
         }
         match event_result {
             Ok(Event::Start(ref e)) | Ok(Event::Empty(ref e)) => {
diff --git a/fuzz/fuzz_targets/structured_roundtrip.rs b/fuzz/fuzz_targets/structured_roundtrip.rs
@@ -51,7 +51,7 @@ fn fuzz_round_trip(driver: Driver) -> quick_xml::Result<()> {
         // TODO: Handle error cases.
         use WriterFunc::*;
         match writer_func {
-            WriteEvent(event) => writer.write_event(event)?,
+            WriteEvent(event) => writer.write_event(event.borrow())?,
             WriteBom => writer.write_bom()?,
             WriteIndent => writer.write_indent()?,
             CreateElement {
diff --git a/src/events/attributes.rs b/src/events/attributes.rs
@@ -2,11 +2,12 @@
 //!
 //! Provides an iterator over attributes key/value pairs
 
+use crate::encoding::Decoder;
 use crate::errors::Result as XmlResult;
 use crate::escape::{escape, resolve_predefined_entity, unescape_with};
 use crate::name::QName;
-use crate::reader::Reader;
 use crate::utils::{is_whitespace, write_byte_string, write_cow_string, Bytes};
+
 use std::fmt::{self, Debug, Display, Formatter};
 use std::iter::FusedIterator;
 use std::{borrow::Cow, ops::Range};
@@ -84,23 +85,23 @@ impl<'a> Attribute<'a> {
     ///
     /// This will allocate if the value contains any escape sequences or in
     /// non-UTF-8 encoding.
-    pub fn decode_and_unescape_value<B>(&self, reader: &Reader<B>) -> XmlResult<Cow<'a, str>> {
-        self.decode_and_unescape_value_with(reader, resolve_predefined_entity)
+    pub fn decode_and_unescape_value(&self, decoder: Decoder) -> XmlResult<Cow<'a, str>> {
+        self.decode_and_unescape_value_with(decoder, resolve_predefined_entity)
     }
 
     /// Decodes then unescapes the value with custom entities.
     ///
     /// This will allocate if the value contains any escape sequences or in
     /// non-UTF-8 encoding.
-    pub fn decode_and_unescape_value_with<'entity, B>(
+    pub fn decode_and_unescape_value_with<'entity>(
         &self,
-        reader: &Reader<B>,
+        decoder: Decoder,
         resolve_entity: impl FnMut(&str) -> Option<&'entity str>,
     ) -> XmlResult<Cow<'a, str>> {
         let decoded = match &self.value {
-            Cow::Borrowed(bytes) => reader.decoder().decode(bytes)?,
+            Cow::Borrowed(bytes) => decoder.decode(bytes)?,
             // Convert to owned, because otherwise Cow will be bound with wrong lifetime
-            Cow::Owned(bytes) => reader.decoder().decode(bytes)?.into_owned().into(),
+            Cow::Owned(bytes) => decoder.decode(bytes)?.into_owned().into(),
         };
 
         match unescape_with(&decoded, resolve_entity)? {
diff --git a/src/reader/ns_reader.rs b/src/reader/ns_reader.rs
@@ -337,7 +337,6 @@ impl<R> NsReader<R> {
     /// ```
     /// # use pretty_assertions::assert_eq;
     /// use quick_xml::events::Event;
-    /// use quick_xml::events::attributes::Attribute;
     /// use quick_xml::name::{Namespace, QName, ResolveResult::*};
     /// use quick_xml::reader::NsReader;
     ///
diff --git a/src/writer.rs b/src/writer.rs
@@ -51,7 +51,7 @@ use {crate::de::DeError, serde::Serialize};
 ///         },
 ///         Ok(Event::Eof) => break,
 ///         // we can either move or borrow the event to write, depending on your use-case
-///         Ok(e) => assert!(writer.write_event(e).is_ok()),
+///         Ok(e) => assert!(writer.write_event(e.borrow()).is_ok()),
 ///         Err(e) => panic!("Error at position {}: {:?}", reader.buffer_position(), e),
 ///     }
 /// }
@@ -192,37 +192,37 @@ impl<W: Write> Writer<W> {
     }
 
     /// Writes the given event to the underlying writer.
-    pub fn write_event<'a, E: AsRef<Event<'a>>>(&mut self, event: E) -> Result<()> {
+    pub fn write_event<'a, E: Into<Event<'a>>>(&mut self, event: E) -> Result<()> {
         let mut next_should_line_break = true;
-        let result = match *event.as_ref() {
-            Event::Start(ref e) => {
-                let result = self.write_wrapped(b"<", e, b">");
+        let result = match event.into() {
+            Event::Start(e) => {
+                let result = self.write_wrapped(b"<", &e, b">");
                 if let Some(i) = self.indent.as_mut() {
                     i.grow();
                 }
                 result
             }
-            Event::End(ref e) => {
+            Event::End(e) => {
                 if let Some(i) = self.indent.as_mut() {
                     i.shrink();
                 }
-                self.write_wrapped(b"</", e, b">")
+                self.write_wrapped(b"</", &e, b">")
             }
-            Event::Empty(ref e) => self.write_wrapped(b"<", e, b"/>"),
-            Event::Text(ref e) => {
+            Event::Empty(e) => self.write_wrapped(b"<", &e, b"/>"),
+            Event::Text(e) => {
                 next_should_line_break = false;
-                self.write(e)
+                self.write(&e)
             }
-            Event::Comment(ref e) => self.write_wrapped(b"<!--", e, b"-->"),
-            Event::CData(ref e) => {
+            Event::Comment(e) => self.write_wrapped(b"<!--", &e, b"-->"),
+            Event::CData(e) => {
                 next_should_line_break = false;
                 self.write(b"<![CDATA[")?;
-                self.write(e)?;
+                self.write(&e)?;
                 self.write(b"]]>")
             }
-            Event::Decl(ref e) => self.write_wrapped(b"<?", e, b"?>"),
-            Event::PI(ref e) => self.write_wrapped(b"<?", e, b"?>"),
-            Event::DocType(ref e) => self.write_wrapped(b"<!DOCTYPE ", e, b">"),
+            Event::Decl(e) => self.write_wrapped(b"<?", &e, b"?>"),
+            Event::PI(e) => self.write_wrapped(b"<?", &e, b"?>"),
+            Event::DocType(e) => self.write_wrapped(b"<!DOCTYPE ", &e, b">"),
             Event::Eof => Ok(()),
         };
         if let Some(i) = self.indent.as_mut() {
diff --git a/src/writer/async_tokio.rs b/src/writer/async_tokio.rs
@@ -9,37 +9,37 @@ use crate::{ElementWriter, Writer};
 
 impl<W: AsyncWrite + Unpin> Writer<W> {
     /// Writes the given event to the underlying writer. Async version of [`Writer::write_event`].
-    pub async fn write_event_async<'a, E: AsRef<Event<'a>>>(&mut self, event: E) -> Result<()> {
+    pub async fn write_event_async<'a, E: Into<Event<'a>>>(&mut self, event: E) -> Result<()> {
         let mut next_should_line_break = true;
-        let result = match *event.as_ref() {
-            Event::Start(ref e) => {
-                let result = self.write_wrapped_async(b"<", e, b">").await;
+        let result = match event.into() {
+            Event::Start(e) => {
+                let result = self.write_wrapped_async(b"<", &e, b">").await;
                 if let Some(i) = self.indent.as_mut() {
                     i.grow();
                 }
                 result
             }
-            Event::End(ref e) => {
+            Event::End(e) => {
                 if let Some(i) = self.indent.as_mut() {
                     i.shrink();
                 }
-                self.write_wrapped_async(b"</", e, b">").await
+                self.write_wrapped_async(b"</", &e, b">").await
             }
-            Event::Empty(ref e) => self.write_wrapped_async(b"<", e, b"/>").await,
-            Event::Text(ref e) => {
+            Event::Empty(e) => self.write_wrapped_async(b"<", &e, b"/>").await,
+            Event::Text(e) => {
                 next_should_line_break = false;
-                self.write_async(e).await
+                self.write_async(&e).await
             }
-            Event::Comment(ref e) => self.write_wrapped_async(b"<!--", e, b"-->").await,
-            Event::CData(ref e) => {
+            Event::Comment(e) => self.write_wrapped_async(b"<!--", &e, b"-->").await,
+            Event::CData(e) => {
                 next_should_line_break = false;
                 self.write_async(b"<![CDATA[").await?;
-                self.write_async(e).await?;
+                self.write_async(&e).await?;
                 self.write_async(b"]]>").await
             }
-            Event::Decl(ref e) => self.write_wrapped_async(b"<?", e, b"?>").await,
-            Event::PI(ref e) => self.write_wrapped_async(b"<?", e, b"?>").await,
-            Event::DocType(ref e) => self.write_wrapped_async(b"<!DOCTYPE ", e, b">").await,
+            Event::Decl(e) => self.write_wrapped_async(b"<?", &e, b"?>").await,
+            Event::PI(e) => self.write_wrapped_async(b"<?", &e, b"?>").await,
+            Event::DocType(e) => self.write_wrapped_async(b"<!DOCTYPE ", &e, b">").await,
             Event::Eof => Ok(()),
         };
         if let Some(i) = self.indent.as_mut() {
diff --git a/tests/fuzzing.rs b/tests/fuzzing.rs
@@ -30,9 +30,9 @@ fn fuzz_101() {
         match reader.read_event_into(&mut buf) {
             Ok(Event::Start(e)) | Ok(Event::Empty(e)) => {
                 for a in e.attributes() {
-                    if a.ok()
-                        .map_or(true, |a| a.decode_and_unescape_value(&reader).is_err())
-                    {
+                    if a.ok().map_or(true, |a| {
+                        a.decode_and_unescape_value(reader.decoder()).is_err()
+                    }) {
                         break;
                     }
                 }
diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs
@@ -180,7 +180,7 @@ fn with_trim_ref() {
     loop {
         match reader.read_event().unwrap() {
             Eof => break,
-            e => assert!(writer.write_event(&e).is_ok()), // either `e` or `&e`
+            e => assert!(writer.write_event(e.borrow()).is_ok()), // either `e` or `&e`
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ fn parse_document_from_str(doc: &str) -> XmlResult<()> {`
`50`	`50`	`match criterion::black_box(r.read_event()?) {`
`51`	`51`	`Event::Start(e) \| Event::Empty(e) => {`
`52`	`52`	`for attr in e.attributes() {`
`53`		`- criterion::black_box(attr?.decode_and_unescape_value(&r)?);`
	`53`	`+ criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);`
`54`	`54`	`}`
`55`	`55`	`}`
`56`	`56`	`Event::Text(e) => {`
`@@ -75,7 +75,7 @@ fn parse_document_from_bytes(doc: &[u8]) -> XmlResult<()> {`
`75`	`75`	`match criterion::black_box(r.read_event_into(&mut buf)?) {`
`76`	`76`	`Event::Start(e) \| Event::Empty(e) => {`
`77`	`77`	`for attr in e.attributes() {`
`78`		`- criterion::black_box(attr?.decode_and_unescape_value(&r)?);`
	`78`	`+ criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);`
`79`	`79`	`}`
`80`	`80`	`}`
`81`	`81`	`Event::Text(e) => {`
`@@ -101,7 +101,7 @@ fn parse_document_from_str_with_namespaces(doc: &str) -> XmlResult<()> {`
`101`	`101`	`(resolved_ns, Event::Start(e) \| Event::Empty(e)) => {`
`102`	`102`	`criterion::black_box(resolved_ns);`
`103`	`103`	`for attr in e.attributes() {`
`104`		`- criterion::black_box(attr?.decode_and_unescape_value(&r)?);`
	`104`	`+ criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);`
`105`	`105`	`}`
`106`	`106`	`}`
`107`	`107`	`(resolved_ns, Event::Text(e)) => {`
`@@ -129,7 +129,7 @@ fn parse_document_from_bytes_with_namespaces(doc: &[u8]) -> XmlResult<()> {`
`129`	`129`	`(resolved_ns, Event::Start(e) \| Event::Empty(e)) => {`
`130`	`130`	`criterion::black_box(resolved_ns);`
`131`	`131`	`for attr in e.attributes() {`
`132`		`- criterion::black_box(attr?.decode_and_unescape_value(&r)?);`
	`132`	`+ criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);`
`133`	`133`	`}`
`134`	`134`	`}`
`135`	`135`	`(resolved_ns, Event::Text(e)) => {`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ where`
`26`	`26`	`let _event = black_box(event.borrow());`
`27`	`27`	`let _event = black_box(event.as_ref());`
`28`	`28`	`debug_format!(event);`
`29`		`- debug_format!(writer.write_event(event));`
	`29`	`+ debug_format!(writer.write_event(event.borrow()));`
`30`	`30`	`}`
`31`	`31`	`match event_result {`
`32`	`32`	`Ok(Event::Start(ref e)) \| Ok(Event::Empty(ref e)) => {`
Original file line number	Diff line number	Diff line change
`@@ -30,9 +30,9 @@ fn fuzz_101() {`
`30`	`30`	`match reader.read_event_into(&mut buf) {`
`31`	`31`	`Ok(Event::Start(e)) \| Ok(Event::Empty(e)) => {`
`32`	`32`	`for a in e.attributes() {`
`33`		`- if a.ok()`
`34`		`- .map_or(true, \|a\| a.decode_and_unescape_value(&reader).is_err())`
`35`		`- {`
	`33`	`+ if a.ok().map_or(true, \|a\| {`
	`34`	`+ a.decode_and_unescape_value(reader.decoder()).is_err()`
	`35`	`+ }) {`
`36`	`36`	`break;`
`37`	`37`	`}`
`38`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -180,7 +180,7 @@ fn with_trim_ref() {`
`180`	`180`	`loop {`
`181`	`181`	`match reader.read_event().unwrap() {`
`182`	`182`	`Eof => break,`
`183`		- e => assert!(writer.write_event(&e).is_ok()), // either `e` or `&e`
	`183`	+ e => assert!(writer.write_event(e.borrow()).is_ok()), // either `e` or `&e`
`184`	`184`	`}`
`185`	`185`	`}`
`186`	`186`