Refactor and simplify FixedString implementation

src/length.rs

@@ -70,6 +70,13 @@ impl InvalidStrLength {
     pub fn get_inner(self) -> Box<str> {
         self.original
     }
+
+    pub(crate) unsafe fn from_invalid_length_unchecked(value: InvalidLength<u8>) -> Self {
+        Self {
+            type_name: value.type_name,
+            original: unsafe { alloc::str::from_boxed_utf8_unchecked(value.original) },
+        }
+    }
 }
 
 #[cfg(feature = "std")]
@@ -86,23 +93,6 @@ impl core::fmt::Display for InvalidStrLength {
     }
 }
 
-impl TryFrom<InvalidLength<u8>> for InvalidStrLength {
-    type Error = core::str::Utf8Error;
-
-    fn try_from(value: InvalidLength<u8>) -> Result<Self, Self::Error> {
-        let original = if let Err(err) = core::str::from_utf8(&value.original) {
-            return Err(err);
-        } else {
-            unsafe { alloc::str::from_boxed_utf8_unchecked(value.original) }
-        };
-
-        Ok(Self {
-            original,
-            type_name: value.type_name,
-        })
-    }
-}
-
 #[doc(hidden)]
 pub trait NonZero<Int: ValidLength>:
     sealed::NonZeroSealed + Into<Int> + Sized + Copy + PartialEq + Debug

src/string.rs

@@ -1,6 +1,7 @@
 use alloc::{
-    borrow::{Cow, ToOwned},
+    borrow::{Cow},
     boxed::Box,
+    rc::Rc,
     string::String,
     sync::Arc,
 };
@@ -70,35 +71,51 @@ impl<LenT: ValidLength> FixedString<LenT> {
         Self(FixedStringRepr::Static(StaticStr::from_static_str(val)))
     }
 
-    /// Converts a `&str` into a [`FixedString`], allocating if the value cannot fit "inline".
+    /// Converts a string into a [`FixedString`], **truncating** if the value is larger than `LenT`'s maximum.
     ///
-    /// This method will be more efficent if you would otherwise clone a [`String`] to convert into [`FixedString`],
-    /// but should not be used in the case that [`String`] ownership could be transfered without reallocation.
-    ///
-    /// If the `&str` is `'static`, it is preferred to use [`Self::from_static_trunc`], which does not need to copy the data around.
+    /// If the `&str` value cannot fit "inline" it gets allocated. For owned types the allocation can get reused.
     ///
     /// "Inline" refers to Small String Optimisation which allows for Strings with less than 9 to 11 characters
     /// to be stored without allocation, saving a pointer size and an allocation.
     ///
-    /// See [`Self::from_string_trunc`] for truncation behaviour.
+    /// If the `&str` is `'static`, it is preferred to use [`Self::from_static_trunc`], which does not need to copy the data around.
+    ///
+    /// This allows for infallible conversion, but may be lossy in the case of a value above `LenT`'s max.
+    /// For lossless fallible conversion use [`TryFrom`] or [`Self::try_from_string`].
     #[must_use]
