GH-73991: Make pathlib.Path.delete() private. (#123315)

Per feedback from Paul Moore on GH-123158, it's better to defer making
`Path.delete()` public than ship it with under-designed error handling
capabilities.

We leave a remnant `_delete()` method, which is used by `move()`. Any
functionality not needed by `move()` is deleted.

Author: barneygale

Doc/library/pathlib.rst

@@ -1657,7 +1657,7 @@ Copying, moving and deleting
 .. method:: Path.unlink(missing_ok=False)
 
    Remove this file or symbolic link.  If the path points to a directory,
-   use :func:`Path.rmdir` or :func:`Path.delete` instead.
+   use :func:`Path.rmdir` instead.
 
    If *missing_ok* is false (the default), :exc:`FileNotFoundError` is
    raised if the path does not exist.
@@ -1671,42 +1671,7 @@ Copying, moving and deleting
 
 .. method:: Path.rmdir()
 
-   Remove this directory.  The directory must be empty; use
-   :meth:`Path.delete` to remove a non-empty directory.
-
-
-.. method:: Path.delete(ignore_errors=False, on_error=None)
-
-   Delete this file or directory. If this path refers to a non-empty
-   directory, its files and sub-directories are deleted recursively.
-
-   If *ignore_errors* is true, errors resulting from failed deletions will be
-   ignored. If *ignore_errors* is false or omitted, and a callable is given as
-   the optional *on_error* argument, it will be called with one argument of
-   type :exc:`OSError` each time an exception is raised. The callable can
-   handle the error to continue the deletion process or re-raise it to stop.
-   Note that the filename is available as the :attr:`~OSError.filename`
-   attribute of the exception object. If neither *ignore_errors* nor
-   *on_error* are supplied, exceptions are propagated to the caller.
-
-   .. note::
-
-      When deleting non-empty directories on platforms that lack the necessary
-      file descriptor-based functions, the :meth:`~Path.delete` implementation
-      is susceptible to a symlink attack: given proper timing and
-      circumstances, attackers can manipulate symlinks on the filesystem to
-      delete files they would not be able to access otherwise. Applications
-      can use the :data:`~Path.delete.avoids_symlink_attacks` method attribute
-      to determine whether the implementation is immune to this attack.
-
-   .. attribute:: delete.avoids_symlink_attacks
-
-      Indicates whether the current platform and implementation provides a
-      symlink attack resistant version of :meth:`~Path.delete`.  Currently
-      this is only true for platforms supporting fd-based directory access
-      functions.
-
-   .. versionadded:: 3.14
+   Remove this directory.  The directory must be empty.
 
 
 Permissions and ownership

Doc/whatsnew/3.14.rst

@@ -185,14 +185,13 @@ os
 pathlib
 -------
 
-* Add methods to :class:`pathlib.Path` to recursively copy, move, or remove
-  files and directories:
+* Add methods to :class:`pathlib.Path` to recursively copy or move files and
+  directories:
 
   * :meth:`~pathlib.Path.copy` copies a file or directory tree to a destination.
   * :meth:`~pathlib.Path.copy_into` copies *into* a destination directory.
   * :meth:`~pathlib.Path.move` moves a file or directory tree to a destination.
   * :meth:`~pathlib.Path.move_into` moves *into* a destination directory.
-  * :meth:`~pathlib.Path.delete` removes a file or directory tree.
 
   (Contributed by Barney Gale in :gh:`73991`.)
 

Lib/pathlib/_abc.py

@@ -962,7 +962,7 @@ def move(self, target):
             if err.errno != EXDEV:
                 raise
         target = self.copy(target, follow_symlinks=False, preserve_metadata=True)
-        self.delete()
+        self._delete()
         return target
 
     def move_into(self, target_dir):
@@ -1004,47 +1004,29 @@ def rmdir(self):
         """
         raise UnsupportedOperation(self._unsupported_msg('rmdir()'))
 
-    def delete(self, ignore_errors=False, on_error=None):
+    def _delete(self):
         """
         Delete this file or directory (including all sub-directories).
-
-        If *ignore_errors* is true, exceptions raised from scanning the
-        filesystem and removing files and directories are ignored. Otherwise,
-        if *on_error* is set, it will be called to handle the error. If
-        neither *ignore_errors* nor *on_error* are set, exceptions are
-        propagated to the caller.
         """
-        if ignore_errors:
-            def on_error(err):
-                pass
-        elif on_error is None:
-            def on_error(err):
-                raise err
-        if self.is_dir(follow_symlinks=False):
-            results = self.walk(
-                on_error=on_error,
-                top_down=False,  # So we rmdir() empty directories.
-                follow_symlinks=False)
-            for dirpath, dirnames, filenames in results:
-                for name in filenames:
-                    try:
-                        dirpath.joinpath(name).unlink()
-                    except OSError as err:
-                        on_error(err)
-                for name in dirnames:
-                    try:
-                        dirpath.joinpath(name).rmdir()
-                    except OSError as err:
-                        on_error(err)
-            delete_self = self.rmdir
+        if self.is_symlink() or self.is_junction():
+            self.unlink()
+        elif self.is_dir():
+            self._rmtree()
         else:
-            delete_self = self.unlink
-        try:
-            delete_self()
-        except OSError as err:
-            err.filename = str(self)
-            on_error(err)
-    delete.avoids_symlink_attacks = False
+            self.unlink()
+
+    def _rmtree(self):
+        def on_error(err):
+            raise err
+        results = self.walk(
+            on_error=on_error,
+            top_down=False,  # So we rmdir() empty directories.
+            follow_symlinks=False)
+        for dirpath, _, filenames in results:
+            for filename in filenames:
+                filepath = dirpath / filename
+                filepath.unlink()
+            dirpath.rmdir()
 
     def owner(self, *, follow_symlinks=True):
         """

Lib/pathlib/_local.py

@@ -824,34 +824,7 @@ def rmdir(self):
         """
         os.rmdir(self)
 
-    def delete(self, ignore_errors=False, on_error=None):
-        """
-        Delete this file or directory (including all sub-directories).
-
-        If *ignore_errors* is true, exceptions raised from scanning the
-        filesystem and removing files and directories are ignored. Otherwise,
-        if *on_error* is set, it will be called to handle the error. If
-        neither *ignore_errors* nor *on_error* are set, exceptions are
-        propagated to the caller.
-        """
-        if self.is_dir(follow_symlinks=False):
-            onexc = None
-            if on_error:
-                def onexc(func, filename, err):
-                    err.filename = filename
-                    on_error(err)
-            shutil.rmtree(str(self), ignore_errors, onexc=onexc)
-        else:
-            try:
-                self.unlink()
-            except OSError as err:
-                if not ignore_errors:
-                    if on_error:
-                        on_error(err)
-                    else:
-                        raise
-
-    delete.avoids_symlink_attacks = shutil.rmtree.avoids_symlink_attacks
+    _rmtree = shutil.rmtree
 
     def rename(self, target):
         """