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: do not apply to function arguments? #14796

Closed
inducer opened this issue Dec 5, 2024 · 8 comments · Fixed by #14818
Closed

RUF052: do not apply to function arguments? #14796

inducer opened this issue Dec 5, 2024 · 8 comments · Fixed by #14818
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule

Comments

@inducer
Copy link

inducer commented Dec 5, 2024

I feel that the connotation of a leading underscore is quite different between local variables and function arguments. I agree with the premise of the rule that an underscored local variable is an unused dummy thing. But an underscored function arguments to me is an internal thing, comparable to an underscored method in a class, not to be considered part of the public interface. As such, I would prefer if the rule could separate function argument names from other local variables.

@dylwil3
Copy link
Collaborator

dylwil3 commented Dec 5, 2024

Incidentally, adopting this point of view would also solve #14790

So that makes it tempting to agree - but perhaps for selfish reasons. Curious what other folks think!

@dylwil3 dylwil3 added the rule Implementing or modifying a lint rule label Dec 5, 2024
@zanieb
Copy link
Member

zanieb commented Dec 5, 2024

But an underscored function arguments to me is an internal thing, comparable to an underscored method in a class, not to be considered part of the public interface. As such, I would prefer if the rule could separate function argument names from other local variables.

So... would this be unused-function-argument instead?

@dylwil3
Copy link
Collaborator

dylwil3 commented Dec 5, 2024

It sounds like the OP is saying that an underscored argument should be indicating that it is "private" rather than "unused". So, for example:

def foo(x:int, y:str, _z:bool = False):
# <-- do stuff -->
if _z:
    #<-- something... -->
#<--- more stuff --->

I think (but I could be wrong) that the OP is suggesting that the connotation of _z is "this is a private, internal setting I can use" as opposed to "this is a dummy variable". In particular, the above code snippet should trigger neither used-dummy-variable (RUF052) nor unused-function-argument (ARG001)

@zanieb
Copy link
Member

zanieb commented Dec 5, 2024

Ah yes, of course — yeah I agree it's weird to raise this (RUF052) for function arguments.

My point is if that if we're not going to treat it as a dummy variable then if it's unused we should raise a violation

❯ cat example.py
def foo(x: int, y: str, _z: bool = False):
    return 10

❯ uvx ruff@latest check example.py --select ARG001
Installed 1 package in 1ms
example.py:1:9: ARG001 Unused function argument: `x`
  |
1 | def foo(x: int, y: str, _z: bool = False):
  |         ^ ARG001
2 |     return 10
  |

example.py:1:17: ARG001 Unused function argument: `y`
  |
1 | def foo(x: int, y: str, _z: bool = False):
  |                 ^ ARG001
2 |     return 10
  |

Found 2 errors.

Note we don't flag _z here and the documentation says

If a variable is intentionally defined-but-not-used, it should be prefixed with an underscore, or some other value that adheres to the lint.dummy-variable-rgx pattern.

@dylwil3
Copy link
Collaborator

dylwil3 commented Dec 5, 2024

Yeah that's a little awkward... it kinda seems like both interpretations of the underscore could make sense in different contexts. I would personally find it okay to ignore both checks in the presence of underscores.

I guess I don't have a good sense of how many people would be happy to receive a lint error (either of them) alerting to the case where they did or didn't use a function argument that was named with an underscore. Like - what is the prevalence of people accidentally putting an underscore on a function argument and wishing they hadn't?

@zanieb
Copy link
Member

zanieb commented Dec 5, 2024

I don't really know of a context where you'd define an intentionally unused variable in a function signature. Matching a parent signature comes to mind, but... you can't add an underscore to resolve that because it changes the signature.

@dylwil3
Copy link
Collaborator

dylwil3 commented Dec 5, 2024

I think you can change the name for position-only arguments.

But I agree I don't know who's doing this...

class Foo:
    def bar(self, x:int, /) -> int:
        return x

class Bar(Foo):
    def bar(self, _x:int, /) -> int:
        return 17

@zanieb
Copy link
Member

zanieb commented Dec 5, 2024

Yeah I guess so haha getting increasingly dubious though.

This can be a separate debate, but I guess I find it weird that we would not apply RUF052 because it's not a dummy argument but... we also wouldn't apply ARG001 because it is a dummy argument.

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

3 participants