Skip to content

Commit 466082c

Browse files
Zac-HDZac-HD
authored
Merge pull request #281 from jakkdl/abstract_class_empty_method_no_abstract_decorator
Add B027: empty method in abstract base class with no abstract decorator
2 parents 0fec7e5 + cea0499 commit 466082c

5 files changed

Lines changed: 198 additions & 34 deletions

File tree

‎README.rst‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ positives due to similarly named user-defined functions.
154154
the loop, because `late-binding closures are a classic gotcha
155155
<https://docs.python-guide.org/writing/gotchas/#late-binding-closures>`__.
156156

157-
**B024**: Abstract base class with no abstract method. Remember to use @abstractmethod, @abstractclassmethod, and/or @abstractproperty decorators.
157+
**B024**: Abstract base class with no abstract method. You might have forgotten to add @abstractmethod.
158158

159159
**B025**: ``try-except`` block with duplicate exceptions found.
160160
This check identifies exception types that are specified in multiple ``except``
@@ -167,6 +167,8 @@ There was `cpython discussion of disallowing this syntax
167167
<https://github.com/python/cpython/issues/82741>`_, but legacy usage and parser
168168
limitations make it difficult.
169169

170+
**B027**: Empty method in abstract base class, but has no abstract decorator. Consider adding @abstractmethod.
171+
170172
Opinionated warnings
171173
~~~~~~~~~~~~~~~~~~~~
172174

@@ -294,17 +296,21 @@ MIT
294296

295297
Change Log
296298
----------
299+
Future
300+
~~~~~~~~~
301+
302+
* Add B027: Empty method in abstract base class with no abstract decorator
297303

298304
22.9.23
299305
~~~~~~~~~~
300306

301-
* add B026: find argument unpacking after keyword argument (#287)
307+
* Add B026: find argument unpacking after keyword argument (#287)
302308
* Move to setup.cfg like flake8 (#288)
303309

304310
22.9.11
305311
~~~~~~~~~~
306312

307-
* add B025: find duplicate except clauses (#284)
313+
* Add B025: find duplicate except clauses (#284)
308314

309315
22.8.23
310316
~~~~~~~~~~

‎bugbear.py‎

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ def visit_ClassDef(self, node):
418418
self.check_for_b903(node)
419419
self.check_for_b018(node)
420420
self.check_for_b021(node)
421-
self.check_for_b024(node)
421+
self.check_for_b024_and_b027(node)
422422
self.generic_visit(node)
423423

424424
def visit_Try(self, node):
@@ -612,17 +612,19 @@ def check_for_b023(self, loop_node):
612612
if reassigned_in_loop.issuperset(err.vars):
613613
self.errors.append(err)
614614

615-
def check_for_b024(self, node: ast.ClassDef):
615+
def check_for_b024_and_b027(self, node: ast.ClassDef): # noqa: C901
616616
"""Check for inheritance from abstract classes in abc and lack of
617617
any methods decorated with abstract*"""
618618

619-
def is_abc_class(value):
619+
def is_abc_class(value, name="ABC"):
620+
# class foo(metaclass = [abc.]ABCMeta)
620621
if isinstance(value, ast.keyword):
621-
return value.arg == "metaclass" and is_abc_class(value.value)
622-
abc_names = ("ABC", "ABCMeta")
623-
return (isinstance(value, ast.Name) and value.id in abc_names) or (
622+
return value.arg == "metaclass" and is_abc_class(value.value, "ABCMeta")
623+
# class foo(ABC)
624+
# class foo(abc.ABC)
625+
return (isinstance(value, ast.Name) and value.id == name) or (
624626
isinstance(value, ast.Attribute)
625-
and value.attr in abc_names
627+
and value.attr == name
626628
and isinstance(value.value, ast.Name)
627629
and value.value.id == "abc"
628630
)
@@ -632,16 +634,56 @@ def is_abstract_decorator(expr):
632634
isinstance(expr, ast.Attribute) and expr.attr[:8] == "abstract"
633635
)
634636

637+
def empty_body(body) -> bool:
638+
def is_str_or_ellipsis(node):
639+
# ast.Ellipsis and ast.Str used in python<3.8
640+
return isinstance(node, (ast.Ellipsis, ast.Str)) or (
641+
isinstance(node, ast.Constant)
642+
and (node.value is Ellipsis or isinstance(node.value, str))
643+
)
644+
645+
# Function body consist solely of `pass`, `...`, and/or (doc)string literals
646+
return all(
647+
isinstance(stmt, ast.Pass)
648+
or (isinstance(stmt, ast.Expr) and is_str_or_ellipsis(stmt.value))
649+
for stmt in body
650+
)
651+
652+
# don't check multiple inheritance
653+
# https://github.com/PyCQA/flake8-bugbear/issues/277
654+
if len(node.bases) + len(node.keywords) > 1:
655+
return
656+
657+
# only check abstract classes
635658
if not any(map(is_abc_class, (*node.bases, *node.keywords))):
636659
return
637660

661+
has_abstract_method = False
662+
638663
for stmt in node.body:
639-
if isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef)) and any(
664+
# https://github.com/PyCQA/flake8-bugbear/issues/293
665+
# Ignore abc's that declares a class attribute that must be set
666+
if isinstance(stmt, (ast.AnnAssign, ast.Assign)):
667+
has_abstract_method = True
668+
continue
669+
670+
# only check function defs
671+
if not isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef)):
672+
continue
673+
674+
has_abstract_decorator = any(
640675
map(is_abstract_decorator, stmt.decorator_list)
641-
):
642-
return
676+
)
677+
678+
has_abstract_method |= has_abstract_decorator
679+
680+
if not has_abstract_decorator and empty_body(stmt.body):
681+
self.errors.append(
682+
B027(stmt.lineno, stmt.col_offset, vars=(stmt.name,))
683+
)
643684

644-
self.errors.append(B024(node.lineno, node.col_offset, vars=(node.name,)))
685+
if not has_abstract_method:
686+
self.errors.append(B024(node.lineno, node.col_offset, vars=(node.name,)))
645687

646688
def check_for_b026(self, call: ast.Call):
647689
if not call.keywords:
@@ -1211,8 +1253,8 @@ def visit_Lambda(self, node):
12111253
B024 = Error(
12121254
message=(
12131255
"B024 {} is an abstract base class, but it has no abstract methods. Remember to"
1214-
" use @abstractmethod, @abstractclassmethod and/or @abstractproperty"
1215-
" decorators."
1256+
" use the @abstractmethod decorator, potentially in conjunction with"
1257+
" @classmethod, @property and/or @staticmethod."
12161258
)
12171259
)
12181260
B025 = Error(
@@ -1229,6 +1271,12 @@ def visit_Lambda(self, node):
12291271
"surprise and mislead readers."
12301272
)
12311273
)
1274+
B027 = Error(
1275+
message=(
1276+
"B027 {} is an empty method in an abstract base class, but has no abstract"
1277+
" decorator. Consider adding @abstractmethod."
1278+
)
1279+
)
12321280

