mirror of https://github.com/astral-sh/ruff
[`perflint`] Handle tuples in dictionary comprehensions (`PERF403`) (#19934)
This pull request fixes the bug described in issue [#19153](https://github.com/astral-sh/ruff/issues/19153). The issue occurred when `PERF403` incorrectly flagged cases involving tuple unpacking in a for loop. For example: ```python def f(): v = {} for (o, p), x in [("op", "x")]: v[x] = o, p ``` This code was wrongly suggested to be rewritten into a dictionary comprehension, which changes the semantics. Changes in this PR: Updated the `PERF403` rule to correctly handle tuple unpacking in loop targets. Added regression tests to ensure this case (and similar ones) are no longer flagged incorrectly. Why: This ensures that `PERF403` only triggers when a dictionary comprehension is semantically equivalent to the original loop, preventing false positives. --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
26082e8ec1
commit
5c2d4d8d8f
|
|
@ -192,3 +192,24 @@ def issue_19005_3():
|
|||
c = {}
|
||||
for a[0], a[1] in ():
|
||||
c[a[0]] = a[1]
|
||||
|
||||
|
||||
def issue_19153_1():
|
||||
v = {}
|
||||
for o, (x,) in ["ox"]:
|
||||
v[x,] = o
|
||||
return v
|
||||
|
||||
|
||||
def issue_19153_2():
|
||||
v = {}
|
||||
for (o, p), x in [("op", "x")]:
|
||||
v[x] = o, p
|
||||
return v
|
||||
|
||||
|
||||
def issue_19153_3():
|
||||
v = {}
|
||||
for o, (x,) in ["ox"]:
|
||||
v[(x,)] = o
|
||||
return v
|
||||
|
|
@ -354,21 +354,37 @@ fn convert_to_dict_comprehension(
|
|||
"for"
|
||||
};
|
||||
// Handles the case where `key` has a trailing comma, e.g, `dict[x,] = y`
|
||||
let key_range = if let Expr::Tuple(ast::ExprTuple { elts, .. }) = key {
|
||||
let [expr] = elts.as_slice() else {
|
||||
let key_str = if let Expr::Tuple(ast::ExprTuple {
|
||||
elts,
|
||||
parenthesized,
|
||||
..
|
||||
}) = key
|
||||
{
|
||||
if elts.len() != 1 {
|
||||
return None;
|
||||
};
|
||||
expr.range()
|
||||
}
|
||||
if *parenthesized {
|
||||
locator.slice(key).to_string()
|
||||
} else {
|
||||
format!("({})", locator.slice(key))
|
||||
}
|
||||
} else {
|
||||
key.range()
|
||||
locator.slice(key).to_string()
|
||||
};
|
||||
let elt_str = format!(
|
||||
"{}: {}",
|
||||
locator.slice(key_range),
|
||||
locator.slice(value.range())
|
||||
);
|
||||
|
||||
let comprehension_str = format!("{{{elt_str} {for_type} {target_str} in {iter_str}{if_str}}}");
|
||||
// If the value is a tuple without parentheses, add them
|
||||
let value_str = if let Expr::Tuple(ast::ExprTuple {
|
||||
parenthesized: false,
|
||||
..
|
||||
}) = value
|
||||
{
|
||||
format!("({})", locator.slice(value))
|
||||
} else {
|
||||
locator.slice(value).to_string()
|
||||
};
|
||||
|
||||
let comprehension_str =
|
||||
format!("{{{key_str}: {value_str} {for_type} {target_str} in {iter_str}{if_str}}}");
|
||||
|
||||
let for_loop_inline_comments = comment_strings_in_range(
|
||||
checker,
|
||||
|
|
|
|||
|
|
@ -176,3 +176,36 @@ PERF403 Use a dictionary comprehension instead of a for-loop
|
|||
| ^^^^^^^
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
--> PERF403.py:200:9
|
||||
|
|
||||
198 | v = {}
|
||||
199 | for o, (x,) in ["ox"]:
|
||||
200 | v[x,] = o
|
||||
| ^^^^^^^^^
|
||||
201 | return v
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
--> PERF403.py:207:9
|
||||
|
|
||||
205 | v = {}
|
||||
206 | for (o, p), x in [("op", "x")]:
|
||||
207 | v[x] = o, p
|
||||
| ^^^^^^^^^^^
|
||||
208 | return v
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
|
||||
PERF403 Use a dictionary comprehension instead of a for-loop
|
||||
--> PERF403.py:214:9
|
||||
|
|
||||
212 | v = {}
|
||||
213 | for o, (x,) in ["ox"]:
|
||||
214 | v[(x,)] = o
|
||||
| ^^^^^^^^^^^
|
||||
215 | return v
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
|
|
|
|||
|
|
@ -372,8 +372,72 @@ help: Replace for loop with dict comprehension
|
|||
- v = {}
|
||||
- for o,(x,)in():
|
||||
- v[x,]=o
|
||||
170 + v = {x: o for o,(x,) in ()}
|
||||
170 + v = {(x,): o for o,(x,) in ()}
|
||||
171 |
|
||||
172 |
|
||||
173 | # https://github.com/astral-sh/ruff/issues/19005
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
--> PERF403.py:200:9
|
||||
|
|
||||
198 | v = {}
|
||||
199 | for o, (x,) in ["ox"]:
|
||||
200 | v[x,] = o
|
||||
| ^^^^^^^^^
|
||||
201 | return v
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
195 |
|
||||
196 |
|
||||
197 | def issue_19153_1():
|
||||
- v = {}
|
||||
- for o, (x,) in ["ox"]:
|
||||
- v[x,] = o
|
||||
198 + v = {(x,): o for o, (x,) in ["ox"]}
|
||||
199 | return v
|
||||
200 |
|
||||
201 |
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
--> PERF403.py:207:9
|
||||
|
|
||||
205 | v = {}
|
||||
206 | for (o, p), x in [("op", "x")]:
|
||||
207 | v[x] = o, p
|
||||
| ^^^^^^^^^^^
|
||||
208 | return v
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
202 |
|
||||
203 |
|
||||
204 | def issue_19153_2():
|
||||
- v = {}
|
||||
- for (o, p), x in [("op", "x")]:
|
||||
- v[x] = o, p
|
||||
205 + v = {x: (o, p) for (o, p), x in [("op", "x")]}
|
||||
206 | return v
|
||||
207 |
|
||||
208 |
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
PERF403 [*] Use a dictionary comprehension instead of a for-loop
|
||||
--> PERF403.py:214:9
|
||||
|
|
||||
212 | v = {}
|
||||
213 | for o, (x,) in ["ox"]:
|
||||
214 | v[(x,)] = o
|
||||
| ^^^^^^^^^^^
|
||||
215 | return v
|
||||
|
|
||||
help: Replace for loop with dict comprehension
|
||||
209 |
|
||||
210 |
|
||||
211 | def issue_19153_3():
|
||||
- v = {}
|
||||
- for o, (x,) in ["ox"]:
|
||||
- v[(x,)] = o
|
||||
212 + v = {(x,): o for o, (x,) in ["ox"]}
|
||||
213 | return v
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
|
|
|||
Loading…
Reference in New Issue