Merge rust-bitcoin#3674: Close amounts error types

fd2a5c1 Close amounts error types (Tobin C. Harding)
23c7727 Reduce code comment lines (Tobin C. Harding)
d595f42 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 fd2a5c1; successfully ran local tests; thanks!

Tree-SHA512: f8d68ef821449e0829c926cf527df4b226b29c8d1d41b320a016fbf70b4b39cc54c8c218955caa0c3776158eeeae0ebacc1cc89dab67bafc399b94063324ab0e

Author: apoelstra

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 enum ParseError {
+pub struct ParseError(pub(crate) ParseErrorInner);
+
+#[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 {
-            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
-            ParseError::MissingDenomination(_) =>
+        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.
+            ParseErrorInner::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 {
-            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
-            ParseError::MissingDenomination(_) => None,
+        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.
+            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.
@@ -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::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()?))

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 =>
-                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))),
         }
     }
@@ -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
-        .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<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) {