Mercurial > public > mercurial-scm > hg-stable
comparison mercurial/wireprototypes.py @ 52128:fd200f5bcaea
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 "<console>", line 1, in <module>
TypeError: Can't instantiate abstract class baseprotocolhandler with abstract method name
>>> class fake(wireprototypes.baseprotocolhandler):
... pass
...
>>> x = fake()
Traceback (most recent call last):
File "<console>", line 1, in <module>
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 "<console>", line 1, in <module>
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.
| author | Matt Harbison <matt_harbison@yahoo.com> |
|---|---|
| date | Thu, 24 Oct 2024 22:47:31 -0400 |
| parents | e7812caacc3c |
| children | e08c878b5571 |
comparison
equal
deleted
inserted
replaced
| 52127:e7812caacc3c | 52128:fd200f5bcaea |
|---|---|
| 200 """The name of the protocol implementation. | 200 """The name of the protocol implementation. |
| 201 | 201 |
| 202 Used for uniquely identifying the transport type. | 202 Used for uniquely identifying the transport type. |
| 203 """ | 203 """ |
| 204 | 204 |
| 205 @abc.abstractmethod | |
| 205 def getargs(self, args): | 206 def getargs(self, args): |
| 206 """return the value for arguments in <args> | 207 """return the value for arguments in <args> |
| 207 | 208 |
| 208 For version 1 transports, returns a list of values in the same | 209 For version 1 transports, returns a list of values in the same |
| 209 order they appear in ``args``. For version 2 transports, returns | 210 order they appear in ``args``. For version 2 transports, returns |
| 210 a dict mapping argument name to value. | 211 a dict mapping argument name to value. |
| 211 """ | 212 """ |
| 212 | 213 |
| 214 @abc.abstractmethod | |
| 213 def getprotocaps(self): | 215 def getprotocaps(self): |
| 214 """Returns the list of protocol-level capabilities of client | 216 """Returns the list of protocol-level capabilities of client |
| 215 | 217 |
| 216 Returns a list of capabilities as declared by the client for | 218 Returns a list of capabilities as declared by the client for |
| 217 the current request (or connection for stateful protocol handlers).""" | 219 the current request (or connection for stateful protocol handlers).""" |
| 218 | 220 |
| 221 @abc.abstractmethod | |
| 219 def getpayload(self): | 222 def getpayload(self): |
| 220 """Provide a generator for the raw payload. | 223 """Provide a generator for the raw payload. |
| 221 | 224 |
| 222 The caller is responsible for ensuring that the full payload is | 225 The caller is responsible for ensuring that the full payload is |
| 223 processed. | 226 processed. |
| 224 """ | 227 """ |
| 225 | 228 |
| 229 @abc.abstractmethod | |
| 226 def mayberedirectstdio(self): | 230 def mayberedirectstdio(self): |
| 227 """Context manager to possibly redirect stdio. | 231 """Context manager to possibly redirect stdio. |
| 228 | 232 |
| 229 The context manager yields a file-object like object that receives | 233 The context manager yields a file-object like object that receives |
| 230 stdout and stderr output when the context manager is active. Or it | 234 stdout and stderr output when the context manager is active. Or it |
| 234 so it may be sent in the response. Some transports support streaming | 238 so it may be sent in the response. Some transports support streaming |
| 235 stdio to the client in real time. For these transports, stdio output | 239 stdio to the client in real time. For these transports, stdio output |
| 236 won't be captured. | 240 won't be captured. |
| 237 """ | 241 """ |
| 238 | 242 |
| 243 @abc.abstractmethod | |
| 239 def client(self): | 244 def client(self): |
| 240 """Returns a string representation of this client (as bytes).""" | 245 """Returns a string representation of this client (as bytes).""" |
| 241 | 246 |
| 247 @abc.abstractmethod | |
| 242 def addcapabilities(self, repo, caps): | 248 def addcapabilities(self, repo, caps): |
| 243 """Adds advertised capabilities specific to this protocol. | 249 """Adds advertised capabilities specific to this protocol. |
| 244 | 250 |
| 245 Receives the list of capabilities collected so far. | 251 Receives the list of capabilities collected so far. |
| 246 | 252 |
| 247 Returns a list of capabilities. The passed in argument can be returned. | 253 Returns a list of capabilities. The passed in argument can be returned. |
| 248 """ | 254 """ |
| 249 | 255 |
| 256 @abc.abstractmethod | |
| 250 def checkperm(self, perm): | 257 def checkperm(self, perm): |
| 251 """Validate that the client has permissions to perform a request. | 258 """Validate that the client has permissions to perform a request. |
| 252 | 259 |
| 253 The argument is the permission required to proceed. If the client | 260 The argument is the permission required to proceed. If the client |
| 254 doesn't have that permission, the exception should raise or abort | 261 doesn't have that permission, the exception should raise or abort |