From 85b106f5d875b9fd7e906ebdbce83da7de016bcb Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Tue, 10 Nov 2020 19:34:58 +1100 Subject: [PATCH] Returned Error to being an enum, as other fields were unused --- src/errors.rs | 146 ++++++++++++++++---------------------------------- src/lib.rs | 20 +++---- 2 files changed, 55 insertions(+), 111 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 678fcac..55165f8 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -3,23 +3,26 @@ use crate::FileMode; use crate::Frame; use std::path::{Path, PathBuf}; -#[derive(Debug, Clone, PartialEq)] /// Error type for the xdrfile library -pub struct Error { - kind: ErrorKind, - source: Option>, +#[derive(Debug, Clone, PartialEq)] +pub enum Error { + /// An error code from the C API + CApiError { code: ErrorCode, task: ErrorTask }, + /// Passed in a frame of the wrong size + WrongSizeFrame { expected: usize, found: usize }, + /// C API failed to open a file (No return code provided) + CouldNotOpen { path: PathBuf, mode: FileMode }, + /// A path could not be converted to &OsStr, probably because it is invalid unicode + InvalidOsStr, + /// A path could not be converted to &CStr because it had a null byte + NullInStr(std::ffi::NulError), } impl Error { - /// Get the task being attempted when the error was returned - pub fn kind(&self) -> &ErrorKind { - &self.kind - } - /// Get the error code returned by the C API, if any pub fn code(&self) -> Option { - if let ErrorKind::CApiError { code, .. } = self.kind { - Some(code) + if let Error::CApiError { code, .. } = self { + Some(*code) } else { None } @@ -27,8 +30,8 @@ impl Error { /// Get the task being attempted when the C API returned an error, if any pub fn task(&self) -> Option { - if let ErrorKind::CApiError { task, .. } = self.kind { - Some(task) + if let Error::CApiError { task, .. } = self { + Some(*task) } else { None } @@ -37,7 +40,6 @@ impl Error { /// True if the error is an end of file error, false otherwise pub fn is_eof(&self) -> bool { self.code().map_or(false, |e| e.is_eof()) - | self.source.as_ref().map_or(false, |e| e.is_eof()) } /// Convert an error code and output value from a C call to a Result @@ -51,86 +53,50 @@ impl Error { if let ErrorCode::ExdrOk = code { Ok(value) } else { - Err(Self { - kind: ErrorKind::CApiError { code, task }, - source: None, - }) + Err(Self::CApiError { code, task }) } } } -impl> From for Error { - fn from(kind: K) -> Self { - Self { - kind: kind.into(), - source: None, - } - } -} - -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.kind.fmt(f) - } -} - impl std::error::Error for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - if let Some(err) = &self.source { - Some(err) - } else { - use ErrorKind::*; - match &self.kind { - NullInStr(err) => Some(err), - _ => None, - } + use Error::*; + match &self { + NullInStr(err) => Some(err), + _ => None, } } } -#[derive(Debug, Clone, PartialEq)] -pub enum ErrorKind { - /// An error code from the C API - CApiError { code: ErrorCode, task: ErrorTask }, - /// Passed in a frame of the wrong size - WrongSizeFrame { expected: usize, found: usize }, - /// C API failed to open a file (No return code provided) - CouldNotOpen { path: PathBuf, mode: FileMode }, - /// A path could not be converted to &OsStr, probably because it is invalid unicode - InvalidOsStr, - /// A path could not be converted to &CStr because it had a null byte - NullInStr(std::ffi::NulError), -} - -impl From for ErrorKind { +impl From for Error { fn from(err: std::ffi::NulError) -> Self { Self::NullInStr(err) } } -impl From<(&Path, FileMode)> for ErrorKind { +impl From<(&Path, FileMode)> for Error { fn from(value: (&Path, FileMode)) -> Self { let (path, mode) = value; - ErrorKind::CouldNotOpen { + Error::CouldNotOpen { path: path.to_owned(), mode, } } } -impl From<(&Frame, usize)> for ErrorKind { +impl From<(&Frame, usize)> for Error { fn from(value: (&Frame, usize)) -> Self { let (frame, num_atoms) = value; - ErrorKind::WrongSizeFrame { + Error::WrongSizeFrame { expected: num_atoms, found: frame.coords.len(), } } } -impl std::fmt::Display for ErrorKind { +impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use ErrorKind::*; + use Error::*; match &self { CApiError { code, task } => write!( f, @@ -263,58 +229,40 @@ mod tests { #[test] fn test_is_eof() { - use ErrorKind::CApiError; - let error = Error { - kind: CApiError { - code: c_abi::xdrfile::exdrENDOFFILE.into(), - task: ErrorTask::Read, - }, - source: None, + use Error::CApiError; + let error = CApiError { + code: c_abi::xdrfile::exdrENDOFFILE.into(), + task: ErrorTask::Read, }; assert!(error.is_eof()); - let error = Error { - kind: CApiError { - code: ErrorCode::ExdrEndOfFile, - task: ErrorTask::Read, - }, - source: None, + let error = CApiError { + code: ErrorCode::ExdrEndOfFile, + task: ErrorTask::Read, }; assert!(error.is_eof()); - let error = Error { - kind: CApiError { - code: (c_abi::xdrfile::exdrENDOFFILE + 1).into(), - task: ErrorTask::Read, - }, - source: None, + let error = CApiError { + code: (c_abi::xdrfile::exdrENDOFFILE + 1).into(), + task: ErrorTask::Read, }; assert!(!error.is_eof()); - let error = Error { - kind: CApiError { - code: 0.into(), - task: ErrorTask::Read, - }, - source: None, + let error = CApiError { + code: 0.into(), + task: ErrorTask::Read, }; assert!(!error.is_eof()); - let error = Error { - kind: CApiError { - code: 255.into(), - task: ErrorTask::Read, - }, - source: None, + let error = CApiError { + code: 255.into(), + task: ErrorTask::Read, }; assert!(!error.is_eof()); - let error = Error { - kind: ErrorKind::CouldNotOpen { - path: PathBuf::from("not/a/file"), - mode: FileMode::Read, - }, - source: None, + let error = Error::CouldNotOpen { + path: PathBuf::from("not/a/file"), + mode: FileMode::Read, }; assert!(!error.is_eof()); } diff --git a/src/lib.rs b/src/lib.rs index 0dd84a6..8e8eff8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -101,11 +101,7 @@ impl FileMode { } fn path_to_cstring(path: impl AsRef) -> Result { - use ErrorKind::InvalidOsStr; - let s = path - .as_ref() - .to_str() - .ok_or_else(|| Error::from(InvalidOsStr))?; + let s = path.as_ref().to_str().ok_or_else(|| Error::InvalidOsStr)?; CString::new(s).map_err(|e| Error::from(e)) } @@ -500,7 +496,7 @@ mod tests { let result = xtc_traj.read(&mut frame); if let Err(e) = result { - assert!(if let ErrorKind::WrongSizeFrame { .. } = e.kind() { + assert!(if let Error::WrongSizeFrame { .. } = e { true } else { false @@ -516,9 +512,9 @@ mod tests { let result_invalid = path_to_cstring(PathBuf::from("invalid/\0path")); if let Err(err) = result_invalid { - match err.kind() { - ErrorKind::NullInStr(_) => (), - ErrorKind::InvalidOsStr => (), + match err { + Error::NullInStr(_) => (), + Error::InvalidOsStr => (), _ => panic!("Improper error type for path_to_cstring"), } } else { @@ -537,13 +533,13 @@ mod tests { let path = Path::new(&file_name); if let Err(e) = XDRFile::open(file_name, FileMode::Read) { - if let ErrorKind::CouldNotOpen { + if let Error::CouldNotOpen { path: err_path, mode: err_mode, - } = e.kind() + } = e { assert_eq!(path, err_path); - assert_eq!(FileMode::Read, *err_mode) + assert_eq!(FileMode::Read, err_mode) } else { panic!("Wrong Error type") }