Skip to content

Commit 5c2d4d8

Browse files
liortctntBre
andauthored
[perflint] Handle tuples in dictionary comprehensions (PERF403) (#19934)
This pull request fixes the bug described in issue [#19153](#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 <[email protected]>
1 parent 26082e8 commit 5c2d4d8

File tree

4 files changed

+146
-12
lines changed

4 files changed

+146
-12
lines changed

crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,24 @@ def issue_19005_3():
192192
c = {}
193193
for a[0], a[1] in ():
194194
c[a[0]] = a[1]
195+
196+
197+
def issue_19153_1():
198+
v = {}
199+
for o, (x,) in ["ox"]:
200+
v[x,] = o
201+
return v
202+
203+
204+
def issue_19153_2():
205+
v = {}
206+
for (o, p), x in [("op", "x")]:
207+
v[x] = o, p
208+
return v
209+
210+
211+
def issue_19153_3():
212+
v = {}
213+
for o, (x,) in ["ox"]:
214+
v[(x,)] = o
215+
return v

crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -354,21 +354,37 @@ fn convert_to_dict_comprehension(
354354
"for"
355355
};
356356
// Handles the case where `key` has a trailing comma, e.g, `dict[x,] = y`
357-
let key_range = if let Expr::Tuple(ast::ExprTuple { elts, .. }) = key {
358-
let [expr] = elts.as_slice() else {
357+
let key_str = if let Expr::Tuple(ast::ExprTuple {
358+
elts,
359+
parenthesized,
360+
..
361+
}) = key
362+
{
363+
if elts.len() != 1 {
359364
return None;
360-
};
361-
expr.range()
365+
}
366+
if *parenthesized {
367+
locator.slice(key).to_string()
368+
} else {
369+
format!("({})", locator.slice(key))
370+
}
362371
} else {
363-
key.range()
372+
locator.slice(key).to_string()
373+
};
374+
375+
// If the value is a tuple without parentheses, add them
376+
let value_str = if let Expr::Tuple(ast::ExprTuple {
377+
parenthesized: false,
378+
..
379+
}) = value
380+
{
381+
format!("({})", locator.slice(value))
382+
} else {
383+
locator.slice(value).to_string()
364384
};
365-
let elt_str = format!(
366-
"{}: {}",
367-
locator.slice(key_range),
368-
locator.slice(value.range())
369-
);
370385

371-
let comprehension_str = format!("{{{elt_str} {for_type} {target_str} in {iter_str}{if_str}}}");
386+
let comprehension_str =
387+
format!("{{{key_str}: {value_str} {for_type} {target_str} in {iter_str}{if_str}}}");
372388

373389
let for_loop_inline_comments = comment_strings_in_range(
374390
checker,

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF403_PERF403.py.snap

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,36 @@ PERF403 Use a dictionary comprehension instead of a for-loop
176176
| ^^^^^^^
177177
|
178178
help: Replace for loop with dict comprehension
179+
180+
PERF403 Use a dictionary comprehension instead of a for-loop
181+
--> PERF403.py:200:9
182+
|
183+
198 | v = {}
184+
199 | for o, (x,) in ["ox"]:
185+
200 | v[x,] = o
186+
| ^^^^^^^^^
187+
201 | return v
188+
|
189+
help: Replace for loop with dict comprehension
190+
191+
PERF403 Use a dictionary comprehension instead of a for-loop
192+
--> PERF403.py:207:9
193+
|
194+
205 | v = {}
195+
206 | for (o, p), x in [("op", "x")]:
196+
207 | v[x] = o, p
197+
| ^^^^^^^^^^^
198+
208 | return v
199+
|
200+
help: Replace for loop with dict comprehension
201+
202+
PERF403 Use a dictionary comprehension instead of a for-loop
203+
--> PERF403.py:214:9
204+
|
205+
212 | v = {}
206+
213 | for o, (x,) in ["ox"]:
207+
214 | v[(x,)] = o
208+
| ^^^^^^^^^^^
209+
215 | return v
210+
|
211+
help: Replace for loop with dict comprehension

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF403_PERF403.py.snap

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,72 @@ help: Replace for loop with dict comprehension
372372
- v = {}
373373
- for o,(x,)in():
374374
- v[x,]=o
375-
170 + v = {x: o for o,(x,) in ()}
375+
170 + v = {(x,): o for o,(x,) in ()}
376376
171 |
377377
172 |
378378
173 | # https://github.com/astral-sh/ruff/issues/19005
379379
note: This is an unsafe fix and may change runtime behavior
380+
381+
PERF403 [*] Use a dictionary comprehension instead of a for-loop
382+
--> PERF403.py:200:9
383+
|
384+
198 | v = {}
385+
199 | for o, (x,) in ["ox"]:
386+
200 | v[x,] = o
387+
| ^^^^^^^^^
388+
201 | return v
389+
|
390+
help: Replace for loop with dict comprehension
391+
195 |
392+
196 |
393+
197 | def issue_19153_1():
394+
- v = {}
395+
- for o, (x,) in ["ox"]:
396+
- v[x,] = o
397+
198 + v = {(x,): o for o, (x,) in ["ox"]}
398+
199 | return v
399+
200 |
400+
201 |
401+
note: This is an unsafe fix and may change runtime behavior
402+
403+
PERF403 [*] Use a dictionary comprehension instead of a for-loop
404+
--> PERF403.py:207:9
405+
|
406+
205 | v = {}
407+
206 | for (o, p), x in [("op", "x")]:
408+
207 | v[x] = o, p
409+
| ^^^^^^^^^^^
410+
208 | return v
411+
|
412+
help: Replace for loop with dict comprehension
413+
202 |
414+
203 |
415+
204 | def issue_19153_2():
416+
- v = {}
417+
- for (o, p), x in [("op", "x")]:
418+
- v[x] = o, p
419+
205 + v = {x: (o, p) for (o, p), x in [("op", "x")]}
420+
206 | return v
421+
207 |
422+
208 |
423+
note: This is an unsafe fix and may change runtime behavior
424+
425+
PERF403 [*] Use a dictionary comprehension instead of a for-loop
426+
--> PERF403.py:214:9
427+
|
428+
212 | v = {}
429+
213 | for o, (x,) in ["ox"]:
430+
214 | v[(x,)] = o
431+
| ^^^^^^^^^^^
432+
215 | return v
433+
|
434+
help: Replace for loop with dict comprehension
435+
209 |
436+
210 |
437+
211 | def issue_19153_3():
438+
- v = {}
439+
- for o, (x,) in ["ox"]:
440+
- v[(x,)] = o
441+
212 + v = {(x,): o for o, (x,) in ["ox"]}
442+
213 | return v
443+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)