Make the requirements to be a recursive call stricter (only top-level…

… recursion). Also add some tests.

tests/test_recursion.py

@@ -0,0 +1,90 @@
+from . import check, v
+
+assert v  # Silence pyflakes.
+
+
+def test_recursion1(v):
+    v.scan(
+        """\
+def Rec():
+    Rec()
+"""
+    )
+    check(v.defined_funcs, ["Rec"])
+    check(v.unused_funcs, ["Rec"])
+
+
+def test_recursion2(v):
+    v.scan(
+        """\
+def Rec():
+    Rec()
+
+class MyClass:
+    def __init__(self):
+        pass
+
+    def Rec():
+        Rec() # calls global Rec()
+
+def main():
+    main()
+"""
+    )
+    check(v.defined_funcs, ["Rec", "Rec", "main"])
+    check(v.unused_funcs, ["main"])
+
+
+def test_recursion3(v):
+    v.scan(
+        """\
+class MyClass:
+    def __init__(self):
+        pass
+
+    def Rec():
+        pass
+
+def Rec():
+    MyClass.Rec()
+"""
+    )
+    check(v.defined_funcs, ["Rec", "Rec"])
+    check(v.unused_funcs, [])
+    # MyClass.Rec() is not treated as a recursive call. So, MyClass.Rec is marked as used, causing Rec to also
+    # be marked as used (in Vulture's current behaviour) since they share the same name.
+
+
+def test_recursion4(v):
+    v.scan(
+        """\
+def Rec():
+    Rec()
+
+class MyClass:
+    def __init__(self):
+        pass
+
+    def Rec():
+        pass
+"""
+    )
+    check(v.defined_funcs, ["Rec", "Rec"])
+    check(v.unused_funcs, ["Rec", "Rec"])
+
+
+def test_recursion5(v):
+    v.scan(
+        """\
+def rec():
+    if (5 > 4):
+        rec()
+
+def outer():
+    def inner():
+        outer() # these calls aren't considered for recursion
+        inner()
+"""
+    )
+    check(v.defined_funcs, ["rec", "outer", "inner"])
+    check(v.unused_funcs, ["rec"])

vulture/core.py

@@ -643,7 +643,7 @@ def visit_Name(self, node):
         if (
             isinstance(node.ctx, (ast.Load, ast.Del))
             and node.id not in IGNORED_VARIABLE_NAMES
-            and not utils.recursive_call(node)
+            and not utils.top_lvl_recursive_call(node)
         ):
             self.used_names.add(node.id)
         elif isinstance(node.ctx, (ast.Param, ast.Store)):

vulture/utils.py

@@ -3,7 +3,7 @@
 import pathlib
 import sys
 import tokenize
-from typing import Optional, Union
+from typing import Optional, Tuple
 
 
 class VultureInputException(Exception):
@@ -120,23 +120,33 @@ def add_parent_info(root: ast.AST) -> None:
             child.parent = node
 
 
-def enclosing_function(
-    node: ast.AST,
-) -> Optional[Union[ast.FunctionDef, ast.AsyncFunctionDef]]:
-    while node and not isinstance(
-        node, (ast.FunctionDef, ast.AsyncFunctionDef)
-    ):
-        node = getattr(node, "parent", None)
-    return node
-
-
 def parent(node: ast.AST) -> Optional[ast.AST]:
     return getattr(node, "parent", None)
 
 
-def recursive_call(node: ast.Name) -> bool:
+def ancestor(node: ast.AST, ancestor_type: Tuple[type]) -> Optional[ast.AST]:
+    while node and not isinstance(node, ancestor_type):
+        node = getattr(node, "parent", None)
+    return node
+
+
+def top_lvl_recursive_call(node: ast.Name) -> bool:
+    """Returns true if a recursive call is made from a top level function to itself."""
+    # ideas:
+    # get rid of '.' not in ast.unparse check
+    # try to use lineno of func (get from call_node.func)
     assert isinstance(node, ast.Name)
-    return node.id == getattr(enclosing_function(node), "name", None)
+    return (
+        (call_node := ancestor(node, (ast.Call,)))
+        and (
+            enclosing_func := ancestor(
+                node, (ast.FunctionDef, ast.AsyncFunctionDef)
+            )
+        )
+        and node.id == enclosing_func.name
+        and enclosing_func.col_offset == 0
+        and "." not in ast.unparse(call_node)
+    )
 
 
 class LoggingList(list):