Make all self-move operations raise IllegalDestination error

fs/base.py

@@ -1091,15 +1091,15 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False):
                 and ``create`` is `False`.
             fs.errors.DirectoryExpected: if ``src_path`` or one of its
                 ancestors is not a directory.
+            fs.errors.IllegalDestination: when attempting to move the
+                folder at ``src_path`` to itself or one of its subfolders.
 
         """
         from .move import move_dir
 
         with self._lock:
             _src_path = self.validatepath(src_path)
             _dst_path = self.validatepath(dst_path)
-            if _src_path == _dst_path:
-                return
             if isbase(_src_path, _dst_path):
                 raise errors.IllegalDestination(dst_path)
             if not create and not self.exists(dst_path):
@@ -1169,6 +1169,8 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False):
                 and ``overwrite`` is `False`.
             fs.errors.ResourceNotFound: If a parent directory of
                 ``dst_path`` does not exist.
+            fs.errors.IllegalDestination: If ``src_path`` and ``dst_path``
+                refer to the same resource, and ``overwrite`` is `True`.
 
         """
         _src_path = self.validatepath(src_path)
@@ -1179,7 +1181,7 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False):
             raise errors.FileExpected(src_path)
         if _src_path == _dst_path:
             # early exit when moving a file onto itself
-            return
+            raise errors.IllegalDestination(dst_path)
         if self.getmeta().get("supports_rename", False):
             try:
                 src_sys_path = self.getsyspath(_src_path)

fs/memoryfs.py

@@ -465,8 +465,9 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False):
             # handle moving a file onto itself
             if src_dir == dst_dir and src_name == dst_name:
                 if overwrite:
-                    return
-                raise errors.DestinationExists(dst_path)
+                    raise errors.IllegalDestination(dst_path)
+                else:
+                    raise errors.DestinationExists(dst_path)
 
             # move the entry from the src folder to the dst folder
             dst_dir_entry.set_entry(dst_name, src_entry)
@@ -483,10 +484,7 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False):
         dst_dir, dst_name = split(_dst_path)
         src_dir, src_name = split(_src_path)
 
-        # move a dir onto itself
-        if _src_path == _dst_path:
-            return
-        # move a dir into itself
+        # move a dir onto or into itself
         if isbase(_src_path, _dst_path):
             raise errors.IllegalDestination(dst_path)
 

fs/test.py

@@ -1813,20 +1813,20 @@ def test_move_file_temp(self):
 
     def test_move_file_onto_itself(self):
         self.fs.writetext("file.txt", "Hello")
-        self.fs.move("file.txt", "file.txt", overwrite=True)
-        self.assert_text("file.txt", "Hello")
-
+        with self.assertRaises(errors.IllegalDestination):
+            self.fs.move("file.txt", "file.txt", overwrite=True)
         with self.assertRaises(errors.DestinationExists):
             self.fs.move("file.txt", "file.txt", overwrite=False)
+        self.assert_text("file.txt", "Hello")
 
     def test_move_file_onto_itself_relpath(self):
         subdir = self.fs.makedir("sub")
         subdir.writetext("file.txt", "Hello")
-        self.fs.move("sub/file.txt", "sub/../sub/file.txt", overwrite=True)
-        self.assert_text("sub/file.txt", "Hello")
-
+        with self.assertRaises(errors.IllegalDestination):
+            self.fs.move("sub/file.txt", "sub/../sub/file.txt", overwrite=True)
         with self.assertRaises(errors.DestinationExists):
             self.fs.move("sub/file.txt", "sub/../sub/file.txt", overwrite=False)
+        self.assert_text("sub/file.txt", "Hello")
 
     def test_copy_file_onto_itself(self):
         self.fs.writetext("file.txt", "Hello")
@@ -1911,8 +1911,8 @@ def test_movedir_onto_itself(self):
         folder.writetext("file1.txt", "Hello1")
         sub = folder.makedir("sub")
         sub.writetext("file2.txt", "Hello2")
-
-        self.fs.movedir("folder", "folder")
+        with self.assertRaises(errors.IllegalDestination):
+            self.fs.movedir("folder", "folder")
         self.assert_text("folder/file1.txt", "Hello1")
         self.assert_text("folder/sub/file2.txt", "Hello2")
 

tests/test_move.py

@@ -11,7 +11,7 @@
 
 import fs.move
 from fs import open_fs
-from fs.errors import FSError, ResourceReadOnly
+from fs.errors import FSError, ResourceReadOnly, IllegalDestination
 from fs.path import join
 from fs.wrap import read_only
 
@@ -175,7 +175,8 @@ def test_move_file_overwrite_itself(self, _, fs_url):
         # behaves like the regular one (TempFS tests the optmized code path).
         with open_fs(fs_url) as tmp:
             tmp.writetext("file.txt", "content")
-            fs.move.move_file(tmp, "file.txt", tmp, "file.txt")
+            with self.assertRaises(IllegalDestination):
+                fs.move.move_file(tmp, "file.txt", tmp, "file.txt")
             self.assertTrue(tmp.exists("file.txt"))
             self.assertEquals(tmp.readtext("file.txt"), "content")
 
@@ -186,7 +187,8 @@ def test_move_file_overwrite_itself_relpath(self, _, fs_url):
         with open_fs(fs_url) as tmp:
             new_dir = tmp.makedir("dir")
             new_dir.writetext("file.txt", "content")
-            fs.move.move_file(tmp, "dir/../dir/file.txt", tmp, "dir/file.txt")
+            with self.assertRaises(IllegalDestination):
+                fs.move.move_file(tmp, "dir/../dir/file.txt", tmp, "dir/file.txt")
             self.assertTrue(tmp.exists("dir/file.txt"))
             self.assertEquals(tmp.readtext("dir/file.txt"), "content")