Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade mutable-argument-defaults to unsafe #8108

Merged
merged 2 commits into from Oct 21, 2023
Merged

Upgrade mutable-argument-defaults to unsafe #8108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a42887
Upgrade mutable-argument-defaults to unsafe
cjolowicz Oct 21, 2023
9edcb67
Add `Known problems` section to explain why the fix is unsafe
cjolowicz Oct 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs
View file
Open in desktop
Expand Up @@ -30,6 +30,12 @@ use crate::checkers::ast::Checker;
/// Types outside of the standard library can be marked as immutable with the
/// [`flake8-bugbear.extend-immutable-calls`] configuration option.
///
/// ## Known problems
/// Mutable argument defaults can be used intentionally to cache computation
/// results. Replacing the default with `None` or an immutable data structure
/// does not work for such usages. Instead, prefer the `@functools.lru_cache`
/// decorator from the standard library.
///
/// ## Example
/// ```python
/// def add_to_list(item, some_list=[]):
Expand Down Expand Up @@ -197,5 +203,5 @@ fn move_initialization(
}

let initialization_edit = Edit::insertion(content, pos);
Some(Fix::display_edits(default_edit, [initialization_edit]))
Some(Fix::unsafe_edits(default_edit, [initialization_edit]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @charliermarsh

Can we add a note here explaining why the fix is unsafe? I'm not sure why it was marked as unapplicable originally as the fix looks relatively safe to me?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented on this in the issue: #8104 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a Known problems section to explain why the fix is unsafe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

}
4 changes: 2 additions & 2 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_1.py.snap
View file
Open in desktop
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_1.py:3:22: B006 Do not use mutable data structures for argument defaults
B006_1.py:3:22: B006 [*] Do not use mutable data structures for argument defaults
|
1 | # Docstring followed by a newline
2 |
Expand All @@ -12,7 +12,7 @@ B006_1.py:3:22: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
1 1 | # Docstring followed by a newline
2 2 |
3 |-def foobar(foor, bar={}):
Expand Down
4 changes: 2 additions & 2 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_2.py.snap
View file
Open in desktop
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_2.py:4:22: B006 Do not use mutable data structures for argument defaults
B006_2.py:4:22: B006 [*] Do not use mutable data structures for argument defaults
|
2 | # Regression test for https://github.com/astral-sh/ruff/issues/7155
3 |
Expand All @@ -12,7 +12,7 @@ B006_2.py:4:22: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
1 1 | # Docstring followed by whitespace with no newline
2 2 | # Regression test for https://github.com/astral-sh/ruff/issues/7155
3 3 |
Expand Down
4 changes: 2 additions & 2 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_3.py.snap
View file
Open in desktop
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_3.py:4:22: B006 Do not use mutable data structures for argument defaults
B006_3.py:4:22: B006 [*] Do not use mutable data structures for argument defaults
|
4 | def foobar(foor, bar={}):
| ^^ B006
Expand All @@ -10,7 +10,7 @@ B006_3.py:4:22: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
1 1 | # Docstring with no newline
2 2 |
3 3 |
Expand Down
4 changes: 2 additions & 2 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_4.py.snap
View file
Open in desktop
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B006_4.py:7:26: B006 Do not use mutable data structures for argument defaults
B006_4.py:7:26: B006 [*] Do not use mutable data structures for argument defaults
|
6 | class FormFeedIndent:
7 | def __init__(self, a=[]):
Expand All @@ -10,7 +10,7 @@ B006_4.py:7:26: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
4 4 |
5 5 |
6 6 | class FormFeedIndent:
Expand Down
52 changes: 26 additions & 26 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_5.py.snap
View file
Open in desktop
@@ -1,15 +1,15 @@
---
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
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
ℹ Suggested fix
2 2 | # https://github.com/astral-sh/ruff/issues/7616
3 3 |
4 4 |
Expand All @@ -22,15 +22,15 @@ B006_5.py:5:49: B006 Do not use mutable data structures for argument defaults
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
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
ℹ Suggested fix
6 6 | import os
7 7 |
8 8 |
Expand All @@ -44,7 +44,7 @@ B006_5.py:9:61: B006 Do not use mutable data structures for argument defaults
13 15 |
14 16 |

B006_5.py:15:50: B006 Do not use mutable data structures for argument defaults
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
Expand All @@ -53,7 +53,7 @@ B006_5.py:15:50: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
12 12 | return 2
13 13 |
14 14 |
Expand All @@ -68,15 +68,15 @@ B006_5.py:15:50: B006 Do not use mutable data structures for argument defaults
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
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
ℹ Suggested fix
18 18 | import itertools
19 19 |
20 20 |
Expand All @@ -89,7 +89,7 @@ B006_5.py:21:54: B006 Do not use mutable data structures for argument defaults
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
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
Expand All @@ -98,7 +98,7 @@ B006_5.py:25:55: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
22 22 | from os import path
23 23 |
24 24 |
Expand All @@ -112,7 +112,7 @@ B006_5.py:25:55: B006 Do not use mutable data structures for argument defaults
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
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
Expand All @@ -121,7 +121,7 @@ B006_5.py:30:66: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
27 27 | from sys import version_info
28 28 |
29 29 |
Expand All @@ -135,7 +135,7 @@ B006_5.py:30:66: B006 Do not use mutable data structures for argument defaults
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
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
Expand All @@ -144,7 +144,7 @@ B006_5.py:35:59: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
32 32 | from sys import version_info
33 33 |
34 34 |
Expand All @@ -158,7 +158,7 @@ B006_5.py:35:59: B006 Do not use mutable data structures for argument defaults
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
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
Expand All @@ -167,7 +167,7 @@ B006_5.py:40:49: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
37 37 | import os
38 38 |
39 39 |
Expand All @@ -181,7 +181,7 @@ B006_5.py:40:49: B006 Do not use mutable data structures for argument defaults
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
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
Expand All @@ -190,7 +190,7 @@ B006_5.py:45:49: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
42 42 | import os; import sys
43 43 |
44 44 |
Expand All @@ -203,7 +203,7 @@ B006_5.py:45:49: B006 Do not use mutable data structures for argument defaults
48 50 |
49 51 |

B006_5.py:50:49: B006 Do not use mutable data structures for argument defaults
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
Expand All @@ -212,7 +212,7 @@ B006_5.py:50:49: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
47 47 | import os; import sys; x = 1
48 48 |
49 49 |
Expand All @@ -226,15 +226,15 @@ B006_5.py:50:49: B006 Do not use mutable data structures for argument defaults
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
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
ℹ Suggested fix
52 52 | import os; import sys
53 53 |
54 54 |
Expand All @@ -247,15 +247,15 @@ B006_5.py:55:49: B006 Do not use mutable data structures for argument defaults
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
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
ℹ Suggested fix
56 56 | import os; import sys
57 57 |
58 58 |
Expand All @@ -267,15 +267,15 @@ B006_5.py:59:49: B006 Do not use mutable data structures for argument defaults
61 63 |
62 64 |

B006_5.py:63:49: B006 Do not use mutable data structures for argument defaults
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
ℹ Suggested fix
60 60 | import os; import sys; x = 1
61 61 |
62 62 |
Expand Down
4 changes: 2 additions & 2 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_6.py.snap
View file
Open in desktop
@@ -1,7 +1,7 @@
---
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
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 |
Expand All @@ -11,7 +11,7 @@ B006_6.py:4:22: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
1 1 | # Import followed by whitespace with no newline
2 2 | # Same as B006_2.py, but import instead of docstring
3 3 |
Expand Down
4 changes: 2 additions & 2 deletions ...s/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B006_B006_7.py.snap
View file
Open in desktop
@@ -1,7 +1,7 @@
---
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
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 |
Expand All @@ -11,7 +11,7 @@ B006_7.py:4:22: B006 Do not use mutable data structures for argument defaults
|
= help: Replace with `None`; initialize within function

ℹ Possible fix
ℹ Suggested fix
1 1 | # Import with no newline
2 2 | # Same as B006_3.py, but import instead of docstring
3 3 |
Expand Down