Add TryWriteable with internal Writeable refactoring

CHANGELOG.md

@@ -59,6 +59,9 @@
         - Add `ZeroTrieSimpleAsciiCursor` for manual iteration (https://github.com/unicode-org/icu4x/pull/4383)
     - `zerovec`
         - Change `ZeroHashMap` to use `twox-hash` (https://github.com/unicode-org/icu4x/pull/4592)
+    - `writeable`
+        - Add `TryWriteable` for fallibility (https://github.com/unicode-org/icu4x/pull/4787)
+        - Add `write_cmp_bytes` for more efficient comparison (https://github.com/unicode-org/icu4x/pull/4402)
 
 ## icu4x 1.4.x
 

components/datetime/src/format/datetime.rs

@@ -618,7 +618,7 @@ mod tests {
             .into_japanese_date()
             .to_any();
 
-        writeable::assert_writeable_eq!(dtf.format(&date).unwrap(), "Sep 1, 12 kansei-1789")
+        writeable::assert_writeable_eq!(dtf.format(&date).unwrap(), "Sep 1, 12 kansei-1789");
     }
 
     #[test]

utils/writeable/examples/writeable_message.rs

@@ -47,7 +47,8 @@ impl<V: Writeable> fmt::Display for WriteableMessage<V> {
 fn main() {
     icu_benchmark_macros::main_setup!();
 
-    let (string, parts) = writeable_to_parts_for_test(&WriteableMessage("world")).unwrap();
+    let (string, parts) =
+        writeable::_internal::writeable_to_parts_for_test(&WriteableMessage("world"));
 
     assert_eq!(string, "Hello world 😅");
 

utils/writeable/src/helpers.rs

@@ -0,0 +1,106 @@
+// This file is part of ICU4X. For terms of use, please see the file
+// called LICENSE at the top level of the ICU4X source tree
+// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
+
+use crate::*;
+use alloc::string::String;
+use alloc::vec::Vec;
+
+pub(crate) struct TestWriter {
+    pub(crate) string: String,
+    pub(crate) parts: Vec<(usize, usize, Part)>,
+}
+
+impl TestWriter {
+    pub(crate) fn finish(mut self) -> (String, Vec<(usize, usize, Part)>) {
+        // Sort by first open and last closed
+        self.parts
+            .sort_unstable_by_key(|(begin, end, _)| (*begin, end.wrapping_neg()));
+        (self.string, self.parts)
+    }
+}
+
+impl fmt::Write for TestWriter {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        self.string.write_str(s)
+    }
+    fn write_char(&mut self, c: char) -> fmt::Result {
+        self.string.write_char(c)
+    }
+}
+
+impl PartsWrite for TestWriter {
+    type SubPartsWrite = Self;
+    fn with_part(
+        &mut self,
+        part: Part,
+        mut f: impl FnMut(&mut Self::SubPartsWrite) -> fmt::Result,
+    ) -> fmt::Result {
+        let start = self.string.len();
+        f(self)?;
+        let end = self.string.len();
+        if start < end {
+            self.parts.push((start, end, part));
+        }
+        Ok(())
+    }
+}
+
+#[allow(clippy::type_complexity)]
+pub fn writeable_to_parts_for_test<W: Writeable>(
+    writeable: &W,
+) -> (String, Vec<(usize, usize, Part)>) {
+    let mut writer = helpers::TestWriter {
+        string: alloc::string::String::new(),
+        parts: Vec::new(),
+    };
+    #[allow(clippy::expect_used)] // for test code
+    writeable
+        .write_to_parts(&mut writer)
+        .expect("String writer infallible");
+    writer.finish()
+}
+
+#[doc(hidden)]
+#[allow(clippy::type_complexity)]
+pub fn try_writeable_to_parts_for_test<W: TryWriteable>(
+    writeable: &W,
+) -> (String, Vec<(usize, usize, Part)>, Option<W::Error>) {
+    let mut writer = helpers::TestWriter {
+        string: alloc::string::String::new(),
+        parts: Vec::new(),
+    };
+    #[allow(clippy::expect_used)] // for test code
+    let result = writeable
+        .try_write_to_parts(&mut writer)
+        .expect("String writer infallible");
+    let (actual_str, actual_parts) = writer.finish();
+    (actual_str, actual_parts, result.err())
+}
+
+pub(crate) struct CoreWriteAsPartsWrite<W: fmt::Write + ?Sized>(pub W);
+
+impl<W: fmt::Write + ?Sized> fmt::Write for CoreWriteAsPartsWrite<W> {
+    #[inline]
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        self.0.write_str(s)
+    }
+
+    #[inline]
+    fn write_char(&mut self, c: char) -> fmt::Result {
+        self.0.write_char(c)
+    }
+}
+
+impl<W: fmt::Write + ?Sized> PartsWrite for CoreWriteAsPartsWrite<W> {
+    type SubPartsWrite = CoreWriteAsPartsWrite<W>;
+
+    #[inline]
+    fn with_part(
+        &mut self,
+        _part: Part,
+        mut f: impl FnMut(&mut Self::SubPartsWrite) -> fmt::Result,
+    ) -> fmt::Result {
+        f(self)
+    }
+}

utils/writeable/src/lib.rs

@@ -69,14 +69,23 @@ extern crate alloc;
 mod cmp;
 #[cfg(feature = "either")]
 mod either;
+mod helpers;
 mod impls;
 mod ops;
+mod try_writeable;
 
 use alloc::borrow::Cow;
 use alloc::string::String;
-use alloc::vec::Vec;
 use core::fmt;
 
+pub use try_writeable::TryWriteable;
+
+#[doc(hidden)]
+pub mod _internal {
+    pub use super::helpers::try_writeable_to_parts_for_test;
+    pub use super::helpers::writeable_to_parts_for_test;
+}
+
 /// A hint to help consumers of `Writeable` pre-allocate bytes before they call
 /// [`write_to`](Writeable::write_to).
 ///
@@ -168,6 +177,16 @@ pub struct Part {
     pub value: &'static str,
 }
 
+impl Part {
+    /// A part that should annotate error segments in [`TryWriteable`] output.
+    ///
+    /// For an example, see [`TryWriteable`].
+    pub const ERROR: Part = Part {
+        category: "writeable",
+        value: "error",
+    };
+}
+
 /// A sink that supports annotating parts of the string with `Part`s.
 pub trait PartsWrite: fmt::Write {
     type SubPartsWrite: PartsWrite + ?Sized;
@@ -185,30 +204,7 @@ pub trait Writeable {
     /// The default implementation delegates to `write_to_parts`, and discards any
     /// `Part` annotations.
     fn write_to<W: fmt::Write + ?Sized>(&self, sink: &mut W) -> fmt::Result {
-        struct CoreWriteAsPartsWrite<W: fmt::Write + ?Sized>(W);
-        impl<W: fmt::Write + ?Sized> fmt::Write for CoreWriteAsPartsWrite<W> {
-            fn write_str(&mut self, s: &str) -> fmt::Result {
-                self.0.write_str(s)
-            }
-
-            fn write_char(&mut self, c: char) -> fmt::Result {
-                self.0.write_char(c)
-            }
-        }
-
-        impl<W: fmt::Write + ?Sized> PartsWrite for CoreWriteAsPartsWrite<W> {
-            type SubPartsWrite = CoreWriteAsPartsWrite<W>;
-
-            fn with_part(
-                &mut self,
-                _part: Part,
-                mut f: impl FnMut(&mut Self::SubPartsWrite) -> fmt::Result,
-            ) -> fmt::Result {
-                f(self)
-            }
-        }
-
-        self.write_to_parts(&mut CoreWriteAsPartsWrite(sink))
+        self.write_to_parts(&mut helpers::CoreWriteAsPartsWrite(sink))
     }
 
     /// Write bytes and `Part` annotations to the given sink. Errors from the
@@ -315,8 +311,7 @@ pub trait Writeable {
     /// ```
     fn write_cmp_bytes(&self, other: &[u8]) -> core::cmp::Ordering {
         let mut wc = cmp::WriteComparator::new(other);
-        #[allow(clippy::unwrap_used)] // infallible impl
-        self.write_to(&mut wc).unwrap();
+        let _ = self.write_to(&mut wc);
         wc.finish().reverse()
     }
 }
@@ -340,12 +335,22 @@ macro_rules! impl_display_with_writeable {
     };
 }
 
-/// Testing macros for types implementing Writeable. The first argument should be a
-/// `Writeable`, the second argument a string, and the third argument (*_parts_eq only)
-/// a list of parts (`[(usize, usize, Part)]`).
+/// Testing macros for types implementing [`Writeable`].
 ///
-/// The macros tests for equality of string content, parts (*_parts_eq only), and
-/// verify the size hint.
+/// Arguments, in order:
+///
+/// 1. The [`Writeable`] under test
+/// 2. The expected string value
+/// 3. [`*_parts_eq`] only: a list of parts (`[(start, end, Part)]`)
+///
+/// Any remaining arguments get passed to `format!`
+///
+/// The macros tests the following:
+///
+/// - Equality of string content
+/// - Equality of parts ([`*_parts_eq`] only)
+/// - Validity of size hint
+/// - Basic validity of `cmp_bytes`
 ///
 /// # Examples
 ///
@@ -389,14 +394,16 @@ macro_rules! impl_display_with_writeable {
 ///     "Hello World"
 /// );
 /// ```
+///
+/// [`*_parts_eq`]: assert_writeable_parts_eq
 #[macro_export]
 macro_rules! assert_writeable_eq {
     ($actual_writeable:expr, $expected_str:expr $(,)?) => {
         $crate::assert_writeable_eq!($actual_writeable, $expected_str, "");
     };
     ($actual_writeable:expr, $expected_str:expr, $($arg:tt)+) => {{
         let actual_writeable = &$actual_writeable;
-        let (actual_str, _) = $crate::writeable_to_parts_for_test(actual_writeable).unwrap();
+        let (actual_str, actual_parts) = $crate::_internal::writeable_to_parts_for_test(actual_writeable);
         assert_eq!(actual_str, $expected_str, $($arg)*);
         assert_eq!(actual_str, $crate::Writeable::write_to_string(actual_writeable), $($arg)+);
         let length_hint = $crate::Writeable::writeable_length_hint(actual_writeable);
@@ -413,6 +420,9 @@ macro_rules! assert_writeable_eq {
             );
         }
         assert_eq!(actual_writeable.to_string(), $expected_str);
+        let ordering = $crate::Writeable::write_cmp_bytes(actual_writeable, $expected_str.as_bytes());
+        assert_eq!(ordering, core::cmp::Ordering::Equal, $($arg)*);
+        actual_parts // return for assert_writeable_parts_eq
     }};
 }
 
@@ -423,65 +433,7 @@ macro_rules! assert_writeable_parts_eq {
         $crate::assert_writeable_parts_eq!($actual_writeable, $expected_str, $expected_parts, "");
     };
     ($actual_writeable:expr, $expected_str:expr, $expected_parts:expr, $($arg:tt)+) => {{
-        let actual_writeable = &$actual_writeable;
-        let (actual_str, actual_parts) = $crate::writeable_to_parts_for_test(actual_writeable).unwrap();
-        assert_eq!(actual_str, $expected_str, $($arg)+);
-        assert_eq!(actual_str, $crate::Writeable::write_to_string(actual_writeable), $($arg)+);
+        let actual_parts = $crate::assert_writeable_eq!($actual_writeable, $expected_str, $($arg)*);
         assert_eq!(actual_parts, $expected_parts, $($arg)+);
-        let length_hint = $crate::Writeable::writeable_length_hint(actual_writeable);
-        assert!(length_hint.0 <= actual_str.len(), $($arg)+);
-        if let Some(upper) = length_hint.1 {
-            assert!(actual_str.len() <= upper, $($arg)+);
-        }
-        assert_eq!(actual_writeable.to_string(), $expected_str);
     }};
 }
-
-#[doc(hidden)]
-#[allow(clippy::type_complexity)]
-pub fn writeable_to_parts_for_test<W: Writeable>(
-    writeable: &W,
-) -> Result<(String, Vec<(usize, usize, Part)>), fmt::Error> {
-    struct State {
-        string: alloc::string::String,
-        parts: Vec<(usize, usize, Part)>,
-    }
-
-    impl fmt::Write for State {
-        fn write_str(&mut self, s: &str) -> fmt::Result {
-            self.string.write_str(s)
-        }
-        fn write_char(&mut self, c: char) -> fmt::Result {
-            self.string.write_char(c)
-        }
-    }
-
-    impl PartsWrite for State {
-        type SubPartsWrite = Self;
-        fn with_part(
-            &mut self,
-            part: Part,
-            mut f: impl FnMut(&mut Self::SubPartsWrite) -> fmt::Result,
-        ) -> fmt::Result {
-            let start = self.string.len();
-            f(self)?;
-            let end = self.string.len();
-            if start < end {
-                self.parts.push((start, end, part));
-            }
-            Ok(())
-        }
-    }
-
-    let mut state = State {
-        string: alloc::string::String::new(),
-        parts: Vec::new(),
-    };
-    writeable.write_to_parts(&mut state)?;
-
-    // Sort by first open and last closed
-    state
-        .parts
-        .sort_unstable_by_key(|(begin, end, _)| (*begin, end.wrapping_neg()));
-    Ok((state.string, state.parts))
-}