Merge #129: Hide error internals

2b88813 Hide error internals (Tobin C. Harding)
10bc300 Remove unnecessary non_exhaustive (Tobin C. Harding)
2e9a64d Use unwrap_err in tests (Tobin C. Harding)
81799cf Remove custom None Error impl (Tobin C. Harding)
b3619b5 Use error getters (Tobin C. Harding)
5d7afc8 Add getters to InvalidLengthError (Tobin C. Harding)

Pull request description:

  In an effort to make give us flexibility in the `1.0` release hide all the error internals.

  - Patch 1: Adds getters to `InvalidLengthError`
  - Patch 2 and 3: Preparatory cleanup
  - Patch 4: Hide the errors
  - Patch 5: Remove unnecessary `non_exhaustive`
  - Patch 6: Adds getters to support `rust-bitcoin` (tested in: rust-bitcoin/rust-bitcoin#3830)

ACKs for top commit:
  apoelstra:
    ACK 2b88813; successfully ran local tests; looks great!

Tree-SHA512: d435c256eade1f8faac079672275ea744dcb6dbab406a549ab271a2cf71f63214e71cd8ecbd4f775d11c50044e93e9044ed1b361d1f39d32e8d8f02b866952d7

src/error.rs

@@ -27,16 +27,45 @@ macro_rules! write_err {
 
 /// Hex decoding error.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub enum HexToBytesError {
+pub struct HexToBytesError(pub(crate) ToBytesError);
+
+impl HexToBytesError {
+    /// Returns a [`ToBytesError`] from this [`HexToBytesError`].
+    // Use clone instead of reference to give use maximum forward flexibility.
+    pub fn parse_error(&self) -> ToBytesError { self.0.clone() }
+}
+
+impl fmt::Display for HexToBytesError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for HexToBytesError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
+impl From<InvalidCharError> for HexToBytesError {
+    #[inline]
+    fn from(e: InvalidCharError) -> Self { Self(e.into()) }
+}
+
+impl From<OddLengthStringError> for HexToBytesError {
+    #[inline]
+    fn from(e: OddLengthStringError) -> Self { Self(e.into()) }
+}
+
+/// Hex decoding error while parsing to a vector of bytes.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum ToBytesError {
     /// Non-hexadecimal character.
     InvalidChar(InvalidCharError),
     /// Purported hex string had odd length.
     OddLengthString(OddLengthStringError),
 }
 
-impl fmt::Display for HexToBytesError {
+impl fmt::Display for ToBytesError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use HexToBytesError::*;
+        use ToBytesError::*;
 
         match *self {
             InvalidChar(ref e) => write_err!(f, "invalid char, failed to create bytes from hex"; e),
@@ -47,9 +76,9 @@ impl fmt::Display for HexToBytesError {
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for HexToBytesError {
+impl std::error::Error for ToBytesError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use HexToBytesError::*;
+        use ToBytesError::*;
 
         match *self {
             InvalidChar(ref e) => Some(e),
@@ -58,12 +87,12 @@ impl std::error::Error for HexToBytesError {
     }
 }
 
-impl From<InvalidCharError> for HexToBytesError {
+impl From<InvalidCharError> for ToBytesError {
     #[inline]
     fn from(e: InvalidCharError) -> Self { Self::InvalidChar(e) }
 }
 
-impl From<OddLengthStringError> for HexToBytesError {
+impl From<OddLengthStringError> for ToBytesError {
     #[inline]
     fn from(e: OddLengthStringError) -> Self { Self::OddLengthString(e) }
 }
@@ -84,14 +113,12 @@ impl InvalidCharError {
 
 impl fmt::Display for InvalidCharError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "invalid hex char {} at pos {}", self.invalid, self.pos)
+        write!(f, "invalid hex char {} at pos {}", self.invalid_char(), self.pos())
     }
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for InvalidCharError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
-}
+impl std::error::Error for InvalidCharError {}
 
 /// Purported hex string had odd length.
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -106,27 +133,54 @@ impl OddLengthStringError {
 
 impl fmt::Display for OddLengthStringError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "odd hex string length {}", self.len)
+        write!(f, "odd hex string length {}", self.length())
     }
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for OddLengthStringError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
-}
+impl std::error::Error for OddLengthStringError {}
 
 /// Hex decoding error.
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub enum HexToArrayError {
+pub struct HexToArrayError(pub(crate) ToArrayError);
+
+impl HexToArrayError {
+    /// Returns a [`ToArrayError`] from this [`HexToArrayError`].
+    // Use clone instead of reference to give use maximum forward flexibility.
+    pub fn parse_error(&self) -> ToArrayError { self.0.clone() }
+}
+
+impl fmt::Display for HexToArrayError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for HexToArrayError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
+impl From<InvalidCharError> for HexToArrayError {
+    #[inline]
+    fn from(e: InvalidCharError) -> Self { Self(e.into()) }
+}
+
+impl From<InvalidLengthError> for HexToArrayError {
+    #[inline]
+    fn from(e: InvalidLengthError) -> Self { Self(e.into()) }
+}
+
+/// Hex decoding error while parsing a byte array.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum ToArrayError {
     /// Non-hexadecimal character.
     InvalidChar(InvalidCharError),
     /// Tried to parse fixed-length hash from a string with the wrong length.
     InvalidLength(InvalidLengthError),
 }
 
-impl fmt::Display for HexToArrayError {
+impl fmt::Display for ToArrayError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use HexToArrayError::*;
+        use ToArrayError::*;
 
         match *self {
             InvalidChar(ref e) => write_err!(f, "failed to parse hex digit"; e),
@@ -136,9 +190,9 @@ impl fmt::Display for HexToArrayError {
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for HexToArrayError {
+impl std::error::Error for ToArrayError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use HexToArrayError::*;
+        use ToArrayError::*;
 
         match *self {
             InvalidChar(ref e) => Some(e),
@@ -147,33 +201,42 @@ impl std::error::Error for HexToArrayError {
     }
 }
 
-impl From<InvalidCharError> for HexToArrayError {
+impl From<InvalidCharError> for ToArrayError {
     #[inline]
     fn from(e: InvalidCharError) -> Self { Self::InvalidChar(e) }
 }
 
-impl From<InvalidLengthError> for HexToArrayError {
+impl From<InvalidLengthError> for ToArrayError {
     #[inline]
     fn from(e: InvalidLengthError) -> Self { Self::InvalidLength(e) }
 }
 
 /// Tried to parse fixed-length hash from a string with the wrong length.
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
 pub struct InvalidLengthError {
     /// The expected length.
-    pub expected: usize,
+    pub(crate) expected: usize,
     /// The invalid length.
-    pub invalid: usize,
+    pub(crate) invalid: usize,
+}
+
+impl InvalidLengthError {
+    /// Returns the expected length.
+    pub fn expected_length(&self) -> usize { self.expected }
+    /// Returns the position of the invalid character byte.
+    pub fn invalid_length(&self) -> usize { self.invalid }
 }
 
 impl fmt::Display for InvalidLengthError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "invilad hex string length {} (expected {})", self.invalid, self.expected)
+        write!(
+            f,
+            "invilad hex string length {} (expected {})",
+            self.invalid_length(),
+            self.expected_length()
+        )
     }
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for InvalidLengthError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
-}
+impl std::error::Error for InvalidLengthError {}

src/iter.rs

@@ -456,23 +456,32 @@ mod tests {
         let hex = "geadbeef";
         let iter = HexToBytesIter::new_unchecked(hex);
         let mut got = [0u8; 4];
-        assert_eq!(iter.drain_to_slice(&mut got), Err(InvalidCharError { invalid: b'g', pos: 0 }));
+        assert_eq!(
+            iter.drain_to_slice(&mut got).unwrap_err(),
+            InvalidCharError { invalid: b'g', pos: 0 }
+        );
     }
 
     #[test]
     fn hex_to_bytes_slice_drain_middle_char_error() {
         let hex = "deadgeef";
         let iter = HexToBytesIter::new_unchecked(hex);
         let mut got = [0u8; 4];
-        assert_eq!(iter.drain_to_slice(&mut got), Err(InvalidCharError { invalid: b'g', pos: 4 }));
+        assert_eq!(
+            iter.drain_to_slice(&mut got).unwrap_err(),
+            InvalidCharError { invalid: b'g', pos: 4 }
+        );
     }
 
     #[test]
     fn hex_to_bytes_slice_drain_end_char_error() {
         let hex = "deadbeeg";
         let iter = HexToBytesIter::new_unchecked(hex);
         let mut got = [0u8; 4];
-        assert_eq!(iter.drain_to_slice(&mut got), Err(InvalidCharError { invalid: b'g', pos: 7 }));
+        assert_eq!(
+            iter.drain_to_slice(&mut got).unwrap_err(),
+            InvalidCharError { invalid: b'g', pos: 7 }
+        );
     }
 
     #[test]
@@ -493,21 +502,21 @@ mod tests {
     fn hex_to_bytes_vec_drain_first_char_error() {
         let hex = "geadbeef";
         let iter = HexToBytesIter::new_unchecked(hex);
-        assert_eq!(iter.drain_to_vec(), Err(InvalidCharError { invalid: b'g', pos: 0 }));
+        assert_eq!(iter.drain_to_vec().unwrap_err(), InvalidCharError { invalid: b'g', pos: 0 });
     }
 
     #[test]
     fn hex_to_bytes_vec_drain_middle_char_error() {
         let hex = "deadgeef";
         let iter = HexToBytesIter::new_unchecked(hex);
-        assert_eq!(iter.drain_to_vec(), Err(InvalidCharError { invalid: b'g', pos: 4 }));
+        assert_eq!(iter.drain_to_vec().unwrap_err(), InvalidCharError { invalid: b'g', pos: 4 });
     }
 
     #[test]
     fn hex_to_bytes_vec_drain_end_char_error() {
         let hex = "deadbeeg";
         let iter = HexToBytesIter::new_unchecked(hex);
-        assert_eq!(iter.drain_to_vec(), Err(InvalidCharError { invalid: b'g', pos: 7 }));
+        assert_eq!(iter.drain_to_vec().unwrap_err(), InvalidCharError { invalid: b'g', pos: 7 });
     }
 
     #[test]

src/parse.rs

@@ -60,22 +60,25 @@ mod tests {
         let badchar2 = "012Y456789abcdeb";
         let badchar3 = "«23456789abcdef";
 
-        assert_eq!(Vec::<u8>::from_hex(oddlen), Err(OddLengthStringError { len: 17 }.into()));
         assert_eq!(
-            <[u8; 4]>::from_hex(oddlen),
-            Err(InvalidLengthError { invalid: 17, expected: 8 }.into())
+            Vec::<u8>::from_hex(oddlen).unwrap_err(),
+            OddLengthStringError { len: 17 }.into()
         );
         assert_eq!(
-            Vec::<u8>::from_hex(badchar1),
-            Err(InvalidCharError { pos: 0, invalid: b'Z' }.into())
+            <[u8; 4]>::from_hex(oddlen).unwrap_err(),
+            InvalidLengthError { invalid: 17, expected: 8 }.into()
         );
         assert_eq!(
-            Vec::<u8>::from_hex(badchar2),
-            Err(InvalidCharError { pos: 3, invalid: b'Y' }.into())
+            Vec::<u8>::from_hex(badchar1).unwrap_err(),
+            InvalidCharError { pos: 0, invalid: b'Z' }.into()
         );
         assert_eq!(
-            Vec::<u8>::from_hex(badchar3),
-            Err(InvalidCharError { pos: 0, invalid: 194 }.into())
+            Vec::<u8>::from_hex(badchar2).unwrap_err(),
+            InvalidCharError { pos: 3, invalid: b'Y' }.into()
+        );
+        assert_eq!(
+            Vec::<u8>::from_hex(badchar3).unwrap_err(),
+            InvalidCharError { pos: 0, invalid: 194 }.into()
         );
     }
 
@@ -88,20 +91,20 @@ mod tests {
         let badpos4 = "0123456789abYdef";
 
         assert_eq!(
-            HexToBytesIter::new(badpos1).unwrap().next().unwrap(),
-            Err(InvalidCharError { pos: 0, invalid: b'Z' })
+            HexToBytesIter::new(badpos1).unwrap().next().unwrap().unwrap_err(),
+            InvalidCharError { pos: 0, invalid: b'Z' }
         );
         assert_eq!(
-            HexToBytesIter::new(badpos2).unwrap().nth(1).unwrap(),
-            Err(InvalidCharError { pos: 3, invalid: b'Y' })
+            HexToBytesIter::new(badpos2).unwrap().nth(1).unwrap().unwrap_err(),
+            InvalidCharError { pos: 3, invalid: b'Y' }
         );
         assert_eq!(
-            HexToBytesIter::new(badpos3).unwrap().next_back().unwrap(),
-            Err(InvalidCharError { pos: 15, invalid: b'Z' })
+            HexToBytesIter::new(badpos3).unwrap().next_back().unwrap().unwrap_err(),
+            InvalidCharError { pos: 15, invalid: b'Z' }
         );
         assert_eq!(
-            HexToBytesIter::new(badpos4).unwrap().nth_back(1).unwrap(),
-            Err(InvalidCharError { pos: 12, invalid: b'Y' })
+            HexToBytesIter::new(badpos4).unwrap().nth_back(1).unwrap().unwrap_err(),
+            InvalidCharError { pos: 12, invalid: b'Y' }
         );
     }
 
@@ -115,8 +118,8 @@ mod tests {
     fn hex_to_array_error() {
         let len_sixteen = "0123456789abcdef";
         assert_eq!(
-            <[u8; 4]>::from_hex(len_sixteen),
-            Err(InvalidLengthError { invalid: 16, expected: 8 }.into())
+            <[u8; 4]>::from_hex(len_sixteen).unwrap_err(),
+            InvalidLengthError { invalid: 16, expected: 8 }.into()
         )
     }