TEMP/NIGHTLY: Use impl Trait as return type for `next_string_reader…

…` and `string_value_writer`

rust-toolchain.toml

@@ -1,4 +1,4 @@
 [toolchain]
-channel = "1.73.0"
+channel = "nightly"
 profile = "minimal"
 components = ["clippy", "rustfmt", "rust-docs"]

src/lib.rs

@@ -14,6 +14,7 @@
 // When `docsrs` configuration flag is set enable banner for features in documentation
 // See https://stackoverflow.com/q/61417452
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![feature(return_position_impl_trait_in_trait)]
 
 //! Struson is an [RFC 8259](https://www.rfc-editor.org/rfc/rfc8259.html) compliant streaming JSON reader and writer.
 //!

src/reader/mod.rs

@@ -1136,6 +1136,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()?;
     ///
@@ -1169,7 +1170,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

@@ -17,7 +17,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)]
@@ -2095,16 +2095,16 @@ 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)?;
         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,
-        }))
+        })
     }
 
     fn next_number_as_string(&mut self) -> Result<String, ReaderError> {
@@ -2399,6 +2399,8 @@ impl<'j, R: Read> Read for StringValueReader<'j, R> {
 
 #[cfg(test)]
 mod tests {
+    use std::io::Write;
+
     use super::*;
     use crate::writer::{
         FiniteNumber, FloatingPointNumber, JsonNumberError, JsonStreamWriter, StringValueWriter,
@@ -4029,12 +4031,32 @@ mod tests {
 
     #[test]
     fn transfer_to_writer_error() {
-        struct FailingJsonWriter;
-
         fn err() -> IoError {
             IoError::new(ErrorKind::Other, "test error")
         }
 
+        struct FailingStringValueWriter;
+        impl Write for FailingStringValueWriter {
+            fn write(&mut self, _: &[u8]) -> std::io::Result<usize> {
+                Err(err())
+            }
+
+            fn flush(&mut self) -> std::io::Result<()> {
+                Err(err())
+            }
+        }
+        impl StringValueWriter for FailingStringValueWriter {
+            fn write_str(&mut self, _: &str) -> Result<(), IoError> {
+                Err(err())
+            }
+
+            fn finish_value(self) -> Result<(), IoError> {
+                Err(err())
+            }
+        }
+
+        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 {
@@ -4070,8 +4092,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::<FailingStringValueWriter, IoError>(err())
             }
 
             fn number_value_from_string(&mut self, _: &str) -> Result<(), JsonNumberError> {

src/writer/mod.rs

@@ -304,6 +304,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);
     ///
@@ -325,13 +326,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
     ///
@@ -555,6 +550,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);
     ///
@@ -581,7 +577,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

@@ -517,16 +517,16 @@ 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,
-        }))
+        })
     }
 }
 
@@ -732,7 +732,7 @@ impl<'j, W: Write> StringValueWriter for StringValueWriterImpl<'j, W> {
         Ok(())
     }
 
-    fn finish_value(self: Box<Self>) -> Result<(), IoError> {
+    fn finish_value(self) -> Result<(), IoError> {
         if self.utf8_pos > 0 {
             return Err(IoError::new(
                 ErrorKind::InvalidData,
@@ -1054,53 +1054,61 @@ 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();
+                #[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")?;
@@ -1110,16 +1118,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")?;
@@ -1129,17 +1137,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")?;
@@ -1149,8 +1157,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

@@ -1,6 +1,7 @@
 // Allow `assert_eq!(true, ...)` because in some cases it is used to check a bool
 // value and not a 'flag' / 'state', and `assert_eq!` makes that more explicit
 #![allow(clippy::bool_assert_comparison)]
+#![feature(return_position_impl_trait_in_trait)]
 
 //! Integration test for a custom `JsonReader` implementation
 //!
@@ -13,6 +14,7 @@
 
 use crate::custom_reader::JsonValueReader;
 use serde_json::json;
+use std::io::Read;
 use struson::{
     reader::{
         json_path::{json_path, JsonPath},
@@ -293,15 +295,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")
             }