Move mac/win to subpackages & fix #64

- Move macOS and Windows implementations to sub packagese to improve organization
- Fix #64 in legacy windows implementation by mapping results to standard error codes

send2trash/__init__.py

@@ -9,9 +9,9 @@ import sys
 from .exceptions import TrashPermissionError  # noqa: F401
 
 if sys.platform == "darwin":
-    from .plat_osx import send2trash
+    from .mac import send2trash
 elif sys.platform == "win32":
-    from .plat_win import send2trash
+    from .win import send2trash
 else:
     try:
         # If we can use gio, let's use it

send2trash/mac/__init__.py

@@ -11,10 +11,10 @@ from sys import version_info
 macos_ver = tuple(int(part) for part in mac_ver()[0].split("."))
 if version_info >= (3, 6) and macos_ver >= (10, 9):
     try:
-        from .plat_osx_pyobjc import send2trash
+        from .modern import send2trash
     except ImportError:
         # Try to fall back to ctypes version, although likely problematic still
-        from .plat_osx_ctypes import send2trash
+        from .legacy import send2trash
 else:
     # Just use the old version otherwise
-    from .plat_osx_ctypes import send2trash  # noqa: F401
+    from .legacy import send2trash  # noqa: F401

send2trash/mac/legacy.py

@@ -9,8 +9,8 @@ from __future__ import unicode_literals
 from ctypes import cdll, byref, Structure, c_char, c_char_p
 from ctypes.util import find_library
 
-from .compat import binary_type
-from .util import preprocess_paths
+from ..compat import binary_type
+from ..util import preprocess_paths
 
 Foundation = cdll.LoadLibrary(find_library("Foundation"))
 CoreServices = cdll.LoadLibrary(find_library("CoreServices"))

send2trash/mac/modern.py

@@ -5,8 +5,8 @@
 # http://www.hardcoded.net/licenses/bsd_license
 
 from Foundation import NSFileManager, NSURL
-from .compat import text_type
-from .util import preprocess_paths
+from ..compat import text_type
+from ..util import preprocess_paths
 
 
 def check_op_result(op_result):

send2trash/plat_other.py

@@ -182,26 +182,23 @@ def send2trash(paths):
             path_b = fsencode(path)
         elif isinstance(path, bytes):
             path_b = path
-        elif hasattr(path, "__fspath__"):
-            # Python 3.6 PathLike protocol
-            return send2trash(path.__fspath__())
         else:
             raise TypeError("str, bytes or PathLike expected, not %r" % type(path))
 
         if not op.exists(path_b):
-            raise OSError("File not found: %s" % path)
+            raise OSError(errno.ENOENT, "File not found: %s" % path)
         # ...should check whether the user has the necessary permissions to delete
         # it, before starting the trashing operation itself. [2]
         if not os.access(path_b, os.W_OK):
-            raise OSError("Permission denied: %s" % path)
-        # if the file to be trashed is on the same device as HOMETRASH we
-        # want to move it there.
-        path_dev = get_dev(path_b)
+            raise OSError(errno.EACCES, "Permission denied: %s" % path)
 
+        path_dev = get_dev(path_b)
         # If XDG_DATA_HOME or HOMETRASH do not yet exist we need to stat the
         # home directory, and these paths will be created further on if needed.
         trash_dev = get_dev(op.expanduser(b"~"))
 
+        # if the file to be trashed is on the same device as HOMETRASH we
+        # want to move it there.
         if path_dev == trash_dev:
             topdir = XDG_DATA_HOME
             dest_trash = HOMETRASH_B

send2trash/win/IFileOperationProgressSink.py

@@ -37,54 +37,10 @@ class FileOperationProgressSink(DesignatedWrapPolicy):
         # but that may need some additional considerations before implementing.
         return 0 if flags & shellcon.TSF_DELETE_RECYCLE_IF_POSSIBLE else 0x80004005  # S_OK, or E_FAIL
 
-    def PostDeleteItem(self, flags, item, hrDelete, newlyCreated):
-        if newlyCreated:
-            self.newItem = newlyCreated.GetDisplayName(shellcon.SHGDN_FORPARSING)
-
-    def StartOperations(self):
-        pass
-
-    def FinishOperations(self, Result):
-        pass
-
-    def PreRenameItem(self, Flags, Item, NewName):
-        pass
-
-    def PostRenameItem(self, Flags, Item, NewName, hrRename, NewlyCreated):
-        pass
-
-    def PreMoveItem(self, Flags, Item, DestinationFolder, NewName):
-        pass
-
-    def PostMoveItem(self, Flags, Item, DestinationFolder, NewName, hrMove, NewlyCreated):
-        pass
-
-    def PreCopyItem(self, Flags, Item, DestinationFolder, NewName):
-        pass
-
-    def PostCopyItem(self, Flags, Item, DestinationFolder, NewName, hrCopy, NewlyCreated):
-        pass
-
-    def PreNewItem(self, Flags, DestinationFolder, NewName):
-        pass
-
-    def PostNewItem(
-        self, Flags, DestinationFolder, NewName, TemplateName, FileAttributes, hrNew, NewItem,
-    ):
-        pass
-
-    def UpdateProgress(self, WorkTotal, WorkSoFar):
-        pass
-
-    def ResetTimer(self):
-        pass
-
-    def PauseTimer(self):
-        pass
-
-    def ResumeTimer(self):
-        pass
+    def PostDeleteItem(self, flags, item, hr_delete, newly_created):
+        if newly_created:
+            self.newItem = newly_created.GetDisplayName(shellcon.SHGDN_FORPARSING)
 
 
-def CreateSink():
+def create_sink():
     return pythoncom.WrapObject(FileOperationProgressSink(), shell.IID_IFileOperationProgressSink)

send2trash/win/__init__.py

@@ -11,10 +11,10 @@ from platform import version
 if int(version().split(".", 1)[0]) >= 6:
     try:
         # Attempt to use pywin32 to use IFileOperation
-        from .plat_win_modern import send2trash
+        from .modern import send2trash
     except ImportError:
         # use SHFileOperation as fallback
-        from .plat_win_legacy import send2trash
+        from .legacy import send2trash
 else:
     # use SHFileOperation as fallback
-    from .plat_win_legacy import send2trash  # noqa: F401
+    from .legacy import send2trash  # noqa: F401

send2trash/win/legacy.py

@@ -6,8 +6,8 @@
 
 from __future__ import unicode_literals
 import os.path as op
