[ty] Add tests for interactions of @classmethod, @staticmethod, and protocol method members#20555
Conversation
…nd protocol method members
AlexWaygood
requested review from
carljm,
dcreager and
sharkdp
as code owners
AlexWaygood
added
testing
| # A nominal type with a `@staticmethod` can satisfy a protocol with a `@classmethod` | ||
| # if the staticmethod duck-types the same as the classmethod member | ||
| # both when accessed on the class and when accessed on an instance of the class | ||
| # The same also applies for a nominal type with a `@classmethod` and a protocol | ||
| # with a `@staticmethod` member |
There was a problem hiding this comment.
I can understand the reasoning, but this seems a bit surprising to me. Can I extend this reasoning to instance methods as well? Does C implement the HasFInstanceMethod protocol? All type checkers seem to think so, but it does feel wrong to me. Shouldn't there be some constraints on the implicit type of the first parameter that would prevent this?
from typing import Protocol
class HasFInstanceMethod(Protocol):
def f(self, x: int, /) -> str: ...
class C:
@classmethod
def f(cls, *args) -> str:
return ""
def accept(_: HasFInstanceMethod): ...
accept(C()) # pyright, mypy, pyrefly, ty: fine!There was a problem hiding this comment.
Does
Cimplement theHasFInstanceMethodprotocol? All type checkers seem to think so, but it does feel wrong to me. Shouldn't there be some constraints on the implicit type of the first parameter that would prevent this?
Yes, I agree with you -- and I added (currently failing) tests in this PR that assert this, too ;) they're up on lines 1791-1795.
The rule I intend to implement for ty is that (given a protocol P, a method member f and a would-be nominal subtype N), two things must be true in order for N to satisfy P
- the get-type of
fwhen accessed on instances ofNmust be consistent with the signature offwhen accessed on instances ofP - the get-type of
fontype[N]must be consistent with the signature offon when accessed on the class objectP
As you note, this is a bit stricter than any other type checker currently implements, so we'll see how it goes; we might have to back out of this. But I'd like to give it a go, at least.
There was a problem hiding this comment.
I added (currently failing) tests in this PR that assert this, too ;) they're up on lines 1791-1795.
They looked slightly different to me, because with the *args signature in my comment, you can even make type checkers think that the classmethod signature is compatible with a non-classmethod protocol signature.
There was a problem hiding this comment.
Oh, sorry -- I missed that. Hmm, I can see how that might be counter-intuitive, but yes, I can't immediately see a principled reason whereby we'd decide that C shouldn't be a subtype of HasFInstanceMethod there...
2 participants
Summary
This PR adds more tests for interactions between
@classmethod,@staticmethod, and subtyping/assignability checks for protocols with method members. This partially addresses @sharkdp's post-merge review comment in #20165 (comment).There are a lot of TODOs in these tests that are tricky to fix right now due to the fact that we currently consider function-types as not being fully static if their signature includes a
selforclsargument that isn't annotated. That means that doing tests regarding the type of a method when accessed on the class object itself is currently quite tricky. I expect it to be a lot easier to resolve these TODOs after #20517 has landed (but I also don't think it's urgent to resolve these TODOs, since@classmethodand@staticmethodmembers are quite rare).