Skip to content

Commit 673167a

Browse files
IDrokin117Igor Drokindylwil3
authored
[flake8-bugbear] Include certain guaranteed-mutable expressions: tuples, generators, and assignment expressions (B006) (#20024)
## Summary Resolves #20004 The implementation now supports guaranteed-mutable expressions in the following cases: - Tuple literals with mutable elements (supporting deep nesting) - Generator expressions - Named expressions (walrus operator) containing mutable components Preserves original formatting for assignment value: ```python # Test case def f5(x=([1, ])): print(x) ``` ```python # Fix before def f5(x=(None)): if x is None: x = [1] print(x) ``` ```python # Fix after def f5(x=None): if x is None: x = ([1, ]) print(x) ``` The expansion of detected expressions and the new fixes gated behind previews. ## Test Plan - Added B006_9.py with a bunch of test cases - Generated snapshots --------- Co-authored-by: Igor Drokin <drokinii1017@gmail.com> Co-authored-by: dylwil3 <dylwil3@gmail.com>
1 parent 805d179 commit 673167a

16 files changed

+1249
-32
lines changed
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
def f1(x=([],)):
2+
print(x)
3+
4+
5+
def f2(x=(x for x in "x")):
6+
print(x)
7+
8+
9+
def f3(x=((x for x in "x"),)):
10+
print(x)
11+
12+
13+
def f4(x=(z := [1, ])):
14+
print(x)
15+
16+
17+
def f5(x=([1, ])):
18+
print(x)
19+
20+
21+
def w1(x=(1,)):
22+
print(x)
23+
24+
25+
def w2(x=(z := 3)):
26+
print(x)

crates/ruff_linter/src/preview.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,3 +245,17 @@ pub(crate) const fn is_a003_class_scope_shadowing_expansion_enabled(
245245
pub(crate) const fn is_refined_submodule_import_match_enabled(settings: &LinterSettings) -> bool {
246246
settings.preview.is_enabled()
247247
}
248+
249+
// github.com/astral-sh/ruff/issues/20004
250+
pub(crate) const fn is_b006_check_guaranteed_mutable_expr_enabled(
251+
settings: &LinterSettings,
252+
) -> bool {
253+
settings.preview.is_enabled()
254+
}
255+
256+
// github.com/astral-sh/ruff/issues/20004
257+
pub(crate) const fn is_b006_unsafe_fix_preserve_assignment_expr_enabled(
258+
settings: &LinterSettings,
259+
) -> bool {
260+
settings.preview.is_enabled()
261+
}

crates/ruff_linter/src/rules/flake8_bugbear/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ mod tests {
4747
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_6.py"))]
4848
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_7.py"))]
4949
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_8.py"))]
50+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_9.py"))]
5051
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
5152
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_1.pyi"))]
5253
#[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
@@ -83,6 +84,17 @@ mod tests {
8384
}
8485

8586
#[test_case(Rule::MapWithoutExplicitStrict, Path::new("B912.py"))]
87+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_1.py"))]
88+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_2.py"))]
89+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_3.py"))]
90+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_4.py"))]
91+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_5.py"))]
92+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_6.py"))]
93+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_7.py"))]
94+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_8.py"))]
95+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_9.py"))]
96+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
97+
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_1.pyi"))]
8698
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
8799
let snapshot = format!(
88100
"preview__{}_{}",

crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs

Lines changed: 73 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,20 @@ use std::fmt::Write;
33
use ruff_macros::{ViolationMetadata, derive_message_formats};
44
use ruff_python_ast::helpers::is_docstring_stmt;
55
use ruff_python_ast::name::QualifiedName;
6-
use ruff_python_ast::{self as ast, Expr, Parameter};
7-
use ruff_python_codegen::{Generator, Stylist};
8-
use ruff_python_index::Indexer;
6+
use ruff_python_ast::parenthesize::parenthesized_range;
7+
use ruff_python_ast::{self as ast, Expr, ParameterWithDefault};
98
use ruff_python_semantic::SemanticModel;
109
use ruff_python_semantic::analyze::function_type::is_stub;
1110
use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_mutable_expr};
1211
use ruff_python_trivia::{indentation_at_offset, textwrap};
1312
use ruff_source_file::LineRanges;
1413
use ruff_text_size::Ranged;
1514

16-
use crate::Locator;
1715
use crate::checkers::ast::Checker;
16+
use crate::preview::{
17+
is_b006_check_guaranteed_mutable_expr_enabled,
18+
is_b006_unsafe_fix_preserve_assignment_expr_enabled,
19+
};
1820
use crate::{Edit, Fix, FixAvailability, Violation};
1921

2022
/// ## What it does
@@ -111,44 +113,50 @@ pub(crate) fn mutable_argument_default(checker: &Checker, function_def: &ast::St
111113
.iter()
112114
.map(|target| QualifiedName::from_dotted_name(target))
113115
.collect();
114-
115-
if is_mutable_expr(default, checker.semantic())
116+
let is_mut_expr = if is_b006_check_guaranteed_mutable_expr_enabled(checker.settings()) {
117+
is_guaranteed_mutable_expr(default, checker.semantic())
118+
} else {
119+
is_mutable_expr(default, checker.semantic())
120+
};
121+
if is_mut_expr
116122
&& !parameter.annotation().is_some_and(|expr| {
117123
is_immutable_annotation(expr, checker.semantic(), extend_immutable_calls.as_slice())
118124
})
119125
{
120126
let mut diagnostic = checker.report_diagnostic(MutableArgumentDefault, default.range());
121127

122128
// If the function body is on the same line as the function def, do not fix
123-
if let Some(fix) = move_initialization(
124-
function_def,
125-
&parameter.parameter,
126-
default,
127-
checker.semantic(),
128-
checker.locator(),
129-
checker.stylist(),
130-
checker.indexer(),
131-
checker.generator(),
132-
) {
129+
if let Some(fix) = move_initialization(function_def, parameter, default, checker) {
133130
diagnostic.set_fix(fix);
134131
}
135132
}
136133
}
137134
}
138135

136+
/// Returns `true` if the expression is guaranteed to create a mutable object.
137+
fn is_guaranteed_mutable_expr(expr: &Expr, semantic: &SemanticModel) -> bool {
138+
match expr {
139+
Expr::Generator(_) => true,
140+
Expr::Tuple(ast::ExprTuple { elts, .. }) => {
141+
elts.iter().any(|e| is_guaranteed_mutable_expr(e, semantic))
142+
}
143+
Expr::Named(ast::ExprNamed { value, .. }) => is_guaranteed_mutable_expr(value, semantic),
144+
_ => is_mutable_expr(expr, semantic),
145+
}
146+
}
147+
139148
/// Generate a [`Fix`] to move a mutable argument default initialization
140149
/// into the function body.
141-
#[expect(clippy::too_many_arguments)]
142150
fn move_initialization(
143151
function_def: &ast::StmtFunctionDef,
144-
parameter: &Parameter,
152+
parameter: &ParameterWithDefault,
145153
default: &Expr,
146-
semantic: &SemanticModel,
147-
locator: &Locator,
148-
stylist: &Stylist,
149-
indexer: &Indexer,
150-
generator: Generator,
154+
checker: &Checker,
151155
) -> Option<Fix> {
156+
let indexer = checker.indexer();
157+
let locator = checker.locator();
158+
let stylist = checker.stylist();
159+
152160
let mut body = function_def.body.iter().peekable();
153161

154162
// Avoid attempting to fix single-line functions.
@@ -157,25 +165,58 @@ fn move_initialization(
157165
return None;
158166
}
159167

168+
let range = match parenthesized_range(
169+
default.into(),
170+
parameter.into(),
171+
checker.comment_ranges(),
172+
checker.source(),
173+
) {
174+
Some(range) => range,
175+
None => default.range(),
176+
};
160177
// Set the default argument value to `None`.
161-
let default_edit = Edit::range_replacement("None".to_string(), default.range());
178+
let default_edit = Edit::range_replacement("None".to_string(), range);
162179

163180
// If the function is a stub, this is the only necessary edit.
164-
if is_stub(function_def, semantic) {
181+
if is_stub(function_def, checker.semantic()) {
165182
return Some(Fix::unsafe_edit(default_edit));
166183
}
167184

168185
// Add an `if`, to set the argument to its original value if still `None`.
169186
let mut content = String::new();
170-
let _ = write!(&mut content, "if {} is None:", parameter.name());
187+
let _ = write!(&mut content, "if {} is None:", parameter.parameter.name());
171188
content.push_str(stylist.line_ending().as_str());
172189
content.push_str(stylist.indentation());
173-
let _ = write!(
174-
&mut content,
175-
"{} = {}",
176-
parameter.name(),
177-
generator.expr(default)
178-
);
190+
if is_b006_unsafe_fix_preserve_assignment_expr_enabled(checker.settings()) {
191+
let annotation = if let Some(ann) = parameter.annotation() {
192+
format!(": {}", locator.slice(ann))
193+
} else {
194+
String::new()
195+
};
196+
let _ = write!(
197+
&mut content,
198+
"{}{} = {}",
199+
parameter.parameter.name(),
200+
annotation,
201+
locator.slice(
202+
parenthesized_range(
203+
default.into(),
204+
parameter.into(),
205+
checker.comment_ranges(),
206+
checker.source()
207+
)
208+
.unwrap_or(default.range())
209+
)
210+
);
211+
} else {
212+
let _ = write!(
213+
&mut content,
214+
"{} = {}",
215+
parameter.name(),
216+
checker.generator().expr(default)
217+
);
218+
}
219+
179220
content.push_str(stylist.line_ending().as_str());
180221

181222
// Determine the indentation depth of the function body.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
B006 [*] Do not use mutable data structures for argument defaults
5+
--> B006_9.py:17:11
6+
|
7+
17 | def f5(x=([1, ])):
8+
| ^^^^^
9+
18 | print(x)
10+
|
11+
help: Replace with `None`; initialize within function
12+
14 | print(x)
13+
15 |
14+
16 |
15+
- def f5(x=([1, ])):
16+
17 + def f5(x=None):
17+
18 + if x is None:
18+
19 + x = [1]
19+
20 | print(x)
20+
21 |
21+
22 |
22+
note: This is an unsafe fix and may change runtime behavior
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
B006 [*] Do not use mutable data structures for argument defaults
5+
--> B006_1.py:3:22
6+
|
7+
1 | # Docstring followed by a newline
8+
2 |
9+
3 | def foobar(foor, bar={}):
10+
| ^^
11+
4 | """
12+
5 | """
13+
|
14+
help: Replace with `None`; initialize within function
15+
1 | # Docstring followed by a newline
16+
2 |
17+
- def foobar(foor, bar={}):
18+
3 + def foobar(foor, bar=None):
19+
4 | """
20+
5 | """
21+
note: This is an unsafe fix and may change runtime behavior
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
B006 [*] Do not use mutable data structures for argument defaults
5+
--> B006_2.py:4:22
6+
|
7+
2 | # Regression test for https://github.com/astral-sh/ruff/issues/7155
8+
3 |
9+
4 | def foobar(foor, bar={}):
10+
| ^^
11+
5 | """
12+
6 | """
13+
|
14+
help: Replace with `None`; initialize within function
15+
1 | # Docstring followed by whitespace with no newline
16+
2 | # Regression test for https://github.com/astral-sh/ruff/issues/7155
17+
3 |
18+
- def foobar(foor, bar={}):
19+
4 + def foobar(foor, bar=None):
20+
5 | """
21+
6 | """
22+
note: This is an unsafe fix and may change runtime behavior
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
B006 [*] Do not use mutable data structures for argument defaults
5+
--> B006_3.py:4:22
6+
|
7+
4 | def foobar(foor, bar={}):
8+
| ^^
9+
5 | """
10+
6 | """
11+
|
12+
help: Replace with `None`; initialize within function
13+
1 | # Docstring with no newline
14+
2 |
15+
3 |
16+
- def foobar(foor, bar={}):
17+
4 + def foobar(foor, bar=None):
18+
5 | """
19+
6 | """
20+
note: This is an unsafe fix and may change runtime behavior
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
B006 [*] Do not use mutable data structures for argument defaults
5+
--> B006_4.py:7:26
6+
|
7+
6 | class FormFeedIndent:
8+
7 | def __init__(self, a=[]):
9+
| ^^
10+
8 | print(a)
11+
|
12+
help: Replace with `None`; initialize within function
13+
4 |
14+
5 |
15+
6 | class FormFeedIndent:
16+
- def __init__(self, a=[]):
17+
7 + def __init__(self, a=None):
18+
8 + if a is None:
19+
9 + a = []
20+
10 | print(a)
21+
11 |
22+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)