From b6c4089d6066047bb18814980a8ac8e0eb743466 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Tue, 5 Sep 2023 18:07:50 +1000 Subject: [PATCH 01/15] Reimplement _util.final as a decorator --- trio/_tests/test_util.py | 12 ++++++++++++ trio/_util.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/trio/_tests/test_util.py b/trio/_tests/test_util.py index 1ab6f825de..34ebc65183 100644 --- a/trio/_tests/test_util.py +++ b/trio/_tests/test_util.py @@ -16,6 +16,7 @@ Final, NoPublicConstructor, coroutine_or_error, + final, fixup_module_metadata, generic_function, is_main_thread, @@ -180,6 +181,17 @@ class SubClass(FinalClass): pass +def test_final_deco() -> None: + @final + class FinalClass: + pass + + with pytest.raises(TypeError): + + class SubClass(FinalClass): # type: ignore[misc] + pass + + def test_no_public_constructor_metaclass(): class SpecialClass(metaclass=NoPublicConstructor): pass diff --git a/trio/_util.py b/trio/_util.py index 73fc024831..f8c741b59c 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -311,6 +311,38 @@ def __getitem__(self, subscript: object) -> Self: return self +def _init_final_cls(cls: type[object]) -> t.NoReturn: + """Raises an exception when a final class is subclassed.""" + raise TypeError(f"{cls.__module__}.{cls.__qualname__} does not support subclassing") + + +def _final_impl(decorated: type[T]) -> type[T]: + """Decorator that enforces a class to be final (i.e., subclass not allowed). + + If a class uses this metaclass like this:: + + @final + class SomeClass: + pass + + The metaclass will ensure that no subclass can be created. + + Raises + ------ + - TypeError if a subclass is created + """ + # Override the method blindly. We're always going to raise, so it doesn't + # matter what the original did (if anything). + decorated.__init_subclass__ = classmethod(_init_final_cls) # type: ignore[assignment] + return decorated + + +if t.TYPE_CHECKING: + from typing import final +else: + final = _final_impl + + class Final(ABCMeta): """Metaclass that enforces a class to be final (i.e., subclass not allowed). From 3c4b6a3b47e4b27d892baf4780d36f78f78cf351 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Tue, 5 Sep 2023 18:11:22 +1000 Subject: [PATCH 02/15] Use _util.final instead of _util.Final --- trio/_core/_local.py | 9 +++++---- trio/_core/_mock_clock.py | 5 +++-- trio/_core/_parking_lot.py | 6 ++++-- trio/_core/_run.py | 15 +++------------ trio/_core/_unbounded_queue.py | 6 ++++-- trio/_dtls.py | 5 +++-- trio/_highlevel_generic.py | 4 ++-- trio/_highlevel_socket.py | 5 +++-- trio/_path.py | 5 +++-- trio/_ssl.py | 8 +++++--- trio/_sync.py | 20 +++++++++++++------- trio/_unix_pipes.py | 5 +++-- trio/testing/_fake_net.py | 5 +++-- trio/testing/_memory_streams.py | 6 ++++-- trio/testing/_sequencer.py | 3 ++- 15 files changed, 60 insertions(+), 47 deletions(-) diff --git a/trio/_core/_local.py b/trio/_core/_local.py index 8286a5578f..f96712bcf6 100644 --- a/trio/_core/_local.py +++ b/trio/_core/_local.py @@ -1,18 +1,18 @@ from __future__ import annotations -from typing import Generic, TypeVar, cast, final +from typing import Generic, TypeVar, cast # Runvar implementations import attr -from .._util import Final, NoPublicConstructor +from .._util import NoPublicConstructor, final from . import _run T = TypeVar("T") @final -class _NoValue(metaclass=Final): +class _NoValue: ... @@ -27,8 +27,9 @@ def _empty(cls, var: RunVar[T]) -> RunVarToken[T]: return cls._create(var) +@final @attr.s(eq=False, hash=False, slots=True, repr=False) -class RunVar(Generic[T], metaclass=Final): +class RunVar(Generic[T]): """The run-local variant of a context variable. :class:`RunVar` objects are similar to context variable objects, diff --git a/trio/_core/_mock_clock.py b/trio/_core/_mock_clock.py index 27a5829076..deb239c417 100644 --- a/trio/_core/_mock_clock.py +++ b/trio/_core/_mock_clock.py @@ -3,7 +3,7 @@ from .. import _core from .._abc import Clock -from .._util import Final +from .._util import final from ._run import GLOBAL_RUN_CONTEXT ################################################################ @@ -14,7 +14,8 @@ # Prior art: # https://twistedmatrix.com/documents/current/api/twisted.internet.task.Clock.html # https://github.com/ztellman/manifold/issues/57 -class MockClock(Clock, metaclass=Final): +@final +class MockClock(Clock): """A user-controllable clock suitable for writing tests. Args: diff --git a/trio/_core/_parking_lot.py b/trio/_core/_parking_lot.py index 6510745e5b..2c83abc8dc 100644 --- a/trio/_core/_parking_lot.py +++ b/trio/_core/_parking_lot.py @@ -79,12 +79,13 @@ import attr from .. import _core -from .._util import Final +from .._util import final if TYPE_CHECKING: from ._run import Task +@final @attr.s(frozen=True, slots=True) class ParkingLotStatistics: """An object containing debugging information for a ParkingLot. @@ -99,8 +100,9 @@ class ParkingLotStatistics: tasks_waiting: int = attr.ib() +@final @attr.s(eq=False, hash=False, slots=True) -class ParkingLot(metaclass=Final): +class ParkingLot: """A fair wait queue with cancellation and requeueing. This class encapsulates the tricky parts of implementing a wait diff --git a/trio/_core/_run.py b/trio/_core/_run.py index b2f3a65ddd..0700853367 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -24,16 +24,7 @@ from math import inf from time import perf_counter from types import TracebackType -from typing import ( - TYPE_CHECKING, - Any, - NoReturn, - Protocol, - TypeVar, - cast, - final, - overload, -) +from typing import TYPE_CHECKING, Any, NoReturn, Protocol, TypeVar, cast, overload import attr from outcome import Error, Outcome, Value, capture @@ -42,7 +33,7 @@ from .. import _core from .._abc import Clock, Instrument -from .._util import Final, NoPublicConstructor, coroutine_or_error +from .._util import NoPublicConstructor, coroutine_or_error, final from ._asyncgens import AsyncGenerators from ._entry_queue import EntryQueue, TrioToken from ._exceptions import Cancelled, RunFinishedError, TrioInternalError @@ -473,7 +464,7 @@ def effective_deadline(self) -> float: @final @attr.s(eq=False, repr=False, slots=True) -class CancelScope(metaclass=Final): +class CancelScope: """A *cancellation scope*: the link between a unit of cancellable work and Trio's cancellation system. diff --git a/trio/_core/_unbounded_queue.py b/trio/_core/_unbounded_queue.py index 1b7dea095f..7901b447c1 100644 --- a/trio/_core/_unbounded_queue.py +++ b/trio/_core/_unbounded_queue.py @@ -6,7 +6,7 @@ from .. import _core from .._deprecate import deprecated -from .._util import Final +from .._util import final T = TypeVar("T") @@ -14,6 +14,7 @@ from typing_extensions import Self +@final @attr.s(slots=True, frozen=True) class UnboundedQueueStatistics: """An object containing debugging information. @@ -30,7 +31,8 @@ class UnboundedQueueStatistics: tasks_waiting: int = attr.ib() -class UnboundedQueue(Generic[T], metaclass=Final): +@final +class UnboundedQueue(Generic[T]): """An unbounded queue suitable for certain unusual forms of inter-task communication. diff --git a/trio/_dtls.py b/trio/_dtls.py index 541144de07..dd55b8e1f9 100644 --- a/trio/_dtls.py +++ b/trio/_dtls.py @@ -33,7 +33,7 @@ import trio -from ._util import Final, NoPublicConstructor +from ._util import NoPublicConstructor, final if TYPE_CHECKING: from types import TracebackType @@ -1154,7 +1154,8 @@ def statistics(self) -> DTLSChannelStatistics: return DTLSChannelStatistics(self._packets_dropped_in_trio) -class DTLSEndpoint(metaclass=Final): +@final +class DTLSEndpoint: """A DTLS endpoint. A single UDP socket can handle arbitrarily many DTLS connections simultaneously, diff --git a/trio/_highlevel_generic.py b/trio/_highlevel_generic.py index e136b2e4bc..9c7878f2b9 100644 --- a/trio/_highlevel_generic.py +++ b/trio/_highlevel_generic.py @@ -5,7 +5,7 @@ import attr import trio -from trio._util import Final +from trio._util import final from .abc import AsyncResource, HalfCloseableStream, ReceiveStream, SendStream @@ -52,11 +52,11 @@ def _is_halfclosable(stream: SendStream) -> TypeGuard[HalfCloseableStream]: return hasattr(stream, "send_eof") +@final @attr.s(eq=False, hash=False) class StapledStream( HalfCloseableStream, Generic[SendStreamT, ReceiveStreamT], - metaclass=Final, ): """This class `staples `__ together two unidirectional streams to make single bidirectional stream. diff --git a/trio/_highlevel_socket.py b/trio/_highlevel_socket.py index f8d01cd755..0067e80a35 100644 --- a/trio/_highlevel_socket.py +++ b/trio/_highlevel_socket.py @@ -9,7 +9,7 @@ import trio from . import socket as tsocket -from ._util import ConflictDetector, Final +from ._util import ConflictDetector, final from .abc import HalfCloseableStream, Listener if TYPE_CHECKING: @@ -42,7 +42,8 @@ def _translate_socket_errors_to_stream_errors() -> Generator[None, None, None]: raise trio.BrokenResourceError(f"socket connection broken: {exc}") from exc -class SocketStream(HalfCloseableStream, metaclass=Final): +@final +class SocketStream(HalfCloseableStream): """An implementation of the :class:`trio.abc.HalfCloseableStream` interface based on a raw network socket. diff --git a/trio/_path.py b/trio/_path.py index c2763e03af..4f676ee572 100644 --- a/trio/_path.py +++ b/trio/_path.py @@ -22,7 +22,7 @@ import trio from trio._file_io import AsyncIOWrapper as _AsyncIOWrapper -from trio._util import Final, async_wraps +from trio._util import async_wraps, final if TYPE_CHECKING: from _typeshed import ( @@ -129,7 +129,7 @@ async def wrapper(cls: type[Path], *args: Any, **kwargs: Any) -> Path: # type: return classmethod(wrapper) -class AsyncAutoWrapperType(Final): +class AsyncAutoWrapperType: _forwards: type _wraps: type _forward_magic: list[str] @@ -197,6 +197,7 @@ def generate_iter(cls, attrs: dict[str, object]) -> None: setattr(cls, attr_name, wrapper) +@final class Path(metaclass=AsyncAutoWrapperType): """A :class:`pathlib.Path` wrapper that executes blocking methods in :meth:`trio.to_thread.run_sync`. diff --git a/trio/_ssl.py b/trio/_ssl.py index f0f01f7583..c31383cec5 100644 --- a/trio/_ssl.py +++ b/trio/_ssl.py @@ -10,7 +10,7 @@ from . import _sync from ._highlevel_generic import aclose_forcefully -from ._util import ConflictDetector, Final +from ._util import ConflictDetector, final from .abc import Listener, Stream # General theory of operation: @@ -240,7 +240,8 @@ def done(self) -> bool: _State = _Enum("_State", ["OK", "BROKEN", "CLOSED"]) -class SSLStream(Stream, metaclass=Final): +@final +class SSLStream(Stream): r"""Encrypted communication using SSL/TLS. :class:`SSLStream` wraps an arbitrary :class:`~trio.abc.Stream`, and @@ -888,7 +889,8 @@ async def wait_send_all_might_not_block(self) -> None: await self.transport_stream.wait_send_all_might_not_block() -class SSLListener(Listener[SSLStream], metaclass=Final): +@final +class SSLListener(Listener[SSLStream]): """A :class:`~trio.abc.Listener` for SSL/TLS-encrypted servers. :class:`SSLListener` wraps around another Listener, and converts diff --git a/trio/_sync.py b/trio/_sync.py index df4790ae74..df0a44c3dc 100644 --- a/trio/_sync.py +++ b/trio/_sync.py @@ -9,7 +9,7 @@ from . import _core from ._core import Abort, ParkingLot, RaiseCancelT, enable_ki_protection -from ._util import Final +from ._util import final if TYPE_CHECKING: from types import TracebackType @@ -32,8 +32,9 @@ class EventStatistics: tasks_waiting: int = attr.ib() +@final @attr.s(repr=False, eq=False, hash=False, slots=True) -class Event(metaclass=Final): +class Event: """A waitable boolean value useful for inter-task synchronization, inspired by :class:`threading.Event`. @@ -158,7 +159,8 @@ class CapacityLimiterStatistics: # Can be a generic type with a default of Task if/when PEP 696 is released # and implemented in type checkers. Making it fully generic would currently # introduce a lot of unnecessary hassle. -class CapacityLimiter(AsyncContextManagerMixin, metaclass=Final): +@final +class CapacityLimiter(AsyncContextManagerMixin): """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 @@ -400,7 +402,8 @@ def statistics(self) -> CapacityLimiterStatistics: ) -class Semaphore(AsyncContextManagerMixin, metaclass=Final): +@final +class Semaphore(AsyncContextManagerMixin): """A `semaphore `__. A semaphore holds an integer value, which can be incremented by @@ -631,7 +634,8 @@ def statistics(self) -> LockStatistics: ) -class Lock(_LockImpl, metaclass=Final): +@final +class Lock(_LockImpl): """A classic `mutex `__. @@ -645,7 +649,8 @@ class Lock(_LockImpl, metaclass=Final): """ -class StrictFIFOLock(_LockImpl, metaclass=Final): +@final +class StrictFIFOLock(_LockImpl): r"""A variant of :class:`Lock` where tasks are guaranteed to acquire the lock in strict first-come-first-served order. @@ -724,7 +729,8 @@ class ConditionStatistics: lock_statistics: LockStatistics = attr.ib() -class Condition(AsyncContextManagerMixin, metaclass=Final): +@final +class Condition(AsyncContextManagerMixin): """A classic `condition variable `__, similar to :class:`threading.Condition`. diff --git a/trio/_unix_pipes.py b/trio/_unix_pipes.py index 1a389e12dd..476d91f6bc 100644 --- a/trio/_unix_pipes.py +++ b/trio/_unix_pipes.py @@ -8,7 +8,7 @@ import trio from ._abc import Stream -from ._util import ConflictDetector, Final +from ._util import ConflictDetector, final if TYPE_CHECKING: from typing import Final as FinalType @@ -84,7 +84,8 @@ def close(self) -> None: self._raw_close() -class FdStream(Stream, metaclass=Final): +@final +class FdStream(Stream): """ Represents a stream given the file descriptor to a pipe, TTY, etc. diff --git a/trio/testing/_fake_net.py b/trio/testing/_fake_net.py index ddf46174f3..948755af6f 100644 --- a/trio/testing/_fake_net.py +++ b/trio/testing/_fake_net.py @@ -16,7 +16,7 @@ import attr import trio -from trio._util import Final, NoPublicConstructor +from trio._util import NoPublicConstructor, final if TYPE_CHECKING: from socket import AddressFamily, SocketKind @@ -138,7 +138,8 @@ async def getnameinfo( raise NotImplementedError("FakeNet doesn't do fake DNS yet") -class FakeNet(metaclass=Final): +@final +class FakeNet: def __init__(self) -> None: # When we need to pick an arbitrary unique ip address/port, use these: self._auto_ipv4_iter = ipaddress.IPv4Network("1.0.0.0/8").hosts() diff --git a/trio/testing/_memory_streams.py b/trio/testing/_memory_streams.py index fc23fae842..2a9bc1a2fd 100644 --- a/trio/testing/_memory_streams.py +++ b/trio/testing/_memory_streams.py @@ -86,7 +86,8 @@ async def get(self, max_bytes: int | None = None) -> bytearray: return self._get_impl(max_bytes) -class MemorySendStream(SendStream, metaclass=_util.Final): +@_util.final +class MemorySendStream(SendStream): """An in-memory :class:`~trio.abc.SendStream`. Args: @@ -200,7 +201,8 @@ def get_data_nowait(self, max_bytes: int | None = None) -> bytearray: return self._outgoing.get_nowait(max_bytes) -class MemoryReceiveStream(ReceiveStream, metaclass=_util.Final): +@_util.final +class MemoryReceiveStream(ReceiveStream): """An in-memory :class:`~trio.abc.ReceiveStream`. Args: diff --git a/trio/testing/_sequencer.py b/trio/testing/_sequencer.py index 137fd3c522..a6e6d8e8eb 100644 --- a/trio/testing/_sequencer.py +++ b/trio/testing/_sequencer.py @@ -12,8 +12,9 @@ from collections.abc import AsyncIterator +@_util.final @attr.s(eq=False, hash=False) -class Sequencer(metaclass=_util.Final): +class Sequencer: """A convenience class for forcing code in different tasks to run in an explicit linear order. From 8744a6c98a35d6ec5bef25723280da2f1db751ce Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Tue, 5 Sep 2023 18:13:35 +1000 Subject: [PATCH 03/15] There is no longer a name collision with typing.Final --- trio/_core/_run.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 0700853367..e5289233bc 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -24,7 +24,16 @@ from math import inf from time import perf_counter from types import TracebackType -from typing import TYPE_CHECKING, Any, NoReturn, Protocol, TypeVar, cast, overload +from typing import ( + TYPE_CHECKING, + Any, + Final, + NoReturn, + Protocol, + TypeVar, + cast, + overload, +) import attr from outcome import Error, Outcome, Value, capture @@ -58,15 +67,12 @@ if TYPE_CHECKING: import contextvars - # An unfortunate name collision here with trio._util.Final - from typing import Final as FinalT - from typing_extensions import Self -DEADLINE_HEAP_MIN_PRUNE_THRESHOLD: FinalT = 1000 +DEADLINE_HEAP_MIN_PRUNE_THRESHOLD: Final = 1000 # Passed as a sentinel -_NO_SEND: FinalT = cast("Outcome[Any]", object()) +_NO_SEND: Final = cast("Outcome[Any]", object()) FnT = TypeVar("FnT", bound="Callable[..., Any]") StatusT = TypeVar("StatusT") @@ -91,7 +97,7 @@ def _public(fn: FnT) -> FnT: # variable to True, and registers the Random instance _r for Hypothesis # to manage for each test case, which together should make Trio's task # scheduling loop deterministic. We have a test for that, of course. -_ALLOW_DETERMINISTIC_SCHEDULING: FinalT = False +_ALLOW_DETERMINISTIC_SCHEDULING: Final = False _r = random.Random() @@ -136,7 +142,7 @@ def function_with_unique_name_xyzzy() -> NoReturn: ) -CONTEXT_RUN_TB_FRAMES: FinalT = _count_context_run_tb_frames() +CONTEXT_RUN_TB_FRAMES: Final = _count_context_run_tb_frames() @attr.s(frozen=True, slots=True) @@ -1419,7 +1425,7 @@ class RunContext(threading.local): task: Task -GLOBAL_RUN_CONTEXT: FinalT = RunContext() +GLOBAL_RUN_CONTEXT: Final = RunContext() @attr.frozen @@ -2389,7 +2395,7 @@ def my_done_callback(run_outcome): # 24 hours is arbitrary, but it avoids issues like people setting timeouts of # 10**20 and then getting integer overflows in the underlying system calls. -_MAX_TIMEOUT: FinalT = 24 * 60 * 60 +_MAX_TIMEOUT: Final = 24 * 60 * 60 # Weird quirk: this is written as a generator in order to support "guest @@ -2647,7 +2653,7 @@ def started(self, value: Any = None) -> None: pass -TASK_STATUS_IGNORED: FinalT[TaskStatus[Any]] = _TaskStatusIgnored() +TASK_STATUS_IGNORED: Final[TaskStatus[Any]] = _TaskStatusIgnored() def current_task() -> Task: From 3573d03dd447856203af5bf4c61e80a1fea35ec9 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Tue, 5 Sep 2023 18:17:27 +1000 Subject: [PATCH 04/15] Use util.final along NoPublicConstructor --- trio/_channel.py | 4 +++- trio/_core/_entry_queue.py | 3 ++- trio/_core/_exceptions.py | 3 ++- trio/_core/_local.py | 1 + trio/_dtls.py | 1 + trio/_subprocess.py | 3 ++- trio/_tests/test_util.py | 5 ----- trio/testing/_fake_net.py | 1 + 8 files changed, 12 insertions(+), 9 deletions(-) diff --git a/trio/_channel.py b/trio/_channel.py index db122d37f5..999a634394 100644 --- a/trio/_channel.py +++ b/trio/_channel.py @@ -13,7 +13,7 @@ from ._abc import ReceiveChannel, ReceiveType, SendChannel, SendType, T from ._core import Abort, RaiseCancelT, Task, enable_ki_protection -from ._util import NoPublicConstructor, generic_function +from ._util import NoPublicConstructor, final, generic_function if TYPE_CHECKING: from typing_extensions import Self @@ -138,6 +138,7 @@ def statistics(self) -> MemoryChannelStats: ) +@final @attr.s(eq=False, repr=False) class MemorySendChannel(SendChannel[SendType], metaclass=NoPublicConstructor): _state: MemoryChannelState[SendType] = attr.ib() @@ -282,6 +283,7 @@ async def aclose(self) -> None: await trio.lowlevel.checkpoint() +@final @attr.s(eq=False, repr=False) class MemoryReceiveChannel(ReceiveChannel[ReceiveType], metaclass=NoPublicConstructor): _state: MemoryChannelState[ReceiveType] = attr.ib() diff --git a/trio/_core/_entry_queue.py b/trio/_core/_entry_queue.py index 468a13462a..cb91025fbb 100644 --- a/trio/_core/_entry_queue.py +++ b/trio/_core/_entry_queue.py @@ -7,7 +7,7 @@ import attr from .. import _core -from .._util import NoPublicConstructor +from .._util import NoPublicConstructor, final from ._wakeup_socketpair import WakeupSocketpair # TODO: Type with TypeVarTuple, at least to an extent where it makes @@ -139,6 +139,7 @@ def run_sync_soon( self.wakeup.wakeup_thread_and_signal_safe() +@final @attr.s(eq=False, hash=False, slots=True) class TrioToken(metaclass=NoPublicConstructor): """An opaque object representing a single call to :func:`trio.run`. diff --git a/trio/_core/_exceptions.py b/trio/_core/_exceptions.py index bdc7b31c21..4996c18f15 100644 --- a/trio/_core/_exceptions.py +++ b/trio/_core/_exceptions.py @@ -1,4 +1,4 @@ -from trio._util import NoPublicConstructor +from trio._util import NoPublicConstructor, final class TrioInternalError(Exception): @@ -26,6 +26,7 @@ class WouldBlock(Exception): """Raised by ``X_nowait`` functions if ``X`` would block.""" +@final class Cancelled(BaseException, metaclass=NoPublicConstructor): """Raised by blocking calls if the surrounding scope has been cancelled. diff --git a/trio/_core/_local.py b/trio/_core/_local.py index f96712bcf6..27252bc78d 100644 --- a/trio/_core/_local.py +++ b/trio/_core/_local.py @@ -16,6 +16,7 @@ class _NoValue: ... +@final @attr.s(eq=False, hash=False, slots=False) class RunVarToken(Generic[T], metaclass=NoPublicConstructor): _var: RunVar[T] = attr.ib() diff --git a/trio/_dtls.py b/trio/_dtls.py index dd55b8e1f9..4a244ecca8 100644 --- a/trio/_dtls.py +++ b/trio/_dtls.py @@ -812,6 +812,7 @@ class DTLSChannelStatistics: incoming_packets_dropped_in_trio: int +@final class DTLSChannel(trio.abc.Channel[bytes], metaclass=NoPublicConstructor): """A DTLS connection. diff --git a/trio/_subprocess.py b/trio/_subprocess.py index 978f7e6188..ffd7483780 100644 --- a/trio/_subprocess.py +++ b/trio/_subprocess.py @@ -23,7 +23,7 @@ wait_child_exiting, ) from ._sync import Lock -from ._util import NoPublicConstructor +from ._util import NoPublicConstructor, final if TYPE_CHECKING: from typing_extensions import TypeAlias @@ -85,6 +85,7 @@ def fileno(self) -> int: ... +@final class Process(AsyncResource, metaclass=NoPublicConstructor): r"""A child process. Like :class:`subprocess.Popen`, but async. diff --git a/trio/_tests/test_util.py b/trio/_tests/test_util.py index 34ebc65183..7b44e80cc6 100644 --- a/trio/_tests/test_util.py +++ b/trio/_tests/test_util.py @@ -199,11 +199,6 @@ class SpecialClass(metaclass=NoPublicConstructor): with pytest.raises(TypeError): SpecialClass() - with pytest.raises(TypeError): - - class SubClass(SpecialClass): - pass - # Private constructor should not raise assert isinstance(SpecialClass._create(), SpecialClass) diff --git a/trio/testing/_fake_net.py b/trio/testing/_fake_net.py index 948755af6f..7cd0c25225 100644 --- a/trio/testing/_fake_net.py +++ b/trio/testing/_fake_net.py @@ -174,6 +174,7 @@ def deliver_packet(self, packet) -> None: pass +@final class FakeSocket(trio.socket.SocketType, metaclass=NoPublicConstructor): def __init__(self, fake_net: FakeNet, family: int, type: int, proto: int): self._fake_net = fake_net From 6a869ecee6e480663433f731da6b9fa3e272e04d Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Wed, 6 Sep 2023 12:36:43 +1000 Subject: [PATCH 05/15] Fix various mistakes --- trio/_highlevel_socket.py | 3 ++- trio/_path.py | 2 +- trio/_util.py | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/trio/_highlevel_socket.py b/trio/_highlevel_socket.py index 0067e80a35..af7ed7278d 100644 --- a/trio/_highlevel_socket.py +++ b/trio/_highlevel_socket.py @@ -355,7 +355,8 @@ def getsockopt(self, level: int, option: int, buffersize: int = 0) -> int | byte pass -class SocketListener(Listener[SocketStream], metaclass=Final): +@final +class SocketListener(Listener[SocketStream]): """A :class:`~trio.abc.Listener` that uses a listening socket to accept incoming connections as :class:`SocketStream` objects. diff --git a/trio/_path.py b/trio/_path.py index 4f676ee572..f46b047cf9 100644 --- a/trio/_path.py +++ b/trio/_path.py @@ -129,7 +129,7 @@ async def wrapper(cls: type[Path], *args: Any, **kwargs: Any) -> Path: # type: return classmethod(wrapper) -class AsyncAutoWrapperType: +class AsyncAutoWrapperType(type): _forwards: type _wraps: type _forward_magic: list[str] diff --git a/trio/_util.py b/trio/_util.py index f8c741b59c..ae36238a01 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -373,6 +373,7 @@ def __new__( return super().__new__(cls, name, bases, cls_namespace) +@final # No subclassing of NoPublicConstructor itself. class NoPublicConstructor(Final): """Metaclass that enforces a class to be final (i.e., subclass not allowed) and ensures a private constructor. From 7d38989b6f72c1eab34e06f392826563a94da2e5 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Wed, 6 Sep 2023 12:40:24 +1000 Subject: [PATCH 06/15] Verify that all NoPublicConstructor classes are also final. --- trio/_tests/test_exports.py | 32 +++++++++++++++++++++++++++----- trio/_util.py | 2 ++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/trio/_tests/test_exports.py b/trio/_tests/test_exports.py index 2f1157db06..f9a5939b90 100644 --- a/trio/_tests/test_exports.py +++ b/trio/_tests/test_exports.py @@ -8,6 +8,8 @@ import json import socket as stdlib_socket import sys +import types +from collections.abc import Iterator from pathlib import Path from types import ModuleType from typing import Protocol @@ -70,10 +72,19 @@ def test_core_is_properly_reexported(): assert found == 1 -def public_modules(module): +def class_is_final(cls: type) -> bool: + """Check if a class has _util.final / typing.final applied.""" + # We use vars() to bypass this being set in superclasses or the metaclass. + return vars(cls).get("__final__", False) + + +def iter_modules( + module: types.ModuleType, + only_public: bool, +) -> Iterator[types.ModuleType]: yield module for name, class_ in module.__dict__.items(): - if name.startswith("_"): # pragma: no cover + if name.startswith("_") and only_public: continue if not isinstance(class_, ModuleType): continue @@ -81,10 +92,11 @@ def public_modules(module): continue if class_ is module: # pragma: no cover continue - yield from public_modules(class_) + yield from iter_modules(class_, only_public) -PUBLIC_MODULES = list(public_modules(trio)) +PUBLIC_MODULES = list(iter_modules(trio, only_public=True)) +ALL_MODULES = list(iter_modules(trio, only_public=False)) PUBLIC_MODULE_NAMES = [m.__name__ for m in PUBLIC_MODULES] @@ -484,6 +496,16 @@ def lookup_symbol(symbol): assert not errors +def test_nopublic_is_final() -> None: + """Check all NoPublicConstructor classes are also @final.""" + assert class_is_final(_util.NoPublicConstructor) # This is itself final. + + for module in ALL_MODULES: + for name, class_ in module.__dict__.items(): + if isinstance(class_, _util.NoPublicConstructor): + assert class_is_final(class_) + + def test_classes_are_final() -> None: for module in PUBLIC_MODULES: for name, class_ in module.__dict__.items(): @@ -519,4 +541,4 @@ def test_classes_are_final() -> None: if name.endswith("Statistics"): continue - assert isinstance(class_, _util.Final) + assert class_is_final(class_), f"{class_.__module__}.{class_.__qualname__}" diff --git a/trio/_util.py b/trio/_util.py index ae36238a01..39bbc87dc5 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -334,6 +334,8 @@ class SomeClass: # Override the method blindly. We're always going to raise, so it doesn't # matter what the original did (if anything). decorated.__init_subclass__ = classmethod(_init_final_cls) # type: ignore[assignment] + # Python 3.11+ sets this attr, do it on all versions so this being present can be tested. + decorated.__final__ = True # type: ignore[attr-defined] return decorated From 235ff2ef2be3ce81bc8f049f92d2646aa97d3da2 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Wed, 6 Sep 2023 12:51:07 +1000 Subject: [PATCH 07/15] Add final decorator to _windows_pipes --- trio/_windows_pipes.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/trio/_windows_pipes.py b/trio/_windows_pipes.py index c1c357b018..a4f59b2c70 100644 --- a/trio/_windows_pipes.py +++ b/trio/_windows_pipes.py @@ -4,7 +4,7 @@ from . import _core from ._abc import ReceiveStream, SendStream from ._core._windows_cffi import _handle, kernel32, raise_winerror -from ._util import ConflictDetector, Final +from ._util import ConflictDetector, final assert sys.platform == "win32" or not TYPE_CHECKING @@ -38,7 +38,8 @@ def __del__(self): self.close() -class PipeSendStream(SendStream, metaclass=Final): +@final +class PipeSendStream(SendStream): """Represents a send stream over a Windows named pipe that has been opened in OVERLAPPED mode. """ @@ -83,7 +84,8 @@ async def aclose(self): await _core.checkpoint() -class PipeReceiveStream(ReceiveStream, metaclass=Final): +@final +class PipeReceiveStream(ReceiveStream): """Represents a receive stream over an os.pipe object.""" def __init__(self, handle: int) -> None: From bcec66ef81c2c4de131d3d45bfad8afe71425ab8 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Wed, 6 Sep 2023 12:52:26 +1000 Subject: [PATCH 08/15] Adjust NoPublicConstructor to require use with final --- trio/_util.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/trio/_util.py b/trio/_util.py index 39bbc87dc5..e2986579cc 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -376,23 +376,24 @@ def __new__( @final # No subclassing of NoPublicConstructor itself. -class NoPublicConstructor(Final): - """Metaclass that enforces a class to be final (i.e., subclass not allowed) - and ensures a private constructor. +class NoPublicConstructor(ABCMeta): + """Metaclass that ensures a private constructor. If a class uses this metaclass like this:: + @final class SomeClass(metaclass=NoPublicConstructor): pass - The metaclass will ensure that no subclass can be created, and that no instance - can be initialized. + The metaclass will ensure that no instance can be initialized. This should always be + used with @final. - If you try to instantiate your class (SomeClass()), a TypeError will be thrown. + If you try to instantiate your class (SomeClass()), a TypeError will be thrown. Use + _create() instead in the class's implementation. Raises ------ - - TypeError if a subclass or an instance is created. + - TypeError if an instance is created. """ def __call__(cls, *args: object, **kwargs: object) -> None: From b62a68df480da5b1e93709282253412f1b72596f Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Wed, 6 Sep 2023 12:52:36 +1000 Subject: [PATCH 09/15] Remove _util.Final --- trio/_tests/test_util.py | 13 +----------- trio/_tests/verify_types_darwin.json | 2 +- trio/_tests/verify_types_linux.json | 2 +- trio/_tests/verify_types_windows.json | 2 +- trio/_util.py | 30 --------------------------- 5 files changed, 4 insertions(+), 45 deletions(-) diff --git a/trio/_tests/test_util.py b/trio/_tests/test_util.py index 7b44e80cc6..b2edf557aa 100644 --- a/trio/_tests/test_util.py +++ b/trio/_tests/test_util.py @@ -13,7 +13,6 @@ ) from .._util import ( ConflictDetector, - Final, NoPublicConstructor, coroutine_or_error, final, @@ -171,17 +170,7 @@ def test_func(arg): assert test_func.__module__ == __name__ -def test_final_metaclass(): - class FinalClass(metaclass=Final): - pass - - with pytest.raises(TypeError): - - class SubClass(FinalClass): - pass - - -def test_final_deco() -> None: +def test_final_decorator() -> None: @final class FinalClass: pass diff --git a/trio/_tests/verify_types_darwin.json b/trio/_tests/verify_types_darwin.json index 2b89d28d8e..3f4c0c0917 100644 --- a/trio/_tests/verify_types_darwin.json +++ b/trio/_tests/verify_types_darwin.json @@ -76,7 +76,7 @@ ], "otherSymbolCounts": { "withAmbiguousType": 0, - "withKnownType": 680, + "withKnownType": 678, "withUnknownType": 0 }, "packageName": "trio" diff --git a/trio/_tests/verify_types_linux.json b/trio/_tests/verify_types_linux.json index ea5af77abc..e61b13782c 100644 --- a/trio/_tests/verify_types_linux.json +++ b/trio/_tests/verify_types_linux.json @@ -64,7 +64,7 @@ ], "otherSymbolCounts": { "withAmbiguousType": 0, - "withKnownType": 680, + "withKnownType": 678, "withUnknownType": 0 }, "packageName": "trio" diff --git a/trio/_tests/verify_types_windows.json b/trio/_tests/verify_types_windows.json index 5d3e29a5dc..551bd67c60 100644 --- a/trio/_tests/verify_types_windows.json +++ b/trio/_tests/verify_types_windows.json @@ -180,7 +180,7 @@ ], "otherSymbolCounts": { "withAmbiguousType": 0, - "withKnownType": 671, + "withKnownType": 669, "withUnknownType": 0 }, "packageName": "trio" diff --git a/trio/_util.py b/trio/_util.py index e2986579cc..6c514a4919 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -345,36 +345,6 @@ class SomeClass: final = _final_impl -class Final(ABCMeta): - """Metaclass that enforces a class to be final (i.e., subclass not allowed). - - If a class uses this metaclass like this:: - - class SomeClass(metaclass=Final): - pass - - The metaclass will ensure that no subclass can be created. - - Raises - ------ - - TypeError if a subclass is created - """ - - def __new__( - cls, - name: str, - bases: tuple[type, ...], - cls_namespace: dict[str, object], - ) -> Final: - for base in bases: - if isinstance(base, Final): - raise TypeError( - f"{base.__module__}.{base.__qualname__} does not support" - " subclassing" - ) - return super().__new__(cls, name, bases, cls_namespace) - - @final # No subclassing of NoPublicConstructor itself. class NoPublicConstructor(ABCMeta): """Metaclass that ensures a private constructor. From 80f85f23db5f1a5f83e40926d3eea693a06a0b11 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Fri, 8 Sep 2023 09:17:04 +1000 Subject: [PATCH 10/15] Add explanation to final-decorator test --- trio/_tests/test_util.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/trio/_tests/test_util.py b/trio/_tests/test_util.py index b2edf557aa..e53f403bec 100644 --- a/trio/_tests/test_util.py +++ b/trio/_tests/test_util.py @@ -171,6 +171,12 @@ def test_func(arg): def test_final_decorator() -> None: + """Test that subclassing a @final-annotated class is not allowed. + + This checks both runtime results, and verifies that type checkers detect + the error statically through the type-ignore comment. + """ + @final class FinalClass: pass From a1c2938609e8a7bfe53584554c1d0dd87bab47aa Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Fri, 8 Sep 2023 09:17:50 +1000 Subject: [PATCH 11/15] Expand no-public-constructor test, check that args are passed through --- trio/_tests/test_util.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/trio/_tests/test_util.py b/trio/_tests/test_util.py index e53f403bec..9cffaa30d3 100644 --- a/trio/_tests/test_util.py +++ b/trio/_tests/test_util.py @@ -188,14 +188,19 @@ class SubClass(FinalClass): # type: ignore[misc] def test_no_public_constructor_metaclass(): + """The NoPublicConstructor metaclass prevents calling the constructor directly.""" + class SpecialClass(metaclass=NoPublicConstructor): - pass + def __init__(self, a: int, b: float): + """Check arguments can be passed to __init__.""" + assert a == 8 + assert b == 3.14 with pytest.raises(TypeError): - SpecialClass() + SpecialClass(8, 3.14) - # Private constructor should not raise - assert isinstance(SpecialClass._create(), SpecialClass) + # Private constructor should not raise, and passes args to __init__. + assert isinstance(SpecialClass._create(8, b=3.14), SpecialClass) def test_fixup_module_metadata(): From 9568bc11dec6001387957ea86002f2a376642e44 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Sat, 9 Sep 2023 10:10:54 +1000 Subject: [PATCH 12/15] Remove this extra fail message, Pytest provides it normally --- trio/_tests/test_exports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trio/_tests/test_exports.py b/trio/_tests/test_exports.py index f9a5939b90..926fe8dbd4 100644 --- a/trio/_tests/test_exports.py +++ b/trio/_tests/test_exports.py @@ -541,4 +541,4 @@ def test_classes_are_final() -> None: if name.endswith("Statistics"): continue - assert class_is_final(class_), f"{class_.__module__}.{class_.__qualname__}" + assert class_is_final(class_) From 85c2ded70f698faef759cd04b09ec2c6abff5f7d Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Sat, 9 Sep 2023 10:18:53 +1000 Subject: [PATCH 13/15] Directly test if classes are final, instead of checking __final__ As a bonus this lets us avoid a special case for enums, which are automatically final if they have members. --- trio/_tests/test_exports.py | 17 ++++++++--------- trio/_util.py | 5 ++--- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/trio/_tests/test_exports.py b/trio/_tests/test_exports.py index 926fe8dbd4..9668338ee1 100644 --- a/trio/_tests/test_exports.py +++ b/trio/_tests/test_exports.py @@ -1,7 +1,6 @@ from __future__ import annotations # isort: split import __future__ # Regular import, not special! -import enum import functools import importlib import inspect @@ -73,9 +72,14 @@ def test_core_is_properly_reexported(): def class_is_final(cls: type) -> bool: - """Check if a class has _util.final / typing.final applied.""" - # We use vars() to bypass this being set in superclasses or the metaclass. - return vars(cls).get("__final__", False) + """Check if a class cannot be subclassed.""" + try: + # new_class() handles metaclasses properly, type(...) does not. + types.new_class("SubclassTester", (cls,)) + except TypeError: + return True + else: + return False def iter_modules( @@ -530,11 +534,6 @@ def test_classes_are_final() -> None: # inspect.isabstract returns False for boring reasons. if class_ is trio.abc.Instrument or class_ is trio.socket.SocketType: continue - # Enums have their own metaclass, so we can't use our metaclasses. - # And I don't think there's a lot of risk from people subclassing - # enums... - if issubclass(class_, enum.Enum): - continue # ... insert other special cases here ... # don't care about the *Statistics classes diff --git a/trio/_util.py b/trio/_util.py index 6c514a4919..ff81d8c4d0 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -334,9 +334,8 @@ class SomeClass: # Override the method blindly. We're always going to raise, so it doesn't # matter what the original did (if anything). decorated.__init_subclass__ = classmethod(_init_final_cls) # type: ignore[assignment] - # Python 3.11+ sets this attr, do it on all versions so this being present can be tested. - decorated.__final__ = True # type: ignore[attr-defined] - return decorated + # Apply the typing decorator, in 3.11+ it adds a __final__ marker attribute. + return t.final(decorated) if t.TYPE_CHECKING: From 49095b82f7f5e0c427bd64db0f144a43533d9890 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Sat, 9 Sep 2023 10:20:14 +1000 Subject: [PATCH 14/15] Revert new final decorator on statistics classes --- trio/_core/_parking_lot.py | 3 +-- trio/_core/_unbounded_queue.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/trio/_core/_parking_lot.py b/trio/_core/_parking_lot.py index 2c83abc8dc..5a38c3ad3b 100644 --- a/trio/_core/_parking_lot.py +++ b/trio/_core/_parking_lot.py @@ -85,12 +85,11 @@ from ._run import Task -@final @attr.s(frozen=True, slots=True) class ParkingLotStatistics: """An object containing debugging information for a ParkingLot. - Currently the following fields are defined: + Currently, the following fields are defined: * ``tasks_waiting`` (int): The number of tasks blocked on this lot's :meth:`trio.lowlevel.ParkingLot.park` method. diff --git a/trio/_core/_unbounded_queue.py b/trio/_core/_unbounded_queue.py index 7901b447c1..7c5c536676 100644 --- a/trio/_core/_unbounded_queue.py +++ b/trio/_core/_unbounded_queue.py @@ -14,12 +14,11 @@ from typing_extensions import Self -@final @attr.s(slots=True, frozen=True) class UnboundedQueueStatistics: """An object containing debugging information. - Currently the following fields are defined: + Currently, the following fields are defined: * ``qsize``: The number of items currently in the queue. * ``tasks_waiting``: The number of tasks blocked on this queue's From 383c4d59c276a1e8068d8e5b4e948e4e41a3eac4 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Sat, 9 Sep 2023 10:37:39 +1000 Subject: [PATCH 15/15] Add some additional class_is_final calls to exercise all code paths --- trio/_tests/test_exports.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/trio/_tests/test_exports.py b/trio/_tests/test_exports.py index 9668338ee1..1c74d58353 100644 --- a/trio/_tests/test_exports.py +++ b/trio/_tests/test_exports.py @@ -511,6 +511,10 @@ def test_nopublic_is_final() -> None: def test_classes_are_final() -> None: + # Sanity checks. + assert not class_is_final(object) + assert class_is_final(bool) + for module in PUBLIC_MODULES: for name, class_ in module.__dict__.items(): if not isinstance(class_, type):