Skip to content

[perflint] Handle tuples in dictionary comprehensions (PERF403)#19934

Merged
ntBre merged 4 commits into
astral-sh:mainfrom
liortct:bugfix/perf403-tuple-error
Aug 28, 2025
Merged

[perflint] Handle tuples in dictionary comprehensions (PERF403)#19934
ntBre merged 4 commits into
astral-sh:mainfrom
liortct:bugfix/perf403-tuple-error

Conversation

@liortct
Copy link
Copy Markdown
Contributor

@liortct liortct commented Aug 16, 2025

This pull request fixes the bug described in issue #19153.

The issue occurred when PERF403 incorrectly flagged cases involving tuple unpacking in a for loop. For example:

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.

@liortct liortct force-pushed the bugfix/perf403-tuple-error branch from 4ef1ddb to dce411b Compare August 16, 2025 09:15
@liortct
Copy link
Copy Markdown
Contributor Author

liortct commented Aug 18, 2025

@ntBre
Hello 👋 just a gentle reminder about this PR. I’d be happy to make any changes if needed. Thanks for reviewing when you get the chance!

@ntBre ntBre self-requested a review August 18, 2025 22:28
@ntBre ntBre added bug Something isn't working fixes Related to suggested fixes for violations labels Aug 18, 2025
@ntBre
Copy link
Copy Markdown
Contributor

ntBre commented Aug 18, 2025

Thank you! I'll try to get to it this week :)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 18, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Copy Markdown
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this looks great! I just had a few small suggestions.

Comment thread crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs Outdated
Comment thread crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs Outdated
Comment thread crates/ruff_linter/resources/test/fixtures/perflint/PERF403.py
Comment thread crates/ruff_linter/src/rules/perflint/rules/manual_dict_comprehension.rs Outdated
@liortct liortct force-pushed the bugfix/perf403-tuple-error branch from da2587d to 63f7ca4 Compare August 23, 2025 10:37
@liortct
Copy link
Copy Markdown
Contributor Author

liortct commented Aug 23, 2025

@ntBre
Thanks a lot for the review and comments!
I’ve addressed all the issues, and the PR is ready for another review.
If there’s anything else that needs to be adjusted, please let me know and I’ll update it promptly.

Thanks again!

Copy link
Copy Markdown
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ntBre ntBre changed the title [perf403] bug handle tuple in dictionary compression [perflint] Handle tuples in dictionary comprehensions (PERF403) Aug 28, 2025
@ntBre ntBre enabled auto-merge (squash) August 28, 2025 21:22
@ntBre ntBre merged commit 5c2d4d8 into astral-sh:main Aug 28, 2025
34 checks passed
second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025
…stral-sh#19934)

This pull request fixes the bug described in issue
[astral-sh#19153](astral-sh#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants