Don't return TinyStr errors over FFI (#4940)

TinyStr is an ICU4X implementation detail, especially over FFI. Return domain-specific errors instead. Also cleaned up UTF-8 errors that were being returned as IO errors.
unicode-org · May 24, 2024 · 79e4f9e · 79e4f9e
1 parent b6dad6c
commit 79e4f9e
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 47 deletions.
diff --git a/ffi/capi/src/casemap.rs b/ffi/capi/src/casemap.rs
@@ -74,9 +74,9 @@ pub mod ffi {
             locale: &ICU4XLocale,
             write: &mut DiplomatWriteable,
         ) -> Result<(), ICU4XError> {
-            let s =
-                core::str::from_utf8(s).map_err(|e| ICU4XError::DataIoError.log_original(&e))?;
-            self.0.lowercase(s, &locale.0.id).write_to(write)?;
+            self.0
+                .lowercase(core::str::from_utf8(s)?, &locale.0.id)
+                .write_to(write)?;
 
             Ok(())
         }
@@ -90,9 +90,9 @@ pub mod ffi {
             locale: &ICU4XLocale,
             write: &mut DiplomatWriteable,
         ) -> Result<(), ICU4XError> {
-            let s =
-                core::str::from_utf8(s).map_err(|e| ICU4XError::DataIoError.log_original(&e))?;
-            self.0.uppercase(s, &locale.0.id).write_to(write)?;
+            self.0
+                .uppercase(core::str::from_utf8(s)?, &locale.0.id)
+                .write_to(write)?;
 
             Ok(())
         }
