Improve the Error type and add create_remove_empty_folder test.

examples/list.rs

@@ -1,5 +1,4 @@
 use trash;
-use trash::TrashItem;
 
 fn main() {
     let list = trash::list().unwrap();

src/lib.rs

@@ -2,6 +2,8 @@ use std::ffi::OsString;
 use std::hash::{Hash, Hasher};
 use std::path::{Path, PathBuf};
 
+use std::fmt;
+
 #[cfg(test)]
 mod tests;
 
@@ -19,9 +21,75 @@ mod platform;
 
 /// Error that might happen during a trash operation.
 #[derive(Debug)]
-pub enum Error {
-    Unknown,
+pub struct Error {
+    source:  Option<Box<dyn std::error::Error + 'static>>,
+    kind: ErrorKind,
+}
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let intro = "Error during a `trash` operation:";
+        if let Some(ref source) = self.source {
+            write!(f, "{} ( {:?} ) Source was '{}'", intro, self.kind, source)
+        } else {
+            write!(f, "{} ( {:?} ) Source error is not specified.", intro, self.kind)
+        }
+    }
+}
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        Some(self.source.as_ref()?.as_ref())
+    }
+}
+impl Error {
+    pub fn new(kind: ErrorKind, source: Box<dyn std::error::Error + 'static>) -> Error {
+        Error { source: Some(source), kind }
+    }
+    pub fn kind_only(kind: ErrorKind) -> Error {
+        Error { source: None, kind, }
+    }
+    pub fn kind(&self) -> &ErrorKind {
+        &self.kind
+    }
+    pub fn into_source(self) -> Option<Box<dyn std::error::Error + 'static>> {
+        self.source
+    }
+    /// Returns `Some` if the source is an `std::io::Error` error. Returns `None` otherwise.
+    /// 
+    /// In other words this is a shorthand for
+    /// `self.source().map(|x| x.downcast_ref::<std::io::Error>())`
+    pub fn io_error_source(&self) -> Option<&std::io::Error> {
+        self.source.as_ref()?.downcast_ref::<std::io::Error>()
+    }
+}
 
