Skip to content
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

Add B027: empty method in abstract base class with no abstract decorator #281

Merged

Conversation

jakkdl
Copy link
Contributor

@jakkdl jakkdl commented Aug 31, 2022

EDIT: I read through the issues and realized that B024 needs changes, so made this a draft for now.

Implements the second suggested check in #273

There seems to be some difference in style on ellipsis vs pass, so one might want to restrict the check to only be if the body is docstring and ellipsis.

When reading the docs on abc I also found out that the abstract decorators inside it other than @abstractmethod are deprecated so I modified the b024 message to not suggest deprecated decorators.
Modified the b024 test to not trigger b027

Fixes #277, fixes #278 (though see #290), and fixes #280.

@jakkdl jakkdl force-pushed the abstract_class_empty_method_no_abstract_decorator branch from 0937a2d to e8ec222 Compare September 29, 2022 09:12
@jakkdl jakkdl changed the title Adding B025: empty method in abstract base class with no abstract decorator Add B027: empty method in abstract base class with no abstract decorator. Sep 29, 2022
@jakkdl jakkdl changed the title Add B027: empty method in abstract base class with no abstract decorator. Add B027: empty method in abstract base class with no abstract decorator Sep 29, 2022
@jakkdl jakkdl force-pushed the abstract_class_empty_method_no_abstract_decorator branch from e8ec222 to 8462c1f Compare September 29, 2022 09:16
@jakkdl jakkdl force-pushed the abstract_class_empty_method_no_abstract_decorator branch 2 times, most recently from b01e327 to a67a8c2 Compare September 29, 2022 10:13
@jakkdl jakkdl marked this pull request as ready for review September 29, 2022 10:17
@jakkdl
Copy link
Contributor Author

jakkdl commented Sep 29, 2022

Updated PR to also fix #277 #280 and the original problem in #278 (but not #278 (comment)).
ready for review and everything

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

Two comments below; otherwise looks good!

README.rst Outdated Show resolved Hide resolved
bugbear.py Outdated Show resolved Hide resolved
@jakkdl jakkdl force-pushed the abstract_class_empty_method_no_abstract_decorator branch from a67a8c2 to 65c141e Compare October 2, 2022 14:42
@jakkdl
Copy link
Contributor Author

jakkdl commented Oct 2, 2022

fixed both comments 👍

@jakkdl jakkdl mentioned this pull request Oct 3, 2022
@jakkdl jakkdl force-pushed the abstract_class_empty_method_no_abstract_decorator branch from 5991c6a to cea0499 Compare October 4, 2022 11:31
@jakkdl
Copy link
Contributor Author

jakkdl commented Oct 4, 2022

Fixing #293 as well now.
Function now runs into C901, but I'm not seeing any great way of getting out of that other than moving out some of the helper functions - but that seems mediocre when they're only used in this check. Splitting the B024 and B027 into two separate functions doesn't seem great either.

@Zac-HD
Copy link
Member

Zac-HD commented Oct 6, 2022

Thanks again @jakkdl!

@Zac-HD Zac-HD merged commit 466082c into PyCQA:main Oct 6, 2022
@jakkdl jakkdl deleted the abstract_class_empty_method_no_abstract_decorator branch January 9, 2023 12:53
@cooperlees cooperlees mentioned this pull request Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants