Report first lineno for decorated objects on Python3.8+

CHANGELOG.md

@@ -1,8 +1,8 @@
 # Unreleased
 
 * Ignore unused import statements which occur in `__init__.py` (RJ722, #192).
-* Report first line number of method definition for unused properties (RJ722,
-  #200). 
+* Report first decorator's line number for decorated objects on Python 3.8+.
+  (RJ722, #200)
 
 # 1.3 (2020-02-03)
 

tests/test_item.py

@@ -17,3 +17,85 @@ def bar():
     )
     for item in v.get_unused_code():
         assert repr(item) == "'{}'".format(item.name)
+
+
+def test_item_attr(v):
+    v.scan("foo.bar = 'bar'")
+    assert len(v.unused_attrs) == 1
+    a = v.unused_attrs[0]
+    assert a.name == "bar"
+    assert a.first_lineno == 1
+    assert a.last_lineno == 1
+
+
+def test_item_class(v):
+    v.scan(
+        """\
+class Foo:
+    pass
+"""
+    )
+    assert len(v.unused_classes) == 1
+    c = v.unused_classes[0]
+    assert c.name == "Foo"
+    assert c.first_lineno == 1
+    assert c.last_lineno == 2
+
+
+def test_item_function(v):
+    v.scan(
+        """\
+def add(a, b):
+    return a + b
+"""
+    )
+    assert len(v.unused_funcs) == 1
+    f = v.unused_funcs[0]
+    assert f.name == "add"
+    assert f.first_lineno == 1
+    assert f.last_lineno == 2
+
+
+def test_item_import(v):
+    v.scan(
+        """\
+import bar
+from foo import *
+"""
+    )
+    assert len(v.unused_imports) == 1
+    i = v.unused_imports[0]
+    assert i.name == "bar"
+    assert i.first_lineno == 1
+    assert i.last_lineno == 1
+
+
+def test_item_property(v):
+    v.scan(
+        """\
+@awesomify
+class Foo:
+    @property
+    @wifi(
+        username='dog',
+        password='cat',
+    )
+    def bar(self):
+        pass
+Foo()
+"""
+    )
+    assert len(v.unused_props) == 1
+    p = v.unused_props[0]
+    assert p.name == "bar"
+    assert p.first_lineno == 3
+    assert p.last_lineno == 9
+
+
+def test_item_variable(v):
+    v.scan("v = 'Vulture'")
+    assert len(v.unused_vars) == 1
+    var = v.unused_vars[0]
+    assert var.name == "v"
+    assert var.first_lineno == 1
+    assert var.last_lineno == 1

tests/test_report.py

@@ -45,10 +45,8 @@ def test_item_report(check_report):
 {filename}:8: unused attribute 'foobar' (60% confidence)
 {filename}:9: unused variable 'foobar' (60% confidence)
 {filename}:11: unreachable code after 'return' (100% confidence)
-{filename}:14: unused property 'myprop' (60% confidence)
+{filename}:13: unused property 'myprop' (60% confidence)
 """
-    if sys.version_info >= (3, 8):
-        expected = expected.replace("{filename}:13:", "{filename}:14:")
     check_report(mock_code, expected)
 
 
@@ -60,8 +58,6 @@ def test_make_whitelist(check_report):
 _.foobar  # unused attribute ({filename}:8)
 foobar  # unused variable ({filename}:9)
 # unreachable code after 'return' ({filename}:11)
-_.myprop  # unused property ({filename}:14)
+_.myprop  # unused property ({filename}:13)
 """
-    if sys.version_info >= (3, 8):
-        expected = expected.replace("{filename}:13)", "{filename}:14)")
     check_report(mock_code, expected, make_whitelist=True)

tests/test_scavenging.py

@@ -653,82 +653,3 @@ def foo(foo_li):
     )
 
     check(v.unused_imports, ["List", "Text"])
-
-
-def test_item_attr(v):
-    v.scan("foo.bar = 'bar'")
-    assert len(v.unused_attrs) == 1
-    a = v.unused_attrs[0]
-    assert a.name == "bar"
-    assert a.first_lineno == 1
-    assert a.last_lineno == 1
-
-
-def test_item_class(v):
-    v.scan(
-        """\
-class Foo:
-    pass
-"""
-    )
-    assert len(v.unused_classes) == 1
-    c = v.unused_classes[0]
-    assert c.name == "Foo"
-    assert c.first_lineno == 1
-    assert c.last_lineno == 2
-
-
-def test_item_function(v):
-    v.scan(
-        """\
-def add(a, b):
-    return a + b
-"""
-    )
-    assert len(v.unused_funcs) == 1
-    f = v.unused_funcs[0]
-    assert f.name == "add"
-    assert f.first_lineno == 1
-    assert f.last_lineno == 2
-
-
-def test_item_import(v):
-    v.scan(
-        """\
-import bar
-from foo import *
-"""
-    )
-    assert len(v.unused_imports) == 1
-    i = v.unused_imports[0]
-    assert i.name == "bar"
-    assert i.first_lineno == 1
-    assert i.last_lineno == 1
-
-
-def test_item_property(v):
-    v.scan(
-        """\
-class Foo:
-    @awesomify
-    @property
-    @wifi
-    def bar(self):
-        pass
-Foo()
-"""
-    )
-    assert len(v.unused_props) == 1
-    p = v.unused_props[0]
-    assert p.name == "bar"
-    assert p.first_lineno == 5
-    assert p.last_lineno == 6
-
-
-def test_item_variable(v):
-    v.scan("v = 'Vulture'")
-    assert len(v.unused_vars) == 1
-    var = v.unused_vars[0]
-    assert var.name == "v"
-    assert var.first_lineno == 1
-    assert var.last_lineno == 1

tests/test_size.py

@@ -22,18 +22,18 @@ def get_last_line_number_slow(node):
     return max(getattr(node, "lineno", -1) for node in ast.walk(node))
 
 
-def count_lines(node):
+def count_lines(node, typ):
     """Estimate the number of lines of the given AST node."""
     last_lineno = lines.get_last_line_number(node)
     assert get_last_line_number_slow(node) == last_lineno
-    return last_lineno - node.lineno + 1
+    return last_lineno - lines.get_first_line_number(node, typ) + 1
 
 
 def check_size(example, size):
     tree = ast.parse(example)
     for node in tree.body:
         if isinstance(node, ast.ClassDef) and node.name == "Foo":
-            assert count_lines(node) == size
+            assert count_lines(node, 'class') == size
             break
     else:
         raise AssertionError('Failed to find top-level class "Foo" in code')
@@ -79,17 +79,15 @@ def bar(self):
             pass
 """
     size = 11
-    if sys.version_info >= (3, 8):
-        size = 10
     check_size(example, size)
 
 
-def test_size_property_def():
+def test_size_decorated_class():
     example = """
+@foo
+@property
+@xoo
 class Foo:
-    @foo
-    @property
-    @xoo
     def zoo(self):
         pass
 """

vulture/core.py

@@ -461,23 +461,14 @@ def _define(
         confidence=DEFAULT_CONFIDENCE,
         ignore=None,
     ):
-        def _get_first_lineno(first_node, typ):
-            if typ == "property" and sys.version_info < (3, 8):
-                # For python < 3.8, increment as many line numbers as there are
-                # decorators on the method.
-                # From Python3.8 onwards, lineno for property is the same as
-                # that of decorated method.
-                return first_node.lineno + len(first_node.decorator_list)
-            return first_node.lineno
-
         last_node = last_node or first_node
         typ = collection.typ
         if (ignore and ignore(self.filename, name)) or _match(
             name, self.ignore_names
         ):
             self._log('Ignoring {typ} "{name}"'.format(**locals()))
         else:
-            first_lineno = _get_first_lineno(first_node, typ)
+            first_lineno = lines.get_first_line_number(first_node, typ)
             last_lineno = lines.get_last_line_number(last_node)
             collection.append(
                 Item(

vulture/lines.py

@@ -1,4 +1,5 @@
 import ast
+import sys
 
 
 def _get_last_child_with_lineno(node):
@@ -63,3 +64,19 @@ def get_last_line_number(node):
             except AttributeError:
                 pass
         node = last_child
+
+
+def get_first_line_number(first_node, typ):
+    decorated_typs = ("class", "function", "property")
+    if typ in decorated_typs and sys.version_info >= (3, 8):
+        # From Python3.8 onwards, lineno for decorated objects is the
+        # line at which the object definition starts, which is
+        # different from what Python < 3.8 did -- reporting the lineno
+        # of the first decorator. To preserve this behaviour of Vulture
+        # for newer Python versions, which is also more accurate for
+        # counting the size of the unused code chunk (if the property
+        # is unused, we also don't need it's decorators), we return the
+        # lineno of the first decorator, if there are any.
+        if first_node.decorator_list:
+            return first_node.decorator_list[0].lineno
+    return first_node.lineno