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 function parameters #14790

Closed
dscorbett opened this issue Dec 5, 2024 · 2 comments · Fixed by #14818
Closed

RUF052 fix is unsafe when renaming function parameters #14790

dscorbett opened this issue Dec 5, 2024 · 2 comments · Fixed by #14818
Labels
bug Something isn't working fixes Related to suggested fixes for violations preview Related to preview mode features

Comments

@dscorbett
Copy link

The fix for used-dummy-variable (RUF052) in Ruff 0.8.2 can introduce errors when renaming a function parameter. It should be marked unsafe in that context unless the parameter is positional-only.

$ cat ruf052.py
def f(_x):
    return _x
print(f(_x=1))

$ python ruf052.py
1

$ ruff check --isolated --preview --select RUF052 ruf052.py --fix
Found 1 error (1 fixed, 0 remaining).

$ cat ruf052.py
def f(x):
    return x
print(f(_x=1))

$ python ruf052.py
Traceback (most recent call last):
  File "ruf052.py", line 3, in <module>
    print(f(_x=1))
          ^^^^^^^
TypeError: f() got an unexpected keyword argument '_x'
@MichaReiser MichaReiser added the bug Something isn't working label Dec 5, 2024
@AlexWaygood AlexWaygood added the fixes Related to suggested fixes for violations label Dec 5, 2024
@AlexWaygood
Copy link
Member

Great catch @dscorbett. We can probably do two things here:

  1. If we see that the binging comes from a function parameter, we can change the autofix so that it iterates through the uses of the function in the function's defining module and also changes those uses
  2. If the function is explicitly marked as private to the module (the module has __all__ and the function does not appear in __all__), we can probably still mark it as safe if we do (1), but otherwise we'd have to mark it as unsafe if the binding comes from a parameter

@MichaReiser
Copy link
Member

I like what you propose, but I think it might also just be fine to mark the fix as unsafe for now when the binding comes from a parameter (there are just so many things to consider; what if the function overrides another method?)

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 a pull request may close this issue.

4 participants