-    pub fn from_str_trunc(val: &str) -> Self {
-        if let Some(inline) = Self::new_inline(val) {
-            inline
-        } else {
-            Self::from_string_trunc(val.to_owned())
+    pub fn from_string_trunc<S>(str: S) -> Self
+    where
+        S: AsRef<str>,
+        Box<str>: From<S>,
+    {
+        match Self::try_from_string::<S>(str) {
+            Ok(val) => val,
+            Err(err) => {
+                Self::from_string_trunc::<String>(truncate_string(err, LenT::MAX.to_usize()))
+            }
         }
     }
 
-    /// Converts a [`String`] into a [`FixedString`], **truncating** if the value is larger than `LenT`'s maximum.
+    /// Converts a string into a [`FixedString`].
     ///
-    /// This allows for infallible conversion, but may be lossy in the case of a value above `LenT`'s max.
-    /// For lossless fallible conversion, convert to [`Box<str>`] using [`String::into_boxed_str`] and use [`TryFrom`].
-    #[must_use]
-    pub fn from_string_trunc(str: String) -> Self {
-        match str.try_into() {
-            Ok(val) => val,
-            Err(err) => Self::from_string_trunc(truncate_string(err, LenT::MAX.to_usize())),
+    /// # Errors
+    ///
+    /// This function will return an error if str is longer than `LenT`'s maximum.
+    pub fn try_from_string<S>(str: S) -> Result<Self, InvalidStrLength>
+    where
+        S: AsRef<str>,
+        Box<str>: From<S>,
+    {
+        if let Some(inline) = Self::new_inline(str.as_ref()) {
+            return Ok(inline);
+        }
+
+        match Box::<str>::from(str).into_boxed_bytes().try_into() {
+            Ok(val) => Ok(Self(FixedStringRepr::Heap(val))),
+            Err(err) => Err(
+                // SAFETY: Box<str> -> Box<[u8]> -> Box<str> always works
+                unsafe { InvalidStrLength::from_invalid_length_unchecked(err) },
+            ),
         }
     }
 
@@ -258,40 +275,55 @@ impl<LenT: ValidLength> FromStr for FixedString<LenT> {
     type Err = InvalidStrLength;
 
     fn from_str(val: &str) -> Result<Self, Self::Err> {
-        if let Some(inline) = Self::new_inline(val) {
-            Ok(inline)
-        } else {
-            Self::try_from(Box::from(val))
-        }
+        Self::try_from_string(val)
     }
 }
 
 impl<LenT: ValidLength> TryFrom<Box<str>> for FixedString<LenT> {
     type Error = InvalidStrLength;
 
     fn try_from(value: Box<str>) -> Result<Self, Self::Error> {
-        if let Some(inline) = Self::new_inline(&value) {
-            return Ok(inline);
-        }
-
-        match value.into_boxed_bytes().try_into() {
-            Ok(val) => Ok(Self(FixedStringRepr::Heap(val))),
-            Err(err) => Err(err
-                .try_into()
-                .expect("Box<str> -> Box<[u8]> should stay valid UTF8")),
-        }
+        Self::try_from_string(value)
     }
 }
 
 impl<LenT: ValidLength> TryFrom<String> for FixedString<LenT> {
     type Error = InvalidStrLength;
 
     fn try_from(value: String) -> Result<Self, Self::Error> {
-        if let Some(inline) = Self::new_inline(&value) {
-            return Ok(inline);
-        }
+        Self::try_from_string(value)
+    }
+}
+
+impl<LenT: ValidLength> TryFrom<&str> for FixedString<LenT> {
+    type Error = InvalidStrLength;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        Self::try_from_string(value)
+    }
+}
+
+impl<LenT: ValidLength> TryFrom<Cow<'_, str>> for FixedString<LenT> {
+    type Error = InvalidStrLength;
+
+    fn try_from(value: Cow<'_, str>) -> Result<Self, Self::Error> {
+        Self::try_from_string(value)
+    }
+}
+
+impl<LenT: ValidLength> TryFrom<Rc<str>> for FixedString<LenT> {
+    type Error = InvalidStrLength;
 
-        value.into_boxed_str().try_into()
+    fn try_from(value: Rc<str>) -> Result<Self, Self::Error> {
+        Self::try_from_string(value.as_ref())
+    }
+}
+
+impl<LenT: ValidLength> TryFrom<Arc<str>> for FixedString<LenT> {
+    type Error = InvalidStrLength;
+
+    fn try_from(value: Arc<str>) -> Result<Self, Self::Error> {
+        Self::try_from_string(value.as_ref())
     }
 }
 
@@ -373,9 +405,33 @@ impl<LenT: ValidLength> AsRef<std::ffi::OsStr> for FixedString<LenT> {
     }
 }
 
+/// With reference counted pointers the allocation can't be re-used,
+/// so allocating a Box<str> just to copy from there to a new alloc
+/// should be avoided. Instead construct the rc from the &str directly.
+fn fixed_string_into_ref_counted<LenT, RC>(value: FixedString<LenT>) -> RC
+where
+    LenT: ValidLength,
+    RC: From<Box<str>> + 'static,
+    for<'a> RC: From<&'a str>,
+{
+    if matches!(value.0, FixedStringRepr::Heap(_)) {
+        // Move existing allocation to Arc/Rc
+        RC::from(Box::<str>::from(value))
+    } else {
+        // Just construct the Arc/Rc directly
+        RC::from(&*value)
+    }
+}
+
 impl<LenT: ValidLength> From<FixedString<LenT>> for Arc<str> {
     fn from(value: FixedString<LenT>) -> Self {
-        Arc::from(value.into_string())
+        fixed_string_into_ref_counted(value)
+    }
+}
+
+impl<LenT: ValidLength> From<FixedString<LenT>> for Rc<str> {
+    fn from(value: FixedString<LenT>) -> Self {
+        fixed_string_into_ref_counted(value)
     }
 }
 
@@ -404,11 +460,11 @@ impl<'de, LenT: ValidLength> serde::Deserialize<'de> for FixedString<LenT> {
             }
 
             fn visit_str<E: serde::de::Error>(self, val: &str) -> Result<Self::Value, E> {
-                FixedString::from_str(val).map_err(E::custom)
+                FixedString::try_from_string(val).map_err(E::custom)
             }
 
             fn visit_string<E: serde::de::Error>(self, val: String) -> Result<Self::Value, E> {
-                FixedString::try_from(val.into_boxed_str()).map_err(E::custom)
+                FixedString::try_from_string(val).map_err(E::custom)
             }
         }
 
