jendrikseipp / vulture

Find dead Python code
MIT License
3.41k stars 150 forks source link

False positive due to distinction between used_attrs and used_names #221

Closed jingw closed 4 years ago

jingw commented 4 years ago

Consider the following code in two files:

# module.py
some_module_var = 1
print(some_module_var)

# module_user.py
import module
module.some_module_var = 2

Running vulture complains that module.some_module_var is an unused attribute

$ vulture module.py module_user.py 
module_user.py:3: unused attribute 'some_module_var' (60% confidence)

How do you feel about this? On one hand, in practice, the cases where I've seen this false positive were test cases that should have been refactored to use mock.patch instead of directly setting module attributes. On the other hand, it's still a legitimate false positive, and someone could conceivably write non-test code like this.

I imagine other classes of false positives may be caused by separating attrs and names. One example that comes to mind is

class Foobar:
    def my_method(self):
        pass

    deprecated_name_for_my_method = my_method

Foobar().deprecated_name_for_my_method()
func.py:2: unused method 'my_method' (60% confidence)

Fixing this can be done by combining used_attrs with used_names in unused_attrs, though that largely defeats the point of separating these. I suppose it's not crazy to collapse them into a single used_names though.

jendrikseipp commented 4 years ago

Good point! I agree that folding used_attrs into used_names makes sense (i.e., storing all used attributes in used_names and dropping the used_attrs set and the get_set function). Do you want to create a pull request for this?

jingw commented 4 years ago

Yep, I can put together that change.