Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide all error internals #44

Merged
merged 2 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<'a> fmt::UpperHex for DisplayALittleBitHexy<'a> {
}

/// Example Error.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Error {
/// Conversion error while parsing hex string.
Conversion(HexToBytesError),
Expand Down
146 changes: 146 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
// SPDX-License-Identifier: CC0-1.0

use core::fmt;

use crate::write_err;

/// Formats error.
///
/// If `std` feature is OFF appends error source (delimited by `: `). We do this because
Expand All @@ -21,3 +25,145 @@ macro_rules! write_err {
}
}
}

/// Hex decoding error.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum HexToBytesError {
/// Non-hexadecimal character.
InvalidChar(InvalidCharError),
/// Purported hex string had odd length.
OddLengthString(OddLengthStringError),
}

impl fmt::Display for HexToBytesError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToBytesError::*;

match *self {
InvalidChar(ref e) => write_err!(f, "invalid char, failed to create bytes from hex"; e),
OddLengthString(ref e) =>
write_err!(f, "odd length, failed to create bytes from hex"; e),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToBytesError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToBytesError::*;

match *self {
InvalidChar(ref e) => Some(e),
OddLengthString(ref e) => Some(e),
}
}
}

impl From<InvalidCharError> for HexToBytesError {
#[inline]
fn from(e: InvalidCharError) -> Self { Self::InvalidChar(e) }
}

impl From<OddLengthStringError> for HexToBytesError {
#[inline]
fn from(e: OddLengthStringError) -> Self { Self::OddLengthString(e) }
}

/// Invalid hex character.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed when at least one field is not pub but I could understand if someone wants to avoid us forgetting to add it in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to put it on every struct regardless because it makes scanning the error types quicker. However, this code is plain old sloppy, I've been not adding Copy when using non_exhaustive.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think naming error type *Error is much better. There are other types that need to be non_exhaustive and are not error types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow your comment, all errors are suffixed with Error. I'll remove the non_exhaustive from all struct errors that exist solely to hide internals. On further thought today it is strange to have unneeded non_exhaustive and there will always be private fields since that is the whole point of these types.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are and should continue to be. That's my point rg 'struct [A-Za-z0-9]*Error' is better than rg non_exhaustive`

pub struct InvalidCharError {
pub(crate) invalid: u8,
}

impl fmt::Display for InvalidCharError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "invalid hex char {}", self.invalid)
}
}

#[cfg(feature = "std")]
impl std::error::Error for InvalidCharError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}

/// Purported hex string had odd length.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not needed

pub struct OddLengthStringError {
pub(crate) len: usize,
}

impl fmt::Display for OddLengthStringError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "odd hex string length {}", self.len)
}
}

#[cfg(feature = "std")]
impl std::error::Error for OddLengthStringError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}

/// Hex decoding error.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum HexToArrayError {
/// Conversion error while parsing hex string.
Conversion(HexToBytesError),
/// Tried to parse fixed-length hash from a string with the wrong length (got, want).
InvalidLength(InvalidLengthError),
}

impl fmt::Display for HexToArrayError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToArrayError::*;

match *self {
Conversion(ref e) =>
crate::write_err!(f, "conversion error, failed to create array from hex"; e),
InvalidLength(ref e) =>
write_err!(f, "invalid length, failed to create array from hex"; e),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToArrayError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToArrayError::*;

match *self {
Conversion(ref e) => Some(e),
InvalidLength(ref e) => Some(e),
}
}
}

impl From<HexToBytesError> for HexToArrayError {
#[inline]
fn from(e: HexToBytesError) -> Self { Self::Conversion(e) }
}

impl From<InvalidLengthError> for HexToArrayError {
#[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 {
pub(crate) expected: usize,
pub(crate) got: usize,
}

impl fmt::Display for InvalidLengthError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "bad hex string length {} (expected {})", self.got, self.expected)
}
}

#[cfg(feature = "std")]
impl std::error::Error for InvalidLengthError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}
11 changes: 7 additions & 4 deletions src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use std::io;
#[cfg(all(feature = "core2", not(feature = "std")))]
use core2::io;

use crate::parse::HexToBytesError;
use crate::error::{InvalidCharError, OddLengthStringError};

#[rustfmt::skip] // Keep public re-exports separate.
pub use crate::error::HexToBytesError;

