Broke out errors module and handle error codes as rust enum

src/errors.rs

@@ -0,0 +1,220 @@
+use crate::c_abi;
+use crate::FileMode;
+use std::path::{Path, PathBuf};
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum ErrorTask {
+    OpenFile(PathBuf, FileMode),
+    ReadNumAtoms,
+    Read,
+    Write,
+    Flush,
+    ToCString(Option<std::ffi::NulError>),
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct Error {
+    code: Option<ErrorCode>,
+    task: ErrorTask,
+}
+
+impl Error {
+    pub fn task(&self) -> &ErrorTask {
+        &self.task
+    }
+
+    pub fn code(&self) -> &Option<ErrorCode> {
+        &self.code
+    }
+
+    pub fn is_eof(&self) -> bool {
+        self.code.as_ref().map_or(false, ErrorCode::is_eof)
+    }
+
+    pub fn from_convert() -> Self {
+        Self {
+            code: None,
+            task: ErrorTask::ToCString(None),
+        }
+    }
+
+    pub fn from_open(path: impl AsRef<Path>, mode: FileMode) -> Self {
+        Self {
+            code: None,
+            task: ErrorTask::OpenFile(path.as_ref().into(), mode),
+        }
+    }
+
+    pub fn from_read_num_atoms(code: impl Into<ErrorCode>) -> Self {
+        Self {
+            code: Some(code.into()),
+            task: ErrorTask::ReadNumAtoms,
+        }
+    }
+
+    pub fn from_read(code: impl Into<ErrorCode>) -> Self {
+        Self {
+            code: Some(code.into()),
+            task: ErrorTask::Read,
+        }
+    }
+
+    pub fn from_write(code: impl Into<ErrorCode>) -> Self {
+        Self {
+            code: Some(code.into()),
+            task: ErrorTask::Write,
+        }
+    }
+
+    pub fn from_flush(code: impl Into<ErrorCode>) -> Self {
+        Self {
+            code: Some(code.into()),
+            task: ErrorTask::Flush,
+        }
+    }
+}
+
+impl From<std::ffi::NulError> for Error {
+    fn from(err: std::ffi::NulError) -> Self {
+        Self {
+            code: None,
+            task: ErrorTask::ToCString(Some(err)),
+        }
+    }
+}
+
+impl std::fmt::Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use ErrorTask::*;
+        match &self.task {
+            OpenFile(path, mode) => write!(
+                f,
+                "Failed to open file at {path:?} with mode {mode:?}",
+                path = path,
+                mode = mode
+            ),
+            ReadNumAtoms => write!(
+                f,
+                "Failed to read atom number from trajectory: C API returned error code {:?}",
+                self.code
+            ),
+            Read => write!(
+                f,
+                "Failed to read trajectory: C API returned error code {:?}",
+                self.code
+            ),
+            Write => write!(
+                f,
+                "Failed to write trajectory: C API returned error code {:?}",
+                self.code
+            ),
+            Flush => write!(
+                f,
+                "Failed to flush trajectory: C API returned error code {:?}",
+                self.code
+            ),
+            ToCString(_) => write!(
+                f,
+                "Path cannot be converted to a C string because it has a null byte"
+            ),
+        }
+    }
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum ErrorCode {
+    ExdrOk,
+    ExdrHeader,
+    ExdrString,
+    ExdrDouble,
+    ExdrInt,
+    ExdrFloat,
+    ExdrUint,
+    Exdr3dx,
+    ExdrClose,
+    ExdrMagic,
+    ExdrNoMem,
+    ExdrEndOfFile,
+    ExdrFileNotFound,
+    ExdrNr,
+    UnmatchedCode(c_abi::xdrfile::BindgenTy1),
+}
+
+impl ErrorCode {
+    pub fn is_eof(&self) -> bool {
+        match self {
+            Self::ExdrEndOfFile => true,
+            _ => false,
+        }
+    }
+}
+
+impl From<c_abi::xdrfile::BindgenTy1> for ErrorCode {
+    fn from(code: c_abi::xdrfile::BindgenTy1) -> Self {
+        match code {
+            c_abi::xdrfile::exdrOK => Self::ExdrOk,
+            c_abi::xdrfile::exdrHEADER => Self::ExdrHeader,
+            c_abi::xdrfile::exdrSTRING => Self::ExdrString,
+            c_abi::xdrfile::exdrDOUBLE => Self::ExdrDouble,
+            c_abi::xdrfile::exdrINT => Self::ExdrInt,
+            c_abi::xdrfile::exdrFLOAT => Self::ExdrFloat,
+            c_abi::xdrfile::exdrUINT => Self::ExdrUint,
+            c_abi::xdrfile::exdr3DX => Self::Exdr3dx,
+            c_abi::xdrfile::exdrCLOSE => Self::ExdrClose,
+            c_abi::xdrfile::exdrMAGIC => Self::ExdrMagic,
+            c_abi::xdrfile::exdrNOMEM => Self::ExdrNoMem,
+            c_abi::xdrfile::exdrENDOFFILE => Self::ExdrEndOfFile,
+            c_abi::xdrfile::exdrFILENOTFOUND => Self::ExdrFileNotFound,
+            c_abi::xdrfile::exdrNR => Self::ExdrNr,
+            code => Self::UnmatchedCode(code),
+        }
+    }
+}
+
+impl std::error::Error for Error {}
+
+pub type Result<T, E = Error> = std::result::Result<T, E>;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_eof() {
+        let error = Error {
+            code: Some(c_abi::xdrfile::exdrENDOFFILE.into()),
+            task: ErrorTask::Read,
+        };
+        assert!(error.is_eof());
+
+        let error = Error {
+            code: Some(ErrorCode::ExdrEndOfFile),
+            task: ErrorTask::Read,
+        };
+        assert!(error.is_eof());
+
+        let error = Error {
+            code: Some((c_abi::xdrfile::exdrENDOFFILE + 1).into()),
+            task: ErrorTask::Read,
+        };
+        assert!(!error.is_eof());
+
+        let error = Error {
+            code: Some(0.into()),
+            task: ErrorTask::Write,
+        };
+        assert!(!error.is_eof());
+
+        let error = Error {
+            code: Some(255.into()),
+            task: ErrorTask::Flush,
+        };
+        assert!(!error.is_eof());
+
+        let error = Error {
+            code: None,
+            task: ErrorTask::OpenFile(PathBuf::from("not/a/file"), FileMode::Read),
+        };
+        assert!(!error.is_eof());
+    }
+}

