From 53552098c4cdcb1d646ac065ea7c5cc5fb53c218 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Tue, 8 Sep 2020 15:10:19 -0400 Subject: [PATCH 01/15] added --absolute-paths option --- .gitignore | 1 + tests/test_config.py | 5 +++++ tests/test_utils.py | 19 +++++++++++++++++++ vulture/config.py | 9 +++++++++ vulture/core.py | 28 +++++++++++++++++++++------- vulture/utils.py | 21 +++++++++++++++++---- 6 files changed, 72 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 1593442a..8ee1e825 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ vulture.egg-info/ .pytest_cache/ .tox/ .venv/ +.idea/ diff --git a/tests/test_config.py b/tests/test_config.py index 4a9a9075..85668e77 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -27,6 +27,7 @@ def test_cli_args(): ignore_names=["name1", "name2"], make_whitelist=True, min_confidence=10, + absolute_paths=True, sort_by_size=True, verbose=True, ) @@ -39,6 +40,7 @@ def test_cli_args(): "--min-confidence=10", "--sort-by-size", "--verbose", + "--absolute-paths", "path1", "path2", ] @@ -97,6 +99,7 @@ def test_config_merging(): min_confidence = 10 sort_by_size = false verbose = false + absolute_paths = true paths = ["toml_path"] """ ) @@ -108,6 +111,7 @@ def test_config_merging(): "--make-whitelist", "--min-confidence=20", "--sort-by-size", + "--absolute-paths", "--verbose", "cli_path", ] @@ -120,6 +124,7 @@ def test_config_merging(): make_whitelist=True, min_confidence=20, sort_by_size=True, + absolute_paths=True, verbose=True, ) assert result == expected diff --git a/tests/test_utils.py b/tests/test_utils.py index 176dc0c7..07e4e74a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,8 +1,27 @@ import ast +import os +import pathlib from vulture import utils +def check_paths(filename, absolute=False): + pathstr = utils.format_path(filename, absolute) + # platform dependencies and path types need to be accounted for + pp = pathlib.PurePath(pathstr) + check = pp.is_absolute() + if absolute: + assert check == True + # even if absolute == True, the path might have been specified absolute + # so can't conclude negatively + +def test_absolute_path(): + check_paths(__file__, absolute=True) + +def test_relative_path(): + check_paths(__file__, absolute=False) + + def check_decorator_names(code, expected_names): decorator_names = [] diff --git a/vulture/config.py b/vulture/config.py index 45ae0c38..28df0775 100644 --- a/vulture/config.py +++ b/vulture/config.py @@ -20,6 +20,7 @@ "make_whitelist": False, "sort_by_size": False, "verbose": False, + "absolute_paths": False } @@ -69,6 +70,7 @@ def _parse_toml(infile): make_whitelist = true min_confidence = 10 sort_by_size = true + absolute_paths = false verbose = true paths = ["path1", "path2"] """ @@ -149,6 +151,13 @@ def csv(exclude): default=missing, help="Sort unused functions and classes by their lines of code.", ) + parser.add_argument( + "--absolute-paths", + action="store_true", + default=False, + required=False, + help="Output absolute file paths.", + ) parser.add_argument( "-v", "--verbose", action="store_true", default=missing ) diff --git a/vulture/core.py b/vulture/core.py index 8a080c69..2966188a 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -103,6 +103,7 @@ class Item: "last_lineno", "message", "confidence", + "absolute_paths" ) def __init__( @@ -114,6 +115,7 @@ def __init__( last_lineno, message="", confidence=DEFAULT_CONFIDENCE, + absolute_paths=False ): self.name = name self.typ = typ @@ -122,6 +124,7 @@ def __init__( self.last_lineno = last_lineno self.message = message or f"unused {typ} '{name}'" self.confidence = confidence + self.absolute_paths = absolute_paths @property def size(self): @@ -135,7 +138,7 @@ def get_report(self, add_size=False): else: size_report = "" return "{}:{:d}: {} ({}% confidence{})".format( - utils.format_path(self.filename), + utils.format_path(self.filename, absolute=self.absolute_paths), self.first_lineno, self.message, self.confidence, @@ -143,7 +146,7 @@ def get_report(self, add_size=False): ) def get_whitelist_string(self): - filename = utils.format_path(self.filename) + filename = utils.format_path(self.filename, self.absolute_paths) if self.typ == "unreachable_code": return f"# {self.message} ({filename}:{self.first_lineno})" else: @@ -171,10 +174,10 @@ 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, absolute_paths=False ): self.verbose = verbose - + self.absolute_paths = absolute_paths def get_list(typ): return utils.LoggingList(typ, self.verbose) @@ -201,17 +204,18 @@ def scan(self, code, filename=""): self.noqa_lines = noqa.parse_noqa(self.code) self.filename = filename + abs_paths = self.absolute_paths def handle_syntax_error(e): text = f' at "{e.text.strip()}"' if e.text else "" print( - f"{utils.format_path(filename)}:{e.lineno}: {e.msg}{text}", + f"{utils.format_path(filename, absolute=abs_paths)}:{e.lineno}: {e.msg}{text}", file=sys.stderr, ) self.found_dead_code_or_error = True try: node = ( - ast.parse(code, filename=self.filename, type_comments=True) + ast.parse(code, filename=self.filename, type_comments=True) # fails if not python 3.8.x if sys.version_info >= (3, 8) # type_comments requires 3.8+ else ast.parse(code, filename=self.filename) ) @@ -220,7 +224,7 @@ def handle_syntax_error(e): except ValueError as err: # ValueError is raised if source contains null bytes. print( - f'{utils.format_path(filename)}: invalid source code "{err}"', + f'{utils.format_path(filename, absolute=self.absolute_paths)}: invalid source code "{err}"', file=sys.stderr, ) self.found_dead_code_or_error = True @@ -452,6 +456,7 @@ def ignored(lineno): last_lineno, message=message, confidence=confidence, + absolute_paths=self.absolute_paths ) ) @@ -733,6 +738,14 @@ def csv(exclude): action="store_true", help="Sort unused functions and classes by their lines of code.", ) + parser.add_argument( + "--absolute-paths", + action="store_true", + default=False, + required=False, + help="Output absolute file paths.", + ) + parser.add_argument("-v", "--verbose", action="store_true") parser.add_argument("--version", action="version", version=version) return parser.parse_args() @@ -744,6 +757,7 @@ def main(): verbose=config["verbose"], ignore_names=config["ignore_names"], ignore_decorators=config["ignore_decorators"], + absolute_paths=config["absolute_paths"] ) vulture.scavenge(config["paths"], exclude=config["exclude"]) sys.exit( diff --git a/vulture/utils.py b/vulture/utils.py index 0951f8ff..f458f5d2 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -2,6 +2,7 @@ import os import sys import tokenize +from pathlib import Path class VultureInputException(Exception): @@ -45,10 +46,20 @@ def condition_is_always_true(condition): return _safe_eval(condition, False) -def format_path(path): +def format_path(path, absolute=False): if not path: return path - relpath = os.path.relpath(path) + if not absolute: + relpath = os.path.relpath(path) + # print(f'relative path {relpath}', file=sys.stderr, flush=True) + else: + # print('absolute paths set', file=sys.stderr) + pp = Path(path) + # make the path absolute, resolving any symlinks + resolved_path = pp.resolve(strict=True) + # relpath = os.path.abspath(path) + relpath = str(resolved_path) + # print(f'abs path {relpath}', file=sys.stderr, flush=True) return relpath if not relpath.startswith("..") else path @@ -96,7 +107,8 @@ class LoggingList(list): def __init__(self, typ, verbose): self.typ = typ self._verbose = verbose - return list.__init__(self) + # return list.__init__(self) + list.__init__(self) def append(self, item): if self._verbose: @@ -108,7 +120,8 @@ class LoggingSet(set): def __init__(self, typ, verbose): self.typ = typ self._verbose = verbose - return set.__init__(self) + # return set.__init__(self) + set.__init__(self) def add(self, name): if self._verbose: From 55c282b80ebf31f016a253398307f245462ec2e9 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Tue, 8 Sep 2020 15:33:29 -0400 Subject: [PATCH 02/15] blackened, de-linted --- tests/test_utils.py | 5 +++-- vulture/config.py | 2 +- vulture/core.py | 26 ++++++++++++++++++-------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 07e4e74a..cb8108c3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,4 @@ import ast -import os import pathlib from vulture import utils @@ -11,13 +10,15 @@ def check_paths(filename, absolute=False): pp = pathlib.PurePath(pathstr) check = pp.is_absolute() if absolute: - assert check == True + assert check # even if absolute == True, the path might have been specified absolute # so can't conclude negatively + def test_absolute_path(): check_paths(__file__, absolute=True) + def test_relative_path(): check_paths(__file__, absolute=False) diff --git a/vulture/config.py b/vulture/config.py index 28df0775..5cf8e3d8 100644 --- a/vulture/config.py +++ b/vulture/config.py @@ -20,7 +20,7 @@ "make_whitelist": False, "sort_by_size": False, "verbose": False, - "absolute_paths": False + "absolute_paths": False, } diff --git a/vulture/core.py b/vulture/core.py index 2966188a..c27b3970 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -103,7 +103,7 @@ class Item: "last_lineno", "message", "confidence", - "absolute_paths" + "absolute_paths", ) def __init__( @@ -115,7 +115,7 @@ def __init__( last_lineno, message="", confidence=DEFAULT_CONFIDENCE, - absolute_paths=False + absolute_paths=False, ): self.name = name self.typ = typ @@ -174,10 +174,15 @@ class Vulture(ast.NodeVisitor): """Find dead code.""" def __init__( - self, verbose=False, ignore_names=None, ignore_decorators=None, absolute_paths=False + self, + verbose=False, + ignore_names=None, + ignore_decorators=None, + absolute_paths=False, ): self.verbose = verbose self.absolute_paths = absolute_paths + def get_list(typ): return utils.LoggingList(typ, self.verbose) @@ -205,17 +210,21 @@ def scan(self, code, filename=""): self.filename = filename abs_paths = self.absolute_paths + def handle_syntax_error(e): text = f' at "{e.text.strip()}"' if e.text else "" print( - f"{utils.format_path(filename, absolute=abs_paths)}:{e.lineno}: {e.msg}{text}", + f"{utils.format_path(filename, absolute=abs_paths)}:\ +{e.lineno}: {e.msg}{text}", file=sys.stderr, ) self.found_dead_code_or_error = True try: node = ( - ast.parse(code, filename=self.filename, type_comments=True) # fails if not python 3.8.x + ast.parse( + code, filename=self.filename, type_comments=True + ) # fails if not python 3.8.x if sys.version_info >= (3, 8) # type_comments requires 3.8+ else ast.parse(code, filename=self.filename) ) @@ -224,7 +233,8 @@ def handle_syntax_error(e): except ValueError as err: # ValueError is raised if source contains null bytes. print( - f'{utils.format_path(filename, absolute=self.absolute_paths)}: invalid source code "{err}"', + f'{utils.format_path(filename, absolute=self.absolute_paths)}: \ + invalid source code "{err}"', file=sys.stderr, ) self.found_dead_code_or_error = True @@ -456,7 +466,7 @@ def ignored(lineno): last_lineno, message=message, confidence=confidence, - absolute_paths=self.absolute_paths + absolute_paths=self.absolute_paths, ) ) @@ -757,7 +767,7 @@ def main(): verbose=config["verbose"], ignore_names=config["ignore_names"], ignore_decorators=config["ignore_decorators"], - absolute_paths=config["absolute_paths"] + absolute_paths=config["absolute_paths"], ) vulture.scavenge(config["paths"], exclude=config["exclude"]) sys.exit( From 891795cc12824e568e28a57731d285ebbc4f4314 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Tue, 8 Sep 2020 21:00:21 -0400 Subject: [PATCH 03/15] CHANGELOG & README --- CHANGELOG.md | 2 ++ README.md | 1 + 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 467ce59b..ba690c88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # unreleased +* added --absolute-paths option to have vulture output absolute paths instead of relative paths (mcooperman, #227) + * Only parse format strings when being used with `locals()` (jingw, #225). # 2.1 (2020-08-19) diff --git a/README.md b/README.md index 0936ddba..0e414af3 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,7 @@ ignore_names = ["visit_*", "do_*"] make_whitelist = true min_confidence = 80 sort_by_size = true +absolute_paths = false verbose = true paths = ["myscript.py", "mydir"] ``` From e2f7b561f4783928594c739a8ed644546daa1ed5 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Sat, 12 Sep 2020 12:40:03 -0400 Subject: [PATCH 04/15] resolve conflicts on PR for issue #227 --- tests/test_utils.py | 6 ++ tox.ini | 3 +- vulture/__main__.py | 2 + vulture/core.py | 195 +++++++++++++++++++++----------------------- vulture/utils.py | 35 +++++--- 5 files changed, 128 insertions(+), 113 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index cb8108c3..57b744e8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,12 @@ +"""module: test-utils""" + +# standard imports import ast import pathlib +# external imports + +# local imports from vulture import utils diff --git a/tox.ini b/tox.ini index 9e09243e..f7fb1fe0 100644 --- a/tox.ini +++ b/tox.ini @@ -25,12 +25,13 @@ deps = flake8-2020 flake8-bugbear flake8-comprehensions + flake8_formatter_abspath pyupgrade whitelist_externals = bash commands = black --check --diff . - flake8 setup.py tests/ vulture/ + flake8 --format=abspath setup.py tests/ vulture/ bash -c "pyupgrade --py36-plus `find dev/ tests/ vulture/ -name '*.py'` setup.py" [testenv:fix-style] diff --git a/vulture/__main__.py b/vulture/__main__.py index 2f3ba754..b0af784f 100644 --- a/vulture/__main__.py +++ b/vulture/__main__.py @@ -1,3 +1,5 @@ +"""module: __main__""" + from vulture.core import main main() diff --git a/vulture/core.py b/vulture/core.py index c27b3970..ce37a24b 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -1,4 +1,11 @@ -import argparse +""" +module: core + +Main classes and main entry point: main() +""" + +# standard imports +# import argparse import ast from fnmatch import fnmatch, fnmatchcase import os.path @@ -7,6 +14,9 @@ import string import sys +# external imports + +# local imports from vulture import lines from vulture import noqa from vulture import utils @@ -78,7 +88,7 @@ def _ignore_method(filename, method_name): ) -def _ignore_variable(filename, varname): +def _ignore_variable(filename, varname): # pylint: disable=unused-argument """ Ignore _ (Python idiom), _x (pylint convention) and __x__ (special variable or method), but not __x. @@ -90,7 +100,7 @@ def _ignore_variable(filename, varname): ) -class Item: +class Item: # pylint: disable=too-many-instance-attributes """ Hold the name, type and location of defined code. """ @@ -106,7 +116,7 @@ class Item: "absolute_paths", ) - def __init__( + def __init__( # pylint: disable=too-many-arguments self, name, typ, @@ -128,10 +138,12 @@ def __init__( @property def size(self): + """property: size""" assert self.last_lineno >= self.first_lineno return self.last_lineno - self.first_lineno + 1 def get_report(self, add_size=False): + """method: get_report""" if add_size: line_format = "line" if self.size == 1 else "lines" size_report = f", {self.size:d} {line_format}" @@ -146,16 +158,17 @@ def get_report(self, add_size=False): ) def get_whitelist_string(self): + """method: get_whitelist_string""" filename = utils.format_path(self.filename, self.absolute_paths) if self.typ == "unreachable_code": return f"# {self.message} ({filename}:{self.first_lineno})" - else: - prefix = "" - if self.typ in ["attribute", "method", "property"]: - prefix = "_." - return "{}{} # unused {} ({}:{:d})".format( - prefix, self.name, self.typ, filename, self.first_lineno - ) + + prefix = "" + if self.typ in ["attribute", "method", "property"]: + prefix = "_." + return "{}{} # unused {} ({}:{:d})".format( + prefix, self.name, self.typ, filename, self.first_lineno + ) def _tuple(self): return (self.filename, self.first_lineno, self.name) @@ -170,7 +183,9 @@ def __hash__(self): return hash(self._tuple()) -class Vulture(ast.NodeVisitor): +class Vulture( + ast.NodeVisitor +): # pylint: disable=too-many-instance-attributes,too-many-public-methods """Find dead code.""" def __init__( @@ -184,6 +199,7 @@ def __init__( self.absolute_paths = absolute_paths def get_list(typ): + """function: get_list""" return utils.LoggingList(typ, self.verbose) self.defined_attrs = get_list("attribute") @@ -203,15 +219,18 @@ def get_list(typ): self.filename = "" self.code = [] self.found_dead_code_or_error = False + self.noqa_lines = {} def scan(self, code, filename=""): + """method: scan""" self.code = code.splitlines() self.noqa_lines = noqa.parse_noqa(self.code) self.filename = filename abs_paths = self.absolute_paths - def handle_syntax_error(e): + def handle_syntax_error(e): # pylint: disable=invalid-name + """function: handle_syntax_error""" text = f' at "{e.text.strip()}"' if e.text else "" print( f"{utils.format_path(filename, absolute=abs_paths)}:\ @@ -246,6 +265,8 @@ def handle_syntax_error(e): handle_syntax_error(err) def scavenge(self, paths, exclude=None): + """method: scavenge""" + def prepare_pattern(pattern): if not any(char in pattern for char in ["*", "?", "["]): pattern = f"*{pattern}*" @@ -254,6 +275,7 @@ def prepare_pattern(pattern): exclude = [prepare_pattern(pattern) for pattern in (exclude or [])] def exclude_file(name): + """function: exclude_file""" return any(fnmatch(name, pattern) for pattern in exclude) for module in utils.get_modules(paths): @@ -291,15 +313,19 @@ def exclude_file(name): def get_unused_code(self, min_confidence=0, sort_by_size=False): """ + method: get_unused code + Return ordered list of unused Item objects. """ if not 0 <= min_confidence <= 100: raise ValueError("min_confidence must be between 0 and 100.") def by_name(item): + """function: by_name""" return (item.filename.lower(), item.first_lineno) def by_size(item): + """method: by_size""" return (item.size,) + by_name(item) unused_code = ( @@ -325,6 +351,8 @@ def report( self, min_confidence=0, sort_by_size=False, make_whitelist=False ): """ + method: report + Print ordered list of Item objects to stdout. """ for item in self.get_unused_code( @@ -340,38 +368,48 @@ def report( @property def unused_classes(self): + """property: unused_classes""" return _get_unused_items(self.defined_classes, self.used_names) @property def unused_funcs(self): + """property: unused_funcss""" return _get_unused_items(self.defined_funcs, self.used_names) @property def unused_imports(self): + """property: unused_imports""" return _get_unused_items(self.defined_imports, self.used_names) @property def unused_methods(self): + """property: unused_methods""" return _get_unused_items(self.defined_methods, self.used_names) @property def unused_props(self): + """property: unused_props""" return _get_unused_items(self.defined_props, self.used_names) @property def unused_vars(self): + """property: unused_vars""" return _get_unused_items(self.defined_vars, self.used_names) @property def unused_attrs(self): + """property: unused_attrs""" return _get_unused_items(self.defined_attrs, self.used_names) def _log(self, *args): + """member: _log""" if self.verbose: print(*args) def _add_aliases(self, node): """ + member: _add_aliases + We delegate to this method instead of using visit_alias() to have access to line numbers and to filter imports from __future__. """ @@ -392,6 +430,7 @@ def _add_aliases(self, node): self.used_names.add(name_and_alias.name) def _handle_conditional_node(self, node, name): + """member: _handle_conditional_node""" if utils.condition_is_always_false(node.test): self._define( self.unreachable_code, @@ -432,7 +471,7 @@ def _handle_conditional_node(self, node, name): confidence=100, ) - def _define( + def _define( # pylint: disable=too-many-arguments self, collection, name, @@ -442,7 +481,10 @@ def _define( confidence=DEFAULT_CONFIDENCE, ignore=None, ): + """member: _define""" + def ignored(lineno): + """function: ignored""" return ( (ignore and ignore(self.filename, name)) or _match(name, self.ignore_names) @@ -483,17 +525,21 @@ def visit_arg(self, node): """Function argument""" self._define_variable(node.arg, node, confidence=100) - def visit_AsyncFunctionDef(self, node): + def visit_AsyncFunctionDef(self, node): # pylint: disable=invalid-name + """method: visit_AsyncFunctionDef""" return self.visit_FunctionDef(node) - def visit_Attribute(self, node): + def visit_Attribute(self, node): # pylint: disable=invalid-name + """method: visit_Attribute""" if isinstance(node.ctx, ast.Store): self._define(self.defined_attrs, node.attr, node) elif isinstance(node.ctx, ast.Load): self.used_names.add(node.attr) - def visit_BinOp(self, node): + def visit_BinOp(self, node): # pylint: disable=invalid-name """ + method: visit_BinOp + Parse variable names in old format strings: "%(my_var)s" % locals() @@ -505,7 +551,8 @@ def visit_BinOp(self, node): ): self.used_names |= set(re.findall(r"%\((\w+)\)", node.left.s)) - def visit_Call(self, node): + def visit_Call(self, node): # pylint: disable=invalid-name + """method: visit_Call""" # Count getattr/hasattr(x, "some_attr", ...) as usage of some_attr. if isinstance(node.func, ast.Name) and ( (node.func.id == "getattr" and 2 <= len(node.args) <= 3) @@ -528,7 +575,9 @@ def visit_Call(self, node): ): self._handle_new_format_string(node.func.value.s) - def _handle_new_format_string(self, s): + def _handle_new_format_string(self, s): # pylint: disable=invalid-name + """method: _handle_new_format_string""" + def is_identifier(name): return bool(re.match(r"[a-zA-Z_][a-zA-Z0-9_]*", name)) @@ -542,8 +591,8 @@ def is_identifier(name): for field_name in names: # Remove brackets and their contents: "a[0][b].c[d].e" -> "a.c.e", # then split the resulting string: "a.b.c" -> ["a", "b", "c"] - vars = re.sub(r"\[\w*\]", "", field_name).split(".") - for var in vars: + _vars = re.sub(r"\[\w*\]", "", field_name).split(".") + for var in _vars: if is_identifier(var): self.used_names.add(var) @@ -558,7 +607,8 @@ def _is_locals_call(node): and not node.keywords ) - def visit_ClassDef(self, node): + def visit_ClassDef(self, node): # pylint: disable=invalid-name + """method: visit_ClassDef""" for decorator in node.decorator_list: if _match( utils.get_decorator_name(decorator), self.ignore_decorators @@ -572,7 +622,8 @@ def visit_ClassDef(self, node): self.defined_classes, node.name, node, ignore=_ignore_class ) - def visit_FunctionDef(self, node): + def visit_FunctionDef(self, node): # pylint: disable=invalid-name + """method: visit_FunctionDef""" decorator_names = [ utils.get_decorator_name(decorator) for decorator in node.decorator_list @@ -606,20 +657,25 @@ def visit_FunctionDef(self, node): self.defined_funcs, node.name, node, ignore=_ignore_function ) - def visit_If(self, node): + def visit_If(self, node): # pylint: disable=invalid-name + """method: visit_If""" self._handle_conditional_node(node, "if") - def visit_IfExp(self, node): + def visit_IfExp(self, node): # pylint: disable=invalid-name + """method: visit_IfExp""" self._handle_conditional_node(node, "ternary") - def visit_Import(self, node): + def visit_Import(self, node): # pylint: disable=invalid-name + """method: visit_Import""" self._add_aliases(node) - def visit_ImportFrom(self, node): + def visit_ImportFrom(self, node): # pylint: disable=invalid-name + """method: visit_ImportFrom""" if node.module != "__future__": self._add_aliases(node) - def visit_Name(self, node): + def visit_Name(self, node): # pylint: disable=invalid-name + """method: visit_Name""" if ( isinstance(node.ctx, ast.Load) and node.id not in IGNORED_VARIABLE_NAMES @@ -628,10 +684,12 @@ def visit_Name(self, node): elif isinstance(node.ctx, (ast.Param, ast.Store)): self._define_variable(node.id, node) - def visit_While(self, node): + def visit_While(self, node): # pylint: disable=invalid-name + """method: visit_While""" self._handle_conditional_node(node, "while") def visit(self, node): + """method: visit""" method = "visit_" + node.__class__.__name__ visitor = getattr(self, method, None) if self.verbose: @@ -658,6 +716,8 @@ def visit(self, node): def _handle_ast_list(self, ast_list): """ + method: _handle_ast_list + Find unreachable nodes in the given sequence of ast nodes. """ for index, node in enumerate(ast_list): @@ -680,7 +740,11 @@ def _handle_ast_list(self, ast_list): return def generic_visit(self, node): - """Called if no explicit visitor function exists for a node.""" + """ + method: generic_visit + + Called if no explicit visitor function exists for a node. + """ for _, value in ast.iter_fields(node): if isinstance(value, list): self._handle_ast_list(value) @@ -691,77 +755,8 @@ def generic_visit(self, node): self.visit(value) -def _parse_args(): - def csv(exclude): - return exclude.split(",") - - usage = "%(prog)s [options] PATH [PATH ...]" - version = f"vulture {__version__}" - glob_help = "Patterns may contain glob wildcards (*, ?, [abc], [!abc])." - parser = argparse.ArgumentParser(prog="vulture", usage=usage) - parser.add_argument( - "paths", - nargs="+", - metavar="PATH", - help="Paths may be Python files or directories. For each directory" - " Vulture analyzes all contained *.py files.", - ) - parser.add_argument( - "--exclude", - metavar="PATTERNS", - type=csv, - help=f"Comma-separated list of paths to ignore (e.g.," - f' "*settings.py,docs/*.py"). {glob_help} A PATTERN without glob' - f" wildcards is treated as *PATTERN*.", - ) - parser.add_argument( - "--ignore-decorators", - metavar="PATTERNS", - type=csv, - help=f"Comma-separated list of decorators. Functions and classes using" - f' these decorators are ignored (e.g., "@app.route,@require_*").' - f" {glob_help}", - ) - parser.add_argument( - "--ignore-names", - metavar="PATTERNS", - type=csv, - default=None, - help=f'Comma-separated list of names to ignore (e.g., "visit_*,do_*").' - f" {glob_help}", - ) - parser.add_argument( - "--make-whitelist", - action="store_true", - help="Report unused code in a format that can be added to a" - " whitelist module.", - ) - parser.add_argument( - "--min-confidence", - type=int, - default=0, - help="Minimum confidence (between 0 and 100) for code to be" - " reported as unused.", - ) - parser.add_argument( - "--sort-by-size", - action="store_true", - help="Sort unused functions and classes by their lines of code.", - ) - parser.add_argument( - "--absolute-paths", - action="store_true", - default=False, - required=False, - help="Output absolute file paths.", - ) - - parser.add_argument("-v", "--verbose", action="store_true") - parser.add_argument("--version", action="version", version=version) - return parser.parse_args() - - def main(): + """function: main""" config = make_config() vulture = Vulture( verbose=config["verbose"], diff --git a/vulture/utils.py b/vulture/utils.py index f458f5d2..2e5a8663 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -1,3 +1,6 @@ +"""module: utils""" + +# standard imports import ast import os import sys @@ -6,7 +9,7 @@ class VultureInputException(Exception): - pass + """class: VultureInputException""" def _safe_eval(node, default): @@ -27,26 +30,28 @@ def _safe_eval(node, default): results = [_safe_eval(value, default) for value in node.values] if isinstance(node.op, ast.And): return all(results) - else: - return any(results) - elif isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not): + return any(results) + if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not): return not _safe_eval(node.operand, not default) - else: - try: - return ast.literal_eval(node) - except ValueError: - return default + + try: + return ast.literal_eval(node) + except ValueError: + return default def condition_is_always_false(condition): + """function: condition_is_always_false""" return not _safe_eval(condition, True) def condition_is_always_true(condition): + """function: condition_is_always_true""" return _safe_eval(condition, False) def format_path(path, absolute=False): + """function: format_path""" if not path: return path if not absolute: @@ -54,7 +59,7 @@ def format_path(path, absolute=False): # print(f'relative path {relpath}', file=sys.stderr, flush=True) else: # print('absolute paths set', file=sys.stderr) - pp = Path(path) + pp = Path(path) # pylint: disable=invalid-name # make the path absolute, resolving any symlinks resolved_path = pp.resolve(strict=True) # relpath = os.path.abspath(path) @@ -64,6 +69,7 @@ def format_path(path, absolute=False): def get_decorator_name(decorator): + """function: get_decorator_name""" if isinstance(decorator, ast.Call): decorator = decorator.func parts = [] @@ -95,15 +101,18 @@ def get_modules(paths, toplevel=True): def read_file(filename): + """function: read_file""" try: # Use encoding detected by tokenize.detect_encoding(). - with tokenize.open(filename) as f: + with tokenize.open(filename) as f: # pylint: disable=invalid-name return f.read() except (SyntaxError, UnicodeDecodeError) as err: - raise VultureInputException(err) + raise VultureInputException(err) from err class LoggingList(list): + """class: LoggingList""" + def __init__(self, typ, verbose): self.typ = typ self._verbose = verbose @@ -117,6 +126,8 @@ def append(self, item): class LoggingSet(set): + """class: LoggingSet""" + def __init__(self, typ, verbose): self.typ = typ self._verbose = verbose From 6d4a084a7aebcaca18415a8a7feeb3326aca847d Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Sat, 12 Sep 2020 12:54:53 -0400 Subject: [PATCH 05/15] resolve conflicts --- vulture/core.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/vulture/core.py b/vulture/core.py index ce37a24b..b2bc9bbd 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -1,11 +1,4 @@ -""" -module: core - -Main classes and main entry point: main() -""" - # standard imports -# import argparse import ast from fnmatch import fnmatch, fnmatchcase import os.path From d80584bdca425d9f8895c4aef6d6015be707310c Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Sat, 12 Sep 2020 12:57:06 -0400 Subject: [PATCH 06/15] resolve conflicts --- vulture/core.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vulture/core.py b/vulture/core.py index b2bc9bbd..d0e217f0 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -1,4 +1,3 @@ -# standard imports import ast from fnmatch import fnmatch, fnmatchcase import os.path From 7f1244be20f58fb5be71ece9d6fecd4986219ead Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Sat, 12 Sep 2020 13:18:43 -0400 Subject: [PATCH 07/15] corrected commit signing key --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba690c88..b1e189a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # unreleased * added --absolute-paths option to have vulture output absolute paths instead of relative paths (mcooperman, #227) + changed git signing key to correct verification * Only parse format strings when being used with `locals()` (jingw, #225). From 54bcd20c3a4a758e44fa4cb128c1d9a2e6706c7d Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Sun, 13 Sep 2020 23:12:34 -0400 Subject: [PATCH 08/15] changes to PR as requested, now [--path_format ] --- README.md | 66 +++++++++++++++++++++++++ tests/test_config.py | 12 +++-- tests/test_utils.py | 29 +++++------ tox.ini | 3 +- vulture/__main__.py | 2 - vulture/config.py | 13 ++--- vulture/core.py | 58 ++++++++++++++-------- vulture/utils.py | 111 ++++++++++++++++++++++++++++++++----------- 8 files changed, 217 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 0e414af3..59cb8bb0 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,54 @@ When using the `--sort-by-size` option, Vulture sorts unused code by its number of lines. This helps developers prioritize where to look for dead code first. +## Path Formatting + +The `—path-format` option allows control of how file paths are emitted in the output of vulture. + +This can be useful when using vulture as a plugin tool for IDEs like PyCharm. Using absolute paths enables ‘jump-to-code’ from the output error messages when vulture is used in PyCharm. + +Currently supported formats are: + +* `relative` (default) this is the original behavior of vulture before this feature was added +* `absolute` absolute file path + +additional formats my be added in the future + +### vulture inside PyCharm + +Reference test platform: *macOS 10.14 (Mojave), anaconda python distribution, PyCharm Community 2019.3* + +Assumes: + +* vulture installed in your (virtual) python environment +* the same (virtual) environment configured into your PyCharm project settings + +Navigate from **PyCharm** menu -> **Preferences** -> **Tools** -> **External Tools** + +**Add a new tool** using the PLUS (+) icon + +Suggested Settings: + +* Name: `vulture` + +* Group: `External Tools` + +* Description: `dead code identification` + +* Tool Settings / Program: `$PyInterpreterDirectory$/python` + +* Tool Settings / Arguments: `-m vulture --path-format absolute $FilePath$` + +* Tool Settings / Working directory: `$ProjectFileDir$` + +* Select all checkboxes under Advanced Options + +* Add these Output Filters: + * `$FILE_PATH$\:$LINE$\:$COLUMN$\:.*` + * `$FILE_PATH$\:$LINE$\:.*` + +Save the settings + ## Examples Consider the following Python script (`dead_code.py`): @@ -210,6 +258,24 @@ Vulture correctly reports "os" and "message" as unused, but it fails to detect that "greet" is actually used. The recommended method to deal with false positives like this is to create a whitelist Python file. +**Absolute Paths** + +Calling: + +``` +$ vulture --path-format absolute dead_code.py +``` + +results in output similar to the following, depending on your exact path: + +``` +/Users//PycharmProjects/vulture/dead_code.py:1: unused import 'os' (90% confidence) +/Users//PycharmProjects/vulture/dead_code.py:4: unused function 'greet' (60% confidence) +/Users//PycharmProjects/vulture/dead_code.py:8: unused variable 'message' (60% confidence) +``` + + + **Preparing whitelists** In a whitelist we simulate the usage of variables, attributes, etc. For diff --git a/tests/test_config.py b/tests/test_config.py index 85668e77..437027ec 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -27,7 +27,7 @@ def test_cli_args(): ignore_names=["name1", "name2"], make_whitelist=True, min_confidence=10, - absolute_paths=True, + path_format="relative", sort_by_size=True, verbose=True, ) @@ -40,7 +40,7 @@ def test_cli_args(): "--min-confidence=10", "--sort-by-size", "--verbose", - "--absolute-paths", + "--path-format=relative", "path1", "path2", ] @@ -62,6 +62,7 @@ def test_toml_config(): min_confidence=10, sort_by_size=True, verbose=True, + path_format="relative", ) data = StringIO( dedent( @@ -75,6 +76,7 @@ def test_toml_config(): sort_by_size = true verbose = true paths = ["path1", "path2"] + path_format = "relative" """ ) ) @@ -99,7 +101,7 @@ def test_config_merging(): min_confidence = 10 sort_by_size = false verbose = false - absolute_paths = true + path_format = "relative" paths = ["toml_path"] """ ) @@ -111,7 +113,7 @@ def test_config_merging(): "--make-whitelist", "--min-confidence=20", "--sort-by-size", - "--absolute-paths", + "--path-format=relative", "--verbose", "cli_path", ] @@ -124,7 +126,7 @@ def test_config_merging(): make_whitelist=True, min_confidence=20, sort_by_size=True, - absolute_paths=True, + path_format="relative", verbose=True, ) assert result == expected diff --git a/tests/test_utils.py b/tests/test_utils.py index 57b744e8..0483d7ac 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,32 +1,33 @@ -"""module: test-utils""" - -# standard imports import ast -import pathlib - -# external imports +from pathlib import Path, PurePath -# local imports from vulture import utils +PATH_FORMATTERS = { + "relative": utils.RelativePathFormat(), + "absolute": utils.AbsolutePathFormat(), +} + -def check_paths(filename, absolute=False): - pathstr = utils.format_path(filename, absolute) +def check_paths(filename, format_name="relative"): + assert format_name in PATH_FORMATTERS + # only checks relative vs absolute right now + pathstr = PATH_FORMATTERS[format_name].m_format_path(Path(filename)) # platform dependencies and path types need to be accounted for - pp = pathlib.PurePath(pathstr) - check = pp.is_absolute() - if absolute: + pure_path = PurePath(pathstr) + check = pure_path.is_absolute() + if format_name == "absolute": assert check # even if absolute == True, the path might have been specified absolute # so can't conclude negatively def test_absolute_path(): - check_paths(__file__, absolute=True) + check_paths(__file__, format_name="absolute") def test_relative_path(): - check_paths(__file__, absolute=False) + check_paths(__file__, format_name="relative") def check_decorator_names(code, expected_names): diff --git a/tox.ini b/tox.ini index f7fb1fe0..9e09243e 100644 --- a/tox.ini +++ b/tox.ini @@ -25,13 +25,12 @@ deps = flake8-2020 flake8-bugbear flake8-comprehensions - flake8_formatter_abspath pyupgrade whitelist_externals = bash commands = black --check --diff . - flake8 --format=abspath setup.py tests/ vulture/ + flake8 setup.py tests/ vulture/ bash -c "pyupgrade --py36-plus `find dev/ tests/ vulture/ -name '*.py'` setup.py" [testenv:fix-style] diff --git a/vulture/__main__.py b/vulture/__main__.py index b0af784f..2f3ba754 100644 --- a/vulture/__main__.py +++ b/vulture/__main__.py @@ -1,5 +1,3 @@ -"""module: __main__""" - from vulture.core import main main() diff --git a/vulture/config.py b/vulture/config.py index 5cf8e3d8..42190cf3 100644 --- a/vulture/config.py +++ b/vulture/config.py @@ -20,7 +20,7 @@ "make_whitelist": False, "sort_by_size": False, "verbose": False, - "absolute_paths": False, + "path_format": "relative", } @@ -70,7 +70,7 @@ def _parse_toml(infile): make_whitelist = true min_confidence = 10 sort_by_size = true - absolute_paths = false + path_format = relative verbose = true paths = ["path1", "path2"] """ @@ -152,11 +152,12 @@ def csv(exclude): help="Sort unused functions and classes by their lines of code.", ) parser.add_argument( - "--absolute-paths", - action="store_true", - default=False, + "--path-format", + type=str, + action="store", + default="relative", required=False, - help="Output absolute file paths.", + help="Specify path format.", ) parser.add_argument( "-v", "--verbose", action="store_true", default=missing diff --git a/vulture/core.py b/vulture/core.py index d0e217f0..fb38765d 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -5,10 +5,8 @@ import re import string import sys +from pathlib import Path -# external imports - -# local imports from vulture import lines from vulture import noqa from vulture import utils @@ -31,6 +29,11 @@ "unreachable_code": "V201", } +PATH_FORMATTERS = { + "relative": utils.RelativePathFormat(), + "absolute": utils.AbsolutePathFormat(), +} + def _get_unused_items(defined_items, used_names): unused_items = [ @@ -105,7 +108,8 @@ class Item: # pylint: disable=too-many-instance-attributes "last_lineno", "message", "confidence", - "absolute_paths", + "_path_format", + "__path_formatter", ) def __init__( # pylint: disable=too-many-arguments @@ -117,7 +121,7 @@ def __init__( # pylint: disable=too-many-arguments last_lineno, message="", confidence=DEFAULT_CONFIDENCE, - absolute_paths=False, + path_format="relative", ): self.name = name self.typ = typ @@ -126,23 +130,27 @@ def __init__( # pylint: disable=too-many-arguments self.last_lineno = last_lineno self.message = message or f"unused {typ} '{name}'" self.confidence = confidence - self.absolute_paths = absolute_paths + self._path_format = path_format + self.__path_formatter = PATH_FORMATTERS[self._path_format] @property def size(self): - """property: size""" assert self.last_lineno >= self.first_lineno return self.last_lineno - self.first_lineno + 1 + @property + def path_format(self): + """property: path_format""" + return self._path_format + def get_report(self, add_size=False): - """method: get_report""" if add_size: line_format = "line" if self.size == 1 else "lines" size_report = f", {self.size:d} {line_format}" else: size_report = "" return "{}:{:d}: {} ({}% confidence{})".format( - utils.format_path(self.filename, absolute=self.absolute_paths), + self.__path_formatter.m_format_path(Path(self.filename)), self.first_lineno, self.message, self.confidence, @@ -150,8 +158,7 @@ def get_report(self, add_size=False): ) def get_whitelist_string(self): - """method: get_whitelist_string""" - filename = utils.format_path(self.filename, self.absolute_paths) + filename = self.__path_formatter.m_format_path(Path(self.filename)) if self.typ == "unreachable_code": return f"# {self.message} ({filename}:{self.first_lineno})" @@ -185,13 +192,13 @@ def __init__( verbose=False, ignore_names=None, ignore_decorators=None, - absolute_paths=False, + path_format="relative", ): self.verbose = verbose - self.absolute_paths = absolute_paths + self._path_format = path_format + self.__path_formatter = PATH_FORMATTERS[self._path_format] def get_list(typ): - """function: get_list""" return utils.LoggingList(typ, self.verbose) self.defined_attrs = get_list("attribute") @@ -214,18 +221,16 @@ def get_list(typ): self.noqa_lines = {} def scan(self, code, filename=""): - """method: scan""" self.code = code.splitlines() self.noqa_lines = noqa.parse_noqa(self.code) self.filename = filename - abs_paths = self.absolute_paths + # abs_paths = self.path_format def handle_syntax_error(e): # pylint: disable=invalid-name - """function: handle_syntax_error""" text = f' at "{e.text.strip()}"' if e.text else "" print( - f"{utils.format_path(filename, absolute=abs_paths)}:\ + f"{self.__path_formatter.m_format_path(Path(filename))}:\ {e.lineno}: {e.msg}{text}", file=sys.stderr, ) @@ -244,7 +249,7 @@ def handle_syntax_error(e): # pylint: disable=invalid-name except ValueError as err: # ValueError is raised if source contains null bytes. print( - f'{utils.format_path(filename, absolute=self.absolute_paths)}: \ + f'{self.__path_formatter.m_format_path(Path(filename))}: \ invalid source code "{err}"', file=sys.stderr, ) @@ -500,7 +505,7 @@ def ignored(lineno): last_lineno, message=message, confidence=confidence, - absolute_paths=self.absolute_paths, + path_format=self._path_format, ) ) @@ -750,11 +755,22 @@ def generic_visit(self, node): def main(): """function: main""" config = make_config() + if config["path_format"] not in PATH_FORMATTERS: + print( + "--path-format {} not recognized.".format(config["path_format"]), + file=sys.stderr, + flush=True, + ) + print("available formats are:", file=sys.stderr, flush=True) + for format_name in list(PATH_FORMATTERS): + print(f"\t{format_name}", file=sys.stderr, flush=True) + sys.exit(1) + vulture = Vulture( verbose=config["verbose"], ignore_names=config["ignore_names"], ignore_decorators=config["ignore_decorators"], - absolute_paths=config["absolute_paths"], + path_format=config["path_format"], ) vulture.scavenge(config["paths"], exclude=config["exclude"]) sys.exit( diff --git a/vulture/utils.py b/vulture/utils.py index 2e5a8663..058eb870 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -6,10 +6,93 @@ import sys import tokenize from pathlib import Path +from abc import ABC, abstractmethod + +EMPTY_PATH = "" class VultureInputException(Exception): - """class: VultureInputException""" + pass + + +class PathFormat(ABC): + """clsss: PathFormat""" + + def __init__(self, format_str=None): + self._format_str = format_str + + @abstractmethod + def m_format_path(self, path: Path, *args) -> str: + """abstractmethod: m_format_path""" + + @classmethod + def __subclasshook__(cls, C): + """classmethod: __subsclasshook__""" + if cls is PathFormat: + if any("m_format_path" in B.__dict__ for B in C.__mro__): + return True + return NotImplemented + + +class RelativePathFormat(PathFormat): + """class: RelativePathFormat""" + + # def __init__(self, format_str=None): + # super().__init__(format_str) + + def m_format_path(self, path: Path, *args) -> str: + """method: m_format_path""" + if not path: + return EMPTY_PATH + + path_str = str(path) + relpath_str = os.path.relpath(path_str) + + if relpath_str.startswith(".."): + if self._format_str: + formatted_path_str = self._format_str.format(path, *args) + else: + formatted_path_str = path + else: + if self._format_str: + formatted_path_str = self._format_str.format( + relpath_str, *args + ) + else: + formatted_path_str = relpath_str + + return formatted_path_str + + +class AbsolutePathFormat(PathFormat): + """clsss: AbsolutePathFormat""" + + # def __init__(self, format_str=None): + # super().__init__(format_str) + + def m_format_path(self, path: Path, *args) -> str: + """method: m_format_path""" + if not path: + return EMPTY_PATH + + path_str = str(path) + resolved_path = path.resolve(strict=True) + resolved_path_str = str(resolved_path) + + if resolved_path_str.startswith(".."): + if self._format_str: + formatted_path_str = self._format_str.format(path_str, *args) + else: + formatted_path_str = path_str + else: + if self._format_str: + formatted_path_str = self._format_str.format( + resolved_path_str, *args + ) + else: + formatted_path_str = resolved_path_str + + return formatted_path_str def _safe_eval(node, default): @@ -41,35 +124,14 @@ def _safe_eval(node, default): def condition_is_always_false(condition): - """function: condition_is_always_false""" return not _safe_eval(condition, True) def condition_is_always_true(condition): - """function: condition_is_always_true""" return _safe_eval(condition, False) -def format_path(path, absolute=False): - """function: format_path""" - if not path: - return path - if not absolute: - relpath = os.path.relpath(path) - # print(f'relative path {relpath}', file=sys.stderr, flush=True) - else: - # print('absolute paths set', file=sys.stderr) - pp = Path(path) # pylint: disable=invalid-name - # make the path absolute, resolving any symlinks - resolved_path = pp.resolve(strict=True) - # relpath = os.path.abspath(path) - relpath = str(resolved_path) - # print(f'abs path {relpath}', file=sys.stderr, flush=True) - return relpath if not relpath.startswith("..") else path - - def get_decorator_name(decorator): - """function: get_decorator_name""" if isinstance(decorator, ast.Call): decorator = decorator.func parts = [] @@ -101,7 +163,6 @@ def get_modules(paths, toplevel=True): def read_file(filename): - """function: read_file""" try: # Use encoding detected by tokenize.detect_encoding(). with tokenize.open(filename) as f: # pylint: disable=invalid-name @@ -111,8 +172,6 @@ def read_file(filename): class LoggingList(list): - """class: LoggingList""" - def __init__(self, typ, verbose): self.typ = typ self._verbose = verbose @@ -126,8 +185,6 @@ def append(self, item): class LoggingSet(set): - """class: LoggingSet""" - def __init__(self, typ, verbose): self.typ = typ self._verbose = verbose From f2631f56fd2267c832241e4151d8c103ce746e1c Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Mon, 14 Sep 2020 00:38:21 -0400 Subject: [PATCH 09/15] more cleanup --- vulture/core.py | 46 ++++------------------------------------------ 1 file changed, 4 insertions(+), 42 deletions(-) diff --git a/vulture/core.py b/vulture/core.py index fb38765d..27988f5b 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -262,8 +262,6 @@ def handle_syntax_error(e): # pylint: disable=invalid-name handle_syntax_error(err) def scavenge(self, paths, exclude=None): - """method: scavenge""" - def prepare_pattern(pattern): if not any(char in pattern for char in ["*", "?", "["]): pattern = f"*{pattern}*" @@ -310,19 +308,15 @@ def exclude_file(name): def get_unused_code(self, min_confidence=0, sort_by_size=False): """ - method: get_unused code - Return ordered list of unused Item objects. """ if not 0 <= min_confidence <= 100: raise ValueError("min_confidence must be between 0 and 100.") def by_name(item): - """function: by_name""" return (item.filename.lower(), item.first_lineno) def by_size(item): - """method: by_size""" return (item.size,) + by_name(item) unused_code = ( @@ -348,8 +342,6 @@ def report( self, min_confidence=0, sort_by_size=False, make_whitelist=False ): """ - method: report - Print ordered list of Item objects to stdout. """ for item in self.get_unused_code( @@ -365,48 +357,42 @@ def report( @property def unused_classes(self): - """property: unused_classes""" return _get_unused_items(self.defined_classes, self.used_names) @property def unused_funcs(self): - """property: unused_funcss""" return _get_unused_items(self.defined_funcs, self.used_names) @property def unused_imports(self): - """property: unused_imports""" return _get_unused_items(self.defined_imports, self.used_names) @property def unused_methods(self): - """property: unused_methods""" return _get_unused_items(self.defined_methods, self.used_names) @property def unused_props(self): - """property: unused_props""" + return _get_unused_items(self.defined_props, self.used_names) + + @property + def unused_items(self): return _get_unused_items(self.defined_props, self.used_names) @property def unused_vars(self): - """property: unused_vars""" return _get_unused_items(self.defined_vars, self.used_names) @property def unused_attrs(self): - """property: unused_attrs""" return _get_unused_items(self.defined_attrs, self.used_names) def _log(self, *args): - """member: _log""" if self.verbose: print(*args) def _add_aliases(self, node): """ - member: _add_aliases - We delegate to this method instead of using visit_alias() to have access to line numbers and to filter imports from __future__. """ @@ -427,7 +413,6 @@ def _add_aliases(self, node): self.used_names.add(name_and_alias.name) def _handle_conditional_node(self, node, name): - """member: _handle_conditional_node""" if utils.condition_is_always_false(node.test): self._define( self.unreachable_code, @@ -478,8 +463,6 @@ def _define( # pylint: disable=too-many-arguments confidence=DEFAULT_CONFIDENCE, ignore=None, ): - """member: _define""" - def ignored(lineno): """function: ignored""" return ( @@ -523,11 +506,9 @@ def visit_arg(self, node): self._define_variable(node.arg, node, confidence=100) def visit_AsyncFunctionDef(self, node): # pylint: disable=invalid-name - """method: visit_AsyncFunctionDef""" return self.visit_FunctionDef(node) def visit_Attribute(self, node): # pylint: disable=invalid-name - """method: visit_Attribute""" if isinstance(node.ctx, ast.Store): self._define(self.defined_attrs, node.attr, node) elif isinstance(node.ctx, ast.Load): @@ -535,8 +516,6 @@ def visit_Attribute(self, node): # pylint: disable=invalid-name def visit_BinOp(self, node): # pylint: disable=invalid-name """ - method: visit_BinOp - Parse variable names in old format strings: "%(my_var)s" % locals() @@ -549,7 +528,6 @@ def visit_BinOp(self, node): # pylint: disable=invalid-name self.used_names |= set(re.findall(r"%\((\w+)\)", node.left.s)) def visit_Call(self, node): # pylint: disable=invalid-name - """method: visit_Call""" # Count getattr/hasattr(x, "some_attr", ...) as usage of some_attr. if isinstance(node.func, ast.Name) and ( (node.func.id == "getattr" and 2 <= len(node.args) <= 3) @@ -573,8 +551,6 @@ def visit_Call(self, node): # pylint: disable=invalid-name self._handle_new_format_string(node.func.value.s) def _handle_new_format_string(self, s): # pylint: disable=invalid-name - """method: _handle_new_format_string""" - def is_identifier(name): return bool(re.match(r"[a-zA-Z_][a-zA-Z0-9_]*", name)) @@ -605,7 +581,6 @@ def _is_locals_call(node): ) def visit_ClassDef(self, node): # pylint: disable=invalid-name - """method: visit_ClassDef""" for decorator in node.decorator_list: if _match( utils.get_decorator_name(decorator), self.ignore_decorators @@ -620,7 +595,6 @@ def visit_ClassDef(self, node): # pylint: disable=invalid-name ) def visit_FunctionDef(self, node): # pylint: disable=invalid-name - """method: visit_FunctionDef""" decorator_names = [ utils.get_decorator_name(decorator) for decorator in node.decorator_list @@ -655,24 +629,19 @@ def visit_FunctionDef(self, node): # pylint: disable=invalid-name ) def visit_If(self, node): # pylint: disable=invalid-name - """method: visit_If""" self._handle_conditional_node(node, "if") def visit_IfExp(self, node): # pylint: disable=invalid-name - """method: visit_IfExp""" self._handle_conditional_node(node, "ternary") def visit_Import(self, node): # pylint: disable=invalid-name - """method: visit_Import""" self._add_aliases(node) def visit_ImportFrom(self, node): # pylint: disable=invalid-name - """method: visit_ImportFrom""" if node.module != "__future__": self._add_aliases(node) def visit_Name(self, node): # pylint: disable=invalid-name - """method: visit_Name""" if ( isinstance(node.ctx, ast.Load) and node.id not in IGNORED_VARIABLE_NAMES @@ -682,11 +651,9 @@ def visit_Name(self, node): # pylint: disable=invalid-name self._define_variable(node.id, node) def visit_While(self, node): # pylint: disable=invalid-name - """method: visit_While""" self._handle_conditional_node(node, "while") def visit(self, node): - """method: visit""" method = "visit_" + node.__class__.__name__ visitor = getattr(self, method, None) if self.verbose: @@ -713,8 +680,6 @@ def visit(self, node): def _handle_ast_list(self, ast_list): """ - method: _handle_ast_list - Find unreachable nodes in the given sequence of ast nodes. """ for index, node in enumerate(ast_list): @@ -738,8 +703,6 @@ def _handle_ast_list(self, ast_list): def generic_visit(self, node): """ - method: generic_visit - Called if no explicit visitor function exists for a node. """ for _, value in ast.iter_fields(node): @@ -753,7 +716,6 @@ def generic_visit(self, node): def main(): - """function: main""" config = make_config() if config["path_format"] not in PATH_FORMATTERS: print( From e5d2f9f335ab0fee4f8d6f4b62aabb348d8e0e78 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Mon, 14 Sep 2020 13:01:10 -0400 Subject: [PATCH 10/15] more cleanup, better tests, corrections for last comment --- deadcode/deadcode.py | 8 +++++++ tests/__init__.py | 25 +++++++++++++++++++++ tests/test_config.py | 14 ++++++------ tests/test_script.py | 24 +++++++++++++++++++- tests/test_utils.py | 2 +- vulture/config.py | 6 ++--- vulture/core.py | 52 ++++++++++++++++++++------------------------ vulture/utils.py | 46 +++++++++++++++++---------------------- 8 files changed, 110 insertions(+), 67 deletions(-) create mode 100644 deadcode/deadcode.py diff --git a/deadcode/deadcode.py b/deadcode/deadcode.py new file mode 100644 index 00000000..701ef9a5 --- /dev/null +++ b/deadcode/deadcode.py @@ -0,0 +1,8 @@ +import sys + + +def deadcode(): + """ + don't call this function from anywhere + intentional dead code for testing purposes + """ diff --git a/tests/__init__.py b/tests/__init__.py index 9c102e94..f7a905f1 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,6 +1,7 @@ import glob import os.path import subprocess +from subprocess import PIPE, STDOUT import sys import pytest @@ -10,6 +11,7 @@ DIR = os.path.dirname(os.path.abspath(__file__)) REPO = os.path.dirname(DIR) WHITELISTS = glob.glob(os.path.join(REPO, "vulture", "whitelists", "*.py")) +CALL_TIMEOUT_SEC = 60 def call_vulture(args, **kwargs): @@ -18,6 +20,29 @@ def call_vulture(args, **kwargs): ) +def run_vulture(args_list, **kwargs): + check = kwargs.get("check", False) + if "check" in kwargs: + del kwargs["check"] + result = subprocess.run( + [sys.executable, "-m", "vulture"] + args_list, + stdin=None, + input=None, + stdout=PIPE, + stderr=STDOUT, + shell=False, + cwd=REPO, + timeout=CALL_TIMEOUT_SEC, + check=check, + encoding=None, + errors=None, + env=None, + universal_newlines=True, + **kwargs + ) + return result + + def check(items_or_names, expected_names): """items_or_names must be a collection of Items or a set of strings.""" try: diff --git a/tests/test_config.py b/tests/test_config.py index 437027ec..7d0270f4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -27,7 +27,7 @@ def test_cli_args(): ignore_names=["name1", "name2"], make_whitelist=True, min_confidence=10, - path_format="relative", + format="relative", sort_by_size=True, verbose=True, ) @@ -40,7 +40,7 @@ def test_cli_args(): "--min-confidence=10", "--sort-by-size", "--verbose", - "--path-format=relative", + "--format=relative", "path1", "path2", ] @@ -62,7 +62,7 @@ def test_toml_config(): min_confidence=10, sort_by_size=True, verbose=True, - path_format="relative", + format="relative", ) data = StringIO( dedent( @@ -76,7 +76,7 @@ def test_toml_config(): sort_by_size = true verbose = true paths = ["path1", "path2"] - path_format = "relative" + format = "relative" """ ) ) @@ -101,7 +101,7 @@ def test_config_merging(): min_confidence = 10 sort_by_size = false verbose = false - path_format = "relative" + format = "relative" paths = ["toml_path"] """ ) @@ -113,7 +113,7 @@ def test_config_merging(): "--make-whitelist", "--min-confidence=20", "--sort-by-size", - "--path-format=relative", + "--format=relative", "--verbose", "cli_path", ] @@ -126,7 +126,7 @@ def test_config_merging(): make_whitelist=True, min_confidence=20, sort_by_size=True, - path_format="relative", + format="relative", verbose=True, ) assert result == expected diff --git a/tests/test_script.py b/tests/test_script.py index b57b5d02..68eb459b 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -2,8 +2,9 @@ import os.path import subprocess import sys +from pathlib import Path -from . import call_vulture, REPO, WHITELISTS +from . import call_vulture, run_vulture, REPO, WHITELISTS def test_module_with_explicit_whitelists(): @@ -73,3 +74,24 @@ def test_make_whitelist(): == 1 ) assert call_vulture(["vulture/", "--make-whitelist"]) == 0 + + +def test_absolute_paths(): + # assert call_vulture(["--format", "absolute", "deadcode/"]) == 0 + try: + completed_process = run_vulture( + ["--format", "absolute", "deadcode/"], check=False + ) + output_lines = completed_process.stdout.strip().split("\n") + for line in output_lines: + filename = line.split(":")[0] + # make the file resolves to an actual file + # and it's an absolute path + path = Path(filename) + assert path.exists() + path.resolve() + assert path.is_absolute() + except subprocess.TimeoutExpired as time_err: + raise AssertionError from time_err + except subprocess.SubprocessError as sub_err: + raise AssertionError from sub_err diff --git a/tests/test_utils.py b/tests/test_utils.py index 0483d7ac..9d9ee084 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,7 +4,7 @@ from vulture import utils PATH_FORMATTERS = { - "relative": utils.RelativePathFormat(), + "relative": utils.PathFormat(), "absolute": utils.AbsolutePathFormat(), } diff --git a/vulture/config.py b/vulture/config.py index 42190cf3..613acd2d 100644 --- a/vulture/config.py +++ b/vulture/config.py @@ -20,7 +20,7 @@ "make_whitelist": False, "sort_by_size": False, "verbose": False, - "path_format": "relative", + "format": "relative", } @@ -70,7 +70,7 @@ def _parse_toml(infile): make_whitelist = true min_confidence = 10 sort_by_size = true - path_format = relative + format = relative verbose = true paths = ["path1", "path2"] """ @@ -152,7 +152,7 @@ def csv(exclude): help="Sort unused functions and classes by their lines of code.", ) parser.add_argument( - "--path-format", + "--format", type=str, action="store", default="relative", diff --git a/vulture/core.py b/vulture/core.py index 27988f5b..4d62970a 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -30,7 +30,7 @@ } PATH_FORMATTERS = { - "relative": utils.RelativePathFormat(), + "relative": utils.PathFormat(), "absolute": utils.AbsolutePathFormat(), } @@ -83,7 +83,7 @@ def _ignore_method(filename, method_name): ) -def _ignore_variable(filename, varname): # pylint: disable=unused-argument +def _ignore_variable(filename, varname): """ Ignore _ (Python idiom), _x (pylint convention) and __x__ (special variable or method), but not __x. @@ -95,7 +95,7 @@ def _ignore_variable(filename, varname): # pylint: disable=unused-argument ) -class Item: # pylint: disable=too-many-instance-attributes +class Item: """ Hold the name, type and location of defined code. """ @@ -112,7 +112,7 @@ class Item: # pylint: disable=too-many-instance-attributes "__path_formatter", ) - def __init__( # pylint: disable=too-many-arguments + def __init__( self, name, typ, @@ -182,9 +182,7 @@ def __hash__(self): return hash(self._tuple()) -class Vulture( - ast.NodeVisitor -): # pylint: disable=too-many-instance-attributes,too-many-public-methods +class Vulture(ast.NodeVisitor): """Find dead code.""" def __init__( @@ -225,9 +223,7 @@ def scan(self, code, filename=""): self.noqa_lines = noqa.parse_noqa(self.code) self.filename = filename - # abs_paths = self.path_format - - def handle_syntax_error(e): # pylint: disable=invalid-name + def handle_syntax_error(e): text = f' at "{e.text.strip()}"' if e.text else "" print( f"{self.__path_formatter.m_format_path(Path(filename))}:\ @@ -270,7 +266,6 @@ def prepare_pattern(pattern): exclude = [prepare_pattern(pattern) for pattern in (exclude or [])] def exclude_file(name): - """function: exclude_file""" return any(fnmatch(name, pattern) for pattern in exclude) for module in utils.get_modules(paths): @@ -453,7 +448,7 @@ def _handle_conditional_node(self, node, name): confidence=100, ) - def _define( # pylint: disable=too-many-arguments + def _define( self, collection, name, @@ -464,7 +459,6 @@ def _define( # pylint: disable=too-many-arguments ignore=None, ): def ignored(lineno): - """function: ignored""" return ( (ignore and ignore(self.filename, name)) or _match(name, self.ignore_names) @@ -505,16 +499,16 @@ def visit_arg(self, node): """Function argument""" self._define_variable(node.arg, node, confidence=100) - def visit_AsyncFunctionDef(self, node): # pylint: disable=invalid-name + def visit_AsyncFunctionDef(self, node): return self.visit_FunctionDef(node) - def visit_Attribute(self, node): # pylint: disable=invalid-name + 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): self.used_names.add(node.attr) - def visit_BinOp(self, node): # pylint: disable=invalid-name + def visit_BinOp(self, node): """ Parse variable names in old format strings: @@ -527,7 +521,7 @@ def visit_BinOp(self, node): # pylint: disable=invalid-name ): self.used_names |= set(re.findall(r"%\((\w+)\)", node.left.s)) - def visit_Call(self, node): # pylint: disable=invalid-name + def visit_Call(self, node): # Count getattr/hasattr(x, "some_attr", ...) as usage of some_attr. if isinstance(node.func, ast.Name) and ( (node.func.id == "getattr" and 2 <= len(node.args) <= 3) @@ -550,7 +544,7 @@ def visit_Call(self, node): # pylint: disable=invalid-name ): self._handle_new_format_string(node.func.value.s) - def _handle_new_format_string(self, s): # pylint: disable=invalid-name + def _handle_new_format_string(self, s): def is_identifier(name): return bool(re.match(r"[a-zA-Z_][a-zA-Z0-9_]*", name)) @@ -580,7 +574,7 @@ def _is_locals_call(node): and not node.keywords ) - def visit_ClassDef(self, node): # pylint: disable=invalid-name + def visit_ClassDef(self, node): for decorator in node.decorator_list: if _match( utils.get_decorator_name(decorator), self.ignore_decorators @@ -594,7 +588,7 @@ def visit_ClassDef(self, node): # pylint: disable=invalid-name self.defined_classes, node.name, node, ignore=_ignore_class ) - def visit_FunctionDef(self, node): # pylint: disable=invalid-name + def visit_FunctionDef(self, node): decorator_names = [ utils.get_decorator_name(decorator) for decorator in node.decorator_list @@ -628,20 +622,20 @@ def visit_FunctionDef(self, node): # pylint: disable=invalid-name self.defined_funcs, node.name, node, ignore=_ignore_function ) - def visit_If(self, node): # pylint: disable=invalid-name + def visit_If(self, node): self._handle_conditional_node(node, "if") - def visit_IfExp(self, node): # pylint: disable=invalid-name + def visit_IfExp(self, node): self._handle_conditional_node(node, "ternary") - def visit_Import(self, node): # pylint: disable=invalid-name + def visit_Import(self, node): self._add_aliases(node) - def visit_ImportFrom(self, node): # pylint: disable=invalid-name + def visit_ImportFrom(self, node): if node.module != "__future__": self._add_aliases(node) - def visit_Name(self, node): # pylint: disable=invalid-name + def visit_Name(self, node): if ( isinstance(node.ctx, ast.Load) and node.id not in IGNORED_VARIABLE_NAMES @@ -650,7 +644,7 @@ def visit_Name(self, node): # pylint: disable=invalid-name elif isinstance(node.ctx, (ast.Param, ast.Store)): self._define_variable(node.id, node) - def visit_While(self, node): # pylint: disable=invalid-name + def visit_While(self, node): self._handle_conditional_node(node, "while") def visit(self, node): @@ -717,9 +711,9 @@ def generic_visit(self, node): def main(): config = make_config() - if config["path_format"] not in PATH_FORMATTERS: + if config["format"] not in PATH_FORMATTERS: print( - "--path-format {} not recognized.".format(config["path_format"]), + "--format {} not recognized.".format(config["format"]), file=sys.stderr, flush=True, ) @@ -732,7 +726,7 @@ def main(): verbose=config["verbose"], ignore_names=config["ignore_names"], ignore_decorators=config["ignore_decorators"], - path_format=config["path_format"], + path_format=config["format"], ) vulture.scavenge(config["paths"], exclude=config["exclude"]) sys.exit( diff --git a/vulture/utils.py b/vulture/utils.py index 058eb870..2f19a2cd 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -6,7 +6,6 @@ import sys import tokenize from pathlib import Path -from abc import ABC, abstractmethod EMPTY_PATH = "" @@ -15,31 +14,17 @@ class VultureInputException(Exception): pass -class PathFormat(ABC): - """clsss: PathFormat""" +class PathFormat: + """ + class: PathFormat + + Base class for path formatting. + Implements the default behavior of relative path formatting. + """ def __init__(self, format_str=None): self._format_str = format_str - @abstractmethod - def m_format_path(self, path: Path, *args) -> str: - """abstractmethod: m_format_path""" - - @classmethod - def __subclasshook__(cls, C): - """classmethod: __subsclasshook__""" - if cls is PathFormat: - if any("m_format_path" in B.__dict__ for B in C.__mro__): - return True - return NotImplemented - - -class RelativePathFormat(PathFormat): - """class: RelativePathFormat""" - - # def __init__(self, format_str=None): - # super().__init__(format_str) - def m_format_path(self, path: Path, *args) -> str: """method: m_format_path""" if not path: @@ -63,12 +48,21 @@ def m_format_path(self, path: Path, *args) -> str: return formatted_path_str + @classmethod + def __subclasshook__(cls, C): + """classmethod: __subsclasshook__""" + if cls is PathFormat: + if any("m_format_path" in B.__dict__ for B in C.__mro__): + return True + return NotImplemented + class AbsolutePathFormat(PathFormat): - """clsss: AbsolutePathFormat""" + """ + class: AbsolutePathFormat - # def __init__(self, format_str=None): - # super().__init__(format_str) + Changes the default relative path formatting to absolute. + """ def m_format_path(self, path: Path, *args) -> str: """method: m_format_path""" @@ -165,7 +159,7 @@ def get_modules(paths, toplevel=True): def read_file(filename): try: # Use encoding detected by tokenize.detect_encoding(). - with tokenize.open(filename) as f: # pylint: disable=invalid-name + with tokenize.open(filename) as f: return f.read() except (SyntaxError, UnicodeDecodeError) as err: raise VultureInputException(err) from err From d27932484765b4cd1e046cefb4c18f6f33a69890 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Mon, 14 Sep 2020 13:21:49 -0400 Subject: [PATCH 11/15] better tests --- tests/test_script.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_script.py b/tests/test_script.py index 68eb459b..4fc6c759 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -95,3 +95,12 @@ def test_absolute_paths(): raise AssertionError from time_err except subprocess.SubprocessError as sub_err: raise AssertionError from sub_err + + +def test_path_format_config(): + """ + Verify any unrecognized format generates an error. + By definition, implemented format names will be registered, + so no sense testing them. + """ + assert call_vulture(["--format", "unimplemented", "tests/"]) == 1 From 852c153a188f7b3a35fce871df2aedc486df3de5 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Mon, 14 Sep 2020 13:31:52 -0400 Subject: [PATCH 12/15] fixed uncaught bug in test (test_scripts.py) --- tests/test_script.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_script.py b/tests/test_script.py index 4fc6c759..2209846a 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -89,7 +89,7 @@ def test_absolute_paths(): # and it's an absolute path path = Path(filename) assert path.exists() - path.resolve() + path = path.resolve() assert path.is_absolute() except subprocess.TimeoutExpired as time_err: raise AssertionError from time_err From 73892b1e258624dde466de4d940ca63b33ed7cb9 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Sun, 8 Nov 2020 12:22:37 -0500 Subject: [PATCH 13/15] added some deadcode example, fixed flake8 complaint --- deadcode/deadcode.py | 3 -- tests/test_config.py | 66 ++++++++++++++++++++++---------------------- tox.ini | 6 ++-- 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/deadcode/deadcode.py b/deadcode/deadcode.py index 701ef9a5..fdb4a58a 100644 --- a/deadcode/deadcode.py +++ b/deadcode/deadcode.py @@ -1,6 +1,3 @@ -import sys - - def deadcode(): """ don't call this function from anywhere diff --git a/tests/test_config.py b/tests/test_config.py index 7d0270f4..21cd7384 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -20,17 +20,17 @@ def test_cli_args(): """ Ensure that CLI arguments are converted to a config object. """ - expected = dict( - paths=["path1", "path2"], - exclude=["file*.py", "dir/"], - ignore_decorators=["deco1", "deco2"], - ignore_names=["name1", "name2"], - make_whitelist=True, - min_confidence=10, - format="relative", - sort_by_size=True, - verbose=True, - ) + expected = { + "paths": ["path1", "path2"], + "exclude": ["file*.py", "dir/"], + "ignore_decorators": ["deco1", "deco2"], + "ignore_names": ["name1", "name2"], + "make_whitelist": True, + "min_confidence": 10, + "format": "relative", + "sort_by_size": True, + "verbose": True, + } result = _parse_args( [ "--exclude=file*.py,dir/", @@ -53,17 +53,17 @@ def test_toml_config(): """ Ensure parsing of TOML files results in a valid config object. """ - expected = dict( - paths=["path1", "path2"], - exclude=["file*.py", "dir/"], - ignore_decorators=["deco1", "deco2"], - ignore_names=["name1", "name2"], - make_whitelist=True, - min_confidence=10, - sort_by_size=True, - verbose=True, - format="relative", - ) + expected = { + "paths": ["path1", "path2"], + "exclude": ["file*.py", "dir/"], + "ignore_decorators": ["deco1", "deco2"], + "ignore_names": ["name1", "name2"], + "make_whitelist": True, + "min_confidence": 10, + "sort_by_size": True, + "verbose": True, + "format": "relative", + } data = StringIO( dedent( """\ @@ -118,17 +118,17 @@ def test_config_merging(): "cli_path", ] result = make_config(cliargs, toml) - expected = dict( - paths=["cli_path"], - exclude=["cli_exclude"], - ignore_decorators=["cli_deco"], - ignore_names=["cli_name"], - make_whitelist=True, - min_confidence=20, - sort_by_size=True, - format="relative", - verbose=True, - ) + expected = { + "paths": ["cli_path"], + "exclude": ["cli_exclude"], + "ignore_decorators": ["cli_deco"], + "ignore_names": ["cli_name"], + "make_whitelist": True, + "min_confidence": 20, + "sort_by_size": True, + "format": "relative", + "verbose": True, + } assert result == expected diff --git a/tox.ini b/tox.ini index 9e09243e..844cabcf 100644 --- a/tox.ini +++ b/tox.ini @@ -30,8 +30,8 @@ whitelist_externals = bash commands = black --check --diff . - flake8 setup.py tests/ vulture/ - bash -c "pyupgrade --py36-plus `find dev/ tests/ vulture/ -name '*.py'` setup.py" + flake8 setup.py tests/ deadcode/ vulture/ + bash -c "pyupgrade --py36-plus `find dev/ tests/ deadcode/ vulture/ -name '*.py'` setup.py" [testenv:fix-style] basepython = python3 @@ -42,4 +42,4 @@ whitelist_externals = bash commands = black . - bash -c "pyupgrade --py36-plus --exit-zero `find dev/ tests/ vulture/ -name '*.py'` setup.py" + bash -c "pyupgrade --py36-plus --exit-zero `find dev/ tests/ deadcode/ vulture/ -name '*.py'` setup.py" From 4b114e008a48d8d4c047c699e1863050f944d424 Mon Sep 17 00:00:00 2001 From: "Marc S. Cooperman" <4791030+mcooperman@users.noreply.github.com> Date: Thu, 12 Nov 2020 10:02:35 -0500 Subject: [PATCH 14/15] removed stray debug prints --- tests/__init__.py | 12 ------------ tests/test_script.py | 29 +++++++++++++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 97f7a85c..42e539ca 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -16,12 +16,6 @@ def call_vulture(args, **kwargs): - print( - f"DEBUG: {sys.executable} -m vulture args={args} \ - REPO={REPO} kwargs={kwargs}", - file=sys.stderr, - flush=True, - ) return subprocess.call( [sys.executable, "-m", "vulture"] + args, cwd=REPO, **kwargs ) @@ -31,12 +25,6 @@ def run_vulture(args_list, **kwargs): check = kwargs.get("check", False) if "check" in kwargs: del kwargs["check"] - print( - f"DEBUG: {sys.executable} -m vulture args_list={args_list} \ - REPO={REPO} kwargs={kwargs}", - file=sys.stderr, - flush=True, - ) result = subprocess.run( [sys.executable, "-m", "vulture"] + args_list, stdin=None, diff --git a/tests/test_script.py b/tests/test_script.py index 2209846a..166df346 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -1,5 +1,5 @@ import glob -import os.path +import os import subprocess import sys from pathlib import Path @@ -77,20 +77,29 @@ def test_make_whitelist(): def test_absolute_paths(): - # assert call_vulture(["--format", "absolute", "deadcode/"]) == 0 try: completed_process = run_vulture( ["--format", "absolute", "deadcode/"], check=False ) - output_lines = completed_process.stdout.strip().split("\n") + output_lines = completed_process.stdout.strip().split(os.linesep) for line in output_lines: - filename = line.split(":")[0] - # make the file resolves to an actual file - # and it's an absolute path - path = Path(filename) - assert path.exists() - path = path.resolve() - assert path.is_absolute() + if line: + lineparts = line.split(":") + # platform independent logic + # Windows differs from other root paths, uses ':' in volumes + # unix-like platforms should have 3 parts on an output line + # windows (absolute paths) have > 3 (4) including drive spec + partcount = len(lineparts) + filename = lineparts[0] + if partcount >= 3: + for i in range(1, partcount - 2): + filename += f":{lineparts[i]}" + # make sure the file resolves to an actual file + # and it's an absolute path + path = Path(filename) + assert path.exists() + path = path.resolve() + assert path.is_absolute() except subprocess.TimeoutExpired as time_err: raise AssertionError from time_err except subprocess.SubprocessError as sub_err: From 80c7b911af3740d4ae32c2a716b69bf3af1ec139 Mon Sep 17 00:00:00 2001 From: "Cooperman, Marc (CORP)" <4791030+mcooperman@users.noreply.github.com> Date: Sun, 17 Jan 2021 17:40:19 -0500 Subject: [PATCH 15/15] use Path --- vulture/test_format_path.py | 15 +++++++++++++++ vulture/utils.py | 21 ++++++++++++++------- 2 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 vulture/test_format_path.py diff --git a/vulture/test_format_path.py b/vulture/test_format_path.py new file mode 100644 index 00000000..ff8ba6c2 --- /dev/null +++ b/vulture/test_format_path.py @@ -0,0 +1,15 @@ +""" +test_format_path + +temporary testing for vulture.utils.format_path +""" +import sys + +from vulture.utils import format_path + +if __name__ == "__main__": + for format_id in ("relative", "absolute"): + result = format_path( + "vulture/test_format_path.py", None, format_id=format_id + ) + print(f"{format_id}: {result}", file=sys.stderr, flush=True) diff --git a/vulture/utils.py b/vulture/utils.py index 8458820c..c8db4a11 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -6,10 +6,11 @@ import sys import tokenize from pathlib import Path +from typing import Union from .config import RELATIVE_PATH_FORMAT, ABSOLUTE_PATH_FORMAT -EMPTY_PATH = "" +# EMPTY_PATH = "" class VultureInputException(Exception): @@ -53,18 +54,24 @@ def condition_is_always_true(condition): def format_path( - filename_path_str: str, + filename_path: Union[str, os.PathLike], format_str: str, *args, format_id: str = RELATIVE_PATH_FORMAT, ) -> str: - if not filename_path_str: - return EMPTY_PATH + if not filename_path: + raise ValueError("Empty filename/path.") if format_id not in (RELATIVE_PATH_FORMAT, ABSOLUTE_PATH_FORMAT): - raise ValueError(f"path format {format_id} uknown.") - _path = Path(filename_path_str) + raise ValueError(f"path format {format_id} unknown.") + if not isinstance(filename_path, str) and not isinstance( + filename_path, os.PathLike + ): + raise ValueError( + f"filename/path type {type(filename_path)} not supported." + ) + _path = Path(filename_path) if format_id == RELATIVE_PATH_FORMAT: - _path_str = str(filename_path_str) + _path_str = str(filename_path) _relpath_str = os.path.relpath(_path_str, start=os.curdir) _use_path_str = ( _path_str if _relpath_str.startswith("..") else _relpath_str