+///
+/// A type that is contained within [`Error`]. It provides information about why the error was
+/// produced. Some `ErrorKind` variants may promise that calling `source()`
+/// (from `std::error::Error`) on [`Error`] will return a reference to a certain type of 
+/// `std::error::Error`.
+/// 
+/// For example further information can be extracted from a `CanonicalizePath` error
+/// 
+/// ```rust
+/// use std::error::Error;
+/// let result = trash::remove_all(&["non-existing"]);
+/// if let Err(err) = result {
+///     match err.kind() { 
+///         trash::ErrorKind::CanonicalizePath{..} => (), // This is what we expect
+///         _ => panic!()
+///     };
+///     // Long format
+///     let io_kind = err.source().unwrap().downcast_ref::<std::io::Error>().unwrap().kind();
+///     assert_eq!(io_kind, std::io::ErrorKind::NotFound);
+///     // Short format
+///     let io_kind = err.io_error_source().unwrap().kind();
+///     assert_eq!(io_kind, std::io::ErrorKind::NotFound);
+/// }
+/// ```
+/// 
+/// [`Error`]: struct.Error.html
+#[derive(Debug)]
+pub enum ErrorKind {
     /// Any error that might happen during a direct call to a platform specific API.
     ///
     /// `function_name`: the name of the function during which the error occured.
@@ -30,25 +98,37 @@ pub enum Error {
     /// On Windows the `code` will contain the HRESULT that the function returned or that was
     /// obtained with `HRESULT_FROM_WIN32(GetLastError())`
     PlatformApi {
-        function_name: String,
+        function_name: &'static str,
         code: Option<i32>,
     },
 
     /// Error while canonicalizing path.
-    /// `code` contains a raw os error code if accessible.
+    /// 
+    /// The `source()` function of the `Error` will return a reference to an
+    /// `std::io::Error`.
     CanonicalizePath {
-        code: Option<i32>,
+        /// Path that triggered the error.
+        original: PathBuf,
     },
 
+    ///
+    /// NOTE: THIS ERROR WAS REMOVED
+    /// The reason for this is that it provides vauge information of the circumstances
+    /// that caused the error. The user would've had to look into the source of the library
+    /// to understand when this error is produced.
+    /// 
     /// Error while performing the remove operation.
     /// `code` contains a raw os error code if accessible.
-    Remove {
-        code: Option<i32>,
-    },
+    // Remove {
+    //     code: Option<i32>,
+    // },
 
     /// Error while converting an OsString to a String.
-    /// `original` is the string that was attempted to be converted.
+    /// 
+    /// This error kind will not provide a `source()` but it directly corresponds to the error
+    /// returned by https://doc.rust-lang.org/std/ffi/struct.OsString.html#method.into_string
     ConvertOsString {
+        /// The string that was attempted to be converted.
         original: OsString,
     },
 }

src/tests.rs

@@ -27,7 +27,18 @@ fn create_remove() {
 }
 
 #[test]
-fn create_remove_folder() {
+fn create_remove_empty_folder() {
+    let folder_name = get_unique_name();
+    let path = PathBuf::from(folder_name);
+    create_dir(&path).unwrap();
+
+    assert!(path.exists());
+    trash::remove(&path).unwrap();
+    assert!(path.exists() == false);
+}
+
+#[test]
+fn create_remove_folder_with_file() {
     let folder_name = get_unique_name();
     let path = PathBuf::from(folder_name);
     create_dir(&path).unwrap();

src/windows.rs

@@ -1,4 +1,4 @@
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsString;
 use std::mem::MaybeUninit;
 use std::ops::DerefMut;
 use std::os::windows::prelude::*;
@@ -31,7 +31,7 @@ use winapi::{
     um::shlwapi::StrRetToStrW,
     um::shobjidl_core::{
         FileOperation, IContextMenu, IEnumIDList, IFileOperation, IShellFolder, IShellFolder2,
-        IShellItem, SHCreateItemWithParent, CMF_NORMAL, CMIC_MASK_FLAG_NO_UI, CMINVOKECOMMANDINFO,
+        IShellItem, SHCreateItemWithParent, CMF_NORMAL, CMIC_MASK_FLAG_NO_UI,
         CMINVOKECOMMANDINFOEX, SHCONTF_FOLDERS, SHCONTF_NONFOLDERS, SHGDNF, SHGDN_FORPARSING,
         SHGDN_INFOLDER,
     },
@@ -45,16 +45,16 @@ use winapi::{
     Class, Interface,
 };
 
-use crate::{Error, TrashItem};
+use crate::{Error, ErrorKind, TrashItem};
 
 macro_rules! return_err_on_fail {
     {$f_name:ident($($args:tt)*)} => ({
         let hr = $f_name($($args)*);
         if !SUCCEEDED(hr) {
-            return Err(Error::PlatformApi {
+            return Err(Error::kind_only(ErrorKind::PlatformApi {
                 function_name: stringify!($f_name).into(),
                 code: Some(hr)
-            });
+            }));
         }
         hr
     });
@@ -64,10 +64,10 @@ macro_rules! return_err_on_fail {
     {($obj:expr).$f_name:ident($($args:tt)*)} => ({
         let hr = ($obj).$f_name($($args)*);
         if !SUCCEEDED(hr) {
-            return Err(Error::PlatformApi {
+            return Err(Error::kind_only(ErrorKind::PlatformApi {
                 function_name: stringify!($f_name).into(),
                 code: Some(hr)
-            });
+            }));
         }
         hr
     })
@@ -80,19 +80,19 @@ where
 {
     let paths = paths.into_iter();
     let full_paths = paths
-        .map(|x| x.as_ref().canonicalize())
-        .collect::<Result<Vec<_>, _>>()
-        .map_err(|e| Error::CanonicalizePath {
-            code: e.raw_os_error(),
-        })?;
+        .map(|x| {
+            x.as_ref().canonicalize().map_err(|e| {
+                Error::new(ErrorKind::CanonicalizePath {original: x.as_ref().into()}, Box::new(e))
+            })
+        })
+        .collect::<Result<Vec<_>, _>>()?;
     let mut wide_paths = Vec::with_capacity(full_paths.len());
     for path in full_paths.iter() {
         let mut os_string = OsString::from(path);
         os_string.push("\0");
         let mut encode_wide = os_string.as_os_str().encode_wide();
-        // Remove the "\\?\" prefix as `SHFileOperationW` fails if such a prefix is part of the path.
-        // See:
-        // https://docs.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-_shfileopstructa
+        // Remove the "\\?\" prefix as `SHFileOperationW` fails if such a prefix is part of the
+        // path. See: https://docs.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-_shfileopstructa
         assert_eq!(encode_wide.next(), Some('\\' as u16));
         assert_eq!(encode_wide.next(), Some('\\' as u16));
         assert_eq!(encode_wide.next(), Some('?' as u16));
@@ -113,12 +113,17 @@ where
         lpszProgressTitle: std::ptr::null(),
     };
 
-    let result = unsafe { SHFileOperationW(&mut fileop as *mut SHFILEOPSTRUCTW) };
+    let result = unsafe { return_err_on_fail!{
+        SHFileOperationW(&mut fileop as *mut SHFILEOPSTRUCTW)
+    }};
 
     if result == S_OK {
         Ok(())
     } else {
-        Err(Error::Remove { code: Some(result) })
+        Err(Error::kind_only(ErrorKind::PlatformApi{
+            function_name: "SHFileOperationW",
+            code: Some(result),
+        }))
     }
 }
 
@@ -147,10 +152,10 @@ pub fn list() -> Result<Vec<TrashItem>, Error> {
             )
         };
         if hr != S_OK {
-            return Err(Error::PlatformApi {
-                function_name: "EnumObjects".into(),
+            return Err(Error::kind_only(ErrorKind::PlatformApi{
+                function_name: "EnumObjects",
                 code: Some(hr),
-            });
+            }));
         }
         let peidl = peidl.assume_init();
         let mut item_vec = Vec::new();
@@ -168,7 +173,9 @@ pub fn list() -> Result<Vec<TrashItem>, Error> {
                 id,
                 name: name
                     .into_string()
-                    .map_err(|original| Error::ConvertOsString { original })?,
+                    .map_err(|original| 
+                        Error::kind_only( ErrorKind::ConvertOsString { original })
+                    )?,
                 original_parent: PathBuf::from(orig_loc),
                 time_deleted: date_deleted,
             });
@@ -361,10 +368,10 @@ unsafe fn variant_time_to_unix_time(from: f64) -> Result<i64, Error> {
     let st = st.assume_init();
     let mut ft = MaybeUninit::<FILETIME>::uninit();
     if SystemTimeToFileTime(&st, ft.as_mut_ptr()) == 0 {
-        return Err(Error::PlatformApi {
-            function_name: "SystemTimeToFileTime".into(),
+        return Err(Error::kind_only(ErrorKind::PlatformApi{
+            function_name: "SystemTimeToFileTime",
             code: Some(HRESULT_FROM_WIN32(GetLastError())),
-        });
+        }));
     }
     let ft = ft.assume_init();
     // Applying assume init straight away because there's no explicit support to initialize struct
@@ -407,10 +414,10 @@ unsafe fn invoke_verb(pcm: *mut IContextMenu, verb: &'static str) -> Result<(),
     // properties
     let hmenu = CreatePopupMenu();
     if hmenu == std::ptr::null_mut() {
-        return Err(Error::PlatformApi {
-            function_name: "CreatePopupMenu".into(),
+        return Err(Error::kind_only(ErrorKind::PlatformApi {
+            function_name: "CreatePopupMenu",
             code: Some(HRESULT_FROM_WIN32(GetLastError())),
-        });
+        }));
     }
     defer! {{ DestroyMenu(hmenu); }}
     return_err_on_fail! {(*pcm).QueryContextMenu(hmenu, 0, 1, 0x7FFF, CMF_NORMAL)};