Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pyupgrade] Rename private type parameters in PEP 695 generics (UP049) #15862

Merged
merged 52 commits into from
Feb 4, 2025

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Jan 31, 2025

Summary

This is a new rule to implement the renaming of PEP 695 type parameters with leading underscores after they have (presumably) been converted from standalone type variables by either UP046 or UP047. Part of #15642.

I'm not 100% sure the fix is always safe, but I haven't come up with any counterexamples yet. Renamer seems pretty precise, so I don't think the usual issues with comments apply.

I initially tried writing this as a rule that receives a Stmt rather than a Binding, but in that case the checker.semantic().current_scope() was the global scope, rather than the scope of the type parameters as I needed. Most of the other rules using Renamer also used Bindings, but it does have the downside of offering separate diagnostics for each parameter to rename.

Test Plan

New snapshot tests for UP049 alone and the combination of UP046, UP049, and PYI018.

@ntBre ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jan 31, 2025
Copy link
Contributor

github-actions bot commented Jan 31, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Overall this looks great!

@ntBre ntBre marked this pull request as ready for review February 4, 2025 17:04
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Great work!!

@ntBre
Copy link
Contributor Author

ntBre commented Feb 4, 2025

Awesome, thanks for all of your help! Is UP051 an acceptable code to use? I just searched issues and PRs again, and the highest number I see now is UP048 (#15625). I think #15841 was previously UP050, which is probably where I got UP051. I'm happy to switch to UP049 to avoid leaving a gap, if you want.

@AlexWaygood
Copy link
Member

It's nice to avoid leaving a gap where possible with the rule codes, but I don't think it matters too much in cases like this where there's no corresponding "upstream" rule! It is quite important to make sure we don't use the same code as a rule that we used to have but has since been deprecated or removed, however.

@ntBre ntBre changed the title [pyupgrade] Rename private type parameters in PEP 695 generics (UP051) [pyupgrade] Rename private type parameters in PEP 695 generics (UP049) Feb 4, 2025
@ntBre ntBre merged commit 6bb3235 into main Feb 4, 2025
21 checks passed
@ntBre ntBre deleted the brent/rename-typevars branch February 4, 2025 18:22
@AlexWaygood
Copy link
Member

AlexWaygood commented Feb 4, 2025

Oh shoot, I forgot to say. Can we add a mention of this rule to the ## See also sections in the docs for UP046, UP047 and UP040? The autofixes for those rules will work really well if this one is enabled as well

@ntBre
Copy link
Contributor Author

ntBre commented Feb 4, 2025

Ahhh, good catch. I meant to do that the whole time but forgot. I'll open a follow-up!

dcreager added a commit that referenced this pull request Feb 4, 2025
* main: (66 commits)
  [red-knot] Use ternary decision diagrams (TDDs) for visibility constraints (#15861)
  [`pyupgrade`] Rename private type parameters in PEP 695 generics (`UP049`) (#15862)
  Simplify the `StringFlags` trait (#15944)
  [`flake8-pyi`] Make `PYI019` autofixable for `.py` files in preview mode as well as stubs (#15889)
  Docs (`linter.md`): clarify that Python files are always searched for in subdirectories (#15882)
  [`flake8-pyi`] Make PEP-695 functions with multiple type parameters fixable by PYI019 again (#15938)
  [red-knot] Use unambiguous invalid-syntax-construct for suppression comment test (#15933)
  Make `Binding::range()` point to the range of a type parameter's name, not the full type parameter (#15935)
  Update black deviations (#15928)
  [red-knot] MDTest: Fix line numbers in error messages (#15932)
  Preserve triple quotes and prefixes for strings (#15818)
  [red-knot] Hand-written MDTest parser (#15926)
  [`pylint`] Fix missing parens in unsafe fix for `unnecessary-dunder-call` (`PLC2801`) (#15762)
  nit: docs for ignore & select (#15883)
  [airflow] `BashOperator` has been moved to `airflow.providers.standard.operators.bash.BashOperator` (AIR302) (#15922)
  [`flake8-logging`] `.exception()` and `exc_info=` outside exception handlers (`LOG004`, `LOG014`) (#15799)
  [red-knot] Enforce specifying paths for mdtest code blocks in a separate preceding line (#15890)
  [red-knot] Internal refactoring of visibility constraints API (#15913)
  [red-knot] Implicit instance attributes (#15811)
  [`flake8-comprehensions`] Handle extraneous parentheses around list comprehension (`C403`) (#15877)
  ...
ntBre added a commit that referenced this pull request Feb 5, 2025
…P040 (#15956)

## Summary

Minor docs follow-up to #15862 to mention UP049 in the UP046 and UP047
`See also` sections. I wanted to mention it in UP040 too but realized it
didn't have a `See also` section, so I also added that, adapted from the
other two rules.

## Test Plan

cargo test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants