Combine used_attrs and used_names to avoid false positives

Note this produces more false negatives, so this is a trade-off.

Fixes jendrikseipp#221

tests/test_format_strings.py

@@ -10,7 +10,7 @@ def test_old_format_string(v):
 
 def test_new_format_string(v):
     v.scan("'{a}, {b:0d} {c:<30} {d:.2%}'.format(**locals())")
-    check(v.used_names, ["a", "b", "c", "d", "locals"])
+    check(v.used_names, ["a", "b", "c", "d", "format", "locals"])
 
 
 def test_f_string(v):
@@ -27,24 +27,19 @@ def test_f_string(v):
 
 
 def test_new_format_string_access(v):
-    v.scan("'{a.b}, {c.d.e} {f[g]} {h[i][j]}'.format(**locals())")
-    check(v.used_names, ["a", "c", "f", "h", "locals"])
-
-
-def test_new_format_string_attributes(v):
     v.scan("'{a.b}, {c.d.e} {f[g]} {h[i][j].k}'.format(**locals())")
-    check(v.used_names, ["a", "c", "f", "h", "locals"])
-    check(v.used_attrs, ["b", "d", "e", "k", "format"])
+    check(
+        v.used_names,
+        ["a", "b", "c", "d", "e", "f", "format", "h", "k", "locals"],
+    )
 
 
 def test_new_format_string_numbers(v):
     v.scan("'{0.b}, {0.d.e} {0[1]} {0[1][1].k}'.format('foo')")
-    check(v.used_names, [])
-    check(v.used_attrs, ["b", "d", "e", "k", "format"])
+    check(v.used_names, ["b", "d", "e", "k", "format"])
 
 
 def test_incorrect_format_string(v):
     v.scan('"{"')
     v.scan('"{!-a:}"')
     check(v.used_names, [])
-    check(v.used_attrs, [])

tests/test_ignore.py

@@ -90,10 +90,10 @@ def test_attribute():
     code = """\
 class Foo:
     def __init__(self, attr_foo, attr_bar):
-        self.attr_foo = attr_foo
-        self.attr_bar = attr_bar
+        self._attr_foo = attr_foo
+        self._attr_bar = attr_bar
 """
-    check_ignore(code, ["foo", "*_foo"], [], ["Foo", "attr_bar"])
+    check_ignore(code, ["foo", "*_foo"], [], ["Foo", "_attr_bar"])
 
 
 def test_decorated_functions():

tests/test_imports.py

@@ -59,7 +59,7 @@ def test_nested_import(v):
 """
     )
     check(v.defined_imports, ["os"])
-    check(v.used_names, ["os"])
+    check(v.used_names, ["os", "path", "expanduser"])
     check(v.unused_funcs, [])
     check(v.unused_imports, [])
     check(v.unused_vars, [])

tests/test_scavenging.py

@@ -113,7 +113,31 @@ def func():
     check(v.defined_methods, ["func"])
     check(v.unused_classes, ["Bar"])
     check(v.unused_funcs, [])
-    check(v.unused_methods, ["func"])
+    # Bar.func is unused, but it's hard to detect this without producing a
+    # false positive in test_function_and_method2.
+    check(v.unused_methods, [])
+
+
+def test_function_and_method2(v):
+    v.scan(
+        """\
+class Bar(object):
+    def func(self):
+        pass
+
+    other_name_for_func = func
+
+Bar().other_name_for_func()
+"""
+    )
+    check(v.defined_classes, ["Bar"])
+    check(v.defined_funcs, [])
+    check(v.defined_methods, ["func"])
+    check(v.defined_vars, ["other_name_for_func"])
+    check(v.unused_classes, [])
+    check(v.unused_funcs, [])
+    check(v.unused_methods, [])
+    check(v.unused_vars, [])
 
 
 def test_attribute1(v):
@@ -126,7 +150,7 @@ def test_attribute1(v):
     check(v.unused_funcs, [])
     check(v.defined_funcs, [])
     check(v.defined_attrs, ["bar", "bar"])
-    check(v.used_attrs, [])
+    check(v.used_names, ["foo"])
     check(v.unused_attrs, ["bar", "bar"])
 
 
@@ -141,7 +165,7 @@ def test_ignored_attributes(v):
 """
     )
     check(v.defined_attrs, ["_", "_a", "__b", "__c__", "_d_"])
-    check(v.used_attrs, [])
+    check(v.used_names, ["A"])
     check(v.unused_attrs, ["_", "__b", "__c__", "_a", "_d_"])
     check(v.unused_vars, [])
 
@@ -157,7 +181,7 @@ def foo(self):
 b.foo
 """
     )
-    check(v.used_attrs, ["foo"])
+    check(v.used_names, ["Bar", "b", "foo"])
     check(v.defined_classes, ["Bar"])
     check(v.defined_funcs, [])
     check(v.defined_methods, ["foo"])
@@ -172,7 +196,7 @@ class Bar(object):
     pass
 """
     )
-    check(v.used_attrs, [])
+    check(v.used_names, [])
     check(v.defined_classes, ["Bar"])
     check(v.unused_classes, ["Bar"])
 
@@ -187,7 +211,6 @@ class Foo(Bar):
 Foo()
 """
     )
-    check(v.used_attrs, [])
     check(v.used_names, ["Bar", "Foo"])
     check(v.defined_classes, ["Bar", "Foo"])
     check(v.unused_classes, [])
@@ -201,7 +224,6 @@ class Bar():
 [Bar]
 """
     )
-    check(v.used_attrs, [])
     check(v.used_names, ["Bar"])
     check(v.defined_classes, ["Bar"])
     check(v.unused_classes, [])
@@ -215,7 +237,7 @@ class Bar():
 Bar()
 """
     )
-    check(v.used_attrs, [])
+    check(v.used_names, ["Bar"])
     check(v.defined_classes, ["Bar"])
     check(v.unused_classes, [])
 
@@ -228,7 +250,7 @@ class Bar():
 b = Bar()
 """
     )
-    check(v.used_attrs, [])
+    check(v.used_names, ["Bar"])
     check(v.defined_classes, ["Bar"])
     check(v.unused_classes, [])
     check(v.unused_vars, ["b"])
@@ -300,8 +322,7 @@ def test_token_types(v):
     )
     check(v.defined_funcs, [])
     check(v.defined_vars, ["b"])
-    check(v.used_names, ["a", "c", "x"])
-    check(v.used_attrs, ["d"])
+    check(v.used_names, ["a", "c", "d", "x"])
     check(v.unused_attrs, [])
     check(v.unused_funcs, [])
     check(v.unused_props, [])
@@ -320,7 +341,7 @@ def test_variable2(v):
     v.scan("a = 1\nc = b.a")
     check(v.defined_funcs, [])
     check(v.defined_vars, ["a", "c"])
-    check(v.used_names, ["b"])
+    check(v.used_names, ["a", "b"])
     check(v.unused_vars, ["c"])
 
 
@@ -409,7 +430,7 @@ def __init__(self):
     check(v.defined_attrs, ["a"])
     check(v.defined_classes, ["Bar"])
     check(v.defined_vars, [])
-    check(v.used_attrs, [])
+    check(v.used_names, [])
     check(v.unused_attrs, ["a"])
     check(v.unused_classes, ["Bar"])
 
@@ -438,7 +459,6 @@ class OtherClass:
     check(v.defined_classes, ["OtherClass"])
     check(v.defined_funcs, ["other_func"])
     check(v.defined_vars, [])
-    check(v.used_attrs, [])
     check(v.used_names, [])
     check(v.unused_attrs, [])
     check(v.unused_classes, ["OtherClass"])
@@ -498,7 +518,6 @@ class OtherClass:
     check(v.defined_classes, ["BasicTestCase", "OtherClass", "TestClass"])
     check(v.defined_funcs, ["test_func", "other_func"])
     check(v.defined_vars, [])
-    check(v.used_attrs, [])
     check(v.used_names, [])
     check(v.unused_attrs, [])
     check(v.unused_classes, ["BasicTestCase", "OtherClass", "TestClass"])
@@ -520,9 +539,8 @@ def test_global_attribute(v):
     )
     check(v.defined_attrs, ["a"])
     check(v.defined_vars, ["a"])
-    check(v.used_attrs, [])
     check(v.used_names, ["a", "foo"])
-    check(v.unused_attrs, ["a"])
+    check(v.unused_attrs, [])
 
 
 def test_boolean(v):

vulture/core.py

@@ -175,9 +175,6 @@ def __init__(
         def get_list(typ):
             return utils.LoggingList(typ, self.verbose)
 
-        def get_set(typ):
-            return utils.LoggingSet(typ, self.verbose)
-
         self.defined_attrs = get_list("attribute")
         self.defined_classes = get_list("class")
         self.defined_funcs = get_list("function")
@@ -187,8 +184,7 @@ def get_set(typ):
         self.defined_vars = get_list("variable")
         self.unreachable_code = get_list("unreachable_code")
 
-        self.used_attrs = get_set("attribute")
-        self.used_names = get_set("name")
+        self.used_names = utils.LoggingSet("name", self.verbose)
 
         self.ignore_names = ignore_names or []
         self.ignore_decorators = ignore_decorators or []
@@ -315,39 +311,31 @@ def report(
 
     @property
     def unused_classes(self):
-        return _get_unused_items(
-            self.defined_classes, self.used_attrs | self.used_names
-        )
+        return _get_unused_items(self.defined_classes, self.used_names)
 
     @property
     def unused_funcs(self):
-        return _get_unused_items(
-            self.defined_funcs, self.used_attrs | self.used_names
-        )
+        return _get_unused_items(self.defined_funcs, self.used_names)
 
     @property
     def unused_imports(self):
-        return _get_unused_items(
-            self.defined_imports, self.used_names | self.used_attrs
-        )
+        return _get_unused_items(self.defined_imports, self.used_names)
 
     @property
     def unused_methods(self):
-        return _get_unused_items(self.defined_methods, self.used_attrs)
+        return _get_unused_items(self.defined_methods, self.used_names)
 
     @property
     def unused_props(self):
-        return _get_unused_items(self.defined_props, self.used_attrs)
+        return _get_unused_items(self.defined_props, self.used_names)
 
     @property
     def unused_vars(self):
-        return _get_unused_items(
-            self.defined_vars, self.used_attrs | self.used_names
-        )
+        return _get_unused_items(self.defined_vars, self.used_names)
 
     @property
     def unused_attrs(self):
-        return _get_unused_items(self.defined_attrs, self.used_attrs)
+        return _get_unused_items(self.defined_attrs, self.used_names)
 
     def _log(self, *args):
         if self.verbose:
@@ -447,7 +435,7 @@ def is_identifier(name):
                 self.used_names.add(name)
             for attr in name_and_attrs[1:]:
                 if is_identifier(attr):
-                    self.used_attrs.add(attr)
+                    self.used_names.add(attr)
 
     def _define(
         self,
@@ -506,7 +494,7 @@ def visit_Attribute(self, node):
         if isinstance(node.ctx, ast.Store):
             self._define(self.defined_attrs, node.attr, node)
         elif isinstance(node.ctx, ast.Load):
-            self.used_attrs.add(node.attr)
+            self.used_names.add(node.attr)
 
     def visit_ClassDef(self, node):
         for decorator in node.decorator_list: