From c5adbf17da1ec92783eb6a26141c45bcaf1e30e6 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 30 Apr 2024 12:01:08 -0400 Subject: [PATCH] Ignore non-abstract class attributes when enforcing B024 (#11210) ## Summary I think the check included here does make sense, but I don't see why we would allow it if a value is provided for the attribute -- since, in that case, isn't it _not_ abstract? Closes: https://github.com/astral-sh/ruff/issues/11208. --- .../test/fixtures/flake8_bugbear/B024.py | 10 +++++----- .../flake8_bugbear/rules/abstract_base_class.rs | 10 ++++++++-- ...les__flake8_bugbear__tests__B024_B024.py.snap | 16 ++++++++++++++-- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B024.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B024.py index 854e9b08adf0b..51b73453a352f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B024.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B024.py @@ -115,25 +115,25 @@ def method(self): # very invalid code, but that's up to mypy et al to check -class keyword_abc_1(metaclass=ABC): # safe +class keyword_abc_1(metaclass=ABC): # incorrect but outside scope of this check def method(self): foo() -class keyword_abc_2(metaclass=abc.ABC): # safe +class keyword_abc_2(metaclass=abc.ABC): # incorrect but outside scope of this check def method(self): foo() -class abc_set_class_variable_1(ABC): # safe +class abc_set_class_variable_1(ABC): # safe (abstract attribute) foo: int -class abc_set_class_variable_2(ABC): # safe +class abc_set_class_variable_2(ABC): # error (not an abstract attribute) foo = 2 -class abc_set_class_variable_3(ABC): # safe +class abc_set_class_variable_3(ABC): # error (not an abstract attribute) foo: int = 2 diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/abstract_base_class.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/abstract_base_class.rs index b7ec09e599be0..edb47aa5aae8a 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/abstract_base_class.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/abstract_base_class.rs @@ -56,6 +56,7 @@ impl Violation for AbstractBaseClassWithoutAbstractMethod { format!("`{name}` is an abstract base class, but it has no abstract methods") } } + /// ## What it does /// Checks for empty methods in abstract base classes without an abstract /// decorator. @@ -156,8 +157,13 @@ pub(crate) fn abstract_base_class( let mut has_abstract_method = false; for stmt in body { // https://github.com/PyCQA/flake8-bugbear/issues/293 - // Ignore abc's that declares a class attribute that must be set - if let Stmt::AnnAssign(_) | Stmt::Assign(_) = stmt { + // If an ABC declares an attribute by providing a type annotation + // but does not actually assign a value for that attribute, + // assume it is intended to be an "abstract attribute" + if matches!( + stmt, + Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. }) + ) { has_abstract_method = true; continue; } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B024_B024.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B024_B024.py.snap index b2e5c9eb6747b..5f29c87bee74b 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B024_B024.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B024_B024.py.snap @@ -41,6 +41,20 @@ B024.py:92:7: B024 `notabc_Base_1` is an abstract base class, but it has no abst 94 | foo() | +B024.py:132:7: B024 `abc_set_class_variable_2` is an abstract base class, but it has no abstract methods + | +132 | class abc_set_class_variable_2(ABC): # error (not an abstract attribute) + | ^^^^^^^^^^^^^^^^^^^^^^^^ B024 +133 | foo = 2 + | + +B024.py:136:7: B024 `abc_set_class_variable_3` is an abstract base class, but it has no abstract methods + | +136 | class abc_set_class_variable_3(ABC): # error (not an abstract attribute) + | ^^^^^^^^^^^^^^^^^^^^^^^^ B024 +137 | foo: int = 2 + | + B024.py:141:7: B024 `abc_set_class_variable_4` is an abstract base class, but it has no abstract methods | 140 | # this doesn't actually declare a class variable, it's just an expression @@ -48,5 +62,3 @@ B024.py:141:7: B024 `abc_set_class_variable_4` is an abstract base class, but it | ^^^^^^^^^^^^^^^^^^^^^^^^ B024 142 | foo | - -