Repository navigation
[pylint] Mark fixes unsafe when deleting comments (PLR5501, PLW0120) - #29414
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PLW0120 | 2 | 0 | 0 | 0 | 2 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+1 -1 violations, +0 -0 fixes in 1 projects; 57 projects unchanged)
prefecthq/prefect (+1 -1 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --no-fix --output-format concise --preview
- src/prefect/utilities/pydantic.py:274:5: useless-else-on-loop [*] `else` clause on loop without a `break` statement; remove the `else` and dedent its contents + src/prefect/utilities/pydantic.py:274:5: useless-else-on-loop `else` clause on loop without a `break` statement; remove the `else` and dedent its contents
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| useless-else-on-loop | 2 | 1 | 1 | 0 | 0 |
f754448 to
98d4494
Compare
|
Thanks for looking into this! The changes aren't that involved, but I'm kind of leaning toward just marking the fixes unsafe if they would drop comments, instead of trying to retain them. The bit about breaking |
…LR5501`, `PLW0120`) Both fixes replace the `else:` line, so a trailing comment on it (e.g., `else: # fallback`) was deleted while the fix was still marked safe. Rather than moving the comment, mark the fix unsafe when the `else` line has a comment, using the same `comment_ranges().intersects` check as other rules, and document it in a "Fix safety" section. Fixes without a comment on the `else` line, including an inline `else:` body whose trailing comment belongs to the statement, stay safe.
98d4494 to
aca8801
Compare
fixfon
changed the title
else: line comments in PLR5501 and PLW0120 fixespylint] Mark PLR5501 and PLW0120 fixes unsafe when they delete an else: line comment
|
Thanks, that makes sense. I removed the comment-moving logic. Both PLR5501 and PLW0120 now mark the fix as unsafe when it would delete a comment on the |
ntBre
added
bug
ntBre
changed the title
pylint] Mark PLR5501 and PLW0120 fixes unsafe when they delete an else: line commentpylint] Mark fixes unsafe when deleting comments (PLR5501, PLW0120)
2 participants
Fixes #29395.
Both fixes rewrote the
else:line and silently dropped any comment after the colon. Following review, the fix no longer tries to move that comment. Instead, PLR5501 and PLW0120 now mark the fix as unsafe when theelse:line has a comment that would be deleted, the same approach as UP035 (#29413). Without such a comment the fix stays safe. Inline bodies likeelse: x = 1 # calso stay safe, since onlyelse:is removed and the comment belongs to the statement.Both rules got a "Fix safety" section in their docs. The existing PLW0120 snapshots only gain the unsafe-fix note, because their fixtures use
else: # [useless-else-on-loop]. New fixture cases cover the unsafe and safe paths for both rules.I used an AI assistant while working on this and reviewed the changes myself.