From d595f421c67390c13d896a93df2ddfdaf354e1b4 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Nov 2024 13:45:01 +1100 Subject: [PATCH 1/3] Remove whitespace between enum variants We don't tend to put whitespace between enum variants. --- units/src/amount/error.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/units/src/amount/error.rs b/units/src/amount/error.rs index 8c0e79fad..81f3c3b31 100644 --- a/units/src/amount/error.rs +++ b/units/src/amount/error.rs @@ -15,10 +15,8 @@ use super::INPUT_STRING_LEN_LIMIT; pub enum ParseError { /// Invalid amount. Amount(ParseAmountError), - /// Invalid denomination. Denomination(ParseDenominationError), - /// The denomination was not identified. MissingDenomination(MissingDenominationError), } From 23c77275b1ce9b9a8ff69efb8c87cad297b9e506 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Nov 2024 13:57:28 +1100 Subject: [PATCH 2/3] Reduce code comment lines Make the comment more terse by using 'info' instead of 'information' and reduce the line count by 2. --- units/src/amount/error.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/units/src/amount/error.rs b/units/src/amount/error.rs index 81f3c3b31..083e87969 100644 --- a/units/src/amount/error.rs +++ b/units/src/amount/error.rs @@ -56,8 +56,7 @@ impl fmt::Display for ParseError { match self { ParseError::Amount(error) => write_err!(f, "invalid amount"; error), ParseError::Denomination(error) => write_err!(f, "invalid denomination"; error), - // We consider this to not be a source because it currently doesn't contain useful - // information + // We consider this to not be a source because it currently doesn't contain useful info. ParseError::MissingDenomination(_) => f.write_str("the input doesn't contain a denomination"), } @@ -70,8 +69,7 @@ impl std::error::Error for ParseError { match self { ParseError::Amount(error) => Some(error), ParseError::Denomination(error) => Some(error), - // We consider this to not be a source because it currently doesn't contain useful - // information + // We consider this to not be a source because it currently doesn't contain useful info. ParseError::MissingDenomination(_) => None, } } From fd2a5c1ec79f337fb3695c030c9fb6b4671468f2 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 28 Nov 2024 14:17:39 +1100 Subject: [PATCH 3/3] Close amounts error types Close the two pubic enum error types in `units::amounts`. All the other structs are closed already because they either have private fields or marked `non_exhaustive`. --- units/src/amount/error.rs | 66 +++++++++++++++++++++--------------- units/src/amount/mod.rs | 14 ++++---- units/src/amount/signed.rs | 13 ++++--- units/src/amount/tests.rs | 14 ++++---- units/src/amount/unsigned.rs | 7 ++-- 5 files changed, 67 insertions(+), 47 deletions(-) diff --git a/units/src/amount/error.rs b/units/src/amount/error.rs index 083e87969..55939b5ae 100644 --- a/units/src/amount/error.rs +++ b/units/src/amount/error.rs @@ -11,8 +11,10 @@ use super::INPUT_STRING_LEN_LIMIT; /// An error during amount parsing amount with denomination. #[derive(Debug, Clone, PartialEq, Eq)] -#[non_exhaustive] -pub enum ParseError { +pub struct ParseError(pub(crate) ParseErrorInner); + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ParseErrorInner { /// Invalid amount. Amount(ParseAmountError), /// Invalid denomination. @@ -22,42 +24,43 @@ pub enum ParseError { } internals::impl_from_infallible!(ParseError); +internals::impl_from_infallible!(ParseErrorInner); impl From for ParseError { - fn from(e: ParseAmountError) -> Self { Self::Amount(e) } + fn from(e: ParseAmountError) -> Self { Self(ParseErrorInner::Amount(e)) } } impl From for ParseError { - fn from(e: ParseDenominationError) -> Self { Self::Denomination(e) } + fn from(e: ParseDenominationError) -> Self { Self(ParseErrorInner::Denomination(e)) } } impl From for ParseError { - fn from(e: OutOfRangeError) -> Self { Self::Amount(e.into()) } + fn from(e: OutOfRangeError) -> Self { Self(ParseErrorInner::Amount(e.into())) } } impl From for ParseError { - fn from(e: TooPreciseError) -> Self { Self::Amount(e.into()) } + fn from(e: TooPreciseError) -> Self { Self(ParseErrorInner::Amount(e.into())) } } impl From for ParseError { - fn from(e: MissingDigitsError) -> Self { Self::Amount(e.into()) } + fn from(e: MissingDigitsError) -> Self { Self(ParseErrorInner::Amount(e.into())) } } impl From for ParseError { - fn from(e: InputTooLargeError) -> Self { Self::Amount(e.into()) } + fn from(e: InputTooLargeError) -> Self { Self(ParseErrorInner::Amount(e.into())) } } impl From for ParseError { - fn from(e: InvalidCharacterError) -> Self { Self::Amount(e.into()) } + fn from(e: InvalidCharacterError) -> Self { Self(ParseErrorInner::Amount(e.into())) } } impl fmt::Display for ParseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - ParseError::Amount(error) => write_err!(f, "invalid amount"; error), - ParseError::Denomination(error) => write_err!(f, "invalid denomination"; error), + match self.0 { + ParseErrorInner::Amount(ref e) => write_err!(f, "invalid amount"; e), + ParseErrorInner::Denomination(ref e) => write_err!(f, "invalid denomination"; e), // We consider this to not be a source because it currently doesn't contain useful info. - ParseError::MissingDenomination(_) => + ParseErrorInner::MissingDenomination(_) => f.write_str("the input doesn't contain a denomination"), } } @@ -66,19 +69,21 @@ impl fmt::Display for ParseError { #[cfg(feature = "std")] impl std::error::Error for ParseError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - ParseError::Amount(error) => Some(error), - ParseError::Denomination(error) => Some(error), + match self.0 { + ParseErrorInner::Amount(ref e) => Some(e), + ParseErrorInner::Denomination(ref e) => Some(e), // We consider this to not be a source because it currently doesn't contain useful info. - ParseError::MissingDenomination(_) => None, + ParseErrorInner::MissingDenomination(_) => None, } } } /// An error during amount parsing. #[derive(Debug, Clone, PartialEq, Eq)] -#[non_exhaustive] -pub enum ParseAmountError { +pub struct ParseAmountError(pub(crate) ParseAmountErrorInner); + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ParseAmountErrorInner { /// The amount is too big or too small. OutOfRange(OutOfRangeError), /// Amount has higher precision than supported by the type. @@ -92,28 +97,31 @@ pub enum ParseAmountError { } impl From for ParseAmountError { - fn from(value: TooPreciseError) -> Self { Self::TooPrecise(value) } + fn from(value: TooPreciseError) -> Self { Self(ParseAmountErrorInner::TooPrecise(value)) } } impl From for ParseAmountError { - fn from(value: MissingDigitsError) -> Self { Self::MissingDigits(value) } + fn from(value: MissingDigitsError) -> Self { Self(ParseAmountErrorInner::MissingDigits(value)) } } impl From for ParseAmountError { - fn from(value: InputTooLargeError) -> Self { Self::InputTooLarge(value) } + fn from(value: InputTooLargeError) -> Self { Self(ParseAmountErrorInner::InputTooLarge(value)) } } impl From for ParseAmountError { - fn from(value: InvalidCharacterError) -> Self { Self::InvalidCharacter(value) } + fn from(value: InvalidCharacterError) -> Self { + Self(ParseAmountErrorInner::InvalidCharacter(value)) + } } internals::impl_from_infallible!(ParseAmountError); +internals::impl_from_infallible!(ParseAmountErrorInner); impl fmt::Display for ParseAmountError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use ParseAmountError::*; + use ParseAmountErrorInner::*; - match *self { + match self.0 { OutOfRange(ref error) => write_err!(f, "amount out of range"; error), TooPrecise(ref error) => write_err!(f, "amount has a too high precision"; error), MissingDigits(ref error) => write_err!(f, "the input has too few digits"; error), @@ -126,9 +134,9 @@ impl fmt::Display for ParseAmountError { #[cfg(feature = "std")] impl std::error::Error for ParseAmountError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use ParseAmountError::*; + use ParseAmountErrorInner::*; - match *self { + match self.0 { TooPrecise(ref error) => Some(error), InputTooLarge(ref error) => Some(error), OutOfRange(ref error) => Some(error), @@ -195,7 +203,9 @@ impl fmt::Display for OutOfRangeError { impl std::error::Error for OutOfRangeError {} impl From for ParseAmountError { - fn from(value: OutOfRangeError) -> Self { ParseAmountError::OutOfRange(value) } + fn from(value: OutOfRangeError) -> Self { + ParseAmountError(ParseAmountErrorInner::OutOfRange(value)) + } } /// Error returned when the input string has higher precision than satoshis. diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index f78b5c378..21eb9ed8c 100644 --- a/units/src/amount/mod.rs +++ b/units/src/amount/mod.rs @@ -20,7 +20,7 @@ use core::cmp::Ordering; use core::fmt; use core::str::FromStr; -use self::error::MissingDigitsKind; +use self::error::{MissingDigitsKind, ParseAmountErrorInner, ParseErrorInner}; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] @@ -297,10 +297,12 @@ impl InnerParseError { match self { Self::Overflow { is_negative } => OutOfRangeError { is_signed, is_greater_than_max: !is_negative }.into(), - Self::TooPrecise(error) => ParseAmountError::TooPrecise(error), - Self::MissingDigits(error) => ParseAmountError::MissingDigits(error), - Self::InputTooLarge(len) => ParseAmountError::InputTooLarge(InputTooLargeError { len }), - Self::InvalidCharacter(error) => ParseAmountError::InvalidCharacter(error), + Self::TooPrecise(e) => ParseAmountError(ParseAmountErrorInner::TooPrecise(e)), + Self::MissingDigits(e) => ParseAmountError(ParseAmountErrorInner::MissingDigits(e)), + Self::InputTooLarge(len) => + ParseAmountError(ParseAmountErrorInner::InputTooLarge(InputTooLargeError { len })), + Self::InvalidCharacter(e) => + ParseAmountError(ParseAmountErrorInner::InvalidCharacter(e)), } } } @@ -311,7 +313,7 @@ fn split_amount_and_denomination(s: &str) -> Result<(&str, Denomination), ParseE } else { let i = s .find(|c: char| c.is_alphabetic()) - .ok_or(ParseError::MissingDenomination(MissingDenominationError))?; + .ok_or(ParseError(ParseErrorInner::MissingDenomination(MissingDenominationError)))?; (i, i) }; Ok((&s[..i], s[j..].parse()?)) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index b90fdd5e7..632a56267 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -10,6 +10,7 @@ use core::{default, fmt, ops}; #[cfg(feature = "arbitrary")] use arbitrary::{Arbitrary, Unstructured}; +use super::error::{ParseAmountErrorInner, ParseErrorInner}; use super::{ parse_signed_to_satoshi, split_amount_and_denomination, Amount, Denomination, Display, DisplayStyle, OutOfRangeError, ParseAmountError, ParseError, @@ -92,12 +93,14 @@ impl SignedAmount { pub fn from_str_in(s: &str, denom: Denomination) -> Result { match parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(true))? { // (negative, amount) - (false, sat) if sat > i64::MAX as u64 => - Err(ParseAmountError::OutOfRange(OutOfRangeError::too_big(true))), + (false, sat) if sat > i64::MAX as u64 => Err(ParseAmountError( + ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)), + )), (false, sat) => Ok(SignedAmount(sat as i64)), (true, sat) if sat == i64::MIN.unsigned_abs() => Ok(SignedAmount(i64::MIN)), - (true, sat) if sat > i64::MIN.unsigned_abs() => - Err(ParseAmountError::OutOfRange(OutOfRangeError::too_small())), + (true, sat) if sat > i64::MIN.unsigned_abs() => Err(ParseAmountError( + ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small()), + )), (true, sat) => Ok(SignedAmount(-(sat as i64))), } } @@ -415,7 +418,7 @@ impl FromStr for SignedAmount { let result = SignedAmount::from_str_with_denomination(s); match result { - Err(ParseError::MissingDenomination(_)) => { + Err(ParseError(ParseErrorInner::MissingDenomination(_))) => { let d = SignedAmount::from_str_in(s, Denomination::Satoshi); if d == Ok(SignedAmount::ZERO) { diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index e695a443b..24e067545 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -33,7 +33,9 @@ fn from_str_zero() { match s.parse::() { Err(e) => assert_eq!( e, - ParseError::Amount(ParseAmountError::OutOfRange(OutOfRangeError::negative())) + ParseError(ParseErrorInner::Amount(ParseAmountError( + ParseAmountErrorInner::OutOfRange(OutOfRangeError::negative()) + ))) ), Ok(_) => panic!("unsigned amount from {}", s), } @@ -321,7 +323,7 @@ fn parsing() { // more than 50 chars. assert_eq!( p("100000000000000.00000000000000000000000000000000000", Denomination::Bitcoin), - Err(E::InputTooLarge(InputTooLargeError { len: 51 })) + Err(E(ParseAmountErrorInner::InputTooLarge(InputTooLargeError { len: 51 }))) ); } @@ -731,10 +733,10 @@ fn serde_as_btc() { // errors let t: Result = serde_json::from_str("{\"amt\": 1000000.000000001, \"samt\": 1}"); - assert!(t - .unwrap_err() - .to_string() - .contains(&ParseAmountError::TooPrecise(TooPreciseError { position: 16 }).to_string())); + assert!(t.unwrap_err().to_string().contains( + &ParseAmountError(ParseAmountErrorInner::TooPrecise(TooPreciseError { position: 16 })) + .to_string() + )); let t: Result = serde_json::from_str("{\"amt\": -1, \"samt\": 1}"); assert!(t.unwrap_err().to_string().contains(&OutOfRangeError::negative().to_string())); } diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index a68262385..37845c1ce 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -12,6 +12,7 @@ use ::serde::{Deserialize, Serialize}; #[cfg(feature = "arbitrary")] use arbitrary::{Arbitrary, Unstructured}; +use super::error::{ParseAmountErrorInner, ParseErrorInner}; use super::{ parse_signed_to_satoshi, split_amount_and_denomination, Denomination, Display, DisplayStyle, OutOfRangeError, ParseAmountError, ParseError, SignedAmount, @@ -103,7 +104,9 @@ impl Amount { let (negative, satoshi) = parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(false))?; if negative { - return Err(ParseAmountError::OutOfRange(OutOfRangeError::negative())); + return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( + OutOfRangeError::negative(), + ))); } Ok(Amount::from_sat(satoshi)) } @@ -405,7 +408,7 @@ impl FromStr for Amount { let result = Amount::from_str_with_denomination(s); match result { - Err(ParseError::MissingDenomination(_)) => { + Err(ParseError(ParseErrorInner::MissingDenomination(_))) => { let d = Amount::from_str_in(s, Denomination::Satoshi); if d == Ok(Amount::ZERO) {