B024: don't warn on classes without methods (#336)

* B024: don't warn on classes without methods. Also minor rephrasing of
the error message, and changed the message in the README to match it
better.

* Update bugbear.py

Yup, much clearer.

Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: Cooper Lees <me@cooperlees.com>
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

README.rst

@@ -155,7 +155,9 @@ 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. You might have forgotten to add @abstractmethod.
+**B024**: Abstract base class has methods, but none of them are abstract. This
+is not necessarily an error, but you might have forgotten to add the @abstractmethod
+decorator, potentially in conjunction with @classmethod, @property and/or @staticmethod.
 
 **B025**: ``try-except`` block with duplicate exceptions found.
 This check identifies exception types that are specified in multiple ``except``
@@ -312,6 +314,7 @@ Change Log
 Future
 ~~~~~~~~~
 
+* B024: now ignores classes without any methods.
 * B906: Ignore ``visit_`` functions with a ``_fields`` attribute that can't contain ast.AST subnodes. (#330)
 * B017: Don't warn when ``pytest.raises()`` has a ``match`` argument. (#334)
 

bugbear.py

@@ -721,6 +721,7 @@ def is_str_or_ellipsis(node):
         if not any(map(is_abc_class, (*node.bases, *node.keywords))):
             return
 
+        has_method = False
         has_abstract_method = False
 
         for stmt in node.body:
@@ -733,6 +734,7 @@ def is_str_or_ellipsis(node):
             # only check function defs
             if not isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef)):
                 continue
+            has_method = True
 
             has_abstract_decorator = any(
                 map(is_abstract_decorator, stmt.decorator_list)
@@ -749,7 +751,7 @@ def is_str_or_ellipsis(node):
                     B027(stmt.lineno, stmt.col_offset, vars=(stmt.name,))
                 )
 
-        if not has_abstract_method:
+        if has_method and not has_abstract_method:
             self.errors.append(B024(node.lineno, node.col_offset, vars=(node.name,)))
 
     def check_for_b026(self, call: ast.Call):
@@ -1476,8 +1478,9 @@ def visit_Lambda(self, node):
 B023 = Error(message="B023 Function definition does not bind loop variable {!r}.")
 B024 = Error(
     message=(
-        "B024 {} is an abstract base class, but it has no abstract methods. Remember to"
-        " use the @abstractmethod decorator, potentially in conjunction with"
+        "B024 {} is an abstract base class, but none of the methods it defines are"
+        " abstract. This is not necessarily an error, but you might have forgotten to"
+        " add the @abstractmethod decorator, potentially in conjunction with"
         " @classmethod, @property and/or @staticmethod."
     )
 )

tests/b024.py

@@ -125,5 +125,14 @@ class abc_set_class_variable_3(ABC):  # safe
 
 
 # this doesn't actually declare a class variable, it's just an expression
-class abc_set_class_variable_4(ABC):  # error
+# this is now filtered out by not having a method
+class abc_set_class_variable_4(ABC):
     foo
+
+
+class abc_class_no_method_1(ABC):
+    pass
+
+
+class abc_class_no_method_2(ABC):
+    foo()

tests/test_bugbear.py

@@ -383,7 +383,6 @@ def test_b024(self):
             B024(58, 0, vars=("MetaBase_1",)),
             B024(69, 0, vars=("abc_Base_1",)),
             B024(74, 0, vars=("abc_Base_2",)),
-            B024(128, 0, vars=("abc_set_class_variable_4",)),
         )
         self.assertEqual(errors, expected)