src/lib.rs

@@ -62,8 +62,10 @@ extern crate assert_approx_eq;
 extern crate lazy_init;
 
 pub mod c_abi;
+mod errors;
 mod frame;
 mod iterator;
+pub use errors::*;
 pub use frame::Frame;
 pub use iterator::*;
 
@@ -78,75 +80,6 @@ use std::cell::Cell;
 use std::ffi::CString;
 use std::path::{Path, PathBuf};
 
-#[derive(Debug, Clone, PartialEq)]
-pub enum Error {
-    CouldNotOpenFile(std::path::PathBuf, FileMode),
-    CouldNotReadAtomNumber(u32),
-    CouldNotRead(u32),
-    CouldNotWrite(u32),
-    CouldNotFlush(u32),
-    PathInvalidCstring,
-}
-
-impl std::fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        use Error::*;
-        match self {
-            CouldNotOpenFile(path, mode) => write!(
-                f,
-                "Failed to open file at {path:?} with mode {mode:?}",
-                path = path,
-                mode = mode
-            ),
-            CouldNotReadAtomNumber(code) => write!(
-                f,
-                "Failed to read atom number from trajectory: C API returned error code {}",
-                code
-            ),
-            CouldNotRead(code) => write!(
-                f,
-                "Failed to read trajectory: C API returned error code {}",
-                code
-            ),
-            CouldNotWrite(code) => write!(
-                f,
-                "Failed to write trajectory: C API returned error code {}",
-                code
-            ),
-            CouldNotFlush(code) => write!(
-                f,
-                "Failed to flush trajectory: C API returned error code {}",
-                code
-            ),
-            PathInvalidCstring => write!(
-                f,
-                "Path cannot be converted to a C string because it has a 0 byte"
-            ),
-        }
-    }
-}
-
-impl Error {
-    pub fn is_eof(&self) -> bool {
-        use Error::*;
-        match self {
-            &CouldNotReadAtomNumber(code)
-            | &CouldNotRead(code)
-            | &CouldNotWrite(code)
-            | &CouldNotFlush(code)
-                if code == xdrfile::exdrENDOFFILE =>
-            {
-                true
-            }
-            _ => false,
-        }
-    }
-}
-
-impl std::error::Error for Error {}
-
-pub type Result<T, E = Error> = std::result::Result<T, E>;
-
 #[derive(Debug, Clone, PartialEq)]
 pub enum FileMode {
     Write,
@@ -168,8 +101,8 @@ impl FileMode {
 }
 
 fn path_to_cstring(path: impl AsRef<Path>) -> Result<CString> {
-    let s = path.as_ref().to_str().ok_or(Error::PathInvalidCstring)?;
+    let s = path.as_ref().to_str().ok_or_else(Error::from_convert)?;
-    CString::new(s).map_err(|_| Error::PathInvalidCstring)
+    Ok(CString::new(s)?)
 }
 
 /// A safe wrapper around the c implementation of an XDRFile
@@ -202,7 +135,7 @@ impl XDRFile {
                 })
             } else {
                 // Something went wrong. But the C api does not tell us what
-                Err(Error::CouldNotOpenFile(path.into(), filemode))
+                Err(Error::from_open(path, filemode))
             }
         }
     }
@@ -282,9 +215,9 @@ impl Trajectory for XTCTrajectory {
                 &mut self.precision.get(),
             ) as u32;
             frame.step = step as u32;
-            match code {
+            match code.into() {
-                xdrfile::exdrOK => Ok(()),
+                ErrorCode::ExdrOk => Ok(()),
-                _ => Err(Error::CouldNotRead(code)),
+                _ => Err(Error::from_read(code)),
             }
         }
     }
@@ -300,9 +233,9 @@ impl Trajectory for XTCTrajectory {
                 frame.coords[..].as_ptr() as *mut [f32; 3],
                 1000.0,
             ) as u32;
-            match code {
+            match code.into() {
-                xdrfile::exdrOK => Ok(()),
+                ErrorCode::ExdrOk => Ok(()),
-                _ => Err(Error::CouldNotWrite(code)),
+                _ => Err(Error::from_write(code)),
             }
         }
     }
@@ -310,9 +243,9 @@ impl Trajectory for XTCTrajectory {
     fn flush(&mut self) -> Result<()> {
         unsafe {
             let code = xdr_seek::xdr_flush(self.handle.xdrfile) as u32;
-            match code {
+            match code.into() {
-                xdrfile::exdrOK => Ok(()),
+                ErrorCode::ExdrOk => Ok(()),
-                _ => Err(Error::CouldNotFlush(code)),
+                _ => Err(Error::from_flush(code)),
             }
         }
     }
@@ -328,9 +261,10 @@ impl Trajectory for XTCTrajectory {
                         xdrfile_xtc::read_xtc_natoms(path_p, &mut num_atoms as *const i32) as u32;
                     // Reconstitute the CString so it is deallocated correctly
                     let _ = CString::from_raw(path_p);
-                    match code {
+
-                        xdrfile::exdrOK => Ok(num_atoms as u32),
+                    match code.into() {
-                        _ => Err(Error::CouldNotReadAtomNumber(code)),
+                        ErrorCode::ExdrOk => Ok(num_atoms as u32),
+                        _ => Err(Error::from_read_num_atoms(code)),
                     }
                 }
             })
@@ -390,9 +324,9 @@ impl Trajectory for TRRTrajectory {
                 std::ptr::null_mut(),
             ) as u32;
             frame.step = step as u32;
-            match code {
+            match code.into() {
-                xdrfile::exdrOK => Ok(()),
+                ErrorCode::ExdrOk => Ok(()),
-                _ => Err(Error::CouldNotRead(code)),
+                _ => Err(Error::from_read(code)),
             }
         }
     }
@@ -410,9 +344,9 @@ impl Trajectory for TRRTrajectory {
                 std::ptr::null_mut(),
                 std::ptr::null_mut(),
             ) as u32;
-            match code {
+            match code.into() {
-                xdrfile::exdrOK => Ok(()),
+                ErrorCode::ExdrOk => Ok(()),
-                _ => Err(Error::CouldNotWrite(code)),
+                _ => Err(Error::from_write(code)),
             }
         }
     }
