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
 

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/lib.rs

@@ -71,12 +71,22 @@ mod cmp;
 mod either;
 mod impls;
 mod ops;
+mod parts_write_adapter;
+mod testing;
+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::testing::try_writeable_to_parts_for_test;
+    pub use super::testing::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 +178,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 +205,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 parts_write_adapter::CoreWriteAsPartsWrite(sink))
     }
 
     /// Write bytes and `Part` annotations to the given sink. Errors from the
@@ -315,8 +312,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 +336,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`].
+///
+/// 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:
 ///
-/// The macros tests for equality of string content, parts (*_parts_eq only), and
-/// verify the size hint.
+/// - Equality of string content
+/// - Equality of parts ([`*_parts_eq`] only)
+/// - Validity of size hint
+/// - Reflexivity of `cmp_bytes`
 ///
 /// # Examples
 ///
@@ -389,14 +395,19 @@ 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, "");
+        $crate::assert_writeable_eq!($actual_writeable, $expected_str, "")
     };
     ($actual_writeable:expr, $expected_str:expr, $($arg:tt)+) => {{
+        $crate::assert_writeable_eq!(@internal, $actual_writeable, $expected_str, $($arg)*);
+    }};
+    (@internal, $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);
         let actual_len = actual_str.len();
         assert_eq!(actual_str, $expected_str, $($arg)*);
         assert_eq!(actual_str, $crate::Writeable::write_to_string(actual_writeable), $($arg)+);
@@ -415,85 +426,20 @@ 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
     }};
 }
 
 /// See [`assert_writeable_eq`].
 #[macro_export]
 macro_rules! assert_writeable_parts_eq {
     ($actual_writeable:expr, $expected_str:expr, $expected_parts:expr $(,)?) => {
-        $crate::assert_writeable_parts_eq!($actual_writeable, $expected_str, $expected_parts, "");
+        $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();
-        let actual_len = actual_str.len();
-        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!(@internal, $actual_writeable, $expected_str, $($arg)*);
         assert_eq!(actual_parts, $expected_parts, $($arg)+);
-        let length_hint = $crate::Writeable::writeable_length_hint(actual_writeable);
-        let lower = length_hint.0;
-        assert!(
-            lower <= actual_len,
-            "hint lower bound {lower} larger than actual length {actual_len}: {}",
-            format!($($arg)*),
-        );
-        if let Some(upper) = length_hint.1 {
-            assert!(
-                actual_len <= upper,
-                "hint upper bound {upper} smaller than actual length {actual_len}: {}",
-                format!($($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))
-}

utils/writeable/src/parts_write_adapter.rs

@@ -0,0 +1,32 @@
+// 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::*;
+
+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/testing.rs

@@ -0,0 +1,78 @@
+// 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 = 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()
+}
+
+#[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 = 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())
+}