diff --git a/tests/test_recursion.py b/tests/test_recursion.py new file mode 100644 index 00000000..c434aeab --- /dev/null +++ b/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"]) diff --git a/vulture/core.py b/vulture/core.py index a8bbde11..41a68285 100644 --- a/vulture/core.py +++ b/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)): diff --git a/vulture/utils.py b/vulture/utils.py index 6fafdb01..4cdf5c59 100644 --- a/vulture/utils.py +++ b/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):