-from .compat import text_type
-from .util import preprocess_paths
+from ..compat import text_type
+from ..util import preprocess_paths
 
 from ctypes import (
     windll,
@@ -53,6 +53,44 @@ FOF_ALLOWUNDO = 64
 FOF_NOERRORUI = 1024
 
 
+def convert_sh_file_opt_result(result):
+    # map overlapping values from SHFileOpterationW to approximate standard windows errors
+    # ref https://docs.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shfileoperationw#return-value
+    # ref https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
+    results = {
+        0x71: 0x50,  # DE_SAMEFILE -> ERROR_FILE_EXISTS
+        0x72: 0x57,  # DE_MANYSRC1DEST -> ERROR_INVALID_PARAMETER
+        0x73: 0x57,  # DE_DIFFDIR -> ERROR_INVALID_PARAMETER
+        0x74: 0x57,  # DE_ROOTDIR -> ERROR_INVALID_PARAMETER
+        0x75: 0x4C7,  # DE_OPCANCELLED -> ERROR_CANCELLED
+        0x76: 0x57,  # DE_DESTSUBTREE -> ERROR_INVALID_PARAMETER
+        0x78: 0x05,  # DE_ACCESSDENIEDSRC -> ERROR_ACCESS_DENIED
+        0x79: 0x6F,  # DE_PATHTOODEEP -> ERROR_BUFFER_OVERFLOW
+        0x7A: 0x57,  # DE_MANYDEST -> ERROR_INVALID_PARAMETER
+        0x7C: 0xA1,  # DE_INVALIDFILES -> ERROR_BAD_PATHNAME
+        0x7D: 0x57,  # DE_DESTSAMETREE -> ERROR_INVALID_PARAMETER
+        0x7E: 0xB7,  # DE_FLDDESTISFILE -> ERROR_ALREADY_EXISTS
+        0x80: 0xB7,  # DE_FILEDESTISFLD -> ERROR_ALREADY_EXISTS
+        0x81: 0x6F,  # DE_FILENAMETOOLONG -> ERROR_BUFFER_OVERFLOW
+        0x82: 0x13,  # DE_DEST_IS_CDROM -> ERROR_WRITE_PROTECT
+        0x83: 0x13,  # DE_DEST_IS_DVD -> ERROR_WRITE_PROTECT
+        0x84: 0x6F9,  # DE_DEST_IS_CDRECORD -> ERROR_UNRECOGNIZED_MEDIA
+        0x85: 0xDF,  # DE_FILE_TOO_LARGE -> ERROR_FILE_TOO_LARGE
+        0x86: 0x13,  # DE_SRC_IS_CDROM -> ERROR_WRITE_PROTECT
+        0x87: 0x13,  # DE_SRC_IS_DVD -> ERROR_WRITE_PROTECT
+        0x88: 0x6F9,  # DE_SRC_IS_CDRECORD -> ERROR_UNRECOGNIZED_MEDIA
+        0xB7: 0x6F,  # DE_ERROR_MAX -> ERROR_BUFFER_OVERFLOW
+        0x402: 0xA1,  # UNKNOWN -> ERROR_BAD_PATHNAME
+        0x10000: 0x1D,  # ERRORONDEST -> ERROR_WRITE_FAULT
+        0x10074: 0x57,  # DE_ROOTDIR | ERRORONDEST -> ERROR_INVALID_PARAMETER
+    }
+
+    if result in results.keys():
+        return results[result]
+    else:
+        return result
+
+
 def prefix_and_path(path):
     r"""Guess the long-path prefix based on the kind of *path*.
     Local paths (C:\folder\file.ext) and UNC names (\\server\folder\file.ext)
@@ -141,4 +179,5 @@ def send2trash(paths):
     fileop.lpszProgressTitle = None
     result = SHFileOperationW(byref(fileop))
     if result:
-        raise WindowsError(result, FormatError(result), paths)
+        error = convert_sh_file_opt_result(result)
+        raise WindowsError(None, FormatError(error), paths, error)

send2trash/win/modern.py

@@ -6,13 +6,13 @@
 
 from __future__ import unicode_literals
 import os.path as op
-from .compat import text_type
-from .util import preprocess_paths
+from ..compat import text_type
+from ..util import preprocess_paths
 from platform import version
 import pythoncom
 import pywintypes
 from win32com.shell import shell, shellcon
-from .IFileOperationProgressSink import CreateSink
+from .IFileOperationProgressSink import create_sink
 
 
 def send2trash(paths):
@@ -27,7 +27,10 @@ def send2trash(paths):
     pythoncom.CoInitialize()
     # create instance of file operation object
     fileop = pythoncom.CoCreateInstance(
-        shell.CLSID_FileOperation, None, pythoncom.CLSCTX_ALL, shell.IID_IFileOperation,
+        shell.CLSID_FileOperation,
+        None,
+        pythoncom.CLSCTX_ALL,
+        shell.IID_IFileOperation,
     )
     # default flags to use
     flags = shellcon.FOF_NOCONFIRMATION | shellcon.FOF_NOERRORUI | shellcon.FOF_SILENT | shellcon.FOFX_EARLYFAILURE
@@ -42,7 +45,7 @@ def send2trash(paths):
     # actually try to perform the operation, this section may throw a
     # pywintypes.com_error which does not seem to create as nice of an
     # error as OSError so wrapping with try to convert
-    sink = CreateSink()
+    sink = create_sink()
     try:
         for path in paths:
             item = shell.SHCreateItemFromParsingName(path, None, shell.IID_IShellItem)

tests/test_plat_other.py

@@ -13,14 +13,16 @@ except ImportError:
     # py2
     from ConfigParser import ConfigParser  # noqa: F401
 
-from tempfile import mkdtemp, NamedTemporaryFile, mktemp
+from tempfile import mkdtemp, NamedTemporaryFile
 import shutil
 import stat
+import uuid
 
 if sys.platform != "win32":
     import send2trash.plat_other
     from send2trash.plat_other import send2trash as s2t
 
+    INFO_SUFFIX = send2trash.plat_other.INFO_SUFFIX.decode()
     HOMETRASH = send2trash.plat_other.HOMETRASH
 else:
     pytest.skip("Skipping non-windows tests", allow_module_level=True)
@@ -38,7 +40,7 @@ def testfile():
         # Remove trash files if they exist
         if op.exists(op.join(HOMETRASH, "files", name)):
             os.remove(op.join(HOMETRASH, "files", name))
-            os.remove(op.join(HOMETRASH, "info", name + ".trashinfo"))
+            os.remove(op.join(HOMETRASH, "info", name + INFO_SUFFIX))
     if op.exists(file.name):
         os.remove(file.name)
 
@@ -48,7 +50,9 @@ def testfiles():
     files = list(
         map(
             lambda index: NamedTemporaryFile(
-                dir=op.expanduser("~"), prefix="send2trash_test{}".format(index), delete=False,
+                dir=op.expanduser("~"),
+                prefix="send2trash_test{}".format(index),
+                delete=False,
             ),
             range(10),
         )
@@ -58,7 +62,7 @@ def testfiles():
     yield files
     filenames = [op.basename(file.name) for file in files]
     [os.remove(op.join(HOMETRASH, "files", filename)) for filename in filenames]
-    [os.remove(op.join(HOMETRASH, "info", filename + ".trashinfo")) for filename in filenames]
+    [os.remove(op.join(HOMETRASH, "info", filename + INFO_SUFFIX)) for filename in filenames]
 
 
 def test_trash(testfile):
@@ -84,52 +88,53 @@ def _filesys_enc():
 
 
 @pytest.fixture
-def testUnicodefile():
+def gen_unicode_file():
     name = u"send2trash_tést1"
     file = op.join(op.expanduser(b"~"), name.encode("utf-8"))
     touch(file)
     assert op.exists(file) is True
     yield file
     # Cleanup trash files on supported platforms
-    if sys.platform != "win32":
-        # Remove trash files if they exist
-        if op.exists(op.join(HOMETRASH, "files", name)):
-            os.remove(op.join(HOMETRASH, "files", name))
-            os.remove(op.join(HOMETRASH, "info", name + ".trashinfo"))
+    if sys.platform != "win32" and op.exists(op.join(HOMETRASH, "files", name)):
+        os.remove(op.join(HOMETRASH, "files", name))
+        os.remove(op.join(HOMETRASH, "info", name + INFO_SUFFIX))
     if op.exists(file):
         os.remove(file)
 
 
 @pytest.mark.skipif(_filesys_enc() == "ascii", reason="Requires Unicode filesystem")
-def test_trash_bytes(testUnicodefile):
-    s2t(testUnicodefile)
-    assert not op.exists(testUnicodefile)
+def test_trash_bytes(gen_unicode_file):
+    s2t(gen_unicode_file)
+    assert not op.exists(gen_unicode_file)
 
 
 @pytest.mark.skipif(_filesys_enc() == "ascii", reason="Requires Unicode filesystem")
-def test_trash_unicode(testUnicodefile):
-    s2t(testUnicodefile.decode(sys.getfilesystemencoding()))
-    assert not op.exists(testUnicodefile)
+def test_trash_unicode(gen_unicode_file):
+    s2t(gen_unicode_file.decode(sys.getfilesystemencoding()))
+    assert not op.exists(gen_unicode_file)
 
 
 class ExtVol:
     def __init__(self, path):
-        self.trashTopdir = path
+        self.trash_topdir = path
         if PY3:
-            self.trashTopdir_b = os.fsencode(self.trashTopdir)
+            self.trash_topdir_b = os.fsencode(self.trash_topdir)
         else:
-            self.trashTopdir_b = self.trashTopdir
+            self.trash_topdir_b = self.trash_topdir
 
         def s_getdev(path):
             from send2trash.plat_other import is_parent
 
             st = os.lstat(path)
-            if is_parent(self.trashTopdir, path):
+            if is_parent(self.trash_topdir, path):
                 return "dev"
             return st.st_dev
 
         def s_ismount(path):
-            if op.realpath(path) in (op.realpath(self.trashTopdir), op.realpath(self.trashTopdir_b),):
+            if op.realpath(path) in (
+                op.realpath(self.trash_topdir),
+                op.realpath(self.trash_topdir_b),
+            ):
                 return True
             return old_ismount(path)
 
@@ -141,58 +146,86 @@ class ExtVol:
     def cleanup(self):
         send2trash.plat_other.get_dev = self.old_getdev
         send2trash.plat_other.os.path.ismount = self.old_ismount
-        shutil.rmtree(self.trashTopdir)
+        shutil.rmtree(self.trash_topdir)
 
 
 @pytest.fixture
-def testExtVol():
-    trashTopdir = mkdtemp(prefix="s2t")
-    volume = ExtVol(trashTopdir)
-    fileName = "test.txt"
-    filePath = op.join(volume.trashTopdir, fileName)
-    touch(filePath)
-    assert op.exists(filePath) is True
-    yield volume, fileName, filePath
+def gen_ext_vol():
+    trash_topdir = mkdtemp(prefix="s2t")
+    volume = ExtVol(trash_topdir)
+    file_name = "test.txt"
+    file_path = op.join(volume.trash_topdir, file_name)
+    touch(file_path)
+    assert op.exists(file_path) is True
+    yield volume, file_name, file_path
     volume.cleanup()
 
 
-def test_trash_topdir(testExtVol):
-    trashDir = op.join(testExtVol[0].trashTopdir, ".Trash")
-    os.mkdir(trashDir, 0o777 | stat.S_ISVTX)
+def test_trash_topdir(gen_ext_vol):
+    trash_dir = op.join(gen_ext_vol[0].trash_topdir, ".Trash")
+    os.mkdir(trash_dir, 0o777 | stat.S_ISVTX)
 
-    s2t(testExtVol[2])
-    assert op.exists(testExtVol[2]) is False
-    assert op.exists(op.join(trashDir, str(os.getuid()), "files", testExtVol[1])) is True
-    assert op.exists(op.join(trashDir, str(os.getuid()), "info", testExtVol[1] + ".trashinfo",)) is True
+    s2t(gen_ext_vol[2])
+    assert op.exists(gen_ext_vol[2]) is False
+    assert op.exists(op.join(trash_dir, str(os.getuid()), "files", gen_ext_vol[1])) is True
+    assert (
+        op.exists(
+            op.join(
+                trash_dir,
+                str(os.getuid()),
+                "info",
+                gen_ext_vol[1] + INFO_SUFFIX,
+            )
+        )
+        is True
+    )
     # info relative path (if another test is added, with the same fileName/Path,
     # then it gets renamed etc.)
     cfg = ConfigParser()
-    cfg.read(op.join(trashDir, str(os.getuid()), "info", testExtVol[1] + ".trashinfo"))
-    assert (testExtVol[1] == cfg.get("Trash Info", "Path", raw=True)) is True
+    cfg.read(op.join(trash_dir, str(os.getuid()), "info", gen_ext_vol[1] + INFO_SUFFIX))
+    assert (gen_ext_vol[1] == cfg.get("Trash Info", "Path", raw=True)) is True
 
 
-def test_trash_topdir_fallback(testExtVol):
-    s2t(testExtVol[2])
-    assert op.exists(testExtVol[2]) is False
-    assert op.exists(op.join(testExtVol[0].trashTopdir, ".Trash-" + str(os.getuid()), "files", testExtVol[1],)) is True
+def test_trash_topdir_fallback(gen_ext_vol):
+    s2t(gen_ext_vol[2])
+    assert op.exists(gen_ext_vol[2]) is False
+    assert (
+        op.exists(
+            op.join(
+                gen_ext_vol[0].trash_topdir,
+                ".Trash-" + str(os.getuid()),
+                "files",
+                gen_ext_vol[1],
+            )
+        )
+        is True
+    )
 
 
-def test_trash_topdir_failure(testExtVol):
-    os.chmod(testExtVol[0].trashTopdir, 0o500)  # not writable to induce the exception
-    pytest.raises(TrashPermissionError, s2t, [testExtVol[2]])
-    os.chmod(testExtVol[0].trashTopdir, 0o700)  # writable to allow deletion
+def test_trash_topdir_failure(gen_ext_vol):
+    os.chmod(gen_ext_vol[0].trash_topdir, 0o500)  # not writable to induce the exception
+    pytest.raises(TrashPermissionError, s2t, [gen_ext_vol[2]])
+    os.chmod(gen_ext_vol[0].trash_topdir, 0o700)  # writable to allow deletion
 
 
-def test_trash_symlink(testExtVol):
-    # Use mktemp (race conditioney but no symlink equivalent)
-    # Since is_parent uses realpath(), and our getdev uses is_parent,
-    # this should work
-    slDir = mktemp(prefix="s2t", dir=op.expanduser("~"))
-    os.mkdir(op.join(testExtVol[0].trashTopdir, "subdir"), 0o700)
-    filePath = op.join(testExtVol[0].trashTopdir, "subdir", testExtVol[1])
-    touch(filePath)
-    os.symlink(op.join(testExtVol[0].trashTopdir, "subdir"), slDir)
-    s2t(op.join(slDir, testExtVol[1]))
-    assert op.exists(filePath) is False
-    assert op.exists(op.join(testExtVol[0].trashTopdir, ".Trash-" + str(os.getuid()), "files", testExtVol[1],)) is True
-    os.remove(slDir)
+def test_trash_symlink(gen_ext_vol):
+    # Generating a random uuid named path for symlink
+    sl_dir = op.join(op.expanduser("~"), "s2t_" + str(uuid.uuid4()))
+    os.mkdir(op.join(gen_ext_vol[0].trash_topdir, "subdir"), 0o700)
+    file_path = op.join(gen_ext_vol[0].trash_topdir, "subdir", gen_ext_vol[1])
+    touch(file_path)
+    os.symlink(op.join(gen_ext_vol[0].trash_topdir, "subdir"), sl_dir)
+    s2t(op.join(sl_dir, gen_ext_vol[1]))
+    assert op.exists(file_path) is False
+    assert (
+        op.exists(
+            op.join(
+                gen_ext_vol[0].trash_topdir,
+                ".Trash-" + str(os.getuid()),
+                "files",
+                gen_ext_vol[1],
+            )
+        )
+        is True
+    )
+    os.remove(sl_dir)

tests/test_plat_win.py

@@ -9,8 +9,8 @@ from send2trash import send2trash as s2t
 
 # import the two versions as well as the "automatic" version
 if sys.platform == "win32":
-    from send2trash.plat_win_modern import send2trash as s2t_modern
-    from send2trash.plat_win_legacy import send2trash as s2t_legacy
+    from send2trash.win.modern import send2trash as s2t_modern
+    from send2trash.win.legacy import send2trash as s2t_legacy
 else:
     pytest.skip("Skipping windows-only tests", allow_module_level=True)