12331281
# Warnings disabled by default.
12341282
B901 = Error(

‎tests/b024.py‎

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,76 +16,114 @@
1616

1717
class Base_1(ABC): # error
1818
def method(self):
19-
...
19+
foo()
2020

2121

2222
class Base_2(ABC):
2323
@abstractmethod
2424
def method(self):
25-
...
25+
foo()
2626

2727

2828
class Base_3(ABC):
2929
@abc.abstractmethod
3030
def method(self):
31-
...
31+
foo()
3232

3333

3434
class Base_4(ABC):
3535
@notabc.abstractmethod
3636
def method(self):
37-
...
37+
foo()
3838

3939

4040
class Base_5(ABC):
4141
@abstract
4242
def method(self):
43-
...
43+
foo()
4444

4545

4646
class Base_6(ABC):
4747
@abstractaoeuaoeuaoeu
4848
def method(self):
49-
...
49+
foo()
5050

5151

5252
class Base_7(ABC): # error
5353
@notabstract
5454
def method(self):
55-
...
55+
foo()
5656

5757

5858
class MetaBase_1(metaclass=ABCMeta): # error
5959
def method(self):
60-
...
60+
foo()
6161

6262

6363
class MetaBase_2(metaclass=ABCMeta):
6464
@abstractmethod
6565
def method(self):
66-
...
66+
foo()
6767

6868

6969
class abc_Base_1(abc.ABC): # error
7070
def method(self):
71-
...
71+
foo()
7272

7373

7474
class abc_Base_2(metaclass=abc.ABCMeta): # error
7575
def method(self):
76-
...
76+
foo()
7777

7878

7979
class notabc_Base_1(notabc.ABC): # safe
8080
def method(self):
81-
...
81+
foo()
8282

8383

84-
class multi_super_1(notabc.ABC, abc.ABCMeta): # error
84+
class multi_super_1(notabc.ABC, abc.ABCMeta): # safe
8585
def method(self):
86-
...
86+
foo()
8787

8888

89-
class multi_super_2(notabc.ABC, metaclass=abc.ABCMeta): # error
89+
class multi_super_2(notabc.ABC, metaclass=abc.ABCMeta): # safe
9090
def method(self):
91-
...
91+
foo()
92+
93+
94+
class non_keyword_abcmeta_1(ABCMeta): # safe
95+
def method(self):
96+
foo()
97+
98+
99+
class non_keyword_abcmeta_2(abc.ABCMeta): # safe
100+
def method(self):
101+
foo()
102+
103+
104+
# very invalid code, but that's up to mypy et al to check
105+
class keyword_abc_1(metaclass=ABC): # safe
106+
def method(self):
107+
foo()
108+
109+
110+
class keyword_abc_2(metaclass=abc.ABC): # safe
111+
def method(self):
112+
foo()
113+
114+
115+
class abc_set_class_variable_1(ABC): # safe
116+
foo: int
117+
118+
119+
class abc_set_class_variable_2(ABC): # safe
120+
foo = 2
121+
122+
123+
class abc_set_class_variable_3(ABC): # safe
124+
foo: int = 2
125+
126+
127+
# this doesn't actually declare a class variable, it's just an expression
128+
class abc_set_class_variable_4(ABC): # error
129+
foo

‎tests/b027.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import abc
2+
from abc import ABC
3+
from abc import abstractmethod
4+
from abc import abstractmethod as notabstract
5+
6+
"""
7+
Should emit:
8+
B025 - on lines 13, 16, 19, 23, 31
9+
"""
10+
11+
12+
class AbstractClass(ABC):
13+
def empty_1(self): # error
14+
...
15+
16+
def empty_2(self): # error
17+
pass
18+
19+
def empty_3(self): # error
20+
"""docstring"""
21+
...
22+
23+
def empty_4(self): # error
24+
"""multiple ellipsis/pass"""
25+
...
26+
pass
27+
...
28+
pass
29+
30+
@notabstract
31+
def empty_5(self): # error
32+
...
33+
34+
@abstractmethod
35+
def abstract_1(self):
36+
...
37+
38+
@abstractmethod
39+
def abstract_2(self):
40+
pass
41+
42+
@abc.abstractmethod
43+
def abstract_3(self):
44+
...
45+
46+
def body_1(self):
47+
print("foo")
48+
...
49+
50+
def body_2(self):
51+
self.body_1()
52+
53+
54+
class NonAbstractClass:
55+
def empty_1(self): # safe
56+
...
57+
58+
def empty_2(self): # safe
59+
pass

‎tests/test_bugbear.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
B024,
3838
B025,
3939
B026,
40+
B027,
4041
B901,
4142
B902,
4243
B903,
@@ -363,8 +364,7 @@ def test_b024(self):
363364
B024(58, 0, vars=("MetaBase_1",)),
364365
B024(69, 0, vars=("abc_Base_1",)),
365366
B024(74, 0, vars=("abc_Base_2",)),
366-
B024(84, 0, vars=("multi_super_1",)),
367-
B024(89, 0, vars=("multi_super_2",)),
367+
B024(128, 0, vars=("abc_set_class_variable_4",)),
368368
)
369369
self.assertEqual(errors, expected)
370370

@@ -399,6 +399,19 @@ def test_b026(self):
399399
),
400400
)
401401

402+
def test_b027(self):
403+
filename = Path(__file__).absolute().parent / "b027.py"
404+
bbc = BugBearChecker(filename=str(filename))
405+
errors = list(bbc.run())
406+
expected = self.errors(
407+
B027(13, 4, vars=("empty_1",)),
408+
B027(16, 4, vars=("empty_2",)),
409+
B027(19, 4, vars=("empty_3",)),
410+
B027(23, 4, vars=("empty_4",)),
411+
B027(31 if sys.version_info >= (3, 8) else 30, 4, vars=("empty_5",)),
412+
)
413+
self.assertEqual(errors, expected)
414+
402415
def test_b901(self):
403416
filename = Path(__file__).absolute().parent / "b901.py"
404417
bbc = BugBearChecker(filename=str(filename))

0 commit comments

Comments
 (0)