diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/dict_iter_missing_items.py b/crates/ruff_linter/resources/test/fixtures/pylint/dict_iter_missing_items.py new file mode 100644 index 0000000000000..18a5037c8365b --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/dict_iter_missing_items.py @@ -0,0 +1,23 @@ +d = {1: 1, 2: 2} +d_tuple = {(1, 2): 3, (4, 5): 6} +l = [1, 2] +s1 = {1, 2} +s2 = {1, 2, 3} + +# Errors +for k, v in d: + pass + +# False positive, since the keys are all tuples this is valid +for a, b in d_tuple: + pass + +# Non errors +for k, v in d.items(): + pass +for k in d.keys(): + pass +for i, v in enumerate(l): + pass +for i, v in s1.intersection(s2): + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 3ceac945740fc..d10d463fe67ba 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1294,6 +1294,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::IterationOverSet) { pylint::rules::iteration_over_set(checker, iter); } + if checker.enabled(Rule::DictIterMissingItems) { + pylint::rules::dict_iter_missing_items(checker, target, iter); + } if checker.enabled(Rule::ManualListComprehension) { perflint::rules::manual_list_comprehension(checker, target, body); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ac97ac50ba1f2..8e62b9abdd326 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -238,6 +238,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "E0643") => (RuleGroup::Preview, rules::pylint::rules::PotentialIndexError), (Pylint, "E0704") => (RuleGroup::Preview, rules::pylint::rules::MisplacedBareRaise), (Pylint, "E1132") => (RuleGroup::Preview, rules::pylint::rules::RepeatedKeywordArgument), + (Pylint, "E1141") => (RuleGroup::Preview, rules::pylint::rules::DictIterMissingItems), (Pylint, "E1142") => (RuleGroup::Stable, rules::pylint::rules::AwaitOutsideAsync), (Pylint, "E1205") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooManyArgs), (Pylint, "E1206") => (RuleGroup::Stable, rules::pylint::rules::LoggingTooFewArgs), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 518dd781459d5..e9599b963aa70 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -171,6 +171,7 @@ mod tests { #[test_case(Rule::PotentialIndexError, Path::new("potential_index_error.py"))] #[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))] #[test_case(Rule::TooManyNestedBlocks, Path::new("too_many_nested_blocks.py"))] + #[test_case(Rule::DictIterMissingItems, Path::new("dict_iter_missing_items.py"))] #[test_case( Rule::UnnecessaryDictIndexLookup, Path::new("unnecessary_dict_index_lookup.py") diff --git a/crates/ruff_linter/src/rules/pylint/rules/dict_iter_missing_items.rs b/crates/ruff_linter/src/rules/pylint/rules/dict_iter_missing_items.rs new file mode 100644 index 0000000000000..35e3fada8fe58 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/dict_iter_missing_items.rs @@ -0,0 +1,67 @@ +use ruff_python_ast::{Expr, ExprTuple}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_semantic::analyze::typing::is_dict; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for unpacking a dictionary in a for loop without calling `.items()`. +/// +/// ## Why is this bad? +/// You are likely 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 Violation for DictIterMissingItems { + #[derive_message_formats] + fn message(&self) -> String { + format!("Call `items()` when unpacking a dictionary for iteration") + } +} + +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; + } + + checker + .diagnostics + .push(Diagnostic::new(DictIterMissingItems, iter.range())); +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 3ba5d1061b7d2..9963339844a84 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -13,6 +13,7 @@ pub(crate) use compare_to_empty_string::*; pub(crate) use comparison_of_constant::*; pub(crate) use comparison_with_itself::*; pub(crate) use continue_in_finally::*; +pub(crate) use dict_iter_missing_items::*; pub(crate) use duplicate_bases::*; pub(crate) use empty_comment::*; pub(crate) use eq_without_hash::*; @@ -98,6 +99,7 @@ mod compare_to_empty_string; mod comparison_of_constant; mod comparison_with_itself; mod continue_in_finally; +mod dict_iter_missing_items; mod duplicate_bases; mod empty_comment; mod eq_without_hash; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1141_dict_iter_missing_items.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1141_dict_iter_missing_items.py.snap new file mode 100644 index 0000000000000..c2990755322eb --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1141_dict_iter_missing_items.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +dict_iter_missing_items.py:8:13: PLE1141 Call `items()` when unpacking a dictionary for iteration + | +7 | # Errors +8 | for k, v in d: + | ^ PLE1141 +9 | pass + | + +dict_iter_missing_items.py:12:13: PLE1141 Call `items()` when unpacking a dictionary for iteration + | +11 | # False positive, since the keys are all tuples this is valid +12 | for a, b in d_tuple: + | ^^^^^^^ PLE1141 +13 | pass + | + + diff --git a/ruff.schema.json b/ruff.schema.json index c5c9a126a985b..62ffcdfce1a32 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3206,6 +3206,7 @@ "PLE113", "PLE1132", "PLE114", + "PLE1141", "PLE1142", "PLE12", "PLE120",