diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d999d2..79ed3f17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # next (unreleased) +* Add an option to mark most functions only called recursively as unused (John Doknjas, #374). * Add type hints for `get_unused_code` and the fields of the `Item` class (John Doknjas, #361). # 2.13 (2024-10-02) diff --git a/README.md b/README.md index b46c46d5..b0c1302b 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,16 @@ function arguments, e.g., `def foo(x, _y)`. Raise the minimum [confidence value](#types-of-unused-code) with the `--min-confidence` flag. +#### Verbose output + +For more verbose output, use the `--verbose` (or `-v`) flag. + +#### Not counting recursion + +It's possible that a function is only called by itself. The `--recursion` (or `-r`) flag will get +vulture to mark most such functions as unused. Note that this will have some performance cost, +and requires python >= 3.9. + #### Unreachable code If Vulture complains about code like `if False:`, you can use a Boolean diff --git a/tests/__init__.py b/tests/__init__.py index c54b3877..c362e160 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -42,3 +42,8 @@ def check_unreachable(v, lineno, size, name): @pytest.fixture def v(): return core.Vulture(verbose=True) + + +@pytest.fixture +def v_rec(): + return core.Vulture(verbose=True, recursion=True) diff --git a/tests/test_config.py b/tests/test_config.py index 2d55612b..8aed14d3 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -39,6 +39,7 @@ def test_cli_args(): min_confidence=10, sort_by_size=True, verbose=True, + recursion=True, ) result = _parse_args( [ @@ -51,6 +52,7 @@ def test_cli_args(): "--verbose", "path1", "path2", + "-r", ] ) assert isinstance(result, dict) @@ -70,6 +72,7 @@ def test_toml_config(): min_confidence=10, sort_by_size=True, verbose=True, + recursion=True, ) data = get_toml_bytes( dedent( @@ -82,6 +85,7 @@ def test_toml_config(): min_confidence = 10 sort_by_size = true verbose = true + recursion = true paths = ["path1", "path2"] """ ) @@ -146,6 +150,7 @@ def test_config_merging(): min_confidence = 10 sort_by_size = false verbose = false + recursion = false paths = ["toml_path"] """ ) @@ -158,6 +163,7 @@ def test_config_merging(): "--min-confidence=20", "--sort-by-size", "--verbose", + "--recursion", "cli_path", ] result = make_config(cliargs, toml) @@ -171,6 +177,7 @@ def test_config_merging(): min_confidence=20, sort_by_size=True, verbose=True, + recursion=True, ) assert result == expected diff --git a/tests/test_recursion.py b/tests/test_recursion.py new file mode 100644 index 00000000..ecfd4f2a --- /dev/null +++ b/tests/test_recursion.py @@ -0,0 +1,207 @@ +import sys + +from . import check, v, v_rec + +assert v +assert v_rec + +new_version = sys.version_info.minor >= 9 + + +def test_recursion1(v, v_rec): + code = """\ +class MyClass: + def __init__(self): + pass + + def inst(self): + self.inst() + + def inst2(self): + self.inst2() + + def Rec2(): + MyClass.Rec2() + + @classmethod + def Rec3(): + MyClass.Rec3() + + @staticmethod + def Rec4(): + MyClass.Rec4() + +def inst2(): + o = MyClass() + o.inst2() + +def Rec5(): + Rec5() + +def Rec6(): + Rec6() +""" + v_rec.scan(code) + defined_funcs = ["Rec2", "inst2", "Rec5", "Rec6"] + defined_methods = ["inst", "inst2", "Rec3", "Rec4"] + check(v_rec.defined_funcs, defined_funcs) + check(v_rec.defined_methods, defined_methods) + if new_version: + check(v_rec.unused_funcs, ["Rec2", "Rec5", "Rec6"]) + check(v_rec.unused_methods, ["inst", "Rec3", "Rec4"]) + else: + check(v_rec.unused_funcs, []) + check(v_rec.unused_methods, []) + v.scan(code) + check(v.defined_funcs, defined_funcs) + check(v.unused_funcs, []) + check(v.defined_methods, defined_methods) + check(v.unused_methods, []) + + +def test_recursion2(v, v_rec): + code = """\ +def Rec(): + Rec() + +class MyClass: + def __init__(self): + pass + + def Rec(self): + pass +""" + defined_funcs = ["Rec"] + defined_methods = ["Rec"] + v_rec.scan(code) + check(v_rec.defined_funcs, defined_funcs) + check(v_rec.defined_methods, defined_methods) + check(v_rec.unused_funcs, defined_funcs if new_version else []) + check(v_rec.unused_methods, defined_methods if new_version else []) + v.scan(code) + check(v.defined_funcs, defined_funcs) + check(v.defined_methods, defined_methods) + check(v.unused_funcs, []) + check(v.unused_methods, []) + + +def test_recursion3(v, v_rec): + code = """\ +def rec(): + if 5 > 4: + if 5 > 4: + rec() + +def outer(): + def inner(): + # the following calls are within a function within a function, so they + # are disregarded from recursion candidacy (to keep things simple) + outer() + inner() + +class MyClass: + def instMethod(self): + if 5 > 4: + if 5 > 4: + self.instMethod() + def classMethod1(): + if 5 > 4: + if 5 > 4: + MyClass.classMethod1() + @classmethod + def classMethod2(): + if 5 > 4: + if 5 > 4: + MyClass.classMethod2() + @staticmethod + def classMethod3(): + if 5 > 4: + if 5 > 4: + MyClass.classMethod3() +""" + v_rec.scan(code) + defined_funcs = ["rec", "outer", "inner", "classMethod1"] + defined_methods = ["instMethod", "classMethod2", "classMethod3"] + check(v_rec.defined_funcs, defined_funcs) + check(v_rec.defined_methods, defined_methods) + if new_version: + check(v_rec.unused_funcs, ["rec", "classMethod1"]) + check(v_rec.unused_methods, defined_methods) + else: + check(v_rec.unused_funcs, []) + check(v_rec.unused_methods, []) + v.scan(code) + check(v.defined_funcs, defined_funcs) + check(v.defined_methods, defined_methods) + check(v.unused_funcs, []) + check(v.unused_methods, []) + + +def test_recursion4(v, v_rec): + code = """\ +def rec(num: int): + if num > 4: + x = 1 + (rec ((num + num) / 3) / 2) + return x +""" + v_rec.scan(code) + defined_funcs = ["rec"] + check(v_rec.defined_funcs, defined_funcs) + check(v_rec.unused_funcs, defined_funcs if new_version else []) + v.scan(code) + check(v.defined_funcs, defined_funcs) + check(v.unused_funcs, []) + + +def test_recursion5(v, v_rec): + code = """\ +def rec(num: int): + for i in (1, num): + rec(i) +rec(2) + +class myClass: + def instMethod(self, num2): + for i2 in (1, num2): + self.instMethod(i2) + myClass.classMethod1(1) + myClass.classMethod2(1) + myClass.classMethod3(1) + def classMethod1(num3): + for i3 in (1, num3): + myClass.classMethod1(i3) + @classmethod + def classMethod2(num4): + for i4 in (1, num4): + myClass.classMethod2(i4) + @staticmethod + def classMethod3(num5): + for i5 in (1, num5): + myClass.classMethod3(i5) +o = MyClass() +o.instMethod() +""" + defined_funcs = ["rec", "classMethod1"] + defined_methods = ["instMethod", "classMethod2", "classMethod3"] + defined_vars = [ + "num", + "i", + "num2", + "i2", + "num3", + "i3", + "num4", + "i4", + "num5", + "i5", + "o", + ] + v_rec.scan(code) + v.scan(code) + for v_curr in (v_rec, v): + check(v_curr.defined_funcs, defined_funcs) + check(v_curr.unused_funcs, []) + check(v_curr.defined_methods, defined_methods) + check(v_curr.unused_methods, []) + check(v_curr.defined_vars, defined_vars) + check(v_curr.unused_vars, []) diff --git a/vulture/config.py b/vulture/config.py index 87dd5739..0bb3655f 100644 --- a/vulture/config.py +++ b/vulture/config.py @@ -23,6 +23,7 @@ "make_whitelist": False, "sort_by_size": False, "verbose": False, + "recursion": False, } @@ -169,6 +170,9 @@ def csv(exclude): "-v", "--verbose", action="store_true", default=missing ) parser.add_argument("--version", action="version", version=version) + parser.add_argument( + "-r", "--recursion", action="store_true", default=missing + ) namespace = parser.parse_args(args) cli_args = { key: value diff --git a/vulture/core.py b/vulture/core.py index 2cadabb6..d5e68f81 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -196,9 +196,14 @@ class Vulture(ast.NodeVisitor): """Find dead code.""" def __init__( - self, verbose=False, ignore_names=None, ignore_decorators=None + self, + verbose=False, + ignore_names=None, + ignore_decorators=None, + recursion=False, ): self.verbose = verbose + self.recursion = recursion and sys.version_info.minor >= 9 def get_list(typ): return utils.LoggingList(typ, self.verbose) @@ -252,7 +257,9 @@ def handle_syntax_error(e): ) self.exit_code = ExitCode.InvalidInput else: - # When parsing type comments, visiting can throw SyntaxError. + if self.recursion: + utils.add_parents(node) + # When parsing type comments, visiting can throw a SyntaxError: try: self.visit(node) except SyntaxError as err: @@ -513,7 +520,9 @@ def visit_AsyncFunctionDef(self, node): 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): + elif isinstance(node.ctx, ast.Load) and not getattr( + node, "recursive", None + ): self.used_names.add(node.attr) def visit_BinOp(self, node): @@ -552,6 +561,9 @@ def visit_Call(self, node): ): self._handle_new_format_string(node.func.value.value) + if self.recursion and isinstance(node.func, (ast.Name, ast.Attribute)): + node.func.recursive = utils.recursive_call(node.func) + def _handle_new_format_string(self, s): def is_identifier(name): return bool(re.match(r"[a-zA-Z_][a-zA-Z0-9_]*", name)) @@ -647,6 +659,7 @@ def visit_Name(self, node): if ( isinstance(node.ctx, (ast.Load, ast.Del)) and node.id not in IGNORED_VARIABLE_NAMES + and not getattr(node, "recursive", None) ): self.used_names.add(node.id) elif isinstance(node.ctx, (ast.Param, ast.Store)): @@ -737,6 +750,7 @@ def main(): verbose=config["verbose"], ignore_names=config["ignore_names"], ignore_decorators=config["ignore_decorators"], + recursion=config["recursion"], ) vulture.scavenge(config["paths"], exclude=config["exclude"]) sys.exit( diff --git a/vulture/utils.py b/vulture/utils.py index 3382dca3..876d331a 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -3,6 +3,7 @@ import pathlib import sys import tokenize +from typing import Optional, Union class VultureInputException(Exception): @@ -111,6 +112,59 @@ def read_file(filename): raise VultureInputException from err +def add_parents(root: ast.AST) -> None: + # https://stackoverflow.com/a/43311383/7743427: + root.parent = None + for node in ast.walk(root): + for child in ast.iter_child_nodes(node): + child.parent = node + + +def parent(node: Optional[ast.AST]) -> Optional[ast.AST]: + return getattr(node, "parent", None) + + +def ancestor( + node: ast.AST, target_ancestor_types, limit: int +) -> Optional[ast.AST]: + """`limit` is how far back to search before giving up and returning `None` instead""" + assert limit < 100 + if limit == 0: + return None + if not (node := parent(node)) or isinstance(node, target_ancestor_types): + return node + return ancestor(node, target_ancestor_types, limit - 1) + + +def call_info_no_args(call_node: ast.Call) -> str: + """Returns a string of the portion of the function call that's before the parenthesized arg list.""" + assert sys.version_info.minor >= 9 + return ast.unparse(call_node).split("(")[0] + + +def recursive_call(node: Union[ast.Name, ast.Attribute]) -> Optional[bool]: + """Returns a boolean if it can be determined the node is part of a recursive call. + Otherwise if the function is nested in a complicated way, `None` is returned.""" + if not isinstance((call_node := parent(node)), ast.Call) or not ( + func := ancestor(node, (ast.FunctionDef, ast.AsyncFunctionDef), 10) + ): + return False + if isinstance((func_parent := parent(func)), ast.Module): + return call_info_no_args(call_node) == func.name + if not isinstance(func_parent, ast.ClassDef) or not isinstance( + parent(func_parent), ast.Module + ): + return None + return call_info_no_args(call_node).split(".") == [ + ( + "self" + if "self" == next((x.arg for x in func.args.args), None) + else func_parent.name + ), + func.name, + ] + + class LoggingList(list): def __init__(self, typ, verbose): self.typ = typ