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")