@@ -420,9 +354,9 @@ impl Trajectory for TRRTrajectory {
     fn flush(&mut self) -> Result<()> {
         unsafe {
             let code = xdr_seek::xdr_flush(self.handle.xdrfile) as u32;
-            match code {
+            match code.into() {
-                xdrfile::exdrOK => Ok(()),
+                ErrorCode::ExdrOk => Ok(()),
-                _ => Err(Error::CouldNotFlush(code)),
+                _ => Err(Error::from_flush(code)),
             }
         }
     }
@@ -438,9 +372,10 @@ impl Trajectory for TRRTrajectory {
                         xdrfile_trr::read_trr_natoms(path_p, &mut num_atoms as *const i32) as u32;
                     // Reconstitute the CString so it is deallocated correctly
                     let _ = CString::from_raw(path_p);
-                    match code {
+
-                        xdrfile::exdrOK => Ok(num_atoms as u32),
+                    match code.into() {
-                        _ => Err(Error::CouldNotReadAtomNumber(code)),
+                        ErrorCode::ExdrOk => Ok(num_atoms as u32),
+                        _ => Err(Error::from_read_num_atoms(code)),
                     }
                 }
             })
@@ -536,9 +471,21 @@ mod tests {
 
     #[test]
     fn test_path_to_cstring() -> Result<(), Box<dyn std::error::Error>> {
-        let result_invalid = path_to_cstring("invalid/\0path");
+        let result_invalid = path_to_cstring(PathBuf::from("invalid/\0path"));
 
-        assert_eq!(result_invalid, Err(Error::PathInvalidCstring));
+        assert_eq!(
+            result_invalid,
+            CString::new("invalid/\0path").map_err(Error::from)
+        );
+        assert!(!result_invalid.is_ok());
+        if let Err(e) = result_invalid {
+            match e.task() {
+                ErrorTask::ToCString(_) => (),
+                _ => panic!("path_to_cstring's errortask should be ErrorTask::ToCString(_)"),
+            }
+        } else {
+            panic!("path_to_cstring on a NULL-containing string should return an error");
+        }
 
         let result_valid = path_to_cstring("valid/path");
 
@@ -549,12 +496,13 @@ mod tests {
     #[test]
     fn test_err_could_not_open() {
         let file_name = "non-existent.xtc";
-        let expexted_path = Path::new(&file_name);
+
+        let path = Path::new(&file_name);
         if let Err(e) = XDRFile::open(file_name, FileMode::Read) {
-            match e {
+            match e.task() {
-                Error::CouldNotOpenFile(err_path, err_mode) => {
+                ErrorTask::OpenFile(err_path, err_mode) => {
-                    assert_eq!(expexted_path, err_path);
+                    assert_eq!(path, err_path);
-                    assert!(FileMode::Read == err_mode)
+                    assert_eq!(FileMode::Read, *err_mode)
                 }
                 _ => panic!("Wrong Error type"),
             }
@@ -566,9 +514,9 @@ mod tests {
         let file_name = "README.md"; // not a trajectory
         let mut trr = TRRTrajectory::open_read(file_name)?;
         if let Err(e) = trr.get_num_atoms() {
-            match e {
+            match e.task() {
-                Error::CouldNotReadAtomNumber(code) => {
+                ErrorTask::ReadNumAtoms => {
-                    assert_eq!(xdrfile::exdrMAGIC, code);
+                    assert_eq!(Some(ErrorCode::ExdrMagic), *e.code());
                 }
                 _ => panic!("Wrong Error type"),
             }
@@ -582,9 +530,9 @@ mod tests {
         let mut frame = Frame::with_capacity(1);
         let mut trr = TRRTrajectory::open_read(file_name)?;
         if let Err(e) = trr.read(&mut frame) {
-            match e {
+            match e.task() {
-                Error::CouldNotRead(code) => {
+                ErrorTask::Read => {
-                    assert_eq!(xdrfile::exdrMAGIC, code);
+                    assert_eq!(Some(ErrorCode::ExdrMagic), *e.code());
                 }
                 _ => panic!("Wrong Error type"),
             }
@@ -592,24 +540,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_err_eof() {
-        let error = Error::CouldNotRead(xdrfile::exdrENDOFFILE);
-        assert!(error.is_eof());
-
-        let error = Error::CouldNotRead(xdrfile::exdrENDOFFILE + 1);
-        assert!(!error.is_eof());
-
-        let error = Error::CouldNotWrite(0);
-        assert!(!error.is_eof());
-
-        let error = Error::CouldNotFlush(255);
-        assert!(!error.is_eof());
-
-        let error = Error::CouldNotOpenFile(PathBuf::from("not/a/file"), FileMode::Read);
-        assert!(!error.is_eof());
-    }
-
     #[test]
     fn test_err_file_eof() -> Result<(), Box<dyn std::error::Error>> {
         let tempfile = NamedTempFile::new()?;