Issue #15176: Clarified behavior, documentation, and implementation

of os.listdir().

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index bc0d69d0a4ae8dced4ba5ac6ecec6631f9e62a94..0b6ab24cda2c0ce187df708f7d298c44e2a0d581 100644 (file)
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -1488,16 +1488,19 @@ features:
 .. function:: listdir(path='.')
 
    Return a list containing the names of the entries in the directory given by
-   *path* (default: ``'.'``).  The list is in arbitrary order.  It does not
-   include the special entries ``'.'`` and ``'..'`` even if they are present in
-   the directory.
+   *path*.  The list is in arbitrary order, and does not include the special
+   entries ``'.'`` and ``'..'`` even if they are present in the directory.
 
-   This function can be called with a bytes or string argument, and returns
-   filenames of the same datatype.
+   *path* may be either of type ``str`` or of type ``bytes``.  If *path*
+   is of type ``bytes``, the filenames returned will also be of type ``bytes``;
+   in all other circumstances, they will be of type ``str``.
 
    This function can also support :ref:`specifying a file descriptor
    <path_fd>`; the file descriptor must refer to a directory.
 
+   .. note::
+      To encode ``str`` filenames to ``bytes``, use :func:`~os.fsencode`.
+
    Availability: Unix, Windows.
 
    .. versionchanged:: 3.2

diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index 3c6554bbacc3b81086999b862748f7d2b9e03f48..c4f911d28ce732ebff9d4208b3b0ef9cdf8742db 100644 (file)
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -448,16 +448,21 @@ class PosixTester(unittest.TestCase):
             self.assertRaises(OSError, posix.chdir, support.TESTFN)
 
     def test_listdir(self):
-        if hasattr(posix, 'listdir'):
-            self.assertTrue(support.TESTFN in posix.listdir(os.curdir))
+        self.assertTrue(support.TESTFN in posix.listdir(os.curdir))
 
     def test_listdir_default(self):
-        # When listdir is called without argument, it's the same as listdir(os.curdir)
-        if hasattr(posix, 'listdir'):
-            self.assertTrue(support.TESTFN in posix.listdir())
-
-    @unittest.skipUnless(os.listdir in os.supports_fd, "test needs fd support for os.listdir()")
-    def test_flistdir(self):
+        # When listdir is called without argument,
+        # it's the same as listdir(os.curdir).
+        self.assertTrue(support.TESTFN in posix.listdir())
+
+    def test_listdir_bytes(self):
+        # When listdir is called with a bytes object,
+        # the returned strings are of type bytes.
+        self.assertTrue(os.fsencode(support.TESTFN) in posix.listdir(b'.'))
+
+    @unittest.skipUnless(posix.listdir in os.supports_fd,
+                         "test needs fd support for posix.listdir()")
+    def test_listdir_fd(self):
         f = posix.open(posix.getcwd(), posix.O_RDONLY)
         self.addCleanup(posix.close, f)
         self.assertEqual(

diff --git a/Misc/NEWS b/Misc/NEWS
index ebb2f02c1bb9441a18bcabe45c558dd03ed7b288..24a14588f69e40780fa2dca28df3b6f14c4c02b5 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -59,6 +59,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #15176: Clarified behavior, documentation, and implementation
+  of os.listdir().
+  
 - Issue #15164: Change return value of platform.uname() from a
   plain tuple to a collections.namedtuple.
 

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 36f0c4c91c16cb902c68a0a54c07d8b70c3e0e3b..8f2bbfbaa70601cf4866330584ca17884385a29a 100644 (file)
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -3272,14 +3272,16 @@ exit:
 
 
 PyDoc_STRVAR(posix_listdir__doc__,
-"listdir(path='.') -> list_of_strings\n\n\
-Return a list containing the names of the entries in the directory.\n\
-\n\
+"listdir(path='.') -> list_of_filenames\n\n\
+Return a list containing the names of the files in the directory.\n\
 The list is in arbitrary order.  It does not include the special\n\
 entries '.' and '..' even if they are present in the directory.\n\
 \n\
-path can always be specified as a string.\n\
-On some platforms, path may also be specified as an open file descriptor.\n\
+path can be specified as either str or bytes.  If path is bytes,\n\
+  the filenames returned will also be bytes; in all other circumstances\n\
+  the filenames returned will be str.\n\
+On some platforms, path may also be specified as an open file descriptor;\n\
+  the file descriptor must refer to a directory.\n\
   If this functionality is unavailable, using it raises NotImplementedError.");
 
 static PyObject *
@@ -3316,7 +3318,7 @@ posix_listdir(PyObject *self, PyObject *args, PyObject *kwargs)
     PyObject *v;
     DIR *dirp = NULL;
     struct dirent *ep;
-    int arg_is_unicode = 1;
+    int return_str; /* if false, return bytes */
 #endif
 
     memset(&path, 0, sizeof(path));
@@ -3538,11 +3540,6 @@ exit:
 #else
 
     errno = 0;
-    /* v is never read, so it does not need to be initialized yet. */
-    if (path.narrow && !PyArg_ParseTuple(args, "U:listdir", &v)) {
-        arg_is_unicode = 0;
-        PyErr_Clear();
-    }
 #ifdef HAVE_FDOPENDIR
     if (path.fd != -1) {
         /* closedir() closes the FD, so we duplicate it */
@@ -3555,6 +3552,8 @@ exit:
             goto exit;
         }
 
+        return_str = 1;
+
         Py_BEGIN_ALLOW_THREADS
         dirp = fdopendir(fd);
         Py_END_ALLOW_THREADS
@@ -3562,7 +3561,17 @@ exit:
     else
 #endif
     {
-        char *name = path.narrow ? path.narrow : ".";
+        char *name;
+        if (path.narrow) {
+            name = path.narrow;
+            /* only return bytes if they specified a bytes object */
+            return_str = !(PyBytes_Check(path.object));
+        }
+        else {
+            name = ".";
+            return_str = 1;
+        }
+
         Py_BEGIN_ALLOW_THREADS
         dirp = opendir(name);
         Py_END_ALLOW_THREADS
@@ -3593,7 +3602,7 @@ exit:
             (NAMLEN(ep) == 1 ||
              (ep->d_name[1] == '.' && NAMLEN(ep) == 2)))
             continue;
-        if (arg_is_unicode)
+        if (return_str)
             v = PyUnicode_DecodeFSDefaultAndSize(ep->d_name, NAMLEN(ep));
         else
             v = PyBytes_FromStringAndSize(ep->d_name, NAMLEN(ep));