-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow stubtest to raise errors on abstract state mismatch #13323
Conversation
For reference, here is my patch that I worked on ages ago, but never submitted a PR for: AlexWaygood@c031ffd It has a few additional checks to yours, which you might be interested in possibly adding :) |
@@ -825,6 +825,13 @@ def verify_funcitem( | |||
if not callable(runtime): | |||
return | |||
|
|||
if isinstance(stub, nodes.FuncDef): | |||
stub_abstract = stub.abstract_status == nodes.IS_ABSTRACT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can safely skip nodes.IMPLICITLY_ABSTRACT
here. Because it can only happen for a protocol.
@AlexWaygood thanks! I think that if inspect.isabstract(runtime) and not stub.is_abstract: part is not really required. Because mypy sets Short example: >>> import abc, inspect
>>> class A(abc.ABC): ...
...
>>> inspect.isabstract(A)
False So, checking that stubs have >= abstact methods is just fine. Abstract classes will be checked by this as well. Moreover, |
Are abstract properties accounted for? |
Great question! New tests are added. |
Co-authored-by: Alex Waygood <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! :)
There's no diff when running typeshed's |
Awesome! I am going to merge it then 🎉 |
…n the stub is an overloaded method (#14955) #13323 means that stubtest will currently emit an error if the runtime is decorated with `@abstractmethod` but the stub is not, but _only_ if the stub is not an overloaded function, as overloaded functions have a different internal representation in mypy. This PR fixes that discrepancy.
Closes #13322