[flake8-bugbear] Treat raise NotImplemented-only bodies as stub functions (#10990)
## Summary As discussed in https://github.com/astral-sh/ruff/issues/10083#issuecomment-1969653610, stubs detection now also covers the case where the function body raises NotImplementedError and does nothing else. ## Test Plan Tests for the relevant cases were added in B006_8.py
This commit is contained in:
20
crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B006_8.py
vendored
Normal file
20
crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B006_8.py
vendored
Normal file
@@ -0,0 +1,20 @@
|
||||
def foo(a: list = []):
|
||||
raise NotImplementedError("")
|
||||
|
||||
|
||||
def bar(a: dict = {}):
|
||||
""" This one also has a docstring"""
|
||||
raise NotImplementedError("and has some text in here")
|
||||
|
||||
|
||||
def baz(a: list = []):
|
||||
"""This one raises a different exception"""
|
||||
raise IndexError()
|
||||
|
||||
|
||||
def qux(a: list = []):
|
||||
raise NotImplementedError
|
||||
|
||||
|
||||
def quux(a: list = []):
|
||||
raise NotImplemented
|
||||
@@ -41,6 +41,7 @@ mod tests {
|
||||
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_5.py"))]
|
||||
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_6.py"))]
|
||||
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_7.py"))]
|
||||
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_8.py"))]
|
||||
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
|
||||
#[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
|
||||
#[test_case(Rule::RaiseLiteral, Path::new("B016.py"))]
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::helpers::is_docstring_stmt;
|
||||
use ruff_python_ast::helpers::{is_docstring_stmt, map_callable};
|
||||
use ruff_python_ast::name::QualifiedName;
|
||||
use ruff_python_ast::{self as ast, Expr, Parameter, ParameterWithDefault, Stmt};
|
||||
use ruff_python_ast::{self as ast, Expr, Parameter, ParameterWithDefault, Stmt, StmtRaise};
|
||||
use ruff_python_codegen::{Generator, Stylist};
|
||||
use ruff_python_index::Indexer;
|
||||
use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_expr};
|
||||
use ruff_python_semantic::SemanticModel;
|
||||
use ruff_python_trivia::{indentation_at_offset, textwrap};
|
||||
use ruff_source_file::Locator;
|
||||
use ruff_text_size::Ranged;
|
||||
@@ -118,6 +119,7 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast
|
||||
function_def,
|
||||
parameter,
|
||||
default,
|
||||
checker.semantic(),
|
||||
checker.locator(),
|
||||
checker.stylist(),
|
||||
checker.indexer(),
|
||||
@@ -132,10 +134,12 @@ pub(crate) fn mutable_argument_default(checker: &mut Checker, function_def: &ast
|
||||
|
||||
/// Generate a [`Fix`] to move a mutable argument default initialization
|
||||
/// into the function body.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn move_initialization(
|
||||
function_def: &ast::StmtFunctionDef,
|
||||
parameter: &Parameter,
|
||||
default: &Expr,
|
||||
semantic: &SemanticModel,
|
||||
locator: &Locator,
|
||||
stylist: &Stylist,
|
||||
indexer: &Indexer,
|
||||
@@ -153,7 +157,7 @@ fn move_initialization(
|
||||
let default_edit = Edit::range_replacement("None".to_string(), default.range());
|
||||
|
||||
// If the function is a stub, this is the only necessary edit.
|
||||
if is_stub(function_def) {
|
||||
if is_stub(function_def, semantic) {
|
||||
return Some(Fix::unsafe_edit(default_edit));
|
||||
}
|
||||
|
||||
@@ -213,8 +217,8 @@ fn move_initialization(
|
||||
/// Returns `true` if a function has an empty body, and is therefore a stub.
|
||||
///
|
||||
/// A function body is considered to be empty if it contains only `pass` statements, `...` literals,
|
||||
/// and docstrings.
|
||||
fn is_stub(function_def: &ast::StmtFunctionDef) -> bool {
|
||||
/// `NotImplementedError` raises, or string literal statements (docstrings).
|
||||
fn is_stub(function_def: &ast::StmtFunctionDef, semantic: &SemanticModel) -> bool {
|
||||
function_def.body.iter().all(|stmt| match stmt {
|
||||
Stmt::Pass(_) => true,
|
||||
Stmt::Expr(ast::StmtExpr { value, range: _ }) => {
|
||||
@@ -223,6 +227,15 @@ fn is_stub(function_def: &ast::StmtFunctionDef) -> bool {
|
||||
Expr::StringLiteral(_) | Expr::EllipsisLiteral(_)
|
||||
)
|
||||
}
|
||||
Stmt::Raise(StmtRaise {
|
||||
range: _,
|
||||
exc: exception,
|
||||
cause: _,
|
||||
}) => exception.as_ref().is_some_and(|exc| {
|
||||
semantic
|
||||
.resolve_builtin_symbol(map_callable(exc))
|
||||
.is_some_and(|name| matches!(name, "NotImplementedError" | "NotImplemented"))
|
||||
}),
|
||||
_ => false,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
---
|
||||
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
|
||||
---
|
||||
B006_8.py:1:19: B006 [*] Do not use mutable data structures for argument defaults
|
||||
|
|
||||
1 | def foo(a: list = []):
|
||||
| ^^ B006
|
||||
2 | raise NotImplementedError("")
|
||||
|
|
||||
= help: Replace with `None`; initialize within function
|
||||
|
||||
ℹ Unsafe fix
|
||||
1 |-def foo(a: list = []):
|
||||
1 |+def foo(a: list = None):
|
||||
2 2 | raise NotImplementedError("")
|
||||
3 3 |
|
||||
4 4 |
|
||||
|
||||
B006_8.py:5:19: B006 [*] Do not use mutable data structures for argument defaults
|
||||
|
|
||||
5 | def bar(a: dict = {}):
|
||||
| ^^ B006
|
||||
6 | """ This one also has a docstring"""
|
||||
7 | raise NotImplementedError("and has some text in here")
|
||||
|
|
||||
= help: Replace with `None`; initialize within function
|
||||
|
||||
ℹ Unsafe fix
|
||||
2 2 | raise NotImplementedError("")
|
||||
3 3 |
|
||||
4 4 |
|
||||
5 |-def bar(a: dict = {}):
|
||||
5 |+def bar(a: dict = None):
|
||||
6 6 | """ This one also has a docstring"""
|
||||
7 7 | raise NotImplementedError("and has some text in here")
|
||||
8 8 |
|
||||
|
||||
B006_8.py:10:19: B006 [*] Do not use mutable data structures for argument defaults
|
||||
|
|
||||
10 | def baz(a: list = []):
|
||||
| ^^ B006
|
||||
11 | """This one raises a different exception"""
|
||||
12 | raise IndexError()
|
||||
|
|
||||
= help: Replace with `None`; initialize within function
|
||||
|
||||
ℹ Unsafe fix
|
||||
7 7 | raise NotImplementedError("and has some text in here")
|
||||
8 8 |
|
||||
9 9 |
|
||||
10 |-def baz(a: list = []):
|
||||
10 |+def baz(a: list = None):
|
||||
11 11 | """This one raises a different exception"""
|
||||
12 |+ if a is None:
|
||||
13 |+ a = []
|
||||
12 14 | raise IndexError()
|
||||
13 15 |
|
||||
14 16 |
|
||||
|
||||
B006_8.py:15:19: B006 [*] Do not use mutable data structures for argument defaults
|
||||
|
|
||||
15 | def qux(a: list = []):
|
||||
| ^^ B006
|
||||
16 | raise NotImplementedError
|
||||
|
|
||||
= help: Replace with `None`; initialize within function
|
||||
|
||||
ℹ Unsafe fix
|
||||
12 12 | raise IndexError()
|
||||
13 13 |
|
||||
14 14 |
|
||||
15 |-def qux(a: list = []):
|
||||
15 |+def qux(a: list = None):
|
||||
16 16 | raise NotImplementedError
|
||||
17 17 |
|
||||
18 18 |
|
||||
|
||||
B006_8.py:19:20: B006 [*] Do not use mutable data structures for argument defaults
|
||||
|
|
||||
19 | def quux(a: list = []):
|
||||
| ^^ B006
|
||||
20 | raise NotImplemented
|
||||
|
|
||||
= help: Replace with `None`; initialize within function
|
||||
|
||||
ℹ Unsafe fix
|
||||
16 16 | raise NotImplementedError
|
||||
17 17 |
|
||||
18 18 |
|
||||
19 |-def quux(a: list = []):
|
||||
19 |+def quux(a: list = None):
|
||||
20 20 | raise NotImplemented
|
||||
Reference in New Issue
Block a user