flake8-bugbear
flake8-bugbear copied to clipboard
Warn on unused function arguments
Ref https://github.com/PyCQA/pyflakes/issues/147
In many cases, unused function arguments are bugs. As such detection behavior would be inappropriate in Pyflakes per the above, the best place to implement such a rule would be here, analogous to the existing B007.
I like the idea in general. The comments on the original pyflakes issue are fair though: if you are implementing a new function that needs to conform to some known signature, ignoring arguments that you don't care about is fine. This includes subclassing, callbacks, etc.
So, Bugbear is going to raise what is arguably a false positive here. How do you suggest us deal with it?
For positional args, the same approach as in B007 should work – just prefix the argument with an underscore. Also, pragmatically, we should probably ignore *args
.
For keyword args, it's a bit trickier. I would make unused keyword detection a separate rule, and perhaps not enable it by default. This rule should:
- Ignore
**kwargs
- Ignore abstract methods on classes (not sure how easy this is to detect – in principle some combination of looking for
@abstractmethod
andraise NotImplementedError()
orpass
as the function body perhaps)
Personally I'm relatively happy with just # noqa
-ing false positives like that, but the unused position arg detection should be fairly nice.
Having this warning would be nice for pytest fixtures too. Some might be expensive to compute and having a warning when they're not used would be very useful.
For context, fixtures in pytest are defined as function arguments.
This would be really useful for me too. Is this issue still being worked on?
For those interested in this, I've put flake8-unused-arguments up on pypi.
The repo is at https://github.com/nhoad/flake8-unused-arguments/, which includes the documentation on features. I can't edit the description on pypi to include this info until the next release, best I can tell.
Nice. This makes sense as a separate plugin imo.
Yeah, that's how PyPI works. Just spin a real small release/version increment and join your markdown into the long description. Remember to set that it's Markdown in setup.py
like: https://github.com/facebookincubator/ptr/blob/master/setup.py#L53
Yeah I was sure you used to be able to edit descriptions on PyPI, obviously I'm remembering incorrectly though haha. All good though, I did another version bump to get it updated - https://pypi.org/project/flake8-unused-arguments/
From the issue at https://github.com/PyCQA/pyflakes/issues/147
and there's no best practice which would explicitly declare a parameter as intentionally unused.
Isn't it best practice to prefix unused parameters with _
to mark them as intentionally unused? As in
def foo(_param):
pass
Correct, but there are situations where that isn't feasible, e.g. the name might be guarded by someone else's API. You might have something like:
def do_thing(callback):
# ... do some work...
callback(value=5)
user = Mock() # not relevant what it is
def my_callback(user, value):
user.do_something()
do_thing(functools.partial(my_callback, user))
In my contrived and ugly example, my_callback
doesn't use value
, but can't rename the argument, because do_thing
calls the callback with it as a keyword argument. Things get even more difficult when you don't control do_thing
, because you can't change it to use a positional name (thus making the name of the argument in do_thing
irrelevant).
Yeah, I think it's more the recursive nature or callers expecting names of things make this check to hard to implement in my opinion. But down for someone to try and make a calculated test that definitely knows something isn't being used anymore in the function or by callers.
Maybe it could at least check def _methods
as the should be private (once again just a practice, not enforced) to a class or module and if nothing else in the file calls with an optional param we could warn there safely maybe. But many other scenarios I feel will be noise.