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

RUF052 fix is unsafe when renaming TypeVars #14798

Closed
oguzhanmeteozturk opened this issue Dec 5, 2024 · 5 comments · Fixed by #14819
Closed

RUF052 fix is unsafe when renaming TypeVars #14798

oguzhanmeteozturk opened this issue Dec 5, 2024 · 5 comments · Fixed by #14819
Assignees
Labels
bug Something isn't working fixes Related to suggested fixes for violations preview Related to preview mode features rule Implementing or modifying a lint rule

Comments

@oguzhanmeteozturk
Copy link

The automatic fix for RUF052 is unsafe when applied to TypeVar definitions. It renames _T = TypeVar("_T") to T = TypeVar("_T") resulting in invalid code. The TypeVar name string must match the assigned variable name to maintain correctness, and the resulting T = TypeVar("_T") violates this requirement.

The underscore is often used to indicate private or internal TypeVars and is a common and valid pattern in python.

@dylwil3 dylwil3 added bug Something isn't working rule Implementing or modifying a lint rule fixes Related to suggested fixes for violations preview Related to preview mode features labels Dec 5, 2024
@AlexWaygood
Copy link
Member

Thanks for opening the issue! Interesting, are you defining TypeVars inside functions? I thought RUF052 only applied inside function scopes

@oguzhanmeteozturk
Copy link
Author

Tests define TypeVars internally sometimes

@AlexWaygood
Copy link
Member

Just to make sure I understand -- something like this?

def test_foo():
    _T = TypeVar("_T")
    
    def bar(x: _T) -> _T:
        ...  # some logic here

@oguzhanmeteozturk
Copy link
Author

Yes, very similar. Not sure how relevant it is, but the exact point where it broke the test involves _T being passed as an argument to a FUT within a context (pytest.raises in this case).

@AlexWaygood
Copy link
Member

Great, thanks. I'd argue that this is within the purview of RUF052 -- since the TypeVar is a local variable, it's private to the function even without the underscore; leading underscores are more usually used to indicate unused variables in function scopes, I think.

However, we obviously shouldn't be breaking TypeVar definitions by changing the name the TypeVar is assigned to without changing the string passed into the constructor. We either need to make our renaming logic more sophisticated and/or mark the autofix as unsafe in more cases.

Thanks for the report!

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 rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants