Skip to content

[pyupgrade] Skip fix when a defaulted TypeVar precedes a non-defaulted one (UP040, UP046, UP047) - #27133

Merged
ntBre merged 3 commits into
astral-sh:mainfrom
Jayashanker-Padishala:fix-27021-up040-typevar-default-order
Jul 27, 2026
Merged

[pyupgrade] Skip fix when a defaulted TypeVar precedes a non-defaulted one (UP040, UP046, UP047)#27133
ntBre merged 3 commits into
astral-sh:mainfrom
Jayashanker-Padishala:fix-27021-up040-typevar-default-order

Conversation

@Jayashanker-Padishala

Copy link
Copy Markdown
Contributor

Summary

Fixes #27021.

In a PEP 695 type-parameter list, a non-defaulted parameter cannot follow a defaulted one, but the UP040/UP046/UP047 fixes generated exactly that when the source declared e.g. T = TypeVar("T", default=int) before S = TypeVar("S") — producing type Pair[T = int, S] = tuple[T, S], which ruff itself then rejects as invalid syntax.

Reordering the parameters would not be an equivalent fix: parameter order determines how positional arguments bind when subscripting the alias/class/function (Pair[int, str]). So, per the issue's suggestion and following the existing precedent in these rules (duplicate TypeVars and defaults on unsupported targets also suppress the diagnostic; NonPEP695TypeAlias declares FixAvailability::Always, so a fixless diagnostic isn't an option), the diagnostic is skipped when a non-defaulted TypeVar follows a defaulted one.

The guard lives at both choke points (check_type_vars for UP046/UP047 and create_diagnostic for both UP040 paths, including TypeAliasType(..., type_params=...)), so all three rules are covered by one helper.

Test Plan

  • New fixture cases in UP040.py, UP046_0.py, UP047_0.py: defaulted-then-non-defaulted (no diagnostic), non-defaulted-first and all-defaulted (still fixed correctly); snapshots regenerated.
  • cargo test -p ruff_linter — all pass; cargo fmt --check and cargo clippy -p ruff_linter --no-deps -- -D warnings clean.
  • Manually verified the issue's repro: before, ruff emitted the invalid fix and then flagged its own output; after, no diagnostic on the bad ordering while valid orderings still convert.

Developed with AI assistance (Claude Code); I reviewed the change and can speak to it.

…ulted one (`UP040`, `UP046`, `UP047`)

In a PEP 695 type parameter list, a type parameter without a default
cannot follow one with a default, and reordering the parameters would
change the meaning of subscriptions, so there is no valid, equivalent
fix. Bail out of the diagnostic instead of emitting invalid syntax.

Fixes astral-sh#27021
@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre July 23, 2026 15:40
@astral-sh-bot

astral-sh-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

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.

Thanks! The code changes here look good to me. I just had a few minor suggestions to trim down the comments a bit, and a request to use our new mdtest format.

Don't worry too much about the ecosystem check yet, that's just from an old merge base and should clear up if you push another commit.

Comment thread crates/ruff_linter/resources/test/fixtures/pyupgrade/UP040.py Outdated
Comment thread crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs Outdated
Comment thread crates/ruff_linter/src/rules/pyupgrade/rules/pep695/non_pep695_type_alias.rs Outdated
Comment thread crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs Outdated
@ntBre ntBre added bug Something isn't working fixes Related to suggested fixes for violations preview Related to preview mode features labels Jul 24, 2026
- Replace the new UP040/UP046/UP047 fixture cases and snapshot updates
  with an mdtest at resources/mdtest/pyupgrade/pep-695.md covering all
  three PEP 695 rules
- Drop the duplicated call-site comments; the docstring on
  non_default_follows_default explains the check
- Trim the docstring paragraph about fix equivalence
@Jayashanker-Padishala

Copy link
Copy Markdown
Contributor Author

Thanks @ntBre for the suggestions. Addressed all of them.

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

Looks good to me, thank you!

@ntBre
ntBre merged commit 8fbdca7 into astral-sh:main Jul 27, 2026
47 checks passed
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 preview Related to preview mode features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UP040 fix produces invalid syntax when a defaulted TypeVar precedes a non-defaulted TypeVar

2 participants