jendrikseipp / vulture

Find dead Python code
MIT License
3.29k stars 145 forks source link

False positive when an attribute uses `weakref.finalize` #350

Closed trystyncote closed 5 months ago

trystyncote commented 5 months ago

I found a false positive that occurs when using weakref.finalize as an attribute in a class. Consider the following example:

import weakref

__all__ = ["CallsFinalizer"]  # Prevents `CallsFinalizer` from being considered
# unused by Vulture in our example, since it's not the focus.

def close_resource(resource):
    resource.close()

class CallsFinalizer:
    def __init__(self, resource):
        self.resource = resource
        self._finalizer = weakref.finalize(self, close_resource, self.resource)

The _finalizer attribute in CallsFinalizer is used when the instance is set to be garbage collected. When it's being garbage collected, the instance will call the finalizer to close the resource attribute, so that it may be closed gracefully. This is useful for ensuring that a resource, like a file pointer, is closed in cases where compatibility with a with statement is not plausible.

However, Vulture raises an issue: unused attribute '_finalizer' (60% confidence). This, as mentioned before, is a false positive, since the finalizer will always be called in accordance to the behaviour of CallsFinalizer.

Now, I acknowledge that there's a workaround: the --ignore-names flag. Running the command with --ignore-names _finalizer raises no issue with the above code.

That said, I wanted to submit an issue on the repository to let you know that this is a false positive, and a sneaky one at that. I don't see a way to fix this false positive without giving weakref.finalize special behaviour, which I don't believe would be very elegant of a solution, so I'm not asking for a fix, I just wanted to ensure that this was on the record, so to speak.

Happy coding! :)

jendrikseipp commented 5 months ago

Thanks for the report! I didn't know about the weakref module, so I learned something new :-)

I don't see an easy fix for this issue within Vulture. However, the --ignore-names solution is adequate and you could also create a whitelist file instead.