Detect unused whitelist items

[exhuma]

As a project lives on and is maintained, whitelists grow. It is likely that after a time, the whitelist has items in there which no longer exist in the code.

It would be a nice addition if vulture detected those and reports unused whitelist items as well.

mypy does something similar for unused # type: ignore comments. Which is really nice.

I'm willing to work on this myself, but I'm not yet sure where to begin...

[jendrikseipp]

I agree. In my projects that use Vulture I simply run python whitelist.py to check that all whitelisted code still exists. Does that work for you as well? Care to document that approach in the README?

[exhuma]

Good point. I could have thought of that 😄

It works mostly. I have run into a situation where I am overriding a method of a third-party library. In that method I am ignoring a function argument. I really don't need it for that override, but vulture complains about the unused argument.

For example, let's say that this is the full implementation:

class MyHandler(BaseHandler):
    def overridden_method(self, foo: str, bar: int):
        print(foo)

Then I could write in my whitelist something like this:

from handlers import MyHandler

MyHandler.overridden_method

to make it aware of the usage of that method. But I have no way of whitelisting the unused argument.

[jendrikseipp]

In such a case, I'd use _bar. That's a common idiom for unused arguments.

[jendrikseipp]

Python won't complain that the parameter name differs from the method in the base class since it only looks at method names, not their parameters.

[exhuma]

Yes, but then mypy (and I think pylint too) will complain about inconsistent signatures.

Also, the day that code will call the function with keyword args (f.ex. my_handler_instance(bar=10) the code will break. I don't mind using underscored names in independent functions, but not in overridden methods.

I don't actually have any control how this is called. This is inside a sphinx extension that defines how code is extracted (an override of FunctionDocumenter) so I don't have control over how this is called. It is sort of a "hook" or "plugin" and I don't have control over the function signature.

[jendrikseipp]

Then I'd recommend to put the following into your whitelist:

from handlers import MyHandler

MyHandler.overridden_method  # Ensure that method still exists.
bar  # Ignore unused argument.

I guess you could also subclass MyHandler, override the method again and use bar in the overriding method.

[exhuma]

This makes a lot of sense. I really need to think of the whitelist as a completely normal Python module, and not just as a list of rules to ignore. I might update the docs a bit if you don't mind when I get around to that.

There is one final pattern I'm struggling with and maybe you have an idea on how to whitelist that:

I have some higher order functions in an application. A simple example could be:

def make_functions():
    def fun1():
        print(1)
    def fun2():
        print(2)
    return fun1, fun2

These are called at runtime and depend on arguments going into make_functions. It would be a bit cumbersome (albeit possible) to call this in the whitelist.

Do you see a possibility to whitelist this in another way? If not, I'll just call them in the whitelist. It's a really rare corner-case anyway and I'm fine putting in a bit more work into the whitelist for this if there is no alternative.

[jendrikseipp]

I'm not sure what the false positive is in your code since both local functions are returned and therefore "used". Could you give a more complete example, please?

I might update the docs a bit if you don't mind when I get around to that.

That would be great!

[exhuma]

It is actually a Flask Blueprint factory. I don't have my hands on the code right now as it is from the office, but it is along the lines of this:

from flask import Blueprint

def make_bp(name, arg):
    bp = Blueprint(name, __name__)

    @bp.route("/")
    def bp_index():
        return "Hello World"

    return bp

So the route-function itself is not returned from the factory. Flask routes are generally not picked up but are easily whitelisted. This one is a bit trickier.

[jendrikseipp]

I'd use --ignore-decorators @bp.route or --ignore-decorators @*.route for this. Or simply "use" the name bp_index somewhere in the whitelist.

[RJ722]

I think the examples in this thread would be worthwhile to mention in the README as canonical examples for whitelisting, especially the trickier ones, like flask. @jendrikseipp If you give me a green flag, I would like to send a PR!

[jendrikseipp]

That's a great idea! The README is already quite long, so I recommend adding this info under docs/whitelists.md and linking to it from the README.

[sshishov]

Hello guys, for overridden functions and unused arguments, can the vulture follow the same rules as pylint and other tools.

It is described here: https://stackoverflow.com/a/14836005

And this is specified as the suggested solution by PyLint: http://pylint-messages.wikidot.com/messages:w0613

Currently I am trying to use del but vulture is still reporting the variable as unused

[jendrikseipp]

I'm cleaning up the list of issues a bit. Since the main point of this issue is done, I'm closing it. @exhuma and @RJ722: you stated above that you wanted to send PRs that improve the documentation. If you're still up for that, I'm happy to review the PRs.