Skip to content

[pyupgrade] Mark fix unsafe when deleting comments (UP035) - #29413

Merged
ntBre merged 1 commit into
astral-sh:mainfrom
fixfon:t3/up035-preserve-import-comments
Oct 15, 2026
Merged

ntBre merged 1 commit into
astral-sh:mainfrom
fixfon:t3/up035-preserve-import-comments

Conversation

@fixfon

@fixfon fixfon commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #29400.

UP035 rebuilds the whole from typing import (...) statement from the names, so any comment inside it, like # noqa after the paren or a trailing comment on a member, just disappears. The fix was still marked safe.

I went with making the fix unsafe whenever a comment falls inside the import, using the same comment_ranges().intersects check as timeout_error_alias. Actually keeping the comments would need tracking them per member, which felt like too much for this. Imports without comments, or with only a comment after the statement, stay safe. I also added a "Fix safety" section to the rule docs.

Added four cases to UP035.py, covering both repros from the issue, a partial move with an own-line comment, and a trailing comment after the statement.

Heads up: #28394 touches the same files for a different UP035 case, so one of these will probably need a rebase.

I used an AI assistant while working on this and reviewed the changes myself.

…ins comments (`UP035`)

The fix regenerates the `from ... import ...` statement from the imported
names, so comments inside a parenthesized import were deleted even though
the fix was marked safe. Mark the fix unsafe when the import statement's
range contains a comment, and document this under "Fix safety".

Fixes astral-sh#29400
@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre October 15, 2026 06:58
@astral-sh-bot

astral-sh-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +0 -8 fixes in 1 projects; 57 projects unchanged)

python/typeshed (+0 -0 violations, +0 -8 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --no-preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/_collections_abc.pyi:4:1: UP035 Import from `collections.abc` instead: `AsyncGenerator`, `AsyncIterable`, `AsyncIterator`, `Awaitable`, `Callable`, `Collection`, `Container`, `Coroutine`, `Generator`, `Hashable`, `ItemsView`, `Iterable`, `Iterator`, `KeysView`, `Mapping`, `MappingView`, `MutableMapping`, `MutableSequence`, `MutableSet`, `Reversible`, `Sequence`, `Sized`, `ValuesView`
- stdlib/_collections_abc.pyi:4:1: UP035 [*] Import from `collections.abc` instead: `AsyncGenerator`, `AsyncIterable`, `AsyncIterator`, `Awaitable`, `Callable`, `Collection`, `Container`, `Coroutine`, `Generator`, `Hashable`, `ItemsView`, `Iterable`, `Iterator`, `KeysView`, `Mapping`, `MappingView`, `MutableMapping`, `MutableSequence`, `MutableSet`, `Reversible`, `Sequence`, `Sized`, `ValuesView`
+ stdlib/builtins.pyi:40:1: UP035 Import from `collections.abc` instead: `Mapping`, `MutableMapping`, `MutableSequence`, `Sequence`
- stdlib/builtins.pyi:40:1: UP035 [*] Import from `collections.abc` instead: `Mapping`, `MutableMapping`, `MutableSequence`, `Sequence`
+ stdlib/typing_extensions.pyi:33:1: UP035 Import from `collections.abc` instead: `Callable`
+ stdlib/typing_extensions.pyi:33:1: UP035 Import from `collections` instead: `ChainMap`, `Counter`
- stdlib/typing_extensions.pyi:33:1: UP035 [*] Import from `collections.abc` instead: `Callable`
- stdlib/typing_extensions.pyi:33:1: UP035 [*] Import from `collections` instead: `ChainMap`, `Counter`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
UP035 8 0 0 0 8

Linter (preview)

✅ ecosystem check detected no linter changes.

@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.

Thank you!

@ntBre ntBre added the fixes Related to suggested fixes for violations label Oct 15, 2026
@ntBre ntBre changed the title [pyupgrade] Mark UP035 fix as unsafe when the import contains comments [pyupgrade] Mark fix unsafe when deleting comments (UP035) Oct 15, 2026
@ntBre
ntBre merged commit 8f19c4f into astral-sh:main Oct 15, 2026
52 checks passed
ntBre pushed a commit that referenced this pull request Oct 16, 2026
…120`) (#29414)

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
the `else:` line has a comment that would be deleted, the same approach
as UP035 (#29413). Without such a comment the fix stays safe. Inline
bodies like `else: x = 1 # c` also stay safe, since only `else: ` 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UP035 safe fix deletes comments inside a multi-line from typing import (...)

2 participants