-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Define Protocol as abstract to prevent abstract-method FP #7839
Conversation
This comment has been minimized.
This comment has been minimized.
I see I need to add an .rc file so this only applies to py3.8+, but I'll wait to hear from maintainers about if this approach is even the right thing to do. |
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.
Turns out I never submitted my initial review π
"""Test that classes inheriting from protocols should not warn about abstract-method.""" | ||
# pylint: disable=too-few-public-methods,disallowed-name,invalid-name | ||
from abc import abstractmethod | ||
from typing import Protocol, Literal |
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.
"""Test that classes inheriting from protocols should not warn about abstract-method.""" | |
# pylint: disable=too-few-public-methods,disallowed-name,invalid-name | |
from abc import abstractmethod | |
from typing import Protocol, Literal | |
"""Test that classes inheriting from protocols should not warn about abstract-method.""" | |
# pylint: disable=too-few-public-methods,disallowed-name,invalid-name | |
from abc import abstractmethod | |
from typing import Protocol, Literal |
27e56c4
to
26a2c36
Compare
Pull Request Test Coverage Report for Build 3568918104
π - Coveralls |
This comment has been minimized.
This comment has been minimized.
26a2c36
to
b90b442
Compare
return True | ||
except astroid.InferenceError: | ||
continue | ||
return False |
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.
Turns out is_protocol_class
was incorrectly defined since it concluded that if any parent/grandparent class inherited from Protocol, this class was one too, but as we discussed the class has to inherit from Protocol itself. This definition was already in the code base so I just moved it here
This comment has been minimized.
This comment has been minimized.
f15b2b5
to
a7b119d
Compare
(cherry picked from commit 85e7d93)
(cherry picked from commit 85e7d93)
(cherry picked from commit 85e7d93)
) (cherry picked from commit 85e7d93) Co-authored-by: Dani Alcala <[email protected]>
Type of Changes
Description
Closes #7209
I'm not sure if I just got lucky in this very short fix or I'm completely wrong here in adding a Protocol check to the is_abstract utility method....