-
Notifications
You must be signed in to change notification settings - Fork 903
/
dict_iter_missing_items.rs
110 lines (95 loc) · 3.18 KB
/
dict_iter_missing_items.rs
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
use ruff_python_ast::{Expr, ExprTuple};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::analyze::typing::is_dict;
use ruff_python_semantic::{Binding, SemanticModel};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for dictionary unpacking in a for loop without calling `.items()`.
///
/// ## Why is this bad?
/// When iterating over a dictionary in a for loop, if a dictionary is unpacked
/// without calling `.items()`, it could lead to a runtime error if the keys are not
/// a tuple of two elements.
///
/// It is likely that you're looking for an iteration over (key, value) pairs which
/// can only be achieved when calling `.items()`.
///
/// ## Example
/// ```python
/// data = {"Paris": 2_165_423, "New York City": 8_804_190, "Tokyo": 13_988_129}
///
/// for city, population in data:
/// print(f"{city} has population {population}.")
/// ```
///
/// Use instead:
/// ```python
/// data = {"Paris": 2_165_423, "New York City": 8_804_190, "Tokyo": 13_988_129}
///
/// for city, population in data.items():
/// print(f"{city} has population {population}.")
/// ```
#[violation]
pub struct DictIterMissingItems;
impl AlwaysFixableViolation for DictIterMissingItems {
#[derive_message_formats]
fn message(&self) -> String {
format!("Unpacking a dictionary in iteration without calling `.items()`")
}
fn fix_title(&self) -> String {
format!("Add a call to `.items()`")
}
}
pub(crate) fn dict_iter_missing_items(checker: &mut Checker, target: &Expr, iter: &Expr) {
let Expr::Tuple(ExprTuple { elts, .. }) = target else {
return;
};
if elts.len() != 2 {
return;
};
let Some(name) = iter.as_name_expr() else {
return;
};
let Some(binding) = checker
.semantic()
.only_binding(name)
.map(|id| checker.semantic().binding(id))
else {
return;
};
if !is_dict(binding, checker.semantic()) {
return;
}
// If we can reliably determine that a dictionary has keys that are tuples of two we don't warn
if is_dict_key_tuple_with_two_elements(checker.semantic(), binding) {
return;
}
let mut diagnostic = Diagnostic::new(DictIterMissingItems, iter.range());
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
format!("{}.items()", name.id),
iter.range(),
)));
checker.diagnostics.push(diagnostic);
}
/// Returns true if the binding is a dictionary where each key is a tuple with two elements.
fn is_dict_key_tuple_with_two_elements(semantic: &SemanticModel, binding: &Binding) -> bool {
let Some(statement) = binding.statement(semantic) else {
return false;
};
let Some(assign_stmt) = statement.as_assign_stmt() else {
return false;
};
let Some(dict_expr) = assign_stmt.value.as_dict_expr() else {
return false;
};
dict_expr.keys.iter().all(|elt| {
elt.as_ref().is_some_and(|x| {
if let Some(tuple) = x.as_tuple_expr() {
return tuple.elts.len() == 2;
}
false
})
})
}