Adding B025: empty method in abstract base class with no abstract dec…

…orator

README.rst

@@ -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``
@@ -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
 ~~~~~~~~~~~~~~~~~~~~
 
@@ -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
 ~~~~~~~~~~

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(

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()

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

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))