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

[ruff] Teach autofix for used-dummy-variable about TypeVars etc. (RUF052) #14819

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Dec 6, 2024

Summary

For some bindings, such as assignments where the right-hand-side is a TypeVar constructor call, renaming the binding can result in invalid code. For example, this diff that renamer.rs might make would take code that mypy (or any other spec-compliant type checker) would have accepted, and turns it into something a type checker will reject:

  from typing import TypeVar

- _T = TypeVar("_T")
+ T = TypeVar("_T")

This is because the first argument to the TypeVar constructor must be the same as the variable the typevar is being bound to. Detecting this issue in general is impossible (there are lots of classes where this pattern is required), but it seems reasonable to special-case several standard-library classes where this pattern is required.

Fixes #14798

Test Plan

Added some more fixtures to RUF052 to check that the new logic in renamer.rs works as expected.

@AlexWaygood AlexWaygood added bug Something isn't working fixes Related to suggested fixes for violations preview Related to preview mode features labels Dec 6, 2024
Copy link
Contributor

github-actions bot commented Dec 6, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Base automatically changed from alex/ruf052-parameters to main December 6, 2024 14:54
@MichaReiser
Copy link
Member

Do you know what editors do when you rename a variable that's a TypeVar declaration?

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

This is very cool! Nice work

@AlexWaygood
Copy link
Member Author

Do you know what editors do when you rename a variable that's a TypeVar declaration?

VSCode just does this, so we'll do a better job than them with this change ;)

-_T = TypeVar("_T")
+T = TypeVar("_T")

@AlexWaygood AlexWaygood merged commit 2119dca into main Dec 6, 2024
21 checks passed
@AlexWaygood AlexWaygood deleted the alex/typevar-rename branch December 6, 2024 17:05
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.

RUF052 fix is unsafe when renaming TypeVars
2 participants