Warn on unused function arguments about flake8-bugbear HOT 10 OPEN

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.

from flake8-bugbear.

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).

from flake8-bugbear.

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 and raise NotImplementedError() or pass 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.

from flake8-bugbear.

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.

from flake8-bugbear.

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?

from flake8-bugbear.

This would be really useful for me too. Is this issue still being worked on?

from flake8-bugbear.

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

from flake8-bugbear.

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 flake8-bugbear.

From the issue at PyCQA/pyflakes#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

from flake8-bugbear.

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.

from flake8-bugbear.