From 5d9d7449654b47d1da1e779ba03bb52ab0e2c5db Mon Sep 17 00:00:00 2001 From: John Litborn <11260241+jakkdl@users.noreply.github.com> Date: Thu, 19 Jan 2023 22:43:13 +0100 Subject: [PATCH] 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 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: Cooper Lees Co-authored-by: Zac Hatfield-Dodds Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- README.rst | 5 ++++- bugbear.py | 9 ++++++--- tests/b024.py | 11 ++++++++++- tests/test_bugbear.py | 1 - 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index c5acb72..7159b98 100644 --- a/README.rst +++ b/README.rst @@ -155,7 +155,9 @@ 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. 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) diff --git a/bugbear.py b/bugbear.py index 26e89cc..09924f5 100644 --- a/bugbear.py +++ b/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." ) ) diff --git a/tests/b024.py b/tests/b024.py index 256b9ef..238dcd3 100644 --- a/tests/b024.py +++ b/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() diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 950efc5..2e672cb 100644 --- a/tests/test_bugbear.py +++ b/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)