/// Iterator over a hex-encoded string slice which decodes hex and yields bytes.
pub struct HexToBytesIter<'a> {
Expand All @@ -33,7 +36,7 @@ impl<'a> HexToBytesIter<'a> {
#[inline]
pub fn new(s: &'a str) -> Result<HexToBytesIter<'a>, HexToBytesError> {
if s.len() % 2 != 0 {
Err(HexToBytesError::OddLengthString(s.len()))
Err(OddLengthStringError { len: s.len() }.into())
} else {
Ok(HexToBytesIter { iter: s.bytes() })
}
Expand Down Expand Up @@ -93,8 +96,8 @@ impl<'a> io::Read for HexToBytesIter<'a> {

/// `hi` and `lo` are bytes representing hex characters.
fn hex_chars_to_byte(hi: u8, lo: u8) -> Result<u8, HexToBytesError> {
let hih = (hi as char).to_digit(16).ok_or(HexToBytesError::InvalidChar(hi))?;
let loh = (lo as char).to_digit(16).ok_or(HexToBytesError::InvalidChar(lo))?;
let hih = (hi as char).to_digit(16).ok_or(InvalidCharError { invalid: hi })?;
let loh = (lo as char).to_digit(16).ok_or(InvalidCharError { invalid: lo })?;

let ret = (hih << 4) + loh;
Ok(ret as u8)
Expand Down
94 changes: 15 additions & 79 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ use core::{fmt, str};

#[cfg(all(feature = "alloc", not(feature = "std")))]
use crate::alloc::vec::Vec;
use crate::error::InvalidLengthError;
use crate::iter::HexToBytesIter;

#[rustfmt::skip] // Keep public re-exports separate.
pub use crate::error::{HexToBytesError, HexToArrayError};

/// Trait for objects that can be deserialized from hex strings.
pub trait FromHex: Sized {
/// Error type returned while parsing hex string.
Expand Down Expand Up @@ -37,37 +41,6 @@ impl FromHex for Vec<u8> {
}
}

/// Hex decoding error.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum HexToBytesError {
/// Non-hexadecimal character.
InvalidChar(u8),
/// Purported hex string had odd length.
OddLengthString(usize),
}

impl fmt::Display for HexToBytesError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToBytesError::*;

match *self {
InvalidChar(ch) => write!(f, "invalid hex character {}", ch),
OddLengthString(ell) => write!(f, "odd hex string length {}", ell),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToBytesError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToBytesError::*;

match self {
InvalidChar(_) | OddLengthString(_) => None,
}
}
}

macro_rules! impl_fromhex_array {
($len:expr) => {
impl FromHex for [u8; $len] {
Expand All @@ -86,7 +59,7 @@ macro_rules! impl_fromhex_array {
}
Ok(ret)
} else {
Err(HexToArrayError::InvalidLength(2 * $len, 2 * iter.len()))
Err(InvalidLengthError { expected: 2 * $len, got: 2 * iter.len() }.into())
}
}
}
Expand All @@ -113,67 +86,28 @@ impl_fromhex_array!(256);
impl_fromhex_array!(384);
impl_fromhex_array!(512);

/// Hex decoding error.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum HexToArrayError {
/// Conversion error while parsing hex string.
Conversion(HexToBytesError),
/// Tried to parse fixed-length hash from a string with the wrong length (got, want).
InvalidLength(usize, usize),
}

impl fmt::Display for HexToArrayError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToArrayError::*;

match *self {
Conversion(ref e) => crate::write_err!(f, "conversion error"; e),
InvalidLength(got, want) =>
write!(f, "bad hex string length {} (expected {})", got, want),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToArrayError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToArrayError::*;

match *self {
Conversion(ref e) => Some(e),
InvalidLength(_, _) => None,
}
}
}

impl From<HexToBytesError> for HexToArrayError {
#[inline]
fn from(e: HexToBytesError) -> Self { Self::Conversion(e) }
}

#[cfg(test)]
mod tests {
use super::*;
use crate::display::DisplayHex;
use crate::error::{InvalidCharError, InvalidLengthError, OddLengthStringError};

#[test]
#[cfg(feature = "alloc")]
fn hex_error() {
use HexToBytesError::*;

let oddlen = "0123456789abcdef0";
let badchar1 = "Z123456789abcdef";
let badchar2 = "012Y456789abcdeb";
let badchar3 = "«23456789abcdef";

assert_eq!(Vec::<u8>::from_hex(oddlen), Err(OddLengthString(17)));
assert_eq!(Vec::<u8>::from_hex(oddlen), Err(OddLengthStringError { len: 17 }.into()));
assert_eq!(
<[u8; 4]>::from_hex(oddlen),
Err(HexToArrayError::Conversion(OddLengthString(17)))
Err(HexToBytesError::OddLengthString(OddLengthStringError { len: 17 }).into())
);
assert_eq!(Vec::<u8>::from_hex(badchar1), Err(InvalidChar(b'Z')));
assert_eq!(Vec::<u8>::from_hex(badchar2), Err(InvalidChar(b'Y')));
assert_eq!(Vec::<u8>::from_hex(badchar3), Err(InvalidChar(194)));
assert_eq!(Vec::<u8>::from_hex(badchar1), Err(InvalidCharError { invalid: b'Z' }.into()));
assert_eq!(Vec::<u8>::from_hex(badchar2), Err(InvalidCharError { invalid: b'Y' }.into()));
assert_eq!(Vec::<u8>::from_hex(badchar3), Err(InvalidCharError { invalid: 194 }.into()));
}

#[test]
Expand All @@ -183,9 +117,11 @@ mod tests {
}
#[test]
fn hex_to_array_error() {
use HexToArrayError::*;
let len_sixteen = "0123456789abcdef";
assert_eq!(<[u8; 4]>::from_hex(len_sixteen), Err(InvalidLength(8, 16)));
assert_eq!(
<[u8; 4]>::from_hex(len_sixteen),
Err(InvalidLengthError { expected: 8, got: 16 }.into())
)
}

#[test]
Expand Down
Loading