Skip to content

Commit

Permalink
Add PLE1141, DictIterMissingItems
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielNoord committed Feb 5, 2024
1 parent 0ccca40 commit 2e496dc
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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()));
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 2e496dc

Please sign in to comment.