Add PLE1141, DictIterMissingItems

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

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);
             }

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),

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")

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()));
+}

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;

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
+   |
+
+