Use impl Trait as return type for next_string_reader and `string_…

…value_writer`

Resolves #18

src/reader/mod.rs

@@ -1230,6 +1230,7 @@ pub trait JsonReader {
     /// # Examples
     /// ```
     /// # use struson::reader::*;
+    /// # use std::io::Read;
     /// let mut json_reader = JsonStreamReader::new(r#"["hello"]"#.as_bytes());
     /// json_reader.begin_array()?;
     ///
@@ -1271,7 +1272,7 @@ pub trait JsonReader {
     /// when called after the top-level value has already been consumed and multiple top-level
     /// values are not enabled in the [`ReaderSettings`]. Both cases indicate incorrect
     /// usage by the user and are unrelated to the JSON data.
-    fn next_string_reader(&mut self) -> Result<Box<dyn Read + '_>, ReaderError>;
+    fn next_string_reader(&mut self) -> Result<impl Read + '_, ReaderError>;
 
     /// Consumes and returns a JSON number value
     ///

src/reader/stream_reader.rs

@@ -14,7 +14,7 @@ use super::json_path::json_path;
 use crate::{
     json_number::{consume_json_number, NumberBytesProvider},
     utf8,
-    writer::TransferredNumber,
+    writer::{StringValueWriter, TransferredNumber},
 };
 
 #[derive(PartialEq, Clone, Copy, strum::Display, Debug)]
@@ -2170,17 +2170,17 @@ impl<R: Read> JsonReader for JsonStreamReader<R> {
         Ok(str_bytes.get_str(self))
     }
 
-    fn next_string_reader(&mut self) -> Result<Box<dyn Read + '_>, ReaderError> {
+    fn next_string_reader(&mut self) -> Result<impl Read + '_, ReaderError> {
         self.start_expected_value_type(ValueType::String, false)?;
         self.is_string_value_reader_active = true;
-        Ok(Box::new(StringValueReader {
+        Ok(StringValueReader {
             json_reader: self,
             utf8_buf: [0; STRING_VALUE_READER_BUF_SIZE],
             utf8_start_pos: 0,
             utf8_count: 0,
             reached_end: false,
             error: None,
-        }))
+        })
     }
 
     fn next_number_as_string(&mut self) -> Result<String, ReaderError> {
@@ -2475,6 +2475,8 @@ impl<R: Read> Read for StringValueReader<'_, R> {
 
 #[cfg(test)]
 mod tests {
+    use std::io::Write;
+
     use super::*;
     use crate::writer::{
         FiniteNumber, FloatingPointNumber, JsonNumberError, JsonStreamWriter, StringValueWriter,
@@ -4348,12 +4350,32 @@ mod tests {
 
     #[test]
     fn transfer_to_writer_error() {
-        struct FailingJsonWriter;
-
         fn err() -> IoError {
             IoError::new(ErrorKind::Other, "test error")
         }
 
+        struct UnreachableStringValueWriter;
+        impl Write for UnreachableStringValueWriter {
+            fn write(&mut self, _: &[u8]) -> std::io::Result<usize> {
+                unreachable!()
+            }
+
+            fn flush(&mut self) -> std::io::Result<()> {
+                unreachable!()
+            }
+        }
+        impl StringValueWriter for UnreachableStringValueWriter {
+            fn write_str(&mut self, _: &str) -> Result<(), IoError> {
+                unreachable!()
+            }
+
+            fn finish_value(self) -> Result<(), IoError> {
+                unreachable!()
+            }
+        }
+
+        struct FailingJsonWriter;
+
         // JsonWriter which always returns Err(...)
         // Note: If maintaining this becomes too cumbersome when adjusting JsonWriter API, can remove this test
         impl JsonWriter for FailingJsonWriter {
@@ -4389,8 +4411,8 @@ mod tests {
                 Err(err())
             }
 
-            fn string_value_writer(&mut self) -> Result<Box<dyn StringValueWriter + '_>, IoError> {
-                Err(err())
+            fn string_value_writer(&mut self) -> Result<impl StringValueWriter + '_, IoError> {
+                Err::<UnreachableStringValueWriter, IoError>(err())
             }
 
             fn number_value_from_string(&mut self, _: &str) -> Result<(), JsonNumberError> {

src/writer/mod.rs

@@ -313,6 +313,7 @@ pub trait JsonWriter {
     /// # Examples
     /// ```
     /// # use struson::writer::*;
+    /// # use std::io::Write;
     /// let mut writer = Vec::<u8>::new();
     /// let mut json_writer = JsonStreamWriter::new(&mut writer);
     ///
@@ -343,13 +344,7 @@ pub trait JsonWriter {
     /// when called after the top-level value has already been written and multiple top-level
     /// values are not enabled in the [`WriterSettings`]. Both cases indicate incorrect
     /// usage by the user.
-    /*
-     * TODO: Instead of Box<dyn ...> should this directly declare struct as return type
-     *   (and not have separate trait StringValueWriter)?
-     *   But then users might not be able to implement JsonWriter themselves anymore easily;
-     *   would also be inconsistent with JsonReader::next_string_reader
-     */
-    fn string_value_writer(&mut self) -> Result<Box<dyn StringValueWriter + '_>, IoError>;
+    fn string_value_writer(&mut self) -> Result<impl StringValueWriter + '_, IoError>;
 
     /// Writes the string representation of a JSON number value
     ///
@@ -588,6 +583,7 @@ pub trait StringValueWriter: Write {
     /// # Examples
     /// ```
     /// # use struson::writer::*;
+    /// # use std::io::Write;
     /// let mut writer = Vec::<u8>::new();
     /// let mut json_writer = JsonStreamWriter::new(&mut writer);
     ///
@@ -618,7 +614,7 @@ pub trait StringValueWriter: Write {
     /// This method must be called when writing the string value is done to allow
     /// using the original JSON writer again.
     /* Consumes 'self' */
-    fn finish_value(self: Box<Self>) -> Result<(), IoError>;
+    fn finish_value(self) -> Result<(), IoError>;
 }
 
 /// Sealed trait for finite number types such as `u32`

src/writer/stream_writer.rs

@@ -522,17 +522,17 @@ impl<W: Write> JsonWriter for JsonStreamWriter<W> {
         self.flush()
     }
 
-    fn string_value_writer(&mut self) -> Result<Box<dyn StringValueWriter + '_>, IoError> {
+    fn string_value_writer(&mut self) -> Result<impl StringValueWriter + '_, IoError> {
         self.before_value()?;
         self.write_bytes(b"\"")?;
         self.is_string_value_writer_active = true;
-        Ok(Box::new(StringValueWriterImpl {
+        Ok(StringValueWriterImpl {
             json_writer: self,
             utf8_buf: [0; utf8::MAX_BYTES_PER_CHAR],
             utf8_pos: 0,
             utf8_expected_len: 0,
             error: None,
-        }))
+        })
     }
 }
 
@@ -768,7 +768,7 @@ impl<W: Write> StringValueWriter for StringValueWriterImpl<'_, W> {
         Ok(())
     }
 
-    fn finish_value(self: Box<Self>) -> Result<(), IoError> {
+    fn finish_value(self) -> Result<(), IoError> {
         self.check_previous_error()?;
 
         if self.utf8_pos > 0 {
@@ -1092,53 +1092,63 @@ mod tests {
 
     #[test]
     fn string_writer_invalid() {
-        fn assert_invalid_utf8<
-            T,
-            F: FnOnce(Box<dyn StringValueWriter + '_>) -> Result<T, IoError>,
-        >(
-            f: F,
-            expected_custom_message: Option<&str>,
-        ) {
-            let mut writer = Vec::<u8>::new();
-            let mut json_writer = JsonStreamWriter::new(&mut writer);
-
-            match f(json_writer.string_value_writer().unwrap()) {
-                Ok(_) => panic!("Should have failed"),
-                Err(e) => {
-                    assert_eq!(ErrorKind::InvalidData, e.kind());
-
-                    match expected_custom_message {
-                        // None if error message should not be compared, e.g. because it is created by Rust and might change
-                        None => assert!(
-                            e.get_ref().unwrap().is::<Utf8Error>(),
-                            "Inner error is not Utf8Error"
-                        ),
-                        Some(message) => {
-                            assert_eq!(message, e.to_string(), "Custom message does not match")
+        // Uses macro instead of function with FnOnce(Box<...>) as parameter to avoid issues with
+        // calling `StringValueWriter::finish_value` consuming `self`, see https://stackoverflow.com/q/46620790
+        macro_rules! assert_invalid_utf8 {
+            (
+                |$string_value_writer:ident| $writing_expr:expr,
+                $expected_custom_message:expr, // Option<&str>
+            ) => {
+                let mut writer = Vec::<u8>::new();
+                let mut json_writer = JsonStreamWriter::new(&mut writer);
+                let mut $string_value_writer = json_writer.string_value_writer().unwrap();
+
+                // Use a closure here to allow `$writing_expr` to use the `?` operator for error handling
+                #[allow(unused_mut)] // only for some callers the closure has to be mutable
+                let mut writing_function = || -> Result<(), IoError> {
+                    $writing_expr
+                };
+                // Explicitly specify expression type to avoid callers having to specify it
+                let expected_custom_message: Option<&str> = $expected_custom_message;
+
+                match writing_function() {
+                    Ok(_) => panic!("Should have failed"),
+                    Err(e) => {
+                        assert_eq!(ErrorKind::InvalidData, e.kind());
+
+                        match expected_custom_message {
+                            // None if error message should not be compared, e.g. because it is created by Rust and might change
+                            None => assert!(
+                                e.get_ref().unwrap().is::<Utf8Error>(),
+                                "Inner error is not Utf8Error"
+                            ),
+                            Some(message) => {
+                                assert_eq!(message, e.to_string(), "Custom message does not match")
+                            }
                         }
                     }
                 }
             }
         }
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Invalid UTF-8 byte 1111_1000
                 w.write_all(b"\xF8")
             },
             Some("invalid UTF-8 data"),
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Malformed UTF-8; high surrogate U+D800 encoded in UTF-8 (= invalid)
                 w.write_all(b"\xED\xA0\x80")
             },
             None,
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Greater than max code point U+10FFFF; split across multiple bytes
                 w.write_all(b"\xF4")?;
                 w.write_all(b"\x90")?;
@@ -1148,16 +1158,16 @@ mod tests {
             None,
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Overlong encoding for two bytes
                 w.write_all(b"\xC1\xBF")
             },
             None,
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Incomplete four bytes
                 w.write_all(b"\xF0")?;
                 w.write_all(b"\x90")?;
@@ -1167,17 +1177,17 @@ mod tests {
             Some("incomplete multi-byte UTF-8 data"),
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Leading continuation byte
                 w.write_all(b"\x80")?;
                 w.finish_value()
             },
             None,
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Too many continuation bytes
                 w.write_all(b"\xC2")?;
                 w.write_all(b"\x80")?;
@@ -1187,8 +1197,8 @@ mod tests {
             None,
         );
 
-        assert_invalid_utf8(
-            |mut w| {
+        assert_invalid_utf8!(
+            |w| {
                 // Incomplete multi-byte followed by `write_str`
                 w.write_all(b"\xF0")?;
                 w.write_str("")?; // even empty string should trigger this error

tests/custom_json_reader.rs

@@ -9,6 +9,7 @@
 
 use crate::custom_reader::JsonValueReader;
 use serde_json::json;
+use std::io::Read;
 use struson::{
     reader::{
         json_path::{json_path, JsonPath},
@@ -271,15 +272,15 @@ mod custom_reader {
             self.next_str().map(str::to_owned)
         }
 
-        fn next_string_reader(&mut self) -> Result<Box<dyn Read + '_>, ReaderError> {
+        fn next_string_reader(&mut self) -> Result<impl Read + '_, ReaderError> {
             self.begin_value(ValueType::String)?;
             if let Some(Value::String(s)) = self.next_value.take() {
                 self.is_string_value_reader_active = true;
-                Ok(Box::new(StringValueReader {
+                Ok(StringValueReader {
                     delegate: s.as_bytes(),
                     json_reader: self,
                     reached_end: false,
-                }))
+                })
             } else {
                 unreachable!("begin_value should have verified that value is string")
             }

tests/custom_json_writer.rs

@@ -9,9 +9,10 @@
 
 use custom_writer::JsonValueWriter;
 use serde_json::json;
+use std::io::Write;
 use struson::{
     reader::{JsonReader, JsonStreamReader},
-    writer::{JsonNumberError, JsonWriter},
+    writer::{JsonNumberError, JsonWriter, StringValueWriter},
 };
 
 mod custom_writer {
@@ -164,13 +165,13 @@ mod custom_writer {
             Ok(())
         }
 
-        fn string_value_writer(&mut self) -> Result<Box<dyn StringValueWriter + '_>, IoError> {
+        fn string_value_writer(&mut self) -> Result<impl StringValueWriter + '_, IoError> {
             self.check_before_value();
             self.is_string_value_writer_active = true;
-            Ok(Box::new(StringValueWriterImpl {
+            Ok(StringValueWriterImpl {
                 buf: Vec::new(),
                 json_writer: self,
-            }))
+            })
         }
 
         fn number_value_from_string(&mut self, value: &str) -> Result<(), JsonNumberError> {
@@ -279,7 +280,7 @@ mod custom_writer {
         }
     }
     impl StringValueWriter for StringValueWriterImpl<'_, '_> {
-        fn finish_value(self: Box<Self>) -> Result<(), IoError> {
+        fn finish_value(self) -> Result<(), IoError> {
             let string =
                 String::from_utf8(self.buf).map_err(|e| IoError::new(ErrorKind::InvalidData, e))?;
             self.json_writer.add_value(Value::String(string));