From 396e049a7a9d891fab6ea90dd2943e8e5babc1b9 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 20 Nov 2023 11:52:19 +1100 Subject: [PATCH] Use InputString instead of String Done so that we can correctly support builds without an allocator. Add two new error types that wrap `InputString`. --- units/Cargo.toml | 4 +-- units/src/amount.rs | 70 ++++++++++++++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/units/Cargo.toml b/units/Cargo.toml index f564a550..6f677deb 100644 --- a/units/Cargo.toml +++ b/units/Cargo.toml @@ -14,8 +14,8 @@ exclude = ["tests", "contrib"] [features] default = ["std"] -std = ["alloc"] -alloc = [] +std = ["alloc", "internals/std"] +alloc = ["internals/alloc"] [package.metadata.docs.rs] all-features = true diff --git a/units/src/amount.rs b/units/src/amount.rs index 3b72e6f4..e61cfaa6 100644 --- a/units/src/amount.rs +++ b/units/src/amount.rs @@ -12,6 +12,7 @@ use core::{default, ops}; #[cfg(feature = "serde")] use ::serde::{Deserialize, Serialize}; +use internals::error::InputString; use internals::write_err; #[cfg(feature = "alloc")] @@ -129,12 +130,12 @@ impl FromStr for Denomination { use self::ParseDenominationError::*; if CONFUSING_FORMS.contains(&s) { - return Err(PossiblyConfusing(s.to_string())); + return Err(PossiblyConfusing(PossiblyConfusingDenominationError(s.into()))); }; let form = self::Denomination::forms(s); - form.ok_or_else(|| Unknown(s.to_string())) + form.ok_or_else(|| Unknown(UnknownDenominationError(s.into()))) } } @@ -196,9 +197,9 @@ impl From for ParseAmountError { #[non_exhaustive] pub enum ParseDenominationError { /// The denomination was unknown. - Unknown(String), + Unknown(UnknownDenominationError), /// The denomination has multiple possible interpretations. - PossiblyConfusing(String), + PossiblyConfusing(PossiblyConfusingDenominationError), } impl fmt::Display for ParseDenominationError { @@ -206,16 +207,8 @@ impl fmt::Display for ParseDenominationError { use ParseDenominationError::*; match *self { - Unknown(ref d) => write!(f, "unknown denomination: {}", d), - PossiblyConfusing(ref d) => { - let (letter, upper, lower) = match d.chars().next() { - Some('M') => ('M', "Mega", "milli"), - Some('P') => ('P', "Peta", "pico"), - // This panic could be avoided by adding enum ConfusingDenomination { Mega, Peta } but is it worth it? - _ => panic!("invalid error information"), - }; - write!(f, "the '{}' at the beginning of {} should technically mean '{}' but that denomination is uncommon and maybe '{}' was intended", letter, d, upper, lower) - } + Unknown(ref e) => write_err!(f, "denomination parse error"; e), + PossiblyConfusing(ref e) => write_err!(f, "denomination parse error"; e), } } } @@ -231,6 +224,38 @@ impl std::error::Error for ParseDenominationError { } } +/// Parsing error, unknown denomination. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct UnknownDenominationError(InputString); + +impl fmt::Display for UnknownDenominationError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.unknown_variant("bitcoin denomination", f) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for UnknownDenominationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Parsing error, possibly confusing denomination. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct PossiblyConfusingDenominationError(InputString); + +impl fmt::Display for PossiblyConfusingDenominationError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}: possibly confusing denomination - we intentionally do not support 'M' and 'P' so as to not confuse mega/milli and peta/pico", self.0.display_cannot_parse("bitcoin denomination")) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for PossiblyConfusingDenominationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + fn is_too_precise(s: &str, precision: usize) -> bool { match s.find('.') { Some(pos) => @@ -1994,21 +2019,23 @@ mod tests { #[test] #[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin. fn from_str() { + use ParseDenominationError::*; + use super::ParseAmountError as E; assert_eq!(Amount::from_str("x BTC"), Err(E::InvalidCharacter('x'))); assert_eq!( Amount::from_str("xBTC"), - Err(ParseDenominationError::Unknown("xBTC".into()).into()) + Err(Unknown(UnknownDenominationError("xBTC".into())).into()), ); assert_eq!( Amount::from_str("5 BTC BTC"), - Err(ParseDenominationError::Unknown("BTC BTC".into()).into()) + Err(Unknown(UnknownDenominationError("BTC BTC".into())).into()), ); assert_eq!(Amount::from_str("5BTC BTC"), Err(E::InvalidCharacter('B'))); assert_eq!( Amount::from_str("5 5 BTC"), - Err(ParseDenominationError::Unknown("5 BTC".into()).into()) + Err(Unknown(UnknownDenominationError("5 BTC".into())).into()), ); #[track_caller] @@ -2023,7 +2050,7 @@ mod tests { assert_eq!(SignedAmount::from_str(&s.replace(' ', "")), expected); } - case("5 BCH", Err(ParseDenominationError::Unknown("BCH".into()).into())); + case("5 BCH", Err(Unknown(UnknownDenominationError("BCH".into())).into())); case("-1 BTC", Err(E::Negative)); case("-0.0 BTC", Err(E::Negative)); @@ -2133,7 +2160,10 @@ mod tests { #[test] fn to_string_with_denomination_from_str_roundtrip() { + use ParseDenominationError::*; + use super::Denomination as D; + let amt = Amount::from_sat(42); let denom = Amount::to_string_with_denomination; assert_eq!(Amount::from_str(&denom(amt, D::Bitcoin)), Ok(amt)); @@ -2147,11 +2177,11 @@ mod tests { assert_eq!( Amount::from_str("42 satoshi BTC"), - Err(ParseDenominationError::Unknown("satoshi BTC".to_owned()).into()), + Err(Unknown(UnknownDenominationError("satoshi BTC".into())).into()), ); assert_eq!( SignedAmount::from_str("-42 satoshi BTC"), - Err(ParseDenominationError::Unknown("satoshi BTC".to_owned()).into()), + Err(Unknown(UnknownDenominationError("satoshi BTC".into())).into()), ); }