Ensure that B006 autofixes are inserted after imports (#7629)

## Summary

Fixes #7616 by ensuring that
[B006](https://docs.astral.sh/ruff/rules/mutable-argument-default/#mutable-argument-default-b006)
fixes are inserted after module imports.

I have created a new test file, `B006_5.py`. This is mainly because I
have been working on this on and off, and the merge conflicts were
easier to handle in a separate file. If needed, I can move it into
another file.

## Test Plan

`cargo test`
This commit is contained in:
Simon Høxbro Hansen 2023-09-27 03:26:29 +02:00 committed by GitHub
parent 2aef46cb6f
commit fbbc982c29
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 513 additions and 32 deletions

View file

@ -37,6 +37,9 @@ mod tests {
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_2.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_3.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_4.py"))]
#[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::NoExplicitStacklevel, Path::new("B028.py"))]
#[test_case(Rule::RaiseLiteral, Path::new("B016.py"))]
#[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"))]

View file

@ -139,16 +139,14 @@ fn move_initialization(
indexer: &Indexer,
generator: Generator,
) -> Option<Fix> {
let mut body = function_def.body.iter();
let mut body = function_def.body.iter().peekable();
let statement = body.next()?;
if indexer.in_multi_statement_line(statement, locator) {
// Avoid attempting to fix single-line functions.
let statement = body.peek()?;
if indexer.preceded_by_multi_statement_line(statement, locator) {
return None;
}
// Determine the indentation depth of the function body.
let indentation = indentation_at_offset(statement.start(), locator)?;
// Set the default argument value to `None`.
let default_edit = Edit::range_replacement("None".to_string(), default.range());
@ -164,34 +162,43 @@ fn move_initialization(
));
content.push_str(stylist.line_ending().as_str());
// Determine the indentation depth of the function body.
let indentation = indentation_at_offset(statement.start(), locator)?;
// Indent the edit to match the body indentation.
let content = textwrap::indent(&content, indentation).to_string();
let mut content = textwrap::indent(&content, indentation).to_string();
let initialization_edit = if is_docstring_stmt(statement) {
// If the first statement in the function is a docstring, insert _after_ it.
if let Some(statement) = body.next() {
// If there's a second statement, insert _before_ it, but ensure this isn't a
// multi-statement line.
if indexer.in_multi_statement_line(statement, locator) {
return None;
// Find the position to insert the initialization after docstring and imports
let mut pos = locator.line_start(statement.start());
while let Some(statement) = body.next() {
// If the statement is a docstring or an import, insert _after_ it.
if is_docstring_stmt(statement)
|| statement.is_import_stmt()
|| statement.is_import_from_stmt()
{
if let Some(next) = body.peek() {
// If there's a second statement, insert _before_ it, but ensure this isn't a
// multi-statement line.
if indexer.in_multi_statement_line(statement, locator) {
continue;
}
pos = locator.line_start(next.start());
} else if locator.full_line_end(statement.end()) == locator.text_len() {
// If the statement is at the end of the file, without a trailing newline, insert
// _after_ it with an extra newline.
content = format!("{}{}", stylist.line_ending().as_str(), content);
pos = locator.full_line_end(statement.end());
break;
} else {
// If this is the only statement, insert _after_ it.
pos = locator.full_line_end(statement.end());
break;
}
Edit::insertion(content, locator.line_start(statement.start()))
} else if locator.full_line_end(statement.end()) == locator.text_len() {
// If the statement is at the end of the file, without a trailing newline, insert
// _after_ it with an extra newline.
Edit::insertion(
format!("{}{}", stylist.line_ending().as_str(), content),
locator.full_line_end(statement.end()),
)
} else {
// If the docstring is the only statement, insert _after_ it.
Edit::insertion(content, locator.full_line_end(statement.end()))
}
} else {
// Otherwise, insert before the first statement.
let at = locator.line_start(statement.start());
Edit::insertion(content, at)
};
break;
};
}
let initialization_edit = Edit::insertion(content, pos);
Some(Fix::manual_edits(default_edit, [initialization_edit]))
}

View file

@ -0,0 +1,313 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_5.py:5:49: B006 [*] Do not use mutable data structures for argument defaults
|
5 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
6 | import os
|
= help: Replace with `None`; initialize within function
Possible fix
2 2 | # https://github.com/astral-sh/ruff/issues/7616
3 3 |
4 4 |
5 |-def import_module_wrong(value: dict[str, str] = {}):
5 |+def import_module_wrong(value: dict[str, str] = None):
6 6 | import os
7 |+ if value is None:
8 |+ value = {}
7 9 |
8 10 |
9 11 | def import_module_with_values_wrong(value: dict[str, str] = {}):
B006_5.py:9:61: B006 [*] Do not use mutable data structures for argument defaults
|
9 | def import_module_with_values_wrong(value: dict[str, str] = {}):
| ^^ B006
10 | import os
|
= help: Replace with `None`; initialize within function
Possible fix
6 6 | import os
7 7 |
8 8 |
9 |-def import_module_with_values_wrong(value: dict[str, str] = {}):
9 |+def import_module_with_values_wrong(value: dict[str, str] = None):
10 10 | import os
11 11 |
12 |+ if value is None:
13 |+ value = {}
12 14 | return 2
13 15 |
14 16 |
B006_5.py:15:50: B006 [*] Do not use mutable data structures for argument defaults
|
15 | def import_modules_wrong(value: dict[str, str] = {}):
| ^^ B006
16 | import os
17 | import sys
|
= help: Replace with `None`; initialize within function
Possible fix
12 12 | return 2
13 13 |
14 14 |
15 |-def import_modules_wrong(value: dict[str, str] = {}):
15 |+def import_modules_wrong(value: dict[str, str] = None):
16 16 | import os
17 17 | import sys
18 18 | import itertools
19 |+ if value is None:
20 |+ value = {}
19 21 |
20 22 |
21 23 | def from_import_module_wrong(value: dict[str, str] = {}):
B006_5.py:21:54: B006 [*] Do not use mutable data structures for argument defaults
|
21 | def from_import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
22 | from os import path
|
= help: Replace with `None`; initialize within function
Possible fix
18 18 | import itertools
19 19 |
20 20 |
21 |-def from_import_module_wrong(value: dict[str, str] = {}):
21 |+def from_import_module_wrong(value: dict[str, str] = None):
22 22 | from os import path
23 |+ if value is None:
24 |+ value = {}
23 25 |
24 26 |
25 27 | def from_imports_module_wrong(value: dict[str, str] = {}):
B006_5.py:25:55: B006 [*] Do not use mutable data structures for argument defaults
|
25 | def from_imports_module_wrong(value: dict[str, str] = {}):
| ^^ B006
26 | from os import path
27 | from sys import version_info
|
= help: Replace with `None`; initialize within function
Possible fix
22 22 | from os import path
23 23 |
24 24 |
25 |-def from_imports_module_wrong(value: dict[str, str] = {}):
25 |+def from_imports_module_wrong(value: dict[str, str] = None):
26 26 | from os import path
27 27 | from sys import version_info
28 |+ if value is None:
29 |+ value = {}
28 30 |
29 31 |
30 32 | def import_and_from_imports_module_wrong(value: dict[str, str] = {}):
B006_5.py:30:66: B006 [*] Do not use mutable data structures for argument defaults
|
30 | def import_and_from_imports_module_wrong(value: dict[str, str] = {}):
| ^^ B006
31 | import os
32 | from sys import version_info
|
= help: Replace with `None`; initialize within function
Possible fix
27 27 | from sys import version_info
28 28 |
29 29 |
30 |-def import_and_from_imports_module_wrong(value: dict[str, str] = {}):
30 |+def import_and_from_imports_module_wrong(value: dict[str, str] = None):
31 31 | import os
32 32 | from sys import version_info
33 |+ if value is None:
34 |+ value = {}
33 35 |
34 36 |
35 37 | def import_docstring_module_wrong(value: dict[str, str] = {}):
B006_5.py:35:59: B006 [*] Do not use mutable data structures for argument defaults
|
35 | def import_docstring_module_wrong(value: dict[str, str] = {}):
| ^^ B006
36 | """Docstring"""
37 | import os
|
= help: Replace with `None`; initialize within function
Possible fix
32 32 | from sys import version_info
33 33 |
34 34 |
35 |-def import_docstring_module_wrong(value: dict[str, str] = {}):
35 |+def import_docstring_module_wrong(value: dict[str, str] = None):
36 36 | """Docstring"""
37 37 | import os
38 |+ if value is None:
39 |+ value = {}
38 40 |
39 41 |
40 42 | def import_module_wrong(value: dict[str, str] = {}):
B006_5.py:40:49: B006 [*] Do not use mutable data structures for argument defaults
|
40 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
41 | """Docstring"""
42 | import os; import sys
|
= help: Replace with `None`; initialize within function
Possible fix
37 37 | import os
38 38 |
39 39 |
40 |-def import_module_wrong(value: dict[str, str] = {}):
40 |+def import_module_wrong(value: dict[str, str] = None):
41 41 | """Docstring"""
42 42 | import os; import sys
43 |+ if value is None:
44 |+ value = {}
43 45 |
44 46 |
45 47 | def import_module_wrong(value: dict[str, str] = {}):
B006_5.py:45:49: B006 [*] Do not use mutable data structures for argument defaults
|
45 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
46 | """Docstring"""
47 | import os; import sys; x = 1
|
= help: Replace with `None`; initialize within function
Possible fix
42 42 | import os; import sys
43 43 |
44 44 |
45 |-def import_module_wrong(value: dict[str, str] = {}):
45 |+def import_module_wrong(value: dict[str, str] = None):
46 46 | """Docstring"""
47 |+ if value is None:
48 |+ value = {}
47 49 | import os; import sys; x = 1
48 50 |
49 51 |
B006_5.py:50:49: B006 [*] Do not use mutable data structures for argument defaults
|
50 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
51 | """Docstring"""
52 | import os; import sys
|
= help: Replace with `None`; initialize within function
Possible fix
47 47 | import os; import sys; x = 1
48 48 |
49 49 |
50 |-def import_module_wrong(value: dict[str, str] = {}):
50 |+def import_module_wrong(value: dict[str, str] = None):
51 51 | """Docstring"""
52 52 | import os; import sys
53 |+ if value is None:
54 |+ value = {}
53 55 |
54 56 |
55 57 | def import_module_wrong(value: dict[str, str] = {}):
B006_5.py:55:49: B006 [*] Do not use mutable data structures for argument defaults
|
55 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
56 | import os; import sys
|
= help: Replace with `None`; initialize within function
Possible fix
52 52 | import os; import sys
53 53 |
54 54 |
55 |-def import_module_wrong(value: dict[str, str] = {}):
55 |+def import_module_wrong(value: dict[str, str] = None):
56 56 | import os; import sys
57 |+ if value is None:
58 |+ value = {}
57 59 |
58 60 |
59 61 | def import_module_wrong(value: dict[str, str] = {}):
B006_5.py:59:49: B006 [*] Do not use mutable data structures for argument defaults
|
59 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
60 | import os; import sys; x = 1
|
= help: Replace with `None`; initialize within function
Possible fix
56 56 | import os; import sys
57 57 |
58 58 |
59 |-def import_module_wrong(value: dict[str, str] = {}):
59 |+def import_module_wrong(value: dict[str, str] = None):
60 |+ if value is None:
61 |+ value = {}
60 62 | import os; import sys; x = 1
61 63 |
62 64 |
B006_5.py:63:49: B006 [*] Do not use mutable data structures for argument defaults
|
63 | def import_module_wrong(value: dict[str, str] = {}):
| ^^ B006
64 | import os; import sys
|
= help: Replace with `None`; initialize within function
Possible fix
60 60 | import os; import sys; x = 1
61 61 |
62 62 |
63 |-def import_module_wrong(value: dict[str, str] = {}):
63 |+def import_module_wrong(value: dict[str, str] = None):
64 64 | import os; import sys
65 |+ if value is None:
66 |+ value = {}
65 67 |
66 68 |
67 69 | def import_module_wrong(value: dict[str, str] = {}): import os
B006_5.py:67:49: B006 Do not use mutable data structures for argument defaults
|
67 | def import_module_wrong(value: dict[str, str] = {}): import os
| ^^ B006
|
= help: Replace with `None`; initialize within function
B006_5.py:70:49: B006 Do not use mutable data structures for argument defaults
|
70 | def import_module_wrong(value: dict[str, str] = {}): import os; import sys
| ^^ B006
|
= help: Replace with `None`; initialize within function
B006_5.py:73:49: B006 Do not use mutable data structures for argument defaults
|
73 | def import_module_wrong(value: dict[str, str] = {}): \
| ^^ B006
74 | import os
|
= help: Replace with `None`; initialize within function

View file

@ -0,0 +1,25 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_6.py:4:22: B006 [*] Do not use mutable data structures for argument defaults
|
2 | # Same as B006_2.py, but import instead of docstring
3 |
4 | def foobar(foor, bar={}):
| ^^ B006
5 | import os
|
= help: Replace with `None`; initialize within function
Possible fix
1 1 | # Import followed by whitespace with no newline
2 2 | # Same as B006_2.py, but import instead of docstring
3 3 |
4 |-def foobar(foor, bar={}):
5 |- import os
4 |+def foobar(foor, bar=None):
5 |+ import os
6 |+ if bar is None:
7 |+ bar = {}

View file

@ -0,0 +1,25 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_7.py:4:22: B006 [*] Do not use mutable data structures for argument defaults
|
2 | # Same as B006_3.py, but import instead of docstring
3 |
4 | def foobar(foor, bar={}):
| ^^ B006
5 | import os
|
= help: Replace with `None`; initialize within function
Possible fix
1 1 | # Import with no newline
2 2 | # Same as B006_3.py, but import instead of docstring
3 3 |
4 |-def foobar(foor, bar={}):
5 |- import os
4 |+def foobar(foor, bar=None):
5 |+ import os
6 |+ if bar is None:
7 |+ bar = {}

View file

@ -424,7 +424,7 @@ B006_B008.py:288:52: B006 [*] Do not use mutable data structures for argument de
291 293 |
292 294 |
B006_B008.py:293:52: B006 Do not use mutable data structures for argument defaults
B006_B008.py:293:52: B006 [*] Do not use mutable data structures for argument defaults
|
293 | def single_line_func_wrong(value: dict[str, str] = {}):
| ^^ B006
@ -432,7 +432,19 @@ B006_B008.py:293:52: B006 Do not use mutable data structures for argument defaul
|
= help: Replace with `None`; initialize within function
B006_B008.py:297:52: B006 Do not use mutable data structures for argument defaults
Possible fix
290 290 | ...
291 291 |
292 292 |
293 |-def single_line_func_wrong(value: dict[str, str] = {}):
293 |+def single_line_func_wrong(value: dict[str, str] = None):
294 |+ if value is None:
295 |+ value = {}
294 296 | """Docstring"""; ...
295 297 |
296 298 |
B006_B008.py:297:52: B006 [*] Do not use mutable data structures for argument defaults
|
297 | def single_line_func_wrong(value: dict[str, str] = {}):
| ^^ B006
@ -441,6 +453,18 @@ B006_B008.py:297:52: B006 Do not use mutable data structures for argument defaul
|
= help: Replace with `None`; initialize within function
Possible fix
294 294 | """Docstring"""; ...
295 295 |
296 296 |
297 |-def single_line_func_wrong(value: dict[str, str] = {}):
297 |+def single_line_func_wrong(value: dict[str, str] = None):
298 |+ if value is None:
299 |+ value = {}
298 300 | """Docstring"""; \
299 301 | ...
300 302 |
B006_B008.py:302:52: B006 [*] Do not use mutable data structures for argument defaults
|
302 | def single_line_func_wrong(value: dict[str, str] = {