Polish compact decimal API (unicode-org#7759)

unicode-org#3647

## Changelog: N/A

Author: robertbastian

components/decimal/Cargo.toml

@@ -51,12 +51,12 @@ criterion = { workspace = true }
 
 [features]
 default = ["compiled_data"]
-serde = ["dep:serde", "icu_provider/serde", "zerovec/serde", "icu_pattern?/serde", "icu_plurals?/serde"]
+serde = ["alloc", "dep:serde", "icu_provider/serde", "zerovec/serde", "icu_pattern?/serde", "icu_plurals?/serde"]
 datagen = ["serde", "dep:databake", "zerovec/databake", "icu_provider/export", "alloc", "icu_plurals?/datagen"]
 compiled_data = ["dep:icu_decimal_data", "dep:icu_locale", "icu_locale?/compiled_data", "icu_provider/baked", "icu_plurals?/compiled_data"]
 ryu = ["fixed_decimal/ryu"]
 alloc = ["serde?/alloc", "zerovec/alloc"]
-unstable = ["alloc", "dep:icu_pattern", "dep:icu_plurals"]
+unstable = ["dep:icu_pattern", "dep:icu_plurals"]
 
 [[bench]]
 name = "fixed_decimal_format"

components/decimal/src/compact_formatter.rs

@@ -4,15 +4,18 @@
 
 use core::fmt::Display;
 
+use crate::input::Decimal;
+use crate::Cow;
 use crate::{
-    error::ExponentError,
+    error::CompactExponentError,
     options::CompactDecimalFormatterOptions,
     preferences::{CompactDecimalFormatterPreferences, DecimalFormatterPreferences},
     provider::*,
     DecimalFormatter,
 };
-use alloc::borrow::Cow;
-use fixed_decimal::{Decimal, UnsignedDecimal};
+#[cfg(feature = "alloc")]
+use alloc::string::String;
+use fixed_decimal::UnsignedDecimal;
 use icu_pattern::{Pattern, PatternBackend, SinglePlaceholder};
 use icu_plurals::PluralRules;
 use icu_provider::DataError;
@@ -72,9 +75,7 @@ pub struct CompactDecimalFormatter {
 }
 
 impl CompactDecimalFormatter {
-    /// Constructor that takes a selected locale and a list of preferences,
-    /// then collects all compiled data necessary to format numbers in short compact
-    /// decimal notation for the given locale.
+    /// Creates a new short [`CompactDecimalFormatter`] from compiled data and an options bag.
     ///
     /// ✨ *Enabled with the `compiled_data` Cargo feature.*
     ///
@@ -85,11 +86,14 @@ impl CompactDecimalFormatter {
     /// ```
     /// use icu::decimal::CompactDecimalFormatter;
     /// use icu::locale::locale;
+    /// use writeable::assert_writeable_eq;
     ///
-    /// CompactDecimalFormatter::try_new_short(
+    /// let formatter = CompactDecimalFormatter::try_new_short(
     ///     locale!("sv").into(),
     ///     Default::default(),
-    /// );
+    /// ).unwrap();
+    ///
+    /// assert_writeable_eq!(formatter.format(&1234.into()), "1,2 tn");
     /// ```
     #[cfg(feature = "compiled_data")]
     pub fn try_new_short(
@@ -98,10 +102,7 @@ impl CompactDecimalFormatter {
     ) -> Result<Self, DataError> {
         let locale = DecimalCompactShortV1::make_locale(prefs.locale_preferences);
         Ok(Self {
-            decimal_formatter: DecimalFormatter::try_new(
-                (&prefs).into(),
-                options.decimal_formatter_options,
-            )?,
+            decimal_formatter: DecimalFormatter::try_new((&prefs).into(), options.into())?,
             plural_rules: PluralRules::try_new_cardinal((&prefs).into())?,
             compact_data: load_with_fallback::<DecimalCompactShortV1>(
                 &Baked,
@@ -143,7 +144,7 @@ impl CompactDecimalFormatter {
             decimal_formatter: DecimalFormatter::try_new_unstable(
                 provider,
                 (&prefs).into(),
-                options.decimal_formatter_options,
+                options.into(),
             )?,
             plural_rules: PluralRules::try_new_cardinal_unstable(provider, (&prefs).into())?,
             compact_data: load_with_fallback::<DecimalCompactShortV1>(
@@ -158,9 +159,7 @@ impl CompactDecimalFormatter {
         })
     }
 
-    /// Constructor that takes a selected locale and a list of preferences,
-    /// then collects all compiled data necessary to format numbers in short compact
-    /// decimal notation for the given locale.
+    /// Creates a new long [`CompactDecimalFormatter`] from compiled data and an options bag.
     ///
     /// ✨ *Enabled with the `compiled_data` Cargo feature.*
     ///
@@ -171,11 +170,14 @@ impl CompactDecimalFormatter {
     /// ```
     /// use icu::decimal::CompactDecimalFormatter;
     /// use icu::locale::locale;
+    /// use writeable::assert_writeable_eq;
     ///
-    /// CompactDecimalFormatter::try_new_long(
+    /// let formatter = CompactDecimalFormatter::try_new_long(
     ///     locale!("sv").into(),
     ///     Default::default(),
-    /// );
+    /// ).unwrap();
+    ///
+    /// assert_writeable_eq!(formatter.format(&1234.into()), "1,2 tusen");
     /// ```
     #[cfg(feature = "compiled_data")]
     pub fn try_new_long(
@@ -184,10 +186,7 @@ impl CompactDecimalFormatter {
     ) -> Result<Self, DataError> {
         let locale = DecimalCompactLongV1::make_locale(prefs.locale_preferences);
         Ok(Self {
-            decimal_formatter: DecimalFormatter::try_new(
-                (&prefs).into(),
-                options.decimal_formatter_options,
-            )?,
+            decimal_formatter: DecimalFormatter::try_new((&prefs).into(), options.into())?,
             plural_rules: PluralRules::try_new_cardinal((&prefs).into())?,
             compact_data: load_with_fallback::<DecimalCompactLongV1>(
                 &Baked,
@@ -229,7 +228,7 @@ impl CompactDecimalFormatter {
             decimal_formatter: DecimalFormatter::try_new_unstable(
                 provider,
                 (&prefs).into(),
-                options.decimal_formatter_options,
+                options.into(),
             )?,
             plural_rules: PluralRules::try_new_cardinal_unstable(provider, (&prefs).into())?,
             compact_data: load_with_fallback::<DecimalCompactLongV1>(
@@ -249,14 +248,11 @@ impl CompactDecimalFormatter {
     /// The result may have a fractional digit only if it is compact and its
     /// significand is less than 10. Trailing fractional 0s are omitted.
     ///
-    /// Because the Decimal is mutated before formatting, this function
-    /// takes ownership of it.
-    ///
     /// # Examples
     ///
     /// ```
     /// use icu::decimal::CompactDecimalFormatter;
-    /// use icu::decimal::input::{Decimal, FloatPrecision, SignDisplay};
+    /// use icu::decimal::input::{Decimal, SignDisplay};
     /// use icu::locale::locale;
     /// use writeable::assert_writeable_eq;
     ///
@@ -303,14 +299,6 @@ impl CompactDecimalFormatter {
     ///     ),
     ///     "+2.5K"
     /// );
-    ///
-    /// // Floating point inputs should use FloatPrecision::RoundTrip
-    /// assert_writeable_eq!(
-    ///     short_english.format(
-    ///         &Decimal::try_from_f64(999_499.99, FloatPrecision::RoundTrip).unwrap()
-    ///     ),
-    ///     "999K"
-    /// );
     /// ```
     ///
     /// The result is the nearest such compact number, with halfway cases-
@@ -354,6 +342,26 @@ impl CompactDecimalFormatter {
     ///     "0.22"
     /// );
     /// ```
+    ///
+    /// Floating point inputs should use [`FloatPrecision::RoundTrip`](fixed_decimal::FloatPrecision::RoundTrip).
+    ///
+    /// ```
+    /// # use icu::decimal::input::{Decimal, FloatPrecision};
+    /// # use icu::decimal::CompactDecimalFormatter;
+    /// # use icu::locale::locale;
+    /// # use writeable::assert_writeable_eq;
+    /// #
+    /// # let short_english = CompactDecimalFormatter::try_new_short(
+    /// #    locale!("en").into(),
+    /// #    Default::default(),
+    /// # ).unwrap();
+    /// assert_writeable_eq!(
+    ///     short_english.format(
+    ///         &Decimal::try_from_f64(999_499.99, FloatPrecision::RoundTrip).unwrap()
+    ///     ),
+    ///     "999K"
+    /// );
+    /// ```
     pub fn format(&self, value: &Decimal) -> impl Writeable + Display + '_ {
         let (compact_pattern, significand) = self
             .compact_data
@@ -370,6 +378,15 @@ impl CompactDecimalFormatter {
         )
     }
 
+    /// Formats a [`Decimal`], returning a [`String`].
+    ///
+    /// ✨ *Enabled with the `alloc` Cargo feature.*
+    #[cfg(feature = "alloc")]
+    pub fn format_to_string(&self, value: &Decimal) -> String {
+        use writeable::Writeable;
+        self.format(value).write_to_string().into_owned()
+    }
+
     /// Formats a [`Decimal`] with a given exponent according to locale data.
     ///
     /// This is an advanced API; prefer using [`Self::format()`] in simple
@@ -395,7 +412,7 @@ impl CompactDecimalFormatter {
     /// # use icu::locale::locale;
     /// # use writeable::assert_writeable_eq;
     /// # use std::str::FromStr;
-    ///
+    /// #
     /// # let short_french = CompactDecimalFormatter::try_new_short(
     /// #    locale!("fr").into(),
     /// #    Default::default(),
@@ -470,7 +487,7 @@ impl CompactDecimalFormatter {
         &'l self,
         significand: &'l Decimal,
         exponent: u8,
-    ) -> Result<impl Writeable + Display + 'l, ExponentError> {
+    ) -> Result<impl Writeable + Display + 'l, CompactExponentError> {
         let log10_type = significand.absolute.nonzero_magnitude_start() + i16::from(exponent);
 
         let (pattern, expected_exponent) = self
@@ -491,10 +508,10 @@ impl CompactDecimalFormatter {
             .unwrap_or((Pattern::<SinglePlaceholder>::PASS_THROUGH, 0));
 
         if exponent != expected_exponent {
-            return Err(ExponentError {
+            return Err(CompactExponentError {
                 actual: exponent,
                 expected: expected_exponent,
-                log10_type,
+                magnitude: log10_type,
             });
         }
 

components/decimal/src/decimal_formatter.rs

@@ -7,14 +7,15 @@ use core::fmt::Write;
 use writeable::Part;
 
 use crate::grouper;
+use crate::input::Decimal;
 use crate::options::*;
 use crate::parts;
 use crate::provider::*;
 use crate::size_test_macro::size_test;
 use crate::DecimalFormatterPreferences;
 #[cfg(feature = "alloc")]
 use alloc::string::String;
-use fixed_decimal::{Decimal, Sign, UnsignedDecimal};
+use fixed_decimal::{Sign, UnsignedDecimal};
 use icu_provider::prelude::*;
 use writeable::PartsWrite;
 use writeable::Writeable;

components/decimal/src/error.rs

@@ -6,9 +6,8 @@
 
 use displaydoc::Display;
 
-/// An error due to a [`CompactDecimal`](fixed_decimal::CompactDecimal) with an
-/// exponent inconsistent with the compact decimal data for the locale, e.g.,
-/// when formatting 1c5 in English (US).
+/// An error due to incorrect arguments to
+/// [`CompactDecimalFormatter::format_with_exponent`](crate::CompactDecimalFormatter::format_with_exponent).
 ///
 /// <div class="stab unstable">
 /// 🚧 This code is considered unstable; it may change at any time, in breaking or non-breaking ways,
@@ -19,14 +18,14 @@ use displaydoc::Display;
 ///
 /// ✨ *Enabled with the `unstable` Cargo feature.*
 #[derive(Display, Copy, Clone, Debug)]
-#[displaydoc("Expected compact exponent {expected} for 10^{log10_type}, got {actual}")]
-pub struct ExponentError {
+#[displaydoc("Expected compact exponent {expected} for 10^{magnitude}, got {actual}")]
+pub struct CompactExponentError {
     /// The compact decimal exponent passed to the formatter.
     pub(crate) actual: u8,
-    /// The appropriate compact decimal exponent for a number of the given magnitude.
+    /// The appropriate exponent for a number of the given magnitude.
     pub(crate) expected: u8,
     /// The magnitude of the number being formatted.
-    pub(crate) log10_type: i16,
+    pub(crate) magnitude: i16,
 }
 
-impl core::error::Error for ExponentError {}
+impl core::error::Error for CompactExponentError {}

components/decimal/src/lib.rs

@@ -98,15 +98,18 @@ pub use alloc::borrow::Cow;
 #[doc(hidden)] // TODO(#3647): should be private
 pub enum Cow<'a, T> {
     Borrowed(&'a T),
+    Owned(T),
 }
 
 #[cfg(not(feature = "alloc"))]
 impl<'a, T> core::ops::Deref for Cow<'a, T> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {
-        let Self::Borrowed(r) = self;
-        r
+        match self {
+            Self::Borrowed(r) => r,
+            Self::Owned(ref r) => r,
+        }
     }
 }
 

components/decimal/src/options.rs

@@ -82,24 +82,33 @@ pub enum GroupingStrategy {
 #[derive(Debug, Eq, PartialEq, Clone)]
 #[non_exhaustive]
 pub struct CompactDecimalFormatterOptions {
-    /// Options to configure the inner [`crate::DecimalFormatter`].
-    pub decimal_formatter_options: DecimalFormatterOptions,
+    /// When to render grouping separators.
+    ///
+    /// Default is [`GroupingStrategy::Min2`]
+    pub grouping_strategy: Option<GroupingStrategy>,
 }
 
 #[cfg(feature = "unstable")]
 impl Default for CompactDecimalFormatterOptions {
     fn default() -> Self {
-        Self {
-            decimal_formatter_options: GroupingStrategy::Min2.into(),
-        }
+        GroupingStrategy::Min2.into()
     }
 }
 
 #[cfg(feature = "unstable")]
 impl From<DecimalFormatterOptions> for CompactDecimalFormatterOptions {
     fn from(decimal_formatter_options: DecimalFormatterOptions) -> Self {
         Self {
-            decimal_formatter_options,
+            grouping_strategy: decimal_formatter_options.grouping_strategy,
+        }
+    }
+}
+
+#[cfg(feature = "unstable")]
+impl From<CompactDecimalFormatterOptions> for DecimalFormatterOptions {
+    fn from(decimal_formatter_options: CompactDecimalFormatterOptions) -> Self {
+        Self {
+            grouping_strategy: decimal_formatter_options.grouping_strategy,
         }
     }
 }
@@ -108,7 +117,7 @@ impl From<DecimalFormatterOptions> for CompactDecimalFormatterOptions {
 impl From<GroupingStrategy> for CompactDecimalFormatterOptions {
     fn from(grouping_strategy: GroupingStrategy) -> Self {
         Self {
-            decimal_formatter_options: grouping_strategy.into(),
+            grouping_strategy: Some(grouping_strategy),
         }
     }
 }

tools/make/diplomat-coverage/src/allowlist.rs

@@ -296,7 +296,7 @@ lazy_static::lazy_static! {
         // =========================
 
         "icu::decimal::CompactDecimalFormatter",
-        "icu::decimal::error::ExponentError",
+        "icu::decimal::error::CompactExponentError",
         "icu::decimal::options::CompactDecimalFormatterOptions",
 
         "icu::experimental",