From 0a6bd9a577a2e3a898cc34c3d2f83a21af3eec45 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Sat, 5 Oct 2024 16:21:51 -0700 Subject: [PATCH 1/9] If a function is called within a function of the same name, do not count that as being used. --- vulture/core.py | 4 +++- vulture/utils.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/vulture/core.py b/vulture/core.py index b3c845cd..a8bbde11 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -250,7 +250,8 @@ def handle_syntax_error(e): ) self.exit_code = ExitCode.InvalidInput else: - # When parsing type comments, visiting can throw SyntaxError. + utils.add_parent_info(node) + # When parsing type comments, visiting can throw a SyntaxError: try: self.visit(node) except SyntaxError as err: @@ -642,6 +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) ): 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 3382dca3..6fafdb01 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,33 @@ def read_file(filename): raise VultureInputException from err +def add_parent_info(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 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: + assert isinstance(node, ast.Name) + return node.id == getattr(enclosing_function(node), "name", None) + + class LoggingList(list): def __init__(self, typ, verbose): self.typ = typ From 682241af665cd70e1735ee2a4901869c8616e0a9 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Sun, 6 Oct 2024 04:31:40 -0700 Subject: [PATCH 2/9] Make the requirements to be a recursive call stricter (only top-level recursion). Also add some tests. --- tests/test_recursion.py | 90 +++++++++++++++++++++++++++++++++++++++++ vulture/core.py | 2 +- vulture/utils.py | 36 +++++++++++------ 3 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 tests/test_recursion.py 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): From 2d063dc5570519669f892387242100c9051821b5 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Sun, 6 Oct 2024 05:28:26 -0700 Subject: [PATCH 3/9] Don't check for a dot not being in the function call (shouldn't be necessary). --- vulture/utils.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/vulture/utils.py b/vulture/utils.py index 4cdf5c59..ac476980 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -3,7 +3,7 @@ import pathlib import sys import tokenize -from typing import Optional, Tuple +from typing import Optional class VultureInputException(Exception): @@ -120,32 +120,21 @@ def add_parent_info(root: ast.AST) -> None: child.parent = node -def parent(node: ast.AST) -> Optional[ast.AST]: - return getattr(node, "parent", None) - - -def ancestor(node: ast.AST, ancestor_type: Tuple[type]) -> Optional[ast.AST]: - while node and not isinstance(node, ancestor_type): +def ancestor(node: ast.AST, target_ancestor_types) -> Optional[ast.AST]: + while node and not isinstance(node, target_ancestor_types): 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 ( - (call_node := ancestor(node, (ast.Call,))) - and ( - enclosing_func := ancestor( - node, (ast.FunctionDef, ast.AsyncFunctionDef) - ) - ) + enclosing_func = ancestor(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + return bool( + enclosing_func and node.id == enclosing_func.name and enclosing_func.col_offset == 0 - and "." not in ast.unparse(call_node) + and ancestor(node, ast.Call) ) From d202b0a549bccd99cea5c8a17acb03f245aa2747 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Tue, 22 Oct 2024 18:58:35 -0700 Subject: [PATCH 4/9] Get the recursion feature to work for class/instance methods as well. Also, disable the feature by default, and add a `--recursion` flag for if the user wants to do it. --- CHANGELOG.md | 4 + README.md | 9 +++ tests/__init__.py | 5 ++ tests/test_recursion.py | 167 ++++++++++++++++++++++++++++++---------- vulture/config.py | 4 + vulture/core.py | 20 ++++- vulture/utils.py | 60 +++++++++++---- 7 files changed, 206 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a944eb1..bce1f3da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# next (unreleased) + +* Add an option to mark most functions only called recursively as unused (John Doknjas, #374). + # 2.13 (2024-10-02) * Add support for Python 3.13 (Jendrik Seipp, #369). diff --git a/README.md b/README.md index 45bb3c44..3570adb2 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,15 @@ 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. + #### 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_recursion.py b/tests/test_recursion.py index c434aeab..eab920ba 100644 --- a/tests/test_recursion.py +++ b/tests/test_recursion.py @@ -1,63 +1,104 @@ -from . import check, v +from . import check, v, v_rec -assert v # Silence pyflakes. +assert v +assert v_rec -def test_recursion1(v): - v.scan( - """\ +def test_recursion1(v, v_rec): + code = """\ def Rec(): Rec() """ - ) + v_rec.scan(code) + check(v_rec.defined_funcs, ["Rec"]) + check(v_rec.unused_funcs, ["Rec"]) + v.scan(code) check(v.defined_funcs, ["Rec"]) - check(v.unused_funcs, ["Rec"]) - + check(v.unused_funcs, []) -def test_recursion2(v): - v.scan( - """\ -def Rec(): - Rec() +def test_recursion2(v, v_rec): + code = """\ class MyClass: def __init__(self): pass - def Rec(): - Rec() # calls global Rec() + def inst(self): + self.inst() + + def inst2(self): + self.inst2() + + def Rec2(): + MyClass.Rec2() -def main(): - main() + @classmethod + def Rec3(): + MyClass.Rec3() + + @staticmethod + def Rec4(): + MyClass.Rec4() + +def inst2(): + o = MyClass() + o.inst2() + +def Rec3(): + Rec3() + +def Rec4(): + Rec4() """ - ) - check(v.defined_funcs, ["Rec", "Rec", "main"]) - check(v.unused_funcs, ["main"]) + v_rec.scan(code) + check(v_rec.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_funcs, ["Rec2", "Rec3", "Rec4"]) + check(v_rec.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_methods, ["inst", "Rec3", "Rec4"]) + v.scan(code) + check(v.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) + check(v.unused_funcs, []) + check(v.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) + check(v.unused_methods, []) -def test_recursion3(v): - v.scan( - """\ +def test_recursion3(v, v_rec): + code = """\ class MyClass: def __init__(self): pass + @classmethod def Rec(): - pass + MyClass.Rec() -def Rec(): - MyClass.Rec() +def aa(): + aa() """ + v_rec.scan(code) + check( + v_rec.defined_funcs, + [ + "aa", + ], ) - check(v.defined_funcs, ["Rec", "Rec"]) + check(v_rec.defined_methods, ["Rec"]) + check(v_rec.unused_funcs, ["aa"]) + check(v_rec.unused_methods, ["Rec"]) + v.scan(code) + check( + v.defined_funcs, + [ + "aa", + ], + ) + check(v.defined_methods, ["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. + check(v.unused_methods, []) -def test_recursion4(v): - v.scan( - """\ +def test_recursion4(v, v_rec): + code = """\ def Rec(): Rec() @@ -68,23 +109,65 @@ def __init__(self): def Rec(): pass """ - ) + v_rec.scan(code) + check(v_rec.defined_funcs, ["Rec", "Rec"]) + check(v_rec.unused_funcs, ["Rec", "Rec"]) + v.scan(code) check(v.defined_funcs, ["Rec", "Rec"]) - check(v.unused_funcs, ["Rec", "Rec"]) + check(v.unused_funcs, []) -def test_recursion5(v): - v.scan( - """\ +def test_recursion5(v, v_rec): + code = """\ def rec(): - if (5 > 4): - rec() + if 5 > 4: + if 5 > 4: + rec() def outer(): def inner(): - outer() # these calls aren't considered for recursion + # the following calls are within a function within a function, so they + # are disregarded from recursion candidacy (to keep things simple) + outer() inner() """ - ) + v_rec.scan(code) + check(v_rec.defined_funcs, ["rec", "outer", "inner"]) + check(v_rec.unused_funcs, ["rec"]) + v.scan(code) check(v.defined_funcs, ["rec", "outer", "inner"]) - check(v.unused_funcs, ["rec"]) + check(v.unused_funcs, []) + + +def test_recursion6(v, v_rec): + code = """\ +def rec(num: int): + if num > 4: + x = 1 + (rec ((num + num) / 3) / 2) + return x +""" + v_rec.scan(code) + check(v_rec.defined_funcs, ["rec"]) + check(v_rec.unused_funcs, ["rec"]) + v.scan(code) + check(v.defined_funcs, ["rec"]) + check(v.unused_funcs, []) + + +def test_recursion7(v, v_rec): + code = """\ +def rec(num: int): + for i in (1, num): + rec(i) +rec(2) +""" + v_rec.scan(code) + check(v_rec.defined_funcs, ["rec"]) + check(v_rec.unused_funcs, []) + check(v_rec.defined_vars, ["num", "i"]) + check(v_rec.unused_vars, []) + v.scan(code) + check(v.defined_funcs, ["rec"]) + check(v.unused_funcs, []) + check(v.defined_vars, ["num", "i"]) + check(v.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 41a68285..01c86fca 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -195,9 +195,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 def get_list(typ): return utils.LoggingList(typ, self.verbose) @@ -250,7 +255,8 @@ def handle_syntax_error(e): ) self.exit_code = ExitCode.InvalidInput else: - utils.add_parent_info(node) + if self.recursion: + utils.add_parents(node) # When parsing type comments, visiting can throw a SyntaxError: try: self.visit(node) @@ -509,7 +515,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): @@ -548,6 +556,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)) @@ -643,7 +654,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.top_lvl_recursive_call(node) + and not getattr(node, "recursive", None) ): self.used_names.add(node.id) elif isinstance(node.ctx, (ast.Param, ast.Store)): @@ -734,6 +745,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 ac476980..1434ccfc 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -112,7 +112,7 @@ def read_file(filename): raise VultureInputException from err -def add_parent_info(root: ast.AST) -> None: +def add_parents(root: ast.AST) -> None: # https://stackoverflow.com/a/43311383/7743427: root.parent = None for node in ast.walk(root): @@ -120,22 +120,48 @@ def add_parent_info(root: ast.AST) -> None: child.parent = node -def ancestor(node: ast.AST, target_ancestor_types) -> Optional[ast.AST]: - while node and not isinstance(node, target_ancestor_types): - 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.""" - assert isinstance(node, ast.Name) - enclosing_func = ancestor(node, (ast.FunctionDef, ast.AsyncFunctionDef)) - return bool( - enclosing_func - and node.id == enclosing_func.name - and enclosing_func.col_offset == 0 - and ancestor(node, ast.Call) - ) +def parent(node: Optional[ast.AST]) -> ast.AST | None: + 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.""" + return ast.unparse(call_node).split("(")[0] + + +def recursive_call(node: 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): From 3781310fb30123ec7bac6d72ec12e98764d46577 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Tue, 22 Oct 2024 19:36:54 -0700 Subject: [PATCH 5/9] Update config tests to include the recursion option. --- tests/test_config.py | 7 +++++++ 1 file changed, 7 insertions(+) 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 From 8b43c0e08e79ffa86a6ad87f03ff4dc50d78764f Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Tue, 22 Oct 2024 19:48:11 -0700 Subject: [PATCH 6/9] Do not use the `|` type syntax. --- vulture/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vulture/utils.py b/vulture/utils.py index 1434ccfc..18e49977 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -3,7 +3,7 @@ import pathlib import sys import tokenize -from typing import Optional +from typing import Optional, Union class VultureInputException(Exception): @@ -120,7 +120,7 @@ def add_parents(root: ast.AST) -> None: child.parent = node -def parent(node: Optional[ast.AST]) -> ast.AST | None: +def parent(node: Optional[ast.AST]) -> Optional[ast.AST]: return getattr(node, "parent", None) @@ -141,7 +141,7 @@ def call_info_no_args(call_node: ast.Call) -> str: return ast.unparse(call_node).split("(")[0] -def recursive_call(node: ast.Name | ast.Attribute) -> Optional[bool]: +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 ( From c1346c2dd6d9891da5cfb12271840ab21f7b6f31 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Tue, 22 Oct 2024 20:07:39 -0700 Subject: [PATCH 7/9] Do not do the recursion feature if the python version is less than 3.9, since `ast.unparse` was added then. --- vulture/core.py | 2 +- vulture/utils.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/vulture/core.py b/vulture/core.py index fa76feda..d5e68f81 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -203,7 +203,7 @@ def __init__( recursion=False, ): self.verbose = verbose - self.recursion = recursion + self.recursion = recursion and sys.version_info.minor >= 9 def get_list(typ): return utils.LoggingList(typ, self.verbose) diff --git a/vulture/utils.py b/vulture/utils.py index 18e49977..876d331a 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -138,6 +138,7 @@ def ancestor( 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] From fe14db3ca653576ccea26521bf1cf0574705d016 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Tue, 22 Oct 2024 20:15:26 -0700 Subject: [PATCH 8/9] Update tests to account for the recursion feature not applying in python < 3.9. --- README.md | 3 +- tests/test_recursion.py | 62 +++++++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index e1e7bb92..b0c1302b 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,8 @@ 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. +vulture to mark most such functions as unused. Note that this will have some performance cost, +and requires python >= 3.9. #### Unreachable code diff --git a/tests/test_recursion.py b/tests/test_recursion.py index eab920ba..6c6d8538 100644 --- a/tests/test_recursion.py +++ b/tests/test_recursion.py @@ -1,8 +1,12 @@ +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 = """\ @@ -10,8 +14,12 @@ def Rec(): Rec() """ v_rec.scan(code) - check(v_rec.defined_funcs, ["Rec"]) - check(v_rec.unused_funcs, ["Rec"]) + if new_version: + check(v_rec.defined_funcs, ["Rec"]) + check(v_rec.unused_funcs, ["Rec"]) + else: + check(v_rec.defined_funcs, ["Rec"]) + check(v_rec.unused_funcs, []) v.scan(code) check(v.defined_funcs, ["Rec"]) check(v.unused_funcs, []) @@ -51,10 +59,16 @@ def Rec4(): Rec4() """ v_rec.scan(code) - check(v_rec.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) - check(v_rec.unused_funcs, ["Rec2", "Rec3", "Rec4"]) - check(v_rec.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) - check(v_rec.unused_methods, ["inst", "Rec3", "Rec4"]) + if new_version: + check(v_rec.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_funcs, ["Rec2", "Rec3", "Rec4"]) + check(v_rec.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_methods, ["inst", "Rec3", "Rec4"]) + else: + check(v_rec.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_funcs, []) + check(v_rec.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_methods, []) v.scan(code) check(v.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) check(v.unused_funcs, []) @@ -82,9 +96,15 @@ def aa(): "aa", ], ) - check(v_rec.defined_methods, ["Rec"]) - check(v_rec.unused_funcs, ["aa"]) - check(v_rec.unused_methods, ["Rec"]) + if new_version: + check(v_rec.defined_methods, ["Rec"]) + check(v_rec.unused_funcs, ["aa"]) + check(v_rec.unused_methods, ["Rec"]) + else: + check(v_rec.defined_methods, ["Rec"]) + check(v_rec.unused_funcs, []) + check(v_rec.unused_methods, []) + v.scan(code) check( v.defined_funcs, @@ -110,8 +130,12 @@ def Rec(): pass """ v_rec.scan(code) - check(v_rec.defined_funcs, ["Rec", "Rec"]) - check(v_rec.unused_funcs, ["Rec", "Rec"]) + if new_version: + check(v_rec.defined_funcs, ["Rec", "Rec"]) + check(v_rec.unused_funcs, ["Rec", "Rec"]) + else: + check(v_rec.defined_funcs, ["Rec", "Rec"]) + check(v_rec.unused_funcs, []) v.scan(code) check(v.defined_funcs, ["Rec", "Rec"]) check(v.unused_funcs, []) @@ -132,8 +156,12 @@ def inner(): inner() """ v_rec.scan(code) - check(v_rec.defined_funcs, ["rec", "outer", "inner"]) - check(v_rec.unused_funcs, ["rec"]) + if new_version: + check(v_rec.defined_funcs, ["rec", "outer", "inner"]) + check(v_rec.unused_funcs, ["rec"]) + else: + check(v_rec.defined_funcs, ["rec", "outer", "inner"]) + check(v_rec.unused_funcs, []) v.scan(code) check(v.defined_funcs, ["rec", "outer", "inner"]) check(v.unused_funcs, []) @@ -147,8 +175,12 @@ def rec(num: int): return x """ v_rec.scan(code) - check(v_rec.defined_funcs, ["rec"]) - check(v_rec.unused_funcs, ["rec"]) + if new_version: + check(v_rec.defined_funcs, ["rec"]) + check(v_rec.unused_funcs, ["rec"]) + else: + check(v_rec.defined_funcs, ["rec"]) + check(v_rec.unused_funcs, []) v.scan(code) check(v.defined_funcs, ["rec"]) check(v.unused_funcs, []) From 8f58034d4c44e34ba49fc87b203e1b0bd0a24830 Mon Sep 17 00:00:00 2001 From: johndoknjas Date: Wed, 23 Oct 2024 13:26:02 -0700 Subject: [PATCH 9/9] Test class/instance methods more. --- tests/test_recursion.py | 202 ++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 100 deletions(-) diff --git a/tests/test_recursion.py b/tests/test_recursion.py index 6c6d8538..ecfd4f2a 100644 --- a/tests/test_recursion.py +++ b/tests/test_recursion.py @@ -10,23 +10,6 @@ def test_recursion1(v, v_rec): code = """\ -def Rec(): - Rec() -""" - v_rec.scan(code) - if new_version: - check(v_rec.defined_funcs, ["Rec"]) - check(v_rec.unused_funcs, ["Rec"]) - else: - check(v_rec.defined_funcs, ["Rec"]) - check(v_rec.unused_funcs, []) - v.scan(code) - check(v.defined_funcs, ["Rec"]) - check(v.unused_funcs, []) - - -def test_recursion2(v, v_rec): - code = """\ class MyClass: def __init__(self): pass @@ -52,72 +35,31 @@ def inst2(): o = MyClass() o.inst2() -def Rec3(): - Rec3() +def Rec5(): + Rec5() -def Rec4(): - Rec4() +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.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) - check(v_rec.unused_funcs, ["Rec2", "Rec3", "Rec4"]) - check(v_rec.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) + check(v_rec.unused_funcs, ["Rec2", "Rec5", "Rec6"]) check(v_rec.unused_methods, ["inst", "Rec3", "Rec4"]) else: - check(v_rec.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) - check(v_rec.unused_funcs, []) - check(v_rec.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) - check(v_rec.unused_methods, []) - v.scan(code) - check(v.defined_funcs, ["Rec2", "inst2", "Rec3", "Rec4"]) - check(v.unused_funcs, []) - check(v.defined_methods, ["inst", "inst2", "Rec3", "Rec4"]) - check(v.unused_methods, []) - - -def test_recursion3(v, v_rec): - code = """\ -class MyClass: - def __init__(self): - pass - - @classmethod - def Rec(): - MyClass.Rec() - -def aa(): - aa() -""" - v_rec.scan(code) - check( - v_rec.defined_funcs, - [ - "aa", - ], - ) - if new_version: - check(v_rec.defined_methods, ["Rec"]) - check(v_rec.unused_funcs, ["aa"]) - check(v_rec.unused_methods, ["Rec"]) - else: - check(v_rec.defined_methods, ["Rec"]) check(v_rec.unused_funcs, []) check(v_rec.unused_methods, []) - v.scan(code) - check( - v.defined_funcs, - [ - "aa", - ], - ) - check(v.defined_methods, ["Rec"]) + check(v.defined_funcs, defined_funcs) check(v.unused_funcs, []) + check(v.defined_methods, defined_methods) check(v.unused_methods, []) -def test_recursion4(v, v_rec): +def test_recursion2(v, v_rec): code = """\ def Rec(): Rec() @@ -126,22 +68,24 @@ class MyClass: def __init__(self): pass - def Rec(): + def Rec(self): pass """ + defined_funcs = ["Rec"] + defined_methods = ["Rec"] v_rec.scan(code) - if new_version: - check(v_rec.defined_funcs, ["Rec", "Rec"]) - check(v_rec.unused_funcs, ["Rec", "Rec"]) - else: - check(v_rec.defined_funcs, ["Rec", "Rec"]) - check(v_rec.unused_funcs, []) + 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, ["Rec", "Rec"]) + check(v.defined_funcs, defined_funcs) + check(v.defined_methods, defined_methods) check(v.unused_funcs, []) + check(v.unused_methods, []) -def test_recursion5(v, v_rec): +def test_recursion3(v, v_rec): code = """\ def rec(): if 5 > 4: @@ -154,20 +98,46 @@ def inner(): # 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.defined_funcs, ["rec", "outer", "inner"]) - check(v_rec.unused_funcs, ["rec"]) + check(v_rec.unused_funcs, ["rec", "classMethod1"]) + check(v_rec.unused_methods, defined_methods) else: - check(v_rec.defined_funcs, ["rec", "outer", "inner"]) check(v_rec.unused_funcs, []) + check(v_rec.unused_methods, []) v.scan(code) - check(v.defined_funcs, ["rec", "outer", "inner"]) + check(v.defined_funcs, defined_funcs) + check(v.defined_methods, defined_methods) check(v.unused_funcs, []) + check(v.unused_methods, []) -def test_recursion6(v, v_rec): +def test_recursion4(v, v_rec): code = """\ def rec(num: int): if num > 4: @@ -175,31 +145,63 @@ def rec(num: int): return x """ v_rec.scan(code) - if new_version: - check(v_rec.defined_funcs, ["rec"]) - check(v_rec.unused_funcs, ["rec"]) - else: - check(v_rec.defined_funcs, ["rec"]) - check(v_rec.unused_funcs, []) + 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, ["rec"]) + check(v.defined_funcs, defined_funcs) check(v.unused_funcs, []) -def test_recursion7(v, v_rec): +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) - check(v_rec.defined_funcs, ["rec"]) - check(v_rec.unused_funcs, []) - check(v_rec.defined_vars, ["num", "i"]) - check(v_rec.unused_vars, []) v.scan(code) - check(v.defined_funcs, ["rec"]) - check(v.unused_funcs, []) - check(v.defined_vars, ["num", "i"]) - check(v.unused_vars, []) + 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, [])