Merge pull request #1501 from njsmith/metaclasses-for-everyone

docs/source/reference-core.rst

@@ -1444,8 +1444,14 @@ don't have any special access to Trio's internals.)
 .. autoclass:: Semaphore
    :members:
 
+.. We have to use :inherited-members: here because all the actual lock
+   methods are stashed in _LockImpl. Weird side-effect of having both
+   Lock and StrictFIFOLock, but wanting both to be marked Final so
+   neither can inherit from the other.
+
 .. autoclass:: Lock
    :members:
+   :inherited-members:
 
 .. autoclass:: StrictFIFOLock
    :members:

newsfragments/1044.removal.rst

@@ -0,0 +1,17 @@
+Most of the public classes that Trio exports – like `trio.Lock`,
+`trio.SocketStream`, and so on – weren't designed with subclassing in
+mind. And we've noticed that some users were trying to subclass them
+anyway, and ending up with fragile code that we're likely to
+accidentally break in the future, or else be stuck unable to make
+changes for fear of breaking subclasses.
+
+There are also some classes that were explicitly designed to be
+subclassed, like the ones in ``trio.abc``. Subclassing these is still
+supported. However, for all other classes, attempts to subclass will
+now raise a deprecation warning, and in the future will raise an
+error.
+
+If this causes problems for you, feel free to drop by our `chat room
+<https://gitter.im/python-trio/general>`__ or file a bug, to discuss
+alternatives or make a case for why some particular class should be
+designed to support subclassing.

trio/_core/_entry_queue.py

@@ -4,6 +4,7 @@
 import attr
 
 from .. import _core
+from .._util import NoPublicConstructor
 from ._wakeup_socketpair import WakeupSocketpair
 
 __all__ = ["TrioToken"]
@@ -123,7 +124,7 @@ def run_sync_soon(self, sync_fn, *args, idempotent=False):
             self.wakeup.wakeup_thread_and_signal_safe()
 
 
-class TrioToken:
+class TrioToken(metaclass=NoPublicConstructor):
     """An opaque object representing a single call to :func:`trio.run`.
 
     It has no public constructor; instead, see :func:`current_trio_token`.

trio/_core/_local.py

@@ -1,6 +1,8 @@
 # Runvar implementations
 from . import _run
 
+from .._util import SubclassingDeprecatedIn_v0_15_0
+
 __all__ = ["RunVar"]
 
 
@@ -19,7 +21,7 @@ def __init__(self, var, value):
         self.redeemed = False
 
 
-class RunVar:
+class RunVar(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """The run-local variant of a context variable.
 
     :class:`RunVar` objects are similar to context variable objects,

trio/_core/_parking_lot.py

@@ -75,6 +75,7 @@
 from collections import OrderedDict
 
 from .. import _core
+from .._util import SubclassingDeprecatedIn_v0_15_0
 
 __all__ = ["ParkingLot"]
 
@@ -87,7 +88,7 @@ class _ParkingLotStatistics:
 
 
 @attr.s(eq=False, hash=False)
