Skip to content

Commit

Permalink
Adding B025: empty method in abstract base class with no abstract dec…
Browse files Browse the repository at this point in the history
…orator
  • Loading branch information
jakkdl committed Sep 29, 2022
1 parent 846ba72 commit 592b722
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 23 deletions.
10 changes: 9 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ positives due to similarly named user-defined functions.
the loop, because `late-binding closures are a classic gotcha
<https://docs.python-guide.org/writing/gotchas/#late-binding-closures>`__.

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

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

**B027**: Empty method in abstract base class, but has no abstract decorator. Consider adding @abstractmethod.

Opinionated warnings
~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -295,6 +297,7 @@ MIT
Change Log
----------

<<<<<<< HEAD
22.9.23
~~~~~~~~~~

Expand All @@ -305,6 +308,11 @@ Change Log
~~~~~~~~~~

* add B025: find duplicate except clauses (#284)
=======
Future
~~~~~~~~~
* Add B025: Empty method in abstract base class with no abstract decorator.
>>>>>>> 0937a2d (Adding B025: empty method in abstract base class with no abstract decorator)

22.8.23
~~~~~~~~~~
Expand Down
51 changes: 43 additions & 8 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ def visit_ClassDef(self, node):
self.check_for_b903(node)
self.check_for_b018(node)
self.check_for_b021(node)
self.check_for_b024(node)
self.check_for_b024_and_b027(node)
self.generic_visit(node)

def visit_Try(self, node):
Expand Down Expand Up @@ -612,7 +612,7 @@ def check_for_b023(self, loop_node):
if reassigned_in_loop.issuperset(err.vars):
self.errors.append(err)

def check_for_b024(self, node: ast.ClassDef):
def check_for_b024_and_b027(self, node: ast.ClassDef):
"""Check for inheritance from abstract classes in abc and lack of
any methods decorated with abstract*"""

Expand All @@ -632,16 +632,45 @@ def is_abstract_decorator(expr):
isinstance(expr, ast.Attribute) and expr.attr[:8] == "abstract"
)

def empty_body(body) -> bool:
# Function body consist solely of `pass`, `...`, and/or (doc)string literals
return all(
isinstance(stmt, ast.Pass)
or (
isinstance(stmt, ast.Expr)
and isinstance(stmt.value, ast.Constant)
and (
stmt.value.value is Ellipsis
or isinstance(stmt.value.value, str)
)
)
for stmt in body
)

# only check abstract classes
if not any(map(is_abc_class, (*node.bases, *node.keywords))):
return

has_abstract_method = False

for stmt in node.body:
if isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef)) and any(
# only check function defs
if not isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef)):
continue

has_abstract_decorator = any(
map(is_abstract_decorator, stmt.decorator_list)
):
return
)

has_abstract_method |= has_abstract_decorator

if not has_abstract_decorator and empty_body(stmt.body):
self.errors.append(
B025(stmt.lineno, stmt.col_offset, vars=(stmt.name,))
)

self.errors.append(B024(node.lineno, node.col_offset, vars=(node.name,)))
if not has_abstract_method:
self.errors.append(B024(node.lineno, node.col_offset, vars=(node.name,)))

def check_for_b026(self, call: ast.Call):
if not call.keywords:
Expand Down Expand Up @@ -1211,8 +1240,8 @@ def visit_Lambda(self, node):
B024 = Error(
message=(
"B024 {} is an abstract base class, but it has no abstract methods. Remember to"
" use @abstractmethod, @abstractclassmethod and/or @abstractproperty"
" decorators."
" use the @abstractmethod decorator, potentially in conjunction with"
" @classmethod, @property and/or @staticmethod."
)
)
B025 = Error(
Expand All @@ -1229,6 +1258,12 @@ def visit_Lambda(self, node):
"surprise and mislead readers."
)
)
B027 = Error(
message=(
"B027 {} is an empty method in an abstract base class, but has no abstract"
" decorator. Consider adding @abstractmethod."
)
)

# Warnings disabled by default.
B901 = Error(
Expand Down
28 changes: 14 additions & 14 deletions tests/b024.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,76 +16,76 @@

class Base_1(ABC): # error
def method(self):
...
foo()


class Base_2(ABC):
@abstractmethod
def method(self):
...
foo()


class Base_3(ABC):
@abc.abstractmethod
def method(self):
...
foo()


class Base_4(ABC):
@notabc.abstractmethod
def method(self):
...
foo()


class Base_5(ABC):
@abstract
def method(self):
...
foo()


class Base_6(ABC):
@abstractaoeuaoeuaoeu
def method(self):
...
foo()


class Base_7(ABC): # error
@notabstract
def method(self):
...
foo()


class MetaBase_1(metaclass=ABCMeta): # error
def method(self):
...
foo()


class MetaBase_2(metaclass=ABCMeta):
@abstractmethod
def method(self):
...
foo()


class abc_Base_1(abc.ABC): # error
def method(self):
...
foo()


class abc_Base_2(metaclass=abc.ABCMeta): # error
def method(self):
...
foo()


class notabc_Base_1(notabc.ABC): # safe
def method(self):
...
foo()


class multi_super_1(notabc.ABC, abc.ABCMeta): # error
def method(self):
...
foo()


class multi_super_2(notabc.ABC, metaclass=abc.ABCMeta): # error
def method(self):
...
foo()
59 changes: 59 additions & 0 deletions tests/b027.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import abc
from abc import ABC
from abc import abstractmethod
from abc import abstractmethod as notabstract

"""
Should emit:
B025 - on lines 13, 16, 19, 23, 31
"""


class AbstractClass(ABC):
def empty_1(self): # error
...

def empty_2(self): # error
pass

def empty_3(self): # error
"""docstring"""
...

def empty_4(self): # error
"""multiple ellipsis/pass"""
...
pass
...
pass

@notabstract
def empty_5(self): # error
...

@abstractmethod
def abstract_1(self):
...

@abstractmethod
def abstract_2(self):
pass

@abc.abstractmethod
def abstract_3(self):
...

def body_1(self):
print("foo")
...

def body_2(self):
self.body_1()


class NonAbstractClass:
def empty_1(self): # safe
...

def empty_2(self): # safe
pass
14 changes: 14 additions & 0 deletions tests/test_bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
B024,
B025,
B026,
B027,
B901,
B902,
B903,
Expand Down Expand Up @@ -399,6 +400,19 @@ def test_b026(self):
),
)

def test_b027(self):
filename = Path(__file__).absolute().parent / "b025.py"
bbc = BugBearChecker(filename=str(filename))
errors = list(bbc.run())
expected = self.errors(
B025(13, 4, vars=("empty_1",)),
B025(16, 4, vars=("empty_2",)),
B025(19, 4, vars=("empty_3",)),
B025(23, 4, vars=("empty_4",)),
B025(31, 4, vars=("empty_5",)),
)
self.assertEqual(errors, expected)

def test_b901(self):
filename = Path(__file__).absolute().parent / "b901.py"
bbc = BugBearChecker(filename=str(filename))
Expand Down

0 comments on commit 592b722

Please sign in to comment.