@@ -426,6 +482,7 @@ impl<LenT: ValidLength> serde::Serialize for FixedString<LenT> {
 #[cfg(test)]
 mod test {
     use super::*;
+    use core::fmt::Debug;
 
     fn check_u8_roundtrip_generic(to_fixed: fn(String) -> FixedString<u8>) {
         for i in 0..=u8::MAX {
@@ -539,7 +596,7 @@ mod test {
         use to_arraystring::ToArrayString;
 
         check_u8_roundtrip_generic(|original| {
-            FixedString::from_str_trunc(
+            FixedString::<u8>::from_string_trunc(
                 FixedString::from_string_trunc(original)
                     .to_arraystring()
                     .as_str(),
@@ -636,4 +693,111 @@ mod test {
         assert_eq!(s.len(), 4);
         assert!(s.is_inline());
     }
+
+    fn try_from_rountrip<LenT, S>(value: S)
+    where
+        LenT: ValidLength,
+        FixedString<LenT>: TryFrom<S>,
+        <FixedString<LenT> as TryFrom<S>>::Error: Debug,
+        S: AsRef<str>,
+        S: From<FixedString<LenT>>,
+        Box<str>: From<S>,
+    {
+        let string = value.as_ref().to_string();
+
+        let fixed_str: FixedString<LenT> = value.try_into().expect("Try into should work");
+
+        assert_eq!(fixed_str, string);
+
+        let value: S = fixed_str.into();
+
+        assert_eq!(value.as_ref(), string);
+
+        let fixed_str = FixedString::<LenT>::from_string_trunc(value);
+
+        assert_eq!(fixed_str, string);
+
+        let fixed_str = FixedString::<LenT>::from_string_trunc::<FixedString<LenT>>(fixed_str);
+
+        let value: S = fixed_str.into();
+
+        let fixed_str = FixedString::<LenT>::try_from_string(value).expect("try_from_string works");
+
+        assert_eq!(fixed_str, string);
+
+        let fixed_str = FixedString::<LenT>::try_from_string::<FixedString<LenT>>(fixed_str)
+            .expect("try_from_string works");
+
+        assert_eq!(fixed_str, string);
+    }
+
+    #[test]
+    fn test_try_from_string() {
+        let value = "Hello, world!";
+
+        try_from_rountrip::<u8, String>(value.into());
+        try_from_rountrip::<u16, String>(value.into());
+        #[cfg(any(target_pointer_width = "64", target_pointer_width = "32"))]
+        try_from_rountrip::<u32, String>(value.into());
+    }
+
+    #[test]
+    fn test_try_from_boxed_str() {
+        let value = "Hello, world!";
+
+        try_from_rountrip::<u8, Box<str>>(value.into());
+        try_from_rountrip::<u16, Box<str>>(value.into());
+        #[cfg(any(target_pointer_width = "64", target_pointer_width = "32"))]
+        try_from_rountrip::<u32, Box<str>>(value.into());
+    }
+
+    #[test]
+    fn test_try_from_owned_cow_string() {
+        let owned_cow: Cow<'static, str> = Cow::Owned("Hello, world!".into());
+
+        #[cfg(any(target_pointer_width = "64", target_pointer_width = "32"))]
+        try_from_rountrip::<u32, Cow<'static, str>>(owned_cow.clone());
+        try_from_rountrip::<u16, Cow<'static, str>>(owned_cow.clone());
+        try_from_rountrip::<u8, Cow<'static, str>>(owned_cow);
+    }
+
+    #[test]
+    fn test_try_from_cow_string() {
+        let owned_cow: Cow<'_, str> = Cow::Borrowed("Hello, world!");
+
+        #[cfg(any(target_pointer_width = "64", target_pointer_width = "32"))]
+        try_from_rountrip::<u32, Cow<'_, str>>(owned_cow.clone());
+        try_from_rountrip::<u16, Cow<'_, str>>(owned_cow.clone());
+        try_from_rountrip::<u8, Cow<'_, str>>(owned_cow);
+    }
+
+    #[test]
+    fn test_try_from_rc_str() {
+        let static_string = "Test string";
+
+        let rc_string: Rc<str> = static_string.into();
+
+        let value: FixedString::<u8> = rc_string.try_into().unwrap();
+
+        assert_eq!(static_string, value);
+
+        let rc_string: Rc<str> = value.into();
+
+        assert_eq!(static_string, rc_string.as_ref());
+    }
+
+    #[test]
+    fn test_try_from_arc_str() {
+        let static_string = "Test string";
+
+        let arc_string: Arc<str> = static_string.into();
+
+        let value: FixedString::<u8> = arc_string.try_into().unwrap();
+
+        assert_eq!(static_string, value);
+
+        let arc_string: Arc<str> = value.into();
+
+        assert_eq!(static_string, arc_string.as_ref());
+    }
 }