-class ParkingLot:
+class ParkingLot(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """A fair wait queue with cancellation and requeueing.
 
     This class encapsulates the tricky parts of implementing a wait

trio/_core/_run.py

@@ -968,7 +968,7 @@ def __del__(self):
 
 
 @attr.s(eq=False, hash=False, repr=False)
-class Task:
+class Task(metaclass=NoPublicConstructor):
     _parent_nursery = attr.ib()
     coro = attr.ib()
     _runner = attr.ib()
@@ -1353,7 +1353,7 @@ async def python_wrapper(orig_coro):
             LOCALS_KEY_KI_PROTECTION_ENABLED, system_task
         )
 
-        task = Task(
+        task = Task._create(
             coro=coro,
             parent_nursery=nursery,
             runner=self,
@@ -1489,7 +1489,7 @@ def current_trio_token(self):
 
         """
         if self.trio_token is None:
-            self.trio_token = TrioToken(self.entry_queue)
+            self.trio_token = TrioToken._create(self.entry_queue)
         return self.trio_token
 
     ################

trio/_core/_unbounded_queue.py

@@ -2,6 +2,7 @@
 
 from .. import _core
 from .._deprecate import deprecated
+from .._util import SubclassingDeprecatedIn_v0_15_0
 
 __all__ = ["UnboundedQueue"]
 
@@ -12,7 +13,7 @@ class _UnboundedQueueStats:
     tasks_waiting = attr.ib()
 
 
-class UnboundedQueue:
+class UnboundedQueue(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """An unbounded queue suitable for certain unusual forms of inter-task
     communication.
 

trio/_highlevel_generic.py

@@ -3,6 +3,8 @@
 import trio
 from .abc import HalfCloseableStream
 
+from trio._util import SubclassingDeprecatedIn_v0_15_0
+
 
 async def aclose_forcefully(resource):
     """Close an async resource or async generator immediately, without
@@ -35,7 +37,9 @@ async def aclose_forcefully(resource):
 
 
 @attr.s(eq=False, hash=False)
-class StapledStream(HalfCloseableStream):
+class StapledStream(
+    HalfCloseableStream, metaclass=SubclassingDeprecatedIn_v0_15_0
+):
     """This class `staples <https://en.wikipedia.org/wiki/Staple_(fastener)>`__
     together two unidirectional streams to make single bidirectional stream.
 

trio/_highlevel_socket.py

@@ -5,7 +5,7 @@
 
 import trio
 from . import socket as tsocket
-from ._util import ConflictDetector
+from ._util import ConflictDetector, SubclassingDeprecatedIn_v0_15_0
 from .abc import HalfCloseableStream, Listener
 
 __all__ = ["SocketStream", "SocketListener"]
@@ -39,7 +39,9 @@ def _translate_socket_errors_to_stream_errors():
             ) from exc
 
 
-class SocketStream(HalfCloseableStream):
+class SocketStream(
+    HalfCloseableStream, metaclass=SubclassingDeprecatedIn_v0_15_0
+):
     """An implementation of the :class:`trio.abc.HalfCloseableStream`
     interface based on a raw network socket.
 
@@ -322,7 +324,9 @@ def getsockopt(self, level, option, buffersize=0):
         pass
 
 
-class SocketListener(Listener[SocketStream]):
+class SocketListener(
+    Listener[SocketStream], metaclass=SubclassingDeprecatedIn_v0_15_0
+):
     """A :class:`~trio.abc.Listener` that uses a listening socket to accept
     incoming connections as :class:`SocketStream` objects.
 

trio/_path.py

@@ -4,7 +4,7 @@
 import pathlib
 
 import trio
-from trio._util import async_wraps
+from trio._util import async_wraps, SubclassingDeprecatedIn_v0_15_0
 
 __all__ = ['Path']
 
@@ -77,7 +77,7 @@ async def wrapper(cls, *args, **kwargs):
     return wrapper
 
 
-class AsyncAutoWrapperType(type):
+class AsyncAutoWrapperType(SubclassingDeprecatedIn_v0_15_0):
     def __init__(cls, name, bases, attrs):
         super().__init__(name, bases, attrs)
 

trio/_ssl.py

@@ -158,7 +158,7 @@
 from .abc import Stream, Listener
 from ._highlevel_generic import aclose_forcefully
 from . import _sync
-from ._util import ConflictDetector
+from ._util import ConflictDetector, SubclassingDeprecatedIn_v0_15_0
 from ._deprecate import warn_deprecated
 
 ################################################################
@@ -224,7 +224,7 @@ def done(self):
 _State = _Enum("_State", ["OK", "BROKEN", "CLOSED"])
 
 
-class SSLStream(Stream):
+class SSLStream(Stream, metaclass=SubclassingDeprecatedIn_v0_15_0):
     r"""Encrypted communication using SSL/TLS.
 
     :class:`SSLStream` wraps an arbitrary :class:`~trio.abc.Stream`, and
@@ -882,7 +882,9 @@ async def wait_send_all_might_not_block(self):
                 await self.transport_stream.wait_send_all_might_not_block()
 
 
-class SSLListener(Listener[SSLStream]):
+class SSLListener(
+    Listener[SSLStream], metaclass=SubclassingDeprecatedIn_v0_15_0
+):
     """A :class:`~trio.abc.Listener` for SSL/TLS-encrypted servers.
 
     :class:`SSLListener` wraps around another Listener, and converts

trio/_sync.py

@@ -7,6 +7,7 @@
 
 from ._core import enable_ki_protection, ParkingLot
 from ._deprecate import deprecated
+from ._util import SubclassingDeprecatedIn_v0_15_0
 
 __all__ = [
     "Event",
@@ -19,7 +20,7 @@
 
 
 @attr.s(repr=False, eq=False, hash=False)
-class Event:
+class Event(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """A waitable boolean value useful for inter-task synchronization,
     inspired by :class:`threading.Event`.
 
@@ -111,7 +112,7 @@ class _CapacityLimiterStatistics:
 
 
 @async_cm
-class CapacityLimiter:
+class CapacityLimiter(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """An object for controlling access to a resource with limited capacity.
 
     Sometimes you need to put a limit on how many tasks can do something at
@@ -363,7 +364,7 @@ def statistics(self):
 
 
 @async_cm
-class Semaphore:
+class Semaphore(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """A `semaphore <https://en.wikipedia.org/wiki/Semaphore_(programming)>`__.
 
     A semaphore holds an integer value, which can be incremented by
@@ -499,19 +500,7 @@ class _LockStatistics:
 
 @async_cm
 @attr.s(eq=False, hash=False, repr=False)
-class Lock:
-    """A classic `mutex
-    <https://en.wikipedia.org/wiki/Lock_(computer_science)>`__.
-
-    This is a non-reentrant, single-owner lock. Unlike
-    :class:`threading.Lock`, only the owner of the lock is allowed to release
-    it.
-
-    A :class:`Lock` object can be used as an async context manager; it
-    blocks on entry but not on exit.
-
-    """
-
+class _LockImpl:
     _lot = attr.ib(factory=ParkingLot, init=False)
     _owner = attr.ib(default=None, init=False)
 
@@ -606,7 +595,21 @@ def statistics(self):
         )
 
 
-class StrictFIFOLock(Lock):
+class Lock(_LockImpl, metaclass=SubclassingDeprecatedIn_v0_15_0):
+    """A classic `mutex
+    <https://en.wikipedia.org/wiki/Lock_(computer_science)>`__.
+
+    This is a non-reentrant, single-owner lock. Unlike
+    :class:`threading.Lock`, only the owner of the lock is allowed to release
+    it.
+
+    A :class:`Lock` object can be used as an async context manager; it
+    blocks on entry but not on exit.
+
+    """
+
+
+class StrictFIFOLock(_LockImpl, metaclass=SubclassingDeprecatedIn_v0_15_0):
     r"""A variant of :class:`Lock` where tasks are guaranteed to acquire the
     lock in strict first-come-first-served order.
 
@@ -658,7 +661,7 @@ class StrictFIFOLock(Lock):
     :class:`StrictFIFOLock` guarantees that each task will send its data in
     the same order that the state machine generated it.
 
-    Currently, :class:`StrictFIFOLock` is simply an alias for :class:`Lock`,
+    Currently, :class:`StrictFIFOLock` is identical to :class:`Lock`,
     but (a) this may not always be true in the future, especially if Trio ever
     implements `more sophisticated scheduling policies
     <https://github.com/python-trio/trio/issues/32>`__, and (b) the above code
@@ -676,7 +679,7 @@ class _ConditionStatistics:
 
 
 @async_cm
-class Condition:
+class Condition(metaclass=SubclassingDeprecatedIn_v0_15_0):
     """A classic `condition variable
     <https://en.wikipedia.org/wiki/Monitor_(synchronization)>`__, similar to
     :class:`threading.Condition`.

trio/_unix_pipes.py

@@ -2,7 +2,7 @@
 import errno
 
 from ._abc import Stream
-from ._util import ConflictDetector
+from ._util import ConflictDetector, SubclassingDeprecatedIn_v0_15_0
 
 import trio
 
@@ -74,7 +74,7 @@ async def aclose(self):
         await trio.lowlevel.checkpoint()
 
 
-class FdStream(Stream):
+class FdStream(Stream, metaclass=SubclassingDeprecatedIn_v0_15_0):
     """
     Represents a stream given the file descriptor to a pipe, TTY, etc.
 

trio/_util.py

@@ -9,6 +9,8 @@
 import typing as t
 import threading
 
+from ._deprecate import warn_deprecated
+
 # There's a dependency loop here... _core is allowed to use this file (in fact
 # it's the *only* file in the main trio/ package it's allowed to use), but
 # ConflictDetector needs checkpoint so it also has to import
@@ -221,6 +223,20 @@ def __new__(cls, name, bases, cls_namespace):
         return super().__new__(cls, name, bases, cls_namespace)
 
 
+class SubclassingDeprecatedIn_v0_15_0(BaseMeta):
+    def __new__(cls, name, bases, cls_namespace):
+        for base in bases:
+            if isinstance(base, SubclassingDeprecatedIn_v0_15_0):
+                warn_deprecated(
+                    f"subclassing {base.__module__}.{base.__qualname__}",
+                    "0.15.0",
+                    issue=1044,
+                    instead="composition or delegation"
+                )
+                break
+        return super().__new__(cls, name, bases, cls_namespace)
+
+
 class NoPublicConstructor(Final):
     """Metaclass that enforces a class to be final (i.e., subclass not allowed)
     and ensures a private constructor.

trio/_windows_pipes.py

@@ -1,6 +1,6 @@
 from . import _core
 from ._abc import SendStream, ReceiveStream
-from ._util import ConflictDetector
+from ._util import ConflictDetector, Final
 from ._core._windows_cffi import _handle, raise_winerror, kernel32, ffi
 
 # XX TODO: don't just make this up based on nothing.
@@ -37,7 +37,7 @@ def __del__(self):
         self._close()
 
 
-class PipeSendStream(SendStream):
+class PipeSendStream(SendStream, metaclass=Final):
     """Represents a send stream over a Windows named pipe that has been
     opened in OVERLAPPED mode.
     """
@@ -79,7 +79,7 @@ async def aclose(self):
         await self._handle_holder.aclose()
 
 
-class PipeReceiveStream(ReceiveStream):
+class PipeReceiveStream(ReceiveStream, metaclass=Final):
     """Represents a receive stream over an os.pipe object."""
     def __init__(self, handle: int) -> None:
         self._handle_holder = _HandleHolder(handle)