# HG changeset patch # User Matt Harbison # Date 1729824451 14400 # Node ID fd200f5bcaea55a9c1a80bc870ad9f3b1d657956 # Parent e7812caacc3c8d98b6a503bd3773970b8e102351 wireprototypes: make `baseprotocolhandler` methods abstract The documentation says it's an abstract base class, so let's enforce it. The `typing.Protocol` class is already an ABC, but it only prevents instantiation if there are abstract attrs that are missing. For example, from `hg debugshell`: >>> from mercurial import wireprototypes >>> x = wireprototypes.baseprotocolhandler() Traceback (most recent call last): File "", line 1, in TypeError: Can't instantiate abstract class baseprotocolhandler with abstract method name >>> class fake(wireprototypes.baseprotocolhandler): ... pass ... >>> x = fake() Traceback (most recent call last): File "", line 1, in TypeError: Can't instantiate abstract class fake with abstract method name That's great, but it doesn't protect against calling non-abstract methods at runtime, rather it depends on the protocol type hint being added to method signatures or class attrs, and then running a type checker to notice when an instance is assigned that doesn't conform to the protocol. We don't widely use type hints yet, and do have a lot of class hierarchy in the repository area, which could lead to surprises like this: >>> class fake(wireprototypes.baseprotocolhandler): ... @property ... def name(self) -> bytes: ... return b'name' ... >>> z = fake() >>> z.client() >>> print(z.client()) None Oops. That was supposed to return `bytes`. So not only is a bad/unexpected value returned, but it's one that violates the type hints (since the base client() method will be annotated to return bytes). With this change, we get: >>> from mercurial import wireprototypes >>> class fake(wireprototypes.baseprotocolhandler): ... @property ... def name(self) -> bytes: ... return b'name' ... >>> x = fake() Traceback (most recent call last): File "", line 1, in TypeError: Can't instantiate abstract class fake with abstract methods addcapabilities, checkperm, client, getargs, getpayload, getprotocaps, mayberedirectstdio So this looks like a reasonable safety harness to me, and lets us catch problems by running the standard tests while the type hints are being added, and pytype is improved. We should probably do this for all Protocol class methods that don't supply a method implementation. diff -r e7812caacc3c -r fd200f5bcaea mercurial/wireprototypes.py --- a/mercurial/wireprototypes.py Thu Oct 24 22:37:45 2024 -0400 +++ b/mercurial/wireprototypes.py Thu Oct 24 22:47:31 2024 -0400 @@ -202,6 +202,7 @@ Used for uniquely identifying the transport type. """ + @abc.abstractmethod def getargs(self, args): """return the value for arguments in @@ -210,12 +211,14 @@ a dict mapping argument name to value. """ + @abc.abstractmethod def getprotocaps(self): """Returns the list of protocol-level capabilities of client Returns a list of capabilities as declared by the client for the current request (or connection for stateful protocol handlers).""" + @abc.abstractmethod def getpayload(self): """Provide a generator for the raw payload. @@ -223,6 +226,7 @@ processed. """ + @abc.abstractmethod def mayberedirectstdio(self): """Context manager to possibly redirect stdio. @@ -236,9 +240,11 @@ won't be captured. """ + @abc.abstractmethod def client(self): """Returns a string representation of this client (as bytes).""" + @abc.abstractmethod def addcapabilities(self, repo, caps): """Adds advertised capabilities specific to this protocol. @@ -247,6 +253,7 @@ Returns a list of capabilities. The passed in argument can be returned. """ + @abc.abstractmethod def checkperm(self, perm): """Validate that the client has permissions to perform a request.