Skip to content

[perflint] Clarify that PERF402 applies to any iterable - #26242

Merged
ntBre merged 2 commits into
astral-sh:mainfrom
trilamsr:perf402-doc-wording-21593
Jun 24, 2026
Merged

[perflint] Clarify that PERF402 applies to any iterable#26242
ntBre merged 2 commits into
astral-sh:mainfrom
trilamsr:perf402-doc-wording-21593

Conversation

@trilamsr

@trilamsr trilamsr commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

First contribution here, hi! Picking up #21593 as a small intro PR.

PERF402's current "What it does" says it catches copies of an "existing list", but the rule also fires whenever a for loop appends every item of any iterable to a list while the source isn't required to be a list (only the destination is). The issue reporter pointed this out and suggested wording closer to "for loops that can be replaced with list()", which is what I went with here.

Scope is intentionally tiny: just the ## What it does and ## Why is this bad? doc comments. Violation message, example, rule logic, and tests are unchanged so no snapshots move.

Closes #21593.

…ral-sh#21593)

The previous wording described PERF402 as catching copies of an existing
list, but the rule fires whenever a for-loop appends each item of any
iterable to a list. Reword "What it does" and "Why is this bad?" to match
the actual behavior. The violation message and rule logic are unchanged.

Closes astral-sh#21593.

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! Just a couple of small suggestions.

Comment thread crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs Outdated
Comment thread crates/ruff_linter/src/rules/perflint/rules/manual_list_copy.rs Outdated
@ntBre ntBre added the documentation Improvements or additions to documentation label Jun 22, 2026
@ntBre ntBre changed the title [ruff] Reword PERF402 docs to match what the rule actually flags [perflint] Clarify that PERF402 applies to any iterable Jun 22, 2026
@astral-sh-bot

astral-sh-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@trilamsr

trilamsr commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Good calls. I dropped the parens everywhere and re-added the list.copy mention so the docs line up with the violation message. Thanks for the review!

@trilamsr

Copy link
Copy Markdown
Contributor Author

@ntBre pinging here for update. Forgot to @ you at the previous comment!

@ntBre

ntBre commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Thanks, but you don't need to ping us. Any comment or commit puts this back in my review queue. I just haven't had a chance to take another look yet.

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@ntBre
ntBre enabled auto-merge (squash) June 24, 2026 16:06
@ntBre
ntBre merged commit aa96a05 into astral-sh:main Jun 24, 2026
43 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

manual-list-copy rule description says it works on existing lists, but it triggers on any iterable

2 participants