Skip to content

Treat methods with empty bodies in Protocols as abstract #12058

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

Closed
wants to merge 1 commit into from

Conversation

tmke8
Copy link
Contributor

@tmke8 tmke8 commented Jan 24, 2022

Description

Closes #8005

Sending this PR to get feedback. The problem is I don't know where to hook into mypy to implement this. I'm fairly confident about my changes in semanal_classprop.py, but the code in semanal.py is most likely in the wrong place. One problem I have is that I can't figure out how to identify overloads. That's why I'm currently excluding any function with a decorator, which is clearly the wrong thing to do.

My current plan is to treat any method in a protocol as abstract for which all of the following is true:

  1. Function body only consists of pass, ..., and/or a docstring.
  2. Return type is not None or Any.
  3. The definition of the protocol is not from a stub file (because stubs never have function bodies).
  4. Function is not annotated with @overload.

Any help appreciated!

Test Plan

I added tests for the new behavior.

See python#8005 for the motivation.

There are some subtleties to this; for one, we can't apply the rule to
type stubs, because they never have a function body. And of course, if
the return type is `None` or `Any`, then an empty function is completely
valid.
@@ -1079,6 +1079,44 @@ class C2(P):
C()
C2()

[case testCannotInstantiateEmptyMethodExplicitProtocolSubtypes]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we differentiate .py and .pyi files, we 100% need to test it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's a good point.

@tmke8
Copy link
Contributor Author

tmke8 commented Jan 26, 2022

I'm giving up on this for now. Maybe I'll try again some other time.

@tmke8 tmke8 closed this Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extending a Protocol leads to unsafe behavior
2 participants