-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement rule
mutable-fromkeys-value
Implement rule `mutable-fromkeys-value` (`RUF023`) with a fix marked as unsafe. Fix formatting in docs
- Loading branch information
Showing
8 changed files
with
286 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
pierogi_fillings = [ | ||
"cabbage", | ||
"strawberry", | ||
"cheese", | ||
"blueberry", | ||
] | ||
|
||
# Errors. | ||
dict.fromkeys(pierogi_fillings, []) | ||
dict.fromkeys(pierogi_fillings, list()) | ||
dict.fromkeys(pierogi_fillings, {}) | ||
dict.fromkeys(pierogi_fillings, set()) | ||
dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
dict.fromkeys(pierogi_fillings, dict()) | ||
|
||
# Okay. | ||
dict.fromkeys(pierogi_fillings) | ||
dict.fromkeys(pierogi_fillings, None) | ||
dict.fromkeys(pierogi_fillings, 1) | ||
dict.fromkeys(pierogi_fillings) | ||
dict.fromkeys(pierogi_fillings, ("blessed", "tuples", "don't", "mutate")) | ||
dict.fromkeys(pierogi_fillings, "neither do strings") | ||
|
||
class MysteryBox: ... | ||
|
||
dict.fromkeys(pierogi_fillings, MysteryBox) | ||
bar.fromkeys(pierogi_fillings, []) | ||
|
||
|
||
def bad_dict() -> None: | ||
dict = MysteryBox() | ||
dict.fromkeys(pierogi_fillings, []) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
crates/ruff_linter/src/rules/ruff/rules/mutable_fromkeys_value.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; | ||
use ruff_macros::{derive_message_formats, violation}; | ||
use ruff_python_ast::{self as ast, Expr}; | ||
use ruff_python_semantic::analyze::typing::is_mutable_expr; | ||
|
||
use ruff_python_codegen::Generator; | ||
use ruff_text_size::Ranged; | ||
use ruff_text_size::TextRange; | ||
|
||
use crate::checkers::ast::Checker; | ||
|
||
/// ## What it does | ||
/// Checks for mutable objects passed as a value argument to `dict.fromkeys`. | ||
/// | ||
/// ## Why is this bad? | ||
/// All values in the dictionary created by the `dict.fromkeys` classmethod | ||
/// refer to the same instance of an object. If that object is modified, all | ||
/// values are modified, which can lead to unexpected behavior. | ||
/// | ||
/// Instead, use a comprehension to generate a dictionary with distinct values. | ||
/// | ||
/// ## Example | ||
/// ```python | ||
/// cities = dict.fromkeys(["UK", "Poland"], []) | ||
/// cities["UK"].append("London") | ||
/// cities["Poland"].append("Poznan") | ||
/// print(cities) # {'UK': ['London', 'Poznan'], 'Poland': ['London', 'Poznan']} | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```python | ||
/// cities = {country: [] for country in ["UK", "Poland"]} | ||
/// cities["UK"].append("London") | ||
/// cities["Poland"].append("Poznan") | ||
/// print(cities) # {'UK': ['London'], 'Poland': ['Poznan']} | ||
/// ``` | ||
/// | ||
/// ## References | ||
/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#dict.fromkeys) | ||
#[violation] | ||
pub struct MutableFromkeysValue; | ||
|
||
impl Violation for MutableFromkeysValue { | ||
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; | ||
|
||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
format!("Do not pass mutable objects as values to `dict.fromkeys`") | ||
} | ||
|
||
fn fix_title(&self) -> Option<String> { | ||
Some("Replace with a comprehension to generate distinct values".to_string()) | ||
} | ||
} | ||
|
||
/// RUF023 | ||
pub(crate) fn mutable_fromkeys_value(checker: &mut Checker, call: &ast::ExprCall) { | ||
let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = call.func.as_ref() else { | ||
return; | ||
}; | ||
|
||
// Check that the call is to `dict.fromkeys`. | ||
if attr != "fromkeys" { | ||
return; | ||
} | ||
let Some(name_expr) = value.as_name_expr() else { | ||
return; | ||
}; | ||
if name_expr.id != "dict" { | ||
return; | ||
} | ||
if !checker.semantic().is_builtin("dict") { | ||
return; | ||
} | ||
|
||
// Check that the value parameter is a mutable object. | ||
let [keys, value] = call.arguments.args.as_slice() else { | ||
return; | ||
}; | ||
if !is_mutable_expr(value, checker.semantic()) { | ||
return; | ||
} | ||
|
||
let mut diagnostic = Diagnostic::new(MutableFromkeysValue, call.range()); | ||
let replacement = Edit::range_replacement( | ||
generate_dict_comprehension(keys, value, checker.generator()), | ||
call.range(), | ||
); | ||
diagnostic.set_fix(Fix::unsafe_edit(replacement)); | ||
checker.diagnostics.push(diagnostic); | ||
} | ||
|
||
/// Format a code snippet to expression `{key: value for key in keys}`, where | ||
/// `keys` and `value` are the parameters of `dict.fromkeys`. | ||
fn generate_dict_comprehension(keys: &Expr, value: &Expr, generator: Generator) -> String { | ||
// Construct `key`. | ||
let key = ast::ExprName { | ||
id: "key".to_string(), | ||
ctx: ast::ExprContext::Load, | ||
range: TextRange::default(), | ||
}; | ||
// Construct `key in keys`. | ||
let comp = ast::Comprehension { | ||
target: key.clone().into(), | ||
iter: keys.clone(), | ||
ifs: vec![], | ||
range: TextRange::default(), | ||
is_async: false, | ||
}; | ||
// Construct the dict comprehension. | ||
let dict_comp = ast::ExprDictComp { | ||
key: Box::new(key.into()), | ||
value: Box::new(value.clone()), | ||
generators: vec![comp], | ||
range: TextRange::default(), | ||
}; | ||
generator.expr(&dict_comp.into()) | ||
} |
128 changes: 128 additions & 0 deletions
128
...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF023_RUF023.py.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
--- | ||
source: crates/ruff_linter/src/rules/ruff/mod.rs | ||
--- | ||
RUF023.py:9:1: RUF023 [*] Do not pass mutable objects as values to `dict.fromkeys` | ||
| | ||
8 | # Errors. | ||
9 | dict.fromkeys(pierogi_fillings, []) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023 | ||
10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 | dict.fromkeys(pierogi_fillings, {}) | ||
| | ||
= help: Replace with a comprehension to generate distinct values | ||
|
||
ℹ Unsafe fix | ||
6 6 | ] | ||
7 7 | | ||
8 8 | # Errors. | ||
9 |-dict.fromkeys(pierogi_fillings, []) | ||
9 |+{key: [] for key in pierogi_fillings} | ||
10 10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 12 | dict.fromkeys(pierogi_fillings, set()) | ||
|
||
RUF023.py:10:1: RUF023 [*] Do not pass mutable objects as values to `dict.fromkeys` | ||
| | ||
8 | # Errors. | ||
9 | dict.fromkeys(pierogi_fillings, []) | ||
10 | dict.fromkeys(pierogi_fillings, list()) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023 | ||
11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 | dict.fromkeys(pierogi_fillings, set()) | ||
| | ||
= help: Replace with a comprehension to generate distinct values | ||
|
||
ℹ Unsafe fix | ||
7 7 | | ||
8 8 | # Errors. | ||
9 9 | dict.fromkeys(pierogi_fillings, []) | ||
10 |-dict.fromkeys(pierogi_fillings, list()) | ||
10 |+{key: list() for key in pierogi_fillings} | ||
11 11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
|
||
RUF023.py:11:1: RUF023 [*] Do not pass mutable objects as values to `dict.fromkeys` | ||
| | ||
9 | dict.fromkeys(pierogi_fillings, []) | ||
10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 | dict.fromkeys(pierogi_fillings, {}) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023 | ||
12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
| | ||
= help: Replace with a comprehension to generate distinct values | ||
|
||
ℹ Unsafe fix | ||
8 8 | # Errors. | ||
9 9 | dict.fromkeys(pierogi_fillings, []) | ||
10 10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 |-dict.fromkeys(pierogi_fillings, {}) | ||
11 |+{key: {} for key in pierogi_fillings} | ||
12 12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
14 14 | dict.fromkeys(pierogi_fillings, dict()) | ||
|
||
RUF023.py:12:1: RUF023 [*] Do not pass mutable objects as values to `dict.fromkeys` | ||
| | ||
10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 | dict.fromkeys(pierogi_fillings, set()) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023 | ||
13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
14 | dict.fromkeys(pierogi_fillings, dict()) | ||
| | ||
= help: Replace with a comprehension to generate distinct values | ||
|
||
ℹ Unsafe fix | ||
9 9 | dict.fromkeys(pierogi_fillings, []) | ||
10 10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 |-dict.fromkeys(pierogi_fillings, set()) | ||
12 |+{key: set() for key in pierogi_fillings} | ||
13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
14 14 | dict.fromkeys(pierogi_fillings, dict()) | ||
15 15 | | ||
|
||
RUF023.py:13:1: RUF023 [*] Do not pass mutable objects as values to `dict.fromkeys` | ||
| | ||
11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023 | ||
14 | dict.fromkeys(pierogi_fillings, dict()) | ||
| | ||
= help: Replace with a comprehension to generate distinct values | ||
|
||
ℹ Unsafe fix | ||
10 10 | dict.fromkeys(pierogi_fillings, list()) | ||
11 11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 |-dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
13 |+{key: {"pre": "populated!"} for key in pierogi_fillings} | ||
14 14 | dict.fromkeys(pierogi_fillings, dict()) | ||
15 15 | | ||
16 16 | # Okay. | ||
|
||
RUF023.py:14:1: RUF023 [*] Do not pass mutable objects as values to `dict.fromkeys` | ||
| | ||
12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
14 | dict.fromkeys(pierogi_fillings, dict()) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF023 | ||
15 | | ||
16 | # Okay. | ||
| | ||
= help: Replace with a comprehension to generate distinct values | ||
|
||
ℹ Unsafe fix | ||
11 11 | dict.fromkeys(pierogi_fillings, {}) | ||
12 12 | dict.fromkeys(pierogi_fillings, set()) | ||
13 13 | dict.fromkeys(pierogi_fillings, {"pre": "populated!"}) | ||
14 |-dict.fromkeys(pierogi_fillings, dict()) | ||
14 |+{key: dict() for key in pierogi_fillings} | ||
15 15 | | ||
16 16 | # Okay. | ||
17 17 | dict.fromkeys(pierogi_fillings) | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.