Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,19 @@
pass
for a, b in d_tuple_annotated:
pass

# Empty dict cases
empty_dict = {}
empty_dict["x"] = 1
for k, v in empty_dict:
pass

empty_dict_annotated_tuple_keys: dict[tuple[int, str], bool] = {}
empty_dict_annotated_tuple_keys[("x", "y")] = True
for k, v in empty_dict_annotated_tuple_keys:
pass

empty_dict_annotated_str_keys: dict[str, int] = {}
empty_dict_annotated_str_keys["x"] = 1
for k, v in empty_dict_annotated_str_keys:
pass
102 changes: 98 additions & 4 deletions crates/ruff_linter/src/rules/pylint/rules/dict_iter_missing_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,109 @@ fn is_dict_key_tuple_with_two_elements(binding: &Binding, semantic: &SemanticMod
return false;
};

let Stmt::Assign(assign_stmt) = statement else {
return false;
let (dict_expr, annotation) = match statement {
Stmt::Assign(assign_stmt) => {
let Expr::Dict(dict_expr) = &*assign_stmt.value else {
return false;
};
(dict_expr, None)
}
Stmt::AnnAssign(ann_assign_stmt) => {
let Some(value) = ann_assign_stmt.value.as_ref() else {
return false;
};
let Expr::Dict(dict_expr) = value.as_ref() else {
return false;
};
(dict_expr, Some(ann_assign_stmt.annotation.as_ref()))
}
_ => return false,
};

let Expr::Dict(dict_expr) = &*assign_stmt.value else {
// Check if dict is empty
let is_empty = dict_expr.iter_keys().next().is_none();

if is_empty {
// For empty dicts, check type annotation
if let Some(annotation) = annotation {
// Check if annotation is dict[tuple[...], ...] where tuple has 2 elements
return is_annotation_dict_with_tuple_keys(annotation, semantic);
}
// Empty dict without annotation should allow the fix
return false;
};
}

// For non-empty dicts, check if all keys are 2-tuples
dict_expr
.iter_keys()
.all(|key| matches!(key, Some(Expr::Tuple(tuple)) if tuple.len() == 2))
}

/// Returns true if the annotation is `dict[tuple[T1, T2], ...]` where tuple has exactly 2 elements.
fn is_annotation_dict_with_tuple_keys(annotation: &Expr, semantic: &SemanticModel) -> bool {
// Handle stringized annotations
let annotation = if let Expr::StringLiteral(_) = annotation {
// For stringized annotations, we'd need to parse them, but for now,
// we'll be conservative and return false (allow the fix)
return false;
} else {
annotation
};

// Check if it's a subscript: dict[...]
let Expr::Subscript(subscript) = annotation else {
return false;
};

// Check if the value is `dict`
let value_name = match subscript.value.as_ref() {
Expr::Name(name) => name.id.as_str(),
_ => return false,
};

// Check if it's dict or typing.Dict
if value_name != "dict" && !semantic.match_typing_expr(subscript.value.as_ref(), "Dict") {
return false;
}

// Extract the slice (should be a tuple: (key_type, value_type))
let Expr::Tuple(tuple) = subscript.slice.as_ref() else {
return false;
};

// dict[K, V] format - check if K is tuple with 2 elements
if let Some(key_type) = tuple.elts.first() {
return is_tuple_type_with_two_elements(key_type, semantic);
}

false
}

/// Returns true if the expression represents a tuple type with exactly 2 elements.
fn is_tuple_type_with_two_elements(expr: &Expr, semantic: &SemanticModel) -> bool {
// Handle tuple[...] subscript
if let Expr::Subscript(subscript) = expr {
let value_name = match subscript.value.as_ref() {
Expr::Name(name) => name.id.as_str(),
_ => return false,
};

// Check if it's tuple or typing.Tuple
if value_name == "tuple" || semantic.match_typing_expr(subscript.value.as_ref(), "Tuple") {
// Check the slice - tuple[T1, T2] or tuple[T1, T2, ...]
if let Expr::Tuple(tuple_slice) = subscript.slice.as_ref() {
// For PEP 484: tuple[T1, T2, ...], the last element might be ...
// For PEP 585: tuple[T1, T2], just check length
let effective_len = tuple_slice
.elts
.iter()
.take_while(|elt| !matches!(elt, Expr::EllipsisLiteral(_)))
.count();
return effective_len == 2;
}
return false;
}
}

false
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,41 @@ help: Add a call to `.items()`
18 |
19 |
note: This is an unsafe fix and may change runtime behavior

PLE1141 [*] Unpacking a dictionary in iteration without calling `.items()`
--> dict_iter_missing_items.py:37:13
|
35 | empty_dict = {}
36 | empty_dict["x"] = 1
37 | for k, v in empty_dict:
| ^^^^^^^^^^
38 | pass
|
help: Add a call to `.items()`
34 | # Empty dict cases
35 | empty_dict = {}
36 | empty_dict["x"] = 1
- for k, v in empty_dict:
37 + for k, v in empty_dict.items():
38 | pass
39 |
40 | empty_dict_annotated_tuple_keys: dict[tuple[int, str], bool] = {}
note: This is an unsafe fix and may change runtime behavior

PLE1141 [*] Unpacking a dictionary in iteration without calling `.items()`
--> dict_iter_missing_items.py:47:13
|
45 | empty_dict_annotated_str_keys: dict[str, int] = {}
46 | empty_dict_annotated_str_keys["x"] = 1
47 | for k, v in empty_dict_annotated_str_keys:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
48 | pass
|
help: Add a call to `.items()`
44 |
45 | empty_dict_annotated_str_keys: dict[str, int] = {}
46 | empty_dict_annotated_str_keys["x"] = 1
- for k, v in empty_dict_annotated_str_keys:
47 + for k, v in empty_dict_annotated_str_keys.items():
48 | pass
note: This is an unsafe fix and may change runtime behavior