mirror of https://github.com/astral-sh/ruff
[`pylint`] Fix suppression for empty dict without tuple key annotation (`PLE1141`) (#21290)
## Summary Fixes the PLE1141 (`dict-iter-missing-items`) rule to allow fixes for empty dictionaries unless they have type annotations indicating 2-tuple keys. Previously, the fix was incorrectly suppressed for all empty dicts due to vacuous truth in the `all()` function. Fixes #21289 ## Problem Analysis The `is_dict_key_tuple_with_two_elements` function was designed to suppress the fix when a dictionary's keys are all 2-tuples, as unpacking tuple keys directly would change runtime behavior. However, for empty dictionaries, `iter_keys()` returns an empty iterator, and `all()` on an empty iterator returns `true` (vacuous truth). This caused the function to incorrectly suppress fixes for empty dicts, even when there was no indication that future keys would be 2-tuples. ## Approach 1. **Detect empty dictionaries**: Added a check to identify when a dict literal has no keys. 2. **Handle annotated empty dicts**: For empty dicts with type annotations: - Parse the annotation to check if it's `dict[tuple[T1, T2], ...]` where the tuple has exactly 2 elements - Support both PEP 484 (`typing.Dict`, `typing.Tuple`) and PEP 585 (`dict`, `tuple`) syntax - If tuple keys are detected, suppress the fix (correct behavior) - Otherwise, allow the fix 3. **Handle unannotated empty dicts**: For empty dicts without annotations, allow the fix since there's no indication that keys will be 2-tuples. 4. **Preserve existing behavior**: For non-empty dicts, the original logic is unchanged - check if all existing keys are 2-tuples. The implementation includes helper functions: - `is_annotation_dict_with_tuple_keys()`: Checks if a type annotation specifies dict with tuple keys - `is_tuple_type_with_two_elements()`: Checks if a type expression represents a 2-tuple Test cases were added to verify: - Empty dict without annotation triggers the error - Empty dict with `dict[tuple[int, str], bool]` suppresses the error - Empty dict with `dict[str, int]` triggers the error - Existing tests remain unchanged --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
040aa7463b
commit
ddc1417f22
|
|
@ -30,3 +30,23 @@ for a, b in d_tuple:
|
||||||
pass
|
pass
|
||||||
for a, b in d_tuple_annotated:
|
for a, b in d_tuple_annotated:
|
||||||
pass
|
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] = {}
|
||||||
|
for k, v in empty_dict_annotated_tuple_keys:
|
||||||
|
pass
|
||||||
|
|
||||||
|
empty_dict_unannotated = {}
|
||||||
|
empty_dict_unannotated[("x", "y")] = True
|
||||||
|
for k, v in empty_dict_unannotated:
|
||||||
|
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
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
use ruff_python_ast::{Expr, Stmt};
|
use ruff_python_ast::{self as ast, Expr, Stmt};
|
||||||
|
|
||||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||||
use ruff_python_semantic::analyze::typing::is_dict;
|
use ruff_python_semantic::analyze::typing::is_dict;
|
||||||
|
|
@ -108,15 +108,77 @@ fn is_dict_key_tuple_with_two_elements(binding: &Binding, semantic: &SemanticMod
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
|
||||||
let Stmt::Assign(assign_stmt) = statement else {
|
let (value, annotation) = match statement {
|
||||||
|
Stmt::Assign(assign_stmt) => (assign_stmt.value.as_ref(), None),
|
||||||
|
Stmt::AnnAssign(ast::StmtAnnAssign {
|
||||||
|
value: Some(value),
|
||||||
|
annotation,
|
||||||
|
..
|
||||||
|
}) => (value.as_ref(), Some(annotation.as_ref())),
|
||||||
|
_ => return false,
|
||||||
|
};
|
||||||
|
|
||||||
|
let Expr::Dict(dict_expr) = value else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
|
||||||
let Expr::Dict(dict_expr) = &*assign_stmt.value else {
|
// Check if dict is empty
|
||||||
return false;
|
let is_empty = dict_expr.is_empty();
|
||||||
};
|
|
||||||
|
|
||||||
|
if is_empty {
|
||||||
|
// For empty dicts, check type annotation
|
||||||
|
return annotation
|
||||||
|
.is_some_and(|annotation| is_annotation_dict_with_tuple_keys(annotation, semantic));
|
||||||
|
}
|
||||||
|
|
||||||
|
// For non-empty dicts, check if all keys are 2-tuples
|
||||||
dict_expr
|
dict_expr
|
||||||
.iter_keys()
|
.iter_keys()
|
||||||
.all(|key| matches!(key, Some(Expr::Tuple(tuple)) if tuple.len() == 2))
|
.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 {
|
||||||
|
// Check if it's a subscript: dict[...]
|
||||||
|
let Expr::Subscript(subscript) = annotation else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Check if it's dict or typing.Dict
|
||||||
|
if !semantic.match_builtin_expr(subscript.value.as_ref(), "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 [key, _value] = tuple.elts.as_slice() {
|
||||||
|
return is_tuple_type_with_two_elements(key, 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 {
|
||||||
|
// Check if it's tuple or typing.Tuple
|
||||||
|
if semantic.match_builtin_expr(subscript.value.as_ref(), "tuple")
|
||||||
|
|| semantic.match_typing_expr(subscript.value.as_ref(), "Tuple")
|
||||||
|
{
|
||||||
|
// Check the slice - tuple[T1, T2]
|
||||||
|
if let Expr::Tuple(tuple_slice) = subscript.slice.as_ref() {
|
||||||
|
return tuple_slice.elts.len() == 2;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -39,3 +39,61 @@ help: Add a call to `.items()`
|
||||||
18 |
|
18 |
|
||||||
19 |
|
19 |
|
||||||
note: This is an unsafe fix and may change runtime behavior
|
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:46:13
|
||||||
|
|
|
||||||
|
44 | empty_dict_unannotated = {}
|
||||||
|
45 | empty_dict_unannotated[("x", "y")] = True
|
||||||
|
46 | for k, v in empty_dict_unannotated:
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
47 | pass
|
||||||
|
|
|
||||||
|
help: Add a call to `.items()`
|
||||||
|
43 |
|
||||||
|
44 | empty_dict_unannotated = {}
|
||||||
|
45 | empty_dict_unannotated[("x", "y")] = True
|
||||||
|
- for k, v in empty_dict_unannotated:
|
||||||
|
46 + for k, v in empty_dict_unannotated.items():
|
||||||
|
47 | pass
|
||||||
|
48 |
|
||||||
|
49 | empty_dict_annotated_str_keys: dict[str, int] = {}
|
||||||
|
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:51:13
|
||||||
|
|
|
||||||
|
49 | empty_dict_annotated_str_keys: dict[str, int] = {}
|
||||||
|
50 | empty_dict_annotated_str_keys["x"] = 1
|
||||||
|
51 | for k, v in empty_dict_annotated_str_keys:
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
52 | pass
|
||||||
|
|
|
||||||
|
help: Add a call to `.items()`
|
||||||
|
48 |
|
||||||
|
49 | empty_dict_annotated_str_keys: dict[str, int] = {}
|
||||||
|
50 | empty_dict_annotated_str_keys["x"] = 1
|
||||||
|
- for k, v in empty_dict_annotated_str_keys:
|
||||||
|
51 + for k, v in empty_dict_annotated_str_keys.items():
|
||||||
|
52 | pass
|
||||||
|
note: This is an unsafe fix and may change runtime behavior
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue