Add result type for JsonWriter::finish_document

src/reader/stream_reader.rs

@@ -4433,6 +4433,8 @@ mod tests {
         /* Note: If maintaining this becomes too cumbersome when adjusting JsonWriter API, can remove this test */
         struct FailingJsonWriter;
         impl JsonWriter for FailingJsonWriter {
+            type WriterResult = ();
+
             fn begin_object(&mut self) -> Result<(), IoError> {
                 Err(err())
             }

src/writer/mod.rs

@@ -89,6 +89,11 @@ type IoError = std::io::Error;
 /// related to incorrect usage by the user (such as trying to call [`end_object`](Self::end_object) when currently
 /// writing a JSON array).
 pub trait JsonWriter {
+    /// Result returned by [`finish_document`](Self::finish_document)
+    ///
+    /// The type of the result and its meaning depends on the JSON writer implementation.
+    type WriterResult;
+
     // TODO: Should these methods all return &mut Self to allow chaining?
 
     /// Begins writing a JSON object
@@ -521,6 +526,12 @@ pub trait JsonWriter {
 
     /// Verifies that the JSON document is complete and flushes the buffer
     ///
+    /// Returns the [`WriterResult`](Self::WriterResult) on success. Depending on the
+    /// JSON writer implementation this can for example be the written JSON value.
+    /// The result might not be relevant for all JSON writer implementations, for example
+    /// a JSON writer writing to a `Write` will already produce the JSON document during
+    /// usage, so the `WriterResult` returned by this method can be ignored.
+    ///
     /// This method **must** be called explicitly. Dropping the JSON writer will not
     /// automatically flush the buffer.
     ///
@@ -537,12 +548,7 @@ pub trait JsonWriter {
      * Note: Dropping writer will not automatically finish document since that would silently discard errors which might occur
      * TODO: Does consuming 'self' here really guarantee that writer cannot be used afterwards anymore; can this be bypassed with reference somehow?
      */
-    /*
-     * TODO (custom JSON writer support): Instead of returning `()` maybe use an associated type
-     * on JsonWriter as result type; then custom JsonWriter implementations which produce a
-     * value can return it here.
-     */
-    fn finish_document(self) -> Result<(), IoError>;
+    fn finish_document(self) -> Result<Self::WriterResult, IoError>;
 }
 
 /// Writer for lazily writing a JSON string value

src/writer/simple.rs

@@ -287,6 +287,8 @@ mod error_safe_writer {
     }
 
     impl<J: JsonWriter> JsonWriter for ErrorSafeJsonWriter<J> {
+        type WriterResult = J::WriterResult;
+
         fn begin_object(&mut self) -> Result<(), IoError> {
             use_delegate!(self, |w| w.begin_object())
         }
@@ -377,7 +379,7 @@ mod error_safe_writer {
             )
         }
 
-        fn finish_document(self) -> Result<(), IoError> {
+        fn finish_document(self) -> Result<Self::WriterResult, IoError> {
             // Special code instead of `use_delegate!(...)` because this method consumes `self`
             if let Some(error) = self.error {
                 return Err(error_from_stored(&error));
@@ -516,17 +518,20 @@ impl<W: Write> SimpleJsonWriter<JsonStreamWriter<W>> {
 impl<J: JsonWriter> ValueWriter<J> for SimpleJsonWriter<J> {
     fn write_null(mut self) -> Result<(), IoError> {
         self.json_writer.null_value()?;
-        self.json_writer.finish_document()
+        self.json_writer.finish_document()?;
+        Ok(())
     }
 
     fn write_bool(mut self, value: bool) -> Result<(), IoError> {
         self.json_writer.bool_value(value)?;
-        self.json_writer.finish_document()
+        self.json_writer.finish_document()?;
+        Ok(())
     }
 
     fn write_string(mut self, value: &str) -> Result<(), IoError> {
         self.json_writer.string_value(value)?;
-        self.json_writer.finish_document()
+        self.json_writer.finish_document()?;
+        Ok(())
     }
 
     fn write_string_with_writer(
@@ -540,7 +545,8 @@ impl<J: JsonWriter> ValueWriter<J> for SimpleJsonWriter<J> {
 
     fn write_number<N: FiniteNumber>(mut self, value: N) -> Result<(), IoError> {
         self.json_writer.number_value(value)?;
-        self.json_writer.finish_document()
+        self.json_writer.finish_document()?;
+        Ok(())
     }
 
     fn write_fp_number<N: FloatingPointNumber>(mut self, value: N) -> Result<(), JsonNumberError> {

src/writer/stream_writer.rs

@@ -388,6 +388,14 @@ impl<W: Write> JsonStreamWriter<W> {
 }
 
 impl<W: Write> JsonWriter for JsonStreamWriter<W> {
+    /// Result returned by [`finish_document`](Self::finish_document)
+    ///
+    /// This JSON writer implementation returns the underlying `Write` to allow for
+    /// example to reuse it for other purposes. However, the JSON document is already
+    /// written during JSON writer usage, so the returned `Write` can be ignored in
+    /// case it is not needed for anything else.
+    type WriterResult = W;
+
     fn begin_object(&mut self) -> Result<(), IoError> {
         self.before_value()?;
         self.stack.push(StackValue::Object);
@@ -509,7 +517,7 @@ impl<W: Write> JsonWriter for JsonStreamWriter<W> {
         // might not be necessary because Serde's Serialize API enforces this
     }
 
-    fn finish_document(mut self) -> Result<(), IoError> {
+    fn finish_document(mut self) -> Result<Self::WriterResult, IoError> {
         if self.is_string_value_writer_active {
             panic!("Incorrect writer usage: Cannot finish document when string value writer is still active");
         }
@@ -523,7 +531,8 @@ impl<W: Write> JsonWriter for JsonStreamWriter<W> {
         } else {
             panic!("Incorrect writer usage: Cannot finish document when top-level value is not finished");
         }
-        self.flush()
+        self.flush()?;
+        Ok(self.writer)
     }
 
     fn string_value_writer(&mut self) -> Result<impl StringValueWriter + '_, IoError> {
@@ -1585,6 +1594,17 @@ mod tests {
         Ok(())
     }
 
+    /// Verify that `finish_document` returns the wrapped writer.
+    #[test]
+    fn finish_document_result() -> TestResult {
+        let mut json_writer = JsonStreamWriter::new(Vec::<u8>::new());
+        json_writer.string_value("text")?;
+        let written_bytes = json_writer.finish_document()?;
+        assert_eq!("\"text\"", String::from_utf8(written_bytes)?);
+
+        Ok(())
+    }
+
     #[test]
     #[should_panic(
         expected = "Incorrect writer usage: Cannot finish document when no value has been written yet"

tests/custom_json_writer.rs

@@ -29,37 +29,25 @@ mod custom_writer {
         Object(Map<String, Value>),
     }
 
-    pub struct JsonValueWriter<'a> {
+    pub struct JsonValueWriter {
         stack: Vec<StackValue>,
         pending_name: Option<String>,
         is_string_value_writer_active: bool,
         /// Holds the final value until `finish_document` is called
-        final_value_temp: Option<Value>,
-        /// Holds the final value after `finish_document` is called, and which is accessible
-        /// to creator of `JsonValueWriter` (who should still have a reference to this `Option`)
-        final_value_holder: &'a mut Option<Value>,
+        final_value: Option<Value>,
     }
-    impl<'a> JsonValueWriter<'a> {
-        /*
-         * TODO: This approach of taking an `Option` reference and storing the final value in it
-         * matches the current JsonWriter API, however a cleaner approach might be if `finish_document`
-         * could return the result instead, see TODO comment on `JsonWriter::finish_document`
-         */
-        pub fn new(final_value_holder: &'a mut Option<Value>) -> Self {
-            if final_value_holder.is_some() {
-                panic!("Final value holder should be None");
-            }
+    impl JsonValueWriter {
+        pub fn new() -> Self {
             JsonValueWriter {
                 stack: Vec::new(),
                 pending_name: None,
                 is_string_value_writer_active: false,
-                final_value_temp: None,
-                final_value_holder,
+                final_value: None,
             }
         }
     }
 
-    impl JsonValueWriter<'_> {
+    impl JsonValueWriter {
         fn verify_string_writer_inactive(&self) {
             if self.is_string_value_writer_active {
                 panic!("Incorrect writer usage: String value writer is active");
@@ -68,7 +56,7 @@ mod custom_writer {
 
         fn check_before_value(&self) {
             self.verify_string_writer_inactive();
-            if self.final_value_temp.is_some() || self.final_value_holder.is_some() {
+            if self.final_value.is_some() {
                 panic!("Incorrect writer usage: Top-level value has already been written")
             }
             if let Some(StackValue::Object(_)) = self.stack.last() {
@@ -88,10 +76,10 @@ mod custom_writer {
                 };
             } else {
                 debug_assert!(
-                    self.final_value_temp.is_none(),
+                    self.final_value.is_none(),
                     "caller should have verified that final value is not set yet"
                 );
-                self.final_value_temp = Some(value);
+                self.final_value = Some(value);
             }
         }
     }
@@ -101,7 +89,9 @@ mod custom_writer {
             .ok_or_else(|| JsonNumberError::InvalidNumber(format!("non-finite number: {f}")))
     }
 
-    impl JsonWriter for JsonValueWriter<'_> {
+    impl JsonWriter for JsonValueWriter {
+        type WriterResult = Value;
+
         fn begin_object(&mut self) -> Result<(), IoError> {
             self.check_before_value();
             self.stack.push(StackValue::Object(Map::new()));
@@ -253,22 +243,21 @@ mod custom_writer {
             // might not be necessary because Serde's Serialize API enforces this
         }
 
-        fn finish_document(mut self) -> Result<(), IoError> {
+        fn finish_document(self) -> Result<Self::WriterResult, IoError> {
             self.verify_string_writer_inactive();
-            if let Some(value) = self.final_value_temp.take() {
-                *self.final_value_holder = Some(value);
-                Ok(())
+            if let Some(value) = self.final_value {
+                Ok(value)
             } else {
                 panic!("Incorrect writer usage: Top-level value is incomplete")
             }
         }
     }
 
-    struct StringValueWriterImpl<'j, 'a> {
+    struct StringValueWriterImpl<'j> {
         buf: Vec<u8>,
-        json_writer: &'j mut JsonValueWriter<'a>,
+        json_writer: &'j mut JsonValueWriter,
     }
-    impl Write for StringValueWriterImpl<'_, '_> {
+    impl Write for StringValueWriterImpl<'_> {
         fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
             self.buf.extend_from_slice(buf);
             Ok(buf.len())
@@ -279,7 +268,7 @@ mod custom_writer {
             Ok(())
         }
     }
-    impl StringValueWriter for StringValueWriterImpl<'_, '_> {
+    impl StringValueWriter for StringValueWriterImpl<'_> {
         fn finish_value(self) -> Result<(), IoError> {
             let string =
                 String::from_utf8(self.buf).map_err(|e| IoError::new(ErrorKind::InvalidData, e))?;
@@ -303,8 +292,7 @@ fn write() -> Result<(), Box<dyn std::error::Error>> {
         }
     }
 
-    let mut final_value_holder = None;
-    let mut json_writer = JsonValueWriter::new(&mut final_value_holder);
+    let mut json_writer = JsonValueWriter::new();
 
     json_writer.begin_array()?;
 
@@ -343,7 +331,7 @@ fn write() -> Result<(), Box<dyn std::error::Error>> {
     );
 
     json_writer.end_array()?;
-    json_writer.finish_document()?;
+    let written_value = json_writer.finish_document()?;
 
     let expected_json = json!([
         {
@@ -361,23 +349,22 @@ fn write() -> Result<(), Box<dyn std::error::Error>> {
         -67,
         8.9,
     ]);
-    assert_eq!(expected_json, final_value_holder.unwrap());
+    assert_eq!(expected_json, written_value);
 
     Ok(())
 }
 
 #[test]
 fn transfer() -> Result<(), Box<dyn std::error::Error>> {
-    let mut final_value_holder = None;
-    let mut json_writer = JsonValueWriter::new(&mut final_value_holder);
+    let mut json_writer = JsonValueWriter::new();
 
     let mut json_reader = JsonStreamReader::new(
         "[true, 123, {\"name1\": \"value1\", \"name2\": null}, false]".as_bytes(),
     );
     json_reader.transfer_to(&mut json_writer)?;
     json_reader.consume_trailing_whitespace()?;
 
-    json_writer.finish_document()?;
+    let written_value = json_writer.finish_document()?;
 
     let expected_json = json!([
         true,
@@ -389,7 +376,7 @@ fn transfer() -> Result<(), Box<dyn std::error::Error>> {
         },
         false,
     ]);
-    assert_eq!(expected_json, final_value_holder.unwrap());
+    assert_eq!(expected_json, written_value);
 
     Ok(())
 }
@@ -405,16 +392,15 @@ fn serialize() -> Result<(), Box<dyn std::error::Error>> {
         b: &'static str,
     }
 
-    let mut final_value_holder = None;
-    let mut json_writer = JsonValueWriter::new(&mut final_value_holder);
+    let mut json_writer = JsonValueWriter::new();
     json_writer.serialize_value(&CustomStruct { a: 123, b: "test" })?;
-    json_writer.finish_document()?;
+    let written_value = json_writer.finish_document()?;
 
     let expected_json = json!({
         "a": 123,
         "b": "test",
     });
-    assert_eq!(expected_json, final_value_holder.unwrap());
+    assert_eq!(expected_json, written_value);
 
     Ok(())
 }