Merge rust-bitcoin/rust-bitcoin#3674: Close `amounts` error types

fd2a5c1ec7 Close amounts error types (Tobin C. Harding)
23c77275b1 Reduce code comment lines (Tobin C. Harding)
d595f421c6 Remove whitespace between enum variants (Tobin C. Harding)

Pull request description:

  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`.

ACKs for top commit:
  apoelstra:
    ACK fd2a5c1ec79f337fb3695c030c9fb6b4671468f2; successfully ran local tests; thanks!

Tree-SHA512: f8d68ef821449e0829c926cf527df4b226b29c8d1d41b320a016fbf70b4b39cc54c8c218955caa0c3776158eeeae0ebacc1cc89dab67bafc399b94063324ab0e

units/src/amount/error.rs

@@ -11,56 +11,56 @@ use super::INPUT_STRING_LEN_LIMIT;
 
 /// An error during amount parsing amount with denomination.
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
+pub struct ParseError(pub(crate) ParseErrorInner);
-pub enum ParseError {
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) enum ParseErrorInner {
     /// Invalid amount.
     Amount(ParseAmountError),
-
     /// Invalid denomination.
     Denomination(ParseDenominationError),
-
     /// The denomination was not identified.
     MissingDenomination(MissingDenominationError),
 }
 
 internals::impl_from_infallible!(ParseError);
+internals::impl_from_infallible!(ParseErrorInner);
 
 impl From<ParseAmountError> for ParseError {
-    fn from(e: ParseAmountError) -> Self { Self::Amount(e) }
+    fn from(e: ParseAmountError) -> Self { Self(ParseErrorInner::Amount(e)) }
 }
 
 impl From<ParseDenominationError> for ParseError {
-    fn from(e: ParseDenominationError) -> Self { Self::Denomination(e) }
+    fn from(e: ParseDenominationError) -> Self { Self(ParseErrorInner::Denomination(e)) }
 }
 
 impl From<OutOfRangeError> for ParseError {
-    fn from(e: OutOfRangeError) -> Self { Self::Amount(e.into()) }
+    fn from(e: OutOfRangeError) -> Self { Self(ParseErrorInner::Amount(e.into())) }
 }
 
 impl From<TooPreciseError> for ParseError {
-    fn from(e: TooPreciseError) -> Self { Self::Amount(e.into()) }
+    fn from(e: TooPreciseError) -> Self { Self(ParseErrorInner::Amount(e.into())) }
 }
 
 impl From<MissingDigitsError> for ParseError {
-    fn from(e: MissingDigitsError) -> Self { Self::Amount(e.into()) }
+    fn from(e: MissingDigitsError) -> Self { Self(ParseErrorInner::Amount(e.into())) }
 }
 
 impl From<InputTooLargeError> for ParseError {
-    fn from(e: InputTooLargeError) -> Self { Self::Amount(e.into()) }
+    fn from(e: InputTooLargeError) -> Self { Self(ParseErrorInner::Amount(e.into())) }
 }
 
 impl From<InvalidCharacterError> 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 {
+        match self.0 {
-            ParseError::Amount(error) => write_err!(f, "invalid amount"; error),
+            ParseErrorInner::Amount(ref e) => write_err!(f, "invalid amount"; e),
-            ParseError::Denomination(error) => write_err!(f, "invalid denomination"; error),
+            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
+            // We consider this to not be a source because it currently doesn't contain useful info.
-            // information
+            ParseErrorInner::MissingDenomination(_) =>
-            ParseError::MissingDenomination(_) =>
                 f.write_str("the input doesn't contain a denomination"),
         }
     }
@@ -69,20 +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 {
+        match self.0 {
-            ParseError::Amount(error) => Some(error),
+            ParseErrorInner::Amount(ref e) => Some(e),
-            ParseError::Denomination(error) => Some(error),
+            ParseErrorInner::Denomination(ref e) => Some(e),
-            // We consider this to not be a source because it currently doesn't contain useful
+            // We consider this to not be a source because it currently doesn't contain useful info.
-            // information
+            ParseErrorInner::MissingDenomination(_) => None,
-            ParseError::MissingDenomination(_) => None,
         }
     }
 }
 
 /// An error during amount parsing.
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
+pub struct ParseAmountError(pub(crate) ParseAmountErrorInner);
-pub enum ParseAmountError {
+
+#[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.
@@ -96,28 +97,31 @@ pub enum ParseAmountError {
 }
 
 impl From<TooPreciseError> for ParseAmountError {
-    fn from(value: TooPreciseError) -> Self { Self::TooPrecise(value) }
+    fn from(value: TooPreciseError) -> Self { Self(ParseAmountErrorInner::TooPrecise(value)) }
 }
 
 impl From<MissingDigitsError> for ParseAmountError {
-    fn from(value: MissingDigitsError) -> Self { Self::MissingDigits(value) }
+    fn from(value: MissingDigitsError) -> Self { Self(ParseAmountErrorInner::MissingDigits(value)) }
 }
 
 impl From<InputTooLargeError> for ParseAmountError {
-    fn from(value: InputTooLargeError) -> Self { Self::InputTooLarge(value) }
+    fn from(value: InputTooLargeError) -> Self { Self(ParseAmountErrorInner::InputTooLarge(value)) }
 }
 
 impl From<InvalidCharacterError> 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),
@@ -130,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),
@@ -199,7 +203,9 @@ impl fmt::Display for OutOfRangeError {
 impl std::error::Error for OutOfRangeError {}
 
 impl From<OutOfRangeError> 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.

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::TooPrecise(e) => ParseAmountError(ParseAmountErrorInner::TooPrecise(e)),
-            Self::MissingDigits(error) => ParseAmountError::MissingDigits(error),
+            Self::MissingDigits(e) => ParseAmountError(ParseAmountErrorInner::MissingDigits(e)),
-            Self::InputTooLarge(len) => ParseAmountError::InputTooLarge(InputTooLargeError { len }),
+            Self::InputTooLarge(len) =>
-            Self::InvalidCharacter(error) => ParseAmountError::InvalidCharacter(error),
+                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()?))

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,
@@ -107,12 +108,14 @@ impl SignedAmount {
     pub fn from_str_in(s: &str, denom: Denomination) -> Result<SignedAmount, ParseAmountError> {
         match parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(true))? {
             // (negative, amount)
-            (false, sat) if sat > i64::MAX as u64 =>
+            (false, sat) if sat > i64::MAX as u64 => Err(ParseAmountError(
-                Err(ParseAmountError::OutOfRange(OutOfRangeError::too_big(true))),
+                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() =>
+            (true, sat) if sat > i64::MIN.unsigned_abs() => Err(ParseAmountError(
-                Err(ParseAmountError::OutOfRange(OutOfRangeError::too_small())),
+                ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small()),
+            )),
             (true, sat) => Ok(SignedAmount(-(sat as i64))),
         }
     }
@@ -430,7 +433,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) {

units/src/amount/tests.rs

@@ -33,7 +33,9 @@ fn from_str_zero() {
         match s.parse::<Amount>() {
             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<T, serde_json::Error> =
         serde_json::from_str("{\"amt\": 1000000.000000001, \"samt\": 1}");
-    assert!(t
+    assert!(t.unwrap_err().to_string().contains(
-        .unwrap_err()
+        &ParseAmountError(ParseAmountErrorInner::TooPrecise(TooPreciseError { position: 16 }))
             .to_string()
-        .contains(&ParseAmountError::TooPrecise(TooPreciseError { position: 16 }).to_string()));
+    ));
     let t: Result<T, serde_json::Error> = serde_json::from_str("{\"amt\": -1, \"samt\": 1}");
     assert!(t.unwrap_err().to_string().contains(&OutOfRangeError::negative().to_string()));
 }

units/src/amount/unsigned.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, Denomination, Display, DisplayStyle,
     OutOfRangeError, ParseAmountError, ParseError, SignedAmount,
@@ -115,7 +116,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))
     }
@@ -417,7 +420,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) {