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 b024: abstract class with no abstract methods #274

Merged
merged 1 commit into from Aug 18, 2022

Conversation

jakkdl
Copy link
Contributor

@jakkdl jakkdl commented Aug 17, 2022

First requested error in #273

@jakkdl jakkdl force-pushed the abstract_class_no_abstract_methods branch from e09ad12 to 8e751a7 Compare August 17, 2022 12:24
@jakkdl
Copy link
Contributor Author

jakkdl commented Aug 17, 2022

Couple possibilities on how liberal the name checks should be to balance false/missed alarms. You can check tests/b024.py for where the line is drawn currently.

@Zac-HD

@Zac-HD
Copy link
Member

Zac-HD commented Aug 17, 2022

I'd warn only if the name is exactly ABC or abc.ABC, missing the alarm if you [from ...] import ... as seems fine to me.

@jakkdl jakkdl force-pushed the abstract_class_no_abstract_methods branch 2 times, most recently from 0e28577 to c307f23 Compare August 18, 2022 09:10
@jakkdl
Copy link
Contributor Author

jakkdl commented Aug 18, 2022

I was thinking more if any other libraries implement ABC's, but I guess that's very rarely the case (and it that case they might be doing fancy stuff such that this would be a false alarm anyway). Fixed

(Previous CI run failed due to me forgetting to update lines after black formatted the test file)

@jakkdl jakkdl force-pushed the abstract_class_no_abstract_methods branch 2 times, most recently from 4100a78 to 4036ee1 Compare August 18, 2022 09:25
@cooperlees
Copy link
Collaborator

Hmm - Where be the CI? I clicked approve :|

@Zac-HD
Copy link
Member

Zac-HD commented Aug 18, 2022

Stuck in queued, and there's an incident https://www.githubstatus.com/incidents/8gpymn7rystj

Should be fine in a few hours?

@Zac-HD Zac-HD closed this Aug 18, 2022
@Zac-HD Zac-HD reopened this Aug 18, 2022
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.

Looks great - thanks @jakkdl!

@Zac-HD Zac-HD merged commit 881f054 into PyCQA:main Aug 18, 2022
@jakkdl jakkdl deleted the abstract_class_no_abstract_methods branch August 22, 2022 07:35
@decaz decaz mentioned this pull request Aug 22, 2022
jiang1997 added a commit to jiang1997/skywalking-python that referenced this pull request Aug 28, 2022
More details about B024 here PyCQA/flake8-bugbear#274

* ./skywalking/agent/protocol/__init__.py:22:1: B024 Protocol is an abstract base class, but it has no abstract methods.
Set 'heartbeat', 'report' and 'report_log' as abstract method

* ./skywalking/trace/span.py:36:1: B024 Span is an abstract base class, but it has no abstract methods.
Prevent base class Span from being directly instantiated by raising error in __new__ with reference to
https://stackoverflow.com/questions/7989042/preventing-a-class-from-direct-instantiation-in-python

* ./tests/plugin/base.py:35:1: B024 TestPluginBase is an abstract base class
Due to pytest may instantiate parent class for searching test functions, TestPluginBase has to be instantiable.
jiang1997 added a commit to jiang1997/skywalking-python that referenced this pull request Aug 28, 2022
More details about B024 here PyCQA/flake8-bugbear#274

* ./skywalking/agent/protocol/__init__.py:22:1: B024 Protocol is an abstract base class, but it has no abstract methods.
Set 'heartbeat', 'report' and 'report_log' as abstract method

* ./skywalking/trace/span.py:36:1: B024 Span is an abstract base class, but it has no abstract methods.
Prevent base class Span from being directly instantiated by raising error in __new__ with reference to
https://stackoverflow.com/questions/7989042/preventing-a-class-from-direct-instantiation-in-python

* ./tests/plugin/base.py:35:1: B024 TestPluginBase is an abstract base class
Due to pytest may instantiate parent class for searching test functions, TestPluginBase has to be instantiable.
jiang1997 added a commit to jiang1997/skywalking-python that referenced this pull request Aug 28, 2022
More details about B024 here PyCQA/flake8-bugbear#274

* ./skywalking/agent/protocol/__init__.py:22:1: B024 Protocol is an abstract base class, but it has no abstract methods.
Set 'heartbeat', 'report' and 'report_log' as abstract method

* ./skywalking/trace/span.py:36:1: B024 Span is an abstract base class, but it has no abstract methods.
Prevent base class Span from being directly instantiated by raising error in __new__ with reference to
https://stackoverflow.com/questions/7989042/preventing-a-class-from-direct-instantiation-in-python

* ./tests/plugin/base.py:35:1: B024 TestPluginBase is an abstract base class
Due to pytest may instantiate parent class for searching test functions, TestPluginBase has to be instantiable.
jiang1997 added a commit to jiang1997/skywalking-python that referenced this pull request Aug 30, 2022
More details about B024 here PyCQA/flake8-bugbear#274

* ./skywalking/agent/protocol/__init__.py:22:1: B024 Protocol is an abstract base class, but it has no abstract methods.
Set 'heartbeat', 'report' and 'report_log' as abstract method

* ./skywalking/trace/span.py:36:1: B024 Span is an abstract base class, but it has no abstract methods.
Prevent base class Span from being directly instantiated by raising error in __new__ with reference to
https://stackoverflow.com/questions/7989042/preventing-a-class-from-direct-instantiation-in-python

* ./tests/plugin/base.py:35:1: B024 TestPluginBase is an abstract base class
Due to pytest may instantiate parent class for searching test functions, TestPluginBase has to be instantiable.
jiang1997 added a commit to jiang1997/skywalking-python that referenced this pull request Aug 30, 2022
More details about B024 here PyCQA/flake8-bugbear#274

* ./skywalking/agent/protocol/__init__.py:22:1: B024 Protocol is an abstract base class, but it has no abstract methods.
Set 'heartbeat', 'report' and 'report_log' as abstract method

* ./skywalking/trace/span.py:36:1: B024 Span is an abstract base class, but it has no abstract methods.
Prevent base class Span from being directly instantiated by raising error in __new__ with reference to
https://stackoverflow.com/questions/7989042/preventing-a-class-from-direct-instantiation-in-python

* ./tests/plugin/base.py:35:1: B024 TestPluginBase is an abstract base class
Due to pytest instantiating parent class, TestPluginBase has to be instantiable.
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.

None yet

3 participants