diff --git a/README.rst b/README.rst index 0d68160..2691856 100644 --- a/README.rst +++ b/README.rst @@ -154,7 +154,7 @@ positives due to similarly named user-defined functions. the loop, because `late-binding closures are a classic gotcha `__. -**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`` @@ -167,6 +167,8 @@ There was `cpython discussion of disallowing this syntax `_, 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 ~~~~~~~~~~~~~~~~~~~~ @@ -295,6 +297,7 @@ MIT Change Log ---------- +<<<<<<< HEAD 22.9.23 ~~~~~~~~~~ @@ -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 ~~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 2aad5e7..df4e268 100644 --- a/bugbear.py +++ b/bugbear.py @@ -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): @@ -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*""" @@ -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: @@ -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( @@ -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( diff --git a/tests/b024.py b/tests/b024.py index 5b23bb2..936a1f5 100644 --- a/tests/b024.py +++ b/tests/b024.py @@ -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() diff --git a/tests/b027.py b/tests/b027.py new file mode 100644 index 0000000..0e814a5 --- /dev/null +++ b/tests/b027.py @@ -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 diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 28eb964..34f2d9e 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -37,6 +37,7 @@ B024, B025, B026, + B027, B901, B902, B903, @@ -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))