@@ -119,10 +119,12 @@ pub mod ffi {
             options: ICU4XTitlecaseOptionsV1,
             write: &mut DiplomatWriteable,
         ) -> Result<(), ICU4XError> {
-            let s =
-                core::str::from_utf8(s).map_err(|e| ICU4XError::DataIoError.log_original(&e))?;
             self.0
-                .titlecase_segment_with_only_case_data(s, &locale.0.id, options.into())
+                .titlecase_segment_with_only_case_data(
+                    core::str::from_utf8(s)?,
+                    &locale.0.id,
+                    options.into(),
+                )
                 .write_to(write)?;
 
             Ok(())
@@ -136,9 +138,7 @@ pub mod ffi {
             s: &DiplomatStr,
             write: &mut DiplomatWriteable,
         ) -> Result<(), ICU4XError> {
-            let s =
-                core::str::from_utf8(s).map_err(|e| ICU4XError::DataIoError.log_original(&e))?;
-            self.0.fold(s).write_to(write)?;
+            self.0.fold(core::str::from_utf8(s)?).write_to(write)?;
 
             Ok(())
         }
@@ -151,9 +151,9 @@ pub mod ffi {
             s: &DiplomatStr,
             write: &mut DiplomatWriteable,
         ) -> Result<(), ICU4XError> {
-            let s =
-                core::str::from_utf8(s).map_err(|e| ICU4XError::DataIoError.log_original(&e))?;
-            self.0.fold_turkic(s).write_to(write)?;
+            self.0
+                .fold_turkic(core::str::from_utf8(s)?)
+                .write_to(write)?;
 
             Ok(())
         }
@@ -327,10 +327,8 @@ pub mod ffi {
             options: ICU4XTitlecaseOptionsV1,
             write: &mut DiplomatWriteable,
         ) -> Result<(), ICU4XError> {
-            let s =
-                core::str::from_utf8(s).map_err(|e| ICU4XError::DataIoError.log_original(&e))?;
             self.0
-                .titlecase_segment(s, &locale.0.id, options.into())
+                .titlecase_segment(core::str::from_utf8(s)?, &locale.0.id, options.into())
                 .write_to(write)?;
 
             Ok(())

diff --git a/ffi/capi/src/date.rs b/ffi/capi/src/date.rs
@@ -188,8 +188,12 @@ pub mod ffi {
             day: u8,
             calendar: &ICU4XCalendar,
         ) -> Result<Box<ICU4XDate>, ICU4XError> {
-            let era = TinyAsciiStr::from_bytes(era_code)?.into();
-            let month = TinyAsciiStr::from_bytes(month_code)?.into();
+            let era = TinyAsciiStr::from_bytes(era_code)
+                .map_err(|_| ICU4XError::CalendarUnknownEraError)?
+                .into();
+            let month = TinyAsciiStr::from_bytes(month_code)
+                .map_err(|_| ICU4XError::CalendarUnknownMonthCodeError)?
+                .into();
             let cal = calendar.0.clone();
             Ok(Box::new(ICU4XDate(Date::try_new_from_codes(
                 era, year, month, day, cal,

diff --git a/ffi/capi/src/datetime.rs b/ffi/capi/src/datetime.rs
@@ -268,8 +268,12 @@ pub mod ffi {
             nanosecond: u32,
             calendar: &ICU4XCalendar,
         ) -> Result<Box<ICU4XDateTime>, ICU4XError> {
-            let era = TinyAsciiStr::from_bytes(era_code)?.into();
-            let month = TinyAsciiStr::from_bytes(month_code)?.into();
+            let era = TinyAsciiStr::from_bytes(era_code)
+                .map_err(|_| ICU4XError::CalendarUnknownEraError)?
+                .into();
+            let month = TinyAsciiStr::from_bytes(month_code)
+                .map_err(|_| ICU4XError::CalendarUnknownMonthCodeError)?
+                .into();
             let cal = calendar.0.clone();
             let hour = hour.try_into()?;
             let minute = minute.try_into()?;

diff --git a/ffi/capi/src/errors.rs b/ffi/capi/src/errors.rs
@@ -36,7 +36,6 @@ use icu_provider::{DataError, DataErrorKind};
 use icu_segmenter::SegmenterError;
 #[cfg(any(feature = "icu_timezone", feature = "icu_datetime"))]
 use icu_timezone::TimeZoneError;
-use tinystr::TinyStrError;
 
 #[diplomat::bridge]
 pub mod ffi {
@@ -141,6 +140,7 @@ pub mod ffi {
         DateTimeFixedDecimalError = 0x8_07,
         DateTimeMismatchedCalendarError = 0x8_08,
 
+        // dead
         // tinystr errors
         TinyStrTooLargeError = 0x9_00,
         TinyStrContainsNullError = 0x9_01,
@@ -385,18 +385,6 @@ impl From<ParserError> for ICU4XError {
     }
 }
 
-impl From<TinyStrError> for ICU4XError {
-    fn from(e: TinyStrError) -> Self {
-        match e {
-            TinyStrError::TooLarge { .. } => ICU4XError::TinyStrTooLargeError,
-            TinyStrError::ContainsNull => ICU4XError::TinyStrContainsNullError,
-            TinyStrError::NonAscii => ICU4XError::TinyStrNonAsciiError,
-            _ => ICU4XError::UnknownError,
-        }
-        .log_original(&e)
-    }
-}
-
 #[cfg(any(feature = "icu_timezone", feature = "icu_datetime"))]
 impl From<TimeZoneError> for ICU4XError {
     fn from(e: TimeZoneError) -> Self {

diff --git a/ffi/capi/src/properties_sets.rs b/ffi/capi/src/properties_sets.rs
@@ -937,11 +937,7 @@ pub mod ffi {
             provider: &ICU4XDataProvider,
             property_name: &DiplomatStr,
         ) -> Result<Box<ICU4XCodePointSetData>, ICU4XError> {
-            let name = if let Ok(s) = str::from_utf8(property_name) {
-                s
-            } else {
-                return Err(ICU4XError::TinyStrNonAsciiError);
-            };
+            let name = str::from_utf8(property_name)?;
             Ok(Box::new(ICU4XCodePointSetData(call_constructor_unstable!(
                 sets::load_for_ecma262 [r => r.map(|r| r.static_to_owned())],
                 sets::load_for_ecma262_unstable,

diff --git a/ffi/capi/src/timezone.rs b/ffi/capi/src/timezone.rs
@@ -146,7 +146,8 @@ pub mod ffi {
         )]
         pub fn try_set_time_zone_id(&mut self, id: &DiplomatStr) -> Result<(), ICU4XError> {
             self.0.time_zone_id = Some(icu_timezone::TimeZoneBcp47Id(
-                tinystr::TinyAsciiStr::from_bytes(id)?,
+                tinystr::TinyAsciiStr::from_bytes(id)
+                    .map_err(|_| ICU4XError::TimeZoneInvalidIdError)?,
             ));
             Ok(())
         }
@@ -227,9 +228,10 @@ pub mod ffi {
         #[diplomat::rust_link(icu::timezone::MetazoneId, Struct, compact)]
         #[diplomat::rust_link(icu::timezone::MetazoneId::from_str, FnInStruct, hidden)]
         pub fn try_set_metazone_id(&mut self, id: &DiplomatStr) -> Result<(), ICU4XError> {
-            self.0.metazone_id = Some(icu_timezone::MetazoneId(tinystr::TinyAsciiStr::from_bytes(
-                id,
-            )?));
+            self.0.metazone_id = Some(icu_timezone::MetazoneId(
+                tinystr::TinyAsciiStr::from_bytes(id)
+                    .map_err(|_| ICU4XError::TimeZoneInvalidIdError)?,
+            ));
             Ok(())
         }
 
@@ -268,7 +270,8 @@ pub mod ffi {
         #[diplomat::rust_link(icu::timezone::ZoneVariant::from_str, FnInStruct, hidden)]
         pub fn try_set_zone_variant(&mut self, id: &DiplomatStr) -> Result<(), ICU4XError> {
             self.0.zone_variant = Some(icu_timezone::ZoneVariant(
-                tinystr::TinyAsciiStr::from_bytes(id)?,
+                tinystr::TinyAsciiStr::from_bytes(id)
+                    .map_err(|_| ICU4XError::TimeZoneInvalidIdError)?,
             ));
             Ok(())
         }

diff --git a/ffi/capi/src/timezone_mapper.rs b/ffi/capi/src/timezone_mapper.rs
@@ -105,7 +105,9 @@ pub mod ffi {
         ) -> Result<(), ICU4XError> {
             use writeable::Writeable;
             let handle = self.0.as_borrowed();
-            let bcp47_id = TimeZoneBcp47Id(TinyAsciiStr::from_bytes(value)?);
+            let bcp47_id = TimeZoneBcp47Id(
+                TinyAsciiStr::from_bytes(value).map_err(|_| ICU4XError::TimeZoneInvalidIdError)?,
+            );
             if let Some(s) = handle.find_canonical_iana_from_bcp47(bcp47_id) {
                 Ok(s.write_to(write)?)
             } else {
@@ -194,7 +196,9 @@ pub mod ffi {
         ) -> Result<(), ICU4XError> {
             use writeable::Writeable;
             let handle = self.0.as_borrowed();
-            let bcp47_id = TimeZoneBcp47Id(TinyAsciiStr::from_bytes(value)?);
+            let bcp47_id = TimeZoneBcp47Id(
+                TinyAsciiStr::from_bytes(value).map_err(|_| ICU4XError::TimeZoneInvalidIdError)?,
+            );
             if let Some(s) = handle.canonical_iana_from_bcp47(bcp47_id) {
                 Ok(s.write_to(write)?)
             } else {