Issue #22854: Clarify documentation about UnsupportedOperation and add tests

Also change BufferedReader.writable() and BufferedWriter.readable() to always
return False.

6 files changed, 130 insertions, 35 deletions

diff --git a/Lib/_pyio.py b/Lib/_pyio.py
index 37157d5..a467ddd 100644
--- a/Lib/_pyio.py
+++ b/Lib/_pyio.py
@@ -390,7 +390,7 @@ class IOBase(metaclass=abc.ABCMeta):
     def seekable(self):
         """Return a bool indicating whether object supports random access.
 
-        If False, seek(), tell() and truncate() will raise UnsupportedOperation.
+        If False, seek(), tell() and truncate() will raise OSError.
         This method may need to do a test seek().
         """
         return False
@@ -405,7 +405,7 @@ class IOBase(metaclass=abc.ABCMeta):
     def readable(self):
         """Return a bool indicating whether object was opened for reading.
 
-        If False, read() will raise UnsupportedOperation.
+        If False, read() will raise OSError.
         """
         return False
 
@@ -419,7 +419,7 @@ class IOBase(metaclass=abc.ABCMeta):
     def writable(self):
         """Return a bool indicating whether object was opened for writing.
 
-        If False, write() and truncate() will raise UnsupportedOperation.
+        If False, write() and truncate() will raise OSError.
         """
         return False
 
@@ -787,12 +787,6 @@ class _BufferedIOMixin(BufferedIOBase):
     def seekable(self):
         return self.raw.seekable()
 
-    def readable(self):
-        return self.raw.readable()
-
-    def writable(self):
-        return self.raw.writable()
-
     @property
     def raw(self):
         return self._raw
@@ -982,6 +976,9 @@ class BufferedReader(_BufferedIOMixin):
         self._reset_read_buf()
         self._read_lock = Lock()
 
+    def readable(self):
+        return self.raw.readable()
+
     def _reset_read_buf(self):
         self._read_buf = b""
         self._read_pos = 0
@@ -1170,6 +1167,9 @@ class BufferedWriter(_BufferedIOMixin):
         self._write_buf = bytearray()
         self._write_lock = Lock()
 
+    def writable(self):
+        return self.raw.writable()
+
     def write(self, b):
         if self.closed:
             raise ValueError("write to closed file")
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index 86440c5..51c250b 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -203,6 +203,9 @@ class MockUnseekableIO:
     def tell(self, *args):
         raise self.UnsupportedOperation("not seekable")
 
+    def truncate(self, *args):
+        raise self.UnsupportedOperation("not seekable")
+
 class CMockUnseekableIO(MockUnseekableIO, io.BytesIO):
     UnsupportedOperation = io.UnsupportedOperation
 
@@ -361,6 +364,107 @@ class IOTest(unittest.TestCase):
             self.assertRaises(exc, fp.seek, 1, self.SEEK_CUR)
             self.assertRaises(exc, fp.seek, -1, self.SEEK_END)
 
+    def test_optional_abilities(self):
+        # Test for OSError when optional APIs are not supported
+        # The purpose of this test is to try fileno(), reading, writing and
+        # seeking operations with various objects that indicate they do not
+        # support these operations.
+
+        def pipe_reader():
+            [r, w] = os.pipe()
+            os.close(w)  # So that read() is harmless
+            return self.FileIO(r, "r")
+
+        def pipe_writer():
+            [r, w] = os.pipe()
+            self.addCleanup(os.close, r)
+            # Guarantee that we can write into the pipe without blocking
+            thread = threading.Thread(target=os.read, args=(r, 100))
+            thread.start()
+            self.addCleanup(thread.join)
+            return self.FileIO(w, "w")
+
+        def buffered_reader():
+            return self.BufferedReader(self.MockUnseekableIO())
+
+        def buffered_writer():
+            return self.BufferedWriter(self.MockUnseekableIO())
+
+        def buffered_random():
+            return self.BufferedRandom(self.BytesIO())
+
+        def buffered_rw_pair():
+            return self.BufferedRWPair(self.MockUnseekableIO(),
+                self.MockUnseekableIO())
+
+        def text_reader():
+            class UnseekableReader(self.MockUnseekableIO):
+                writable = self.BufferedIOBase.writable
+                write = self.BufferedIOBase.write
+            return self.TextIOWrapper(UnseekableReader(), "ascii")
+
+        def text_writer():
+            class UnseekableWriter(self.MockUnseekableIO):
+                readable = self.BufferedIOBase.readable
+                read = self.BufferedIOBase.read
+            return self.TextIOWrapper(UnseekableWriter(), "ascii")
+
+        tests = (
+            (pipe_reader, "fr"), (pipe_writer, "fw"),
+            (buffered_reader, "r"), (buffered_writer, "w"),
+            (buffered_random, "rws"), (buffered_rw_pair, "rw"),
+            (text_reader, "r"), (text_writer, "w"),
+            (self.BytesIO, "rws"), (self.StringIO, "rws"),
+        )
+        for [test, abilities] in tests:
+            if test is pipe_writer and not threading:
+                continue  # Skip subtest that uses a background thread
+            with self.subTest(test), test() as obj:
+                readable = "r" in abilities
+                self.assertEqual(obj.readable(), readable)
+                writable = "w" in abilities
+                self.assertEqual(obj.writable(), writable)
+                seekable = "s" in abilities
+                self.assertEqual(obj.seekable(), seekable)
+
+                if isinstance(obj, self.TextIOBase):
+                    data = "3"
+                elif isinstance(obj, (self.BufferedIOBase, self.RawIOBase)):
+                    data = b"3"
+                else:
+                    self.fail("Unknown base class")
+
+                if "f" in abilities:
+                    obj.fileno()
+                else:
+                    self.assertRaises(OSError, obj.fileno)
+
+                if readable:
+                    obj.read(1)
+                    obj.read()
+                else:
+                    self.assertRaises(OSError, obj.read, 1)
+                    self.assertRaises(OSError, obj.read)
+
+                if writable:
+                    obj.write(data)
+                else:
+                    self.assertRaises(OSError, obj.write, data)
+
+                if seekable:
+                    obj.tell()
+                    obj.seek(0)
+                else:
+                    self.assertRaises(OSError, obj.tell)
+                    self.assertRaises(OSError, obj.seek, 0)
+
+                if writable and seekable:
+                    obj.truncate()
+                    obj.truncate(0)
+                else:
+                    self.assertRaises(OSError, obj.truncate)
+                    self.assertRaises(OSError, obj.truncate, 0)
+
     def test_open_handles_NUL_chars(self):
         fn_with_NUL = 'foo\0bar'
         self.assertRaises(ValueError, self.open, fn_with_NUL, 'w')
@@ -747,12 +851,6 @@ class CommonBufferedTests:
 
         self.assertEqual(42, bufio.fileno())
 
-    @unittest.skip('test having existential crisis')
-    def test_no_fileno(self):
-        # XXX will we always have fileno() function? If so, kill
-        # this test. Else, write it.
-        pass
-
     def test_invalid_args(self):
         rawio = self.MockRawIO()
         bufio = self.tp(rawio)
@@ -780,13 +878,9 @@ class CommonBufferedTests:
                 super().flush()
         rawio = self.MockRawIO()
         bufio = MyBufferedIO(rawio)
-        writable = bufio.writable()
         del bufio
         support.gc_collect()
-        if writable:
-            self.assertEqual(record, [1, 2, 3])
-        else:
-            self.assertEqual(record, [1, 2])
+        self.assertEqual(record, [1, 2, 3])
 
     def test_context_manager(self):
         # Test usability as a context manager
diff --git a/Misc/NEWS b/Misc/NEWS
index 391bbf2..e2e37a4 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -99,6 +99,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #22854: Change BufferedReader.writable() and
+  BufferedWriter.readable() to always return False.
+
 - Issue #25195: Fix a regression in mock.MagicMock. _Call is a subclass of
   tuple (changeset 3603bae63c13 only works for classes) so we need to
   implement __ne__ ourselves.  Patch by Andrew Plummer.
diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
index ae4af9d..f751446 100644
--- a/Modules/_io/bufferedio.c
+++ b/Modules/_io/bufferedio.c
@@ -2398,7 +2398,6 @@ static PyMethodDef bufferedreader_methods[] = {
     {"close", (PyCFunction)buffered_close, METH_NOARGS},
     {"seekable", (PyCFunction)buffered_seekable, METH_NOARGS},
     {"readable", (PyCFunction)buffered_readable, METH_NOARGS},
-    {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
     {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
     {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
     {"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
@@ -2489,7 +2488,6 @@ static PyMethodDef bufferedwriter_methods[] = {
     {"close", (PyCFunction)buffered_close, METH_NOARGS},
     {"detach", (PyCFunction)buffered_detach, METH_NOARGS},
     {"seekable", (PyCFunction)buffered_seekable, METH_NOARGS},
-    {"readable", (PyCFunction)buffered_readable, METH_NOARGS},
     {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
     {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
     {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
diff --git a/Modules/_io/clinic/iobase.c.h b/Modules/_io/clinic/iobase.c.h
index 3cea079..9762f11 100644
--- a/Modules/_io/clinic/iobase.c.h
+++ b/Modules/_io/clinic/iobase.c.h
@@ -66,7 +66,7 @@ PyDoc_STRVAR(_io__IOBase_seekable__doc__,
 "\n"
 "Return whether object supports random access.\n"
 "\n"
-"If False, seek(), tell() and truncate() will raise UnsupportedOperation.\n"
+"If False, seek(), tell() and truncate() will raise OSError.\n"
 "This method may need to do a test seek().");
 
 #define _IO__IOBASE_SEEKABLE_METHODDEF    \
@@ -87,7 +87,7 @@ PyDoc_STRVAR(_io__IOBase_readable__doc__,
 "\n"
 "Return whether object was opened for reading.\n"
 "\n"
-"If False, read() will raise UnsupportedOperation.");
+"If False, read() will raise OSError.");
 
 #define _IO__IOBASE_READABLE_METHODDEF    \
     {"readable", (PyCFunction)_io__IOBase_readable, METH_NOARGS, _io__IOBase_readable__doc__},
@@ -107,7 +107,7 @@ PyDoc_STRVAR(_io__IOBase_writable__doc__,
 "\n"
 "Return whether object was opened for writing.\n"
 "\n"
-"If False, write() will raise UnsupportedOperation.");
+"If False, write() will raise OSError.");
 
 #define _IO__IOBASE_WRITABLE_METHODDEF    \
     {"writable", (PyCFunction)_io__IOBase_writable, METH_NOARGS, _io__IOBase_writable__doc__},
@@ -127,7 +127,7 @@ PyDoc_STRVAR(_io__IOBase_fileno__doc__,
 "\n"
 "Returns underlying file descriptor if one exists.\n"
 "\n"
-"An IOError is raised if the IO object does not use a file descriptor.");
+"OSError is raised if the IO object does not use a file descriptor.");
 
 #define _IO__IOBASE_FILENO_METHODDEF    \
     {"fileno", (PyCFunction)_io__IOBase_fileno, METH_NOARGS, _io__IOBase_fileno__doc__},
@@ -276,4 +276,4 @@ _io__RawIOBase_readall(PyObject *self, PyObject *Py_UNUSED(ignored))
 {
     return _io__RawIOBase_readall_impl(self);
 }
-/*[clinic end generated code: output=fe034152b6884e65 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=b874952f5cc248a4 input=a9049054013a1b77]*/
diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c
index 025007e..090891d 100644
--- a/Modules/_io/iobase.c
+++ b/Modules/_io/iobase.c
@@ -335,13 +335,13 @@ _io._IOBase.seekable
 
 Return whether object supports random access.
 
-If False, seek(), tell() and truncate() will raise UnsupportedOperation.
+If False, seek(), tell() and truncate() will raise OSError.
 This method may need to do a test seek().
 [clinic start generated code]*/
 
 static PyObject *
 _io__IOBase_seekable_impl(PyObject *self)
-/*[clinic end generated code: output=4c24c67f5f32a43d input=22676eebb81dcf1e]*/
+/*[clinic end generated code: output=4c24c67f5f32a43d input=b976622f7fdf3063]*/
 {
     Py_RETURN_FALSE;
 }
@@ -368,12 +368,12 @@ _io._IOBase.readable
 
 Return whether object was opened for reading.
 
-If False, read() will raise UnsupportedOperation.
+If False, read() will raise OSError.
 [clinic start generated code]*/
 
 static PyObject *
 _io__IOBase_readable_impl(PyObject *self)
-/*[clinic end generated code: output=e48089250686388b input=12fc3d8f6be46434]*/
+/*[clinic end generated code: output=e48089250686388b input=285b3b866a0ec35f]*/
 {
     Py_RETURN_FALSE;
 }
@@ -401,12 +401,12 @@ _io._IOBase.writable
 
 Return whether object was opened for writing.
 
-If False, write() will raise UnsupportedOperation.
+If False, write() will raise OSError.
 [clinic start generated code]*/
 
 static PyObject *
 _io__IOBase_writable_impl(PyObject *self)
-/*[clinic end generated code: output=406001d0985be14f input=c17a0bb6a8dfc590]*/
+/*[clinic end generated code: output=406001d0985be14f input=9dcac18a013a05b5]*/
 {
     Py_RETURN_FALSE;
 }
@@ -456,12 +456,12 @@ _io._IOBase.fileno
 
 Returns underlying file descriptor if one exists.
 
-An IOError is raised if the IO object does not use a file descriptor.
+OSError is raised if the IO object does not use a file descriptor.
 [clinic start generated code]*/
 
 static PyObject *
 _io__IOBase_fileno_impl(PyObject *self)
-/*[clinic end generated code: output=7cc0973f0f5f3b73 input=32773c5df4b7eede]*/
+/*[clinic end generated code: output=7cc0973f0f5f3b73 input=4e37028947dc1cc8]*/
 {
     return iobase_unsupported("fileno");
 }