Repository navigation
[pyupgrade] Mark fix unsafe when deleting comments (UP035) - #29413
Merged
Merged
Conversation
…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
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| UP035 | 8 | 0 | 0 | 0 | 8 |
Linter (preview)
✅ ecosystem check detected no linter changes.
ntBre
changed the title
pyupgrade] Mark fix unsafe when deleting comments (UP035)
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.
rlichtenwalter
added a commit
to rlichtenwalter/dir2text
that referenced
this pull request
Oct 30, 2026
This PR contains the following updates: | Package | Type | Update | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---|---|---| | [DavidAnson/markdownlint-cli2](https://github.com/DavidAnson/markdownlint-cli2) | repository | minor | `v0.23.3` → `v0.24.0` |  |  | | [ruff](https://docs.astral.sh/ruff) ([source](https://github.com/astral-sh/ruff), [changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md)) | dependency-groups | patch | `==0.17.1` → `==0.17.3` |  |  | Note: The `pre-commit` manager in Renovate is not supported by the `pre-commit` maintainers or community. Please do not report any problems there, instead [create a Discussion in the Renovate repository](https://github.com/renovatebot/renovate/discussions/new) if you have any questions. --- ### Release Notes <details> <summary>DavidAnson/markdownlint-cli2</summary> ### [`v0.24.0`](DavidAnson/markdownlint-cli2@v0.23.3...v0.24.0) [Compare Source](DavidAnson/markdownlint-cli2@v0.23.3...v0.24.0) </details> <details> <summary>astral-sh/ruff (ruff)</summary> ### [`v0.17.3`](https://github.com/astral-sh/ruff/blob/HEAD/CHANGELOG.md#0173) [Compare Source](astral-sh/ruff@0.17.2...0.17.3) Released on 2026-10-26. ##### Preview features - \[`suspicious`] Add `shared-clock-mock` ([#​29074](astral-sh/ruff#29074)) ##### Bug fixes - \[`pyupgrade`] Avoid incorrect conversions to f-strings (`UP032`) ([#​25485](astral-sh/ruff#25485)) ##### Performance - Buffer stdout in `ruff format` ([#​29631](astral-sh/ruff#29631)) - Specialize ASCII identifier lexing ([#​29503](astral-sh/ruff#29503)) - Speed up identifier and keyword lexing ([#​29463](astral-sh/ruff#29463)) ##### Documentation - \[`flake8-bandit`] Describe what the rule actually checks for `httpx` (`S113`) ([#​27936](astral-sh/ruff#27936)) ##### Contributors - [@​itsvedantkumar](https://github.com/itsvedantkumar) - [@​charliermarsh](https://github.com/charliermarsh) - [@​anishgirianish](https://github.com/anishgirianish) ### [`v0.17.2`](https://github.com/astral-sh/ruff/blob/HEAD/CHANGELOG.md#0172) [Compare Source](astral-sh/ruff@0.17.1...0.17.2) Released on 2026-10-22. ##### Preview features - \[`complexity`] Add `unnecessary-regular-expression-compile` ([#​25533](astral-sh/ruff#25533)) ##### Bug fixes - Avoid syntax errors for type variables with starred constraints (`UP040`, `UP046`, `UP047`, `RUF053`) ([#​28362](astral-sh/ruff#28362)) - \[`pydocstyle`] Treat backslash-escaped `*args` and `**kwargs` as documented (`D417`) ([#​29536](astral-sh/ruff#29536)) - \[`pylint`] Avoid false positive on starred elements (`PLE0643`) ([#​29483](astral-sh/ruff#29483)) - \[`pylint`] Mark fixes unsafe when deleting comments (`PLR5501`, `PLW0120`) ([#​29414](astral-sh/ruff#29414)) - \[`pyupgrade`] Mark fix unsafe when deleting comments (`UP035`) ([#​29413](astral-sh/ruff#29413)) ##### Performance - Avoid intermediate names in lint name resolution ([#​29506](astral-sh/ruff#29506)) - Prefilter bidirectional Unicode checks by UTF-8 leading bytes ([#​29561](astral-sh/ruff#29561)) - Share postfix parsing and reduce expression copies ([#​29480](astral-sh/ruff#29480)) ##### Configuration - \[`pylint`] Add a configurable allowlist for `magic-value-comparison` (`PLR2004`) ([#​18961](astral-sh/ruff#18961)) ##### Documentation - Clarify documentation and correct stale guidance ([#​29374](astral-sh/ruff#29374)) - \[`pyupgrade`] Remove mentions of variance related to fix safety (`UP040`, `UP047`) ([#​29592](astral-sh/ruff#29592)) ##### Contributors - [@​fixfon](https://github.com/fixfon) - [@​MichaReiser](https://github.com/MichaReiser) - [@​zaniebot](https://github.com/zaniebot) - [@​FrancescoPezzella](https://github.com/FrancescoPezzella) - [@​anishgirianish](https://github.com/anishgirianish) - [@​rajveer3x](https://github.com/rajveer3x) - [@​charliermarsh](https://github.com/charliermarsh) - [@​LckyLike](https://github.com/LckyLke) - [@​DaniBodor](https://github.com/DaniBodor) </details> --- ### Configuration 📅 **Schedule**: (in timezone America/Denver) - Branch creation - Between 02:00 AM and 06:59 AM, only on Tuesday (`* 2-6 * * 2`) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xNDguMyIsInVwZGF0ZWRJblZlciI6IjQ0LjE0OC4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZSJdfQ==--> Reviewed-on: https://git.lichtenwalter.site/rlichten/dir2text/pulls/100
1 task
JerrettDavis
added a commit
to headroomlabs-ai/headroom
that referenced
this pull request
Nov 3, 2026
…ross 1 directory (#4380) Ruff's default `UP035` fix in 0.17.0 can delete comments from a multi-line deprecated typing import. Headroom enables the UP rules and invokes `ruff --fix` in pre-commit, so this can discard contributor explanations during routine checks. Upgrade to 0.17.2, which marks comment-deleting fixes unsafe, and synchronize the exact dev dependency, lockfile, and pre-commit revision. The branch also carries the reviewed glob/minimatch security remediation from #4599 so the resulting tree passes the npm audit gates. It preserves the existing exact-path/version exceptions for npm's bundled dependencies and their November 15 expiry. ## Dependency assessment - **Why Ruff / this version:** existing project linter and formatter; this patch fixes a destructive auto-fix in a rule family already enabled by Headroom. Upstream fix: astral-sh/ruff#29413, released in https://github.com/astral-sh/ruff/releases/tag/0.17.2. This is a bug-fix upgrade, with the default unsafe-fix protection retained. - **Maintenance / security:** maintained by Astral in astral-sh/ruff; the official 0.17.1 and 0.17.2 releases were published October 15 and 22. PyPI's 0.17.2 response reports no known vulnerability entries at qualification time; this is not a guarantee about all security history. - **Install surface:** a development/pre-commit tool, distributed as platform-native Rust binaries in wheels. PyPI 0.17.2 declares no Python runtime dependencies and provides 17 wheels; the installed binary runs successfully. Package installation downloads from the registry; linting itself was exercised without a service/model/API key. No new application-runtime library, transitive package version, or dependency set is added by the Ruff lock update. Lock regeneration also synchronizes the existing lean-extra python-socks declaration. - **Carried security fix:** #4599 documents the glob/minimatch rationale and install surface; the exact manifests, locks, regressions and exceptions were checked against the reviewed fix. No exception was broadened or extended. ## Real behavior proof Windows, Python 3.13.3 for uv lock validation; Node 22.22.0 for OpenCode tests/build; Node 24.22.2 for E2E dependency regressions. Ruff binaries were obtained with `uvx --from ruff==VERSION ruff`. For each Ruff version, write this file and run `ruff check --isolated --select UP035 --target-version py311 --fix fixture.py`: ```python from typing import ( Awaitable, # preserve this contributor explanation Callable, ) ``` Observed: 0.17.0 removes the comment and exits 0 after fixing the import. Version 0.17.2 preserves the comment, leaves the unsafe fix unapplied, and exits 1 with the expected UP035 finding. The test asserted these before/after file contents. This is the intended protection for Headroom's pre-commit auto-fix path. Additional validation on the resulting branch: - `python scripts/verify-ruff-version.py`: Ruff versions aligned at 0.17.2. - `uv lock --check` passes; `uv export --locked --extra dev --no-hashes --no-emit-project` selects `ruff==0.17.2`. - `uvx --from ruff==0.17.2 ruff check . --output-format concise`: all checks passed. - Fresh `npm ci --ignore-scripts --no-audit --no-fund` in `plugins/opencode` and `e2e/wrap/tools`. - OpenCode: `npm test -- --reporter=dot`: 61 passed; `npm run typecheck` and `npm run build` passed; `npm audit --json`: zero vulnerabilities. - E2E tools: `npm exec --yes --package=node@24.22.2 -- node --test --test-concurrency=1 --test-reporter=tap dependency-regression.test.cjs`: 3 passed. The exact Python checker extracted from `.github/workflows/security.yml` accepted only 8 existing bundled advisory/path entries, until 2026-11-15. - `git diff --check` passed. Untested locally: other operating systems, pre-commit environment creation, application runtime/provider paths, and a production release. Hosted CI must qualify the current head; local checks do not claim a rollout. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Kenneth Wong <17061293+KennethWKZ@users.noreply.github.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #29400.
UP035 rebuilds the whole
from typing import (...)statement from the names, so any comment inside it, like# noqaafter 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().intersectscheck astimeout_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.