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/CHANGELOG.md b/CHANGELOG.md index 6f650c6e..3f1a62c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # 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). * Don't override paths in pyproject.toml with empty CLI paths (bcbnz, #228). * Run continuous integration tests for Python 3.9 (ju-sh, #232). diff --git a/README.md b/README.md index 875bbc32..14adbf60 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,7 @@ ignore_names = ["visit_*", "do_*"] make_whitelist = true min_confidence = 80 sort_by_size = true +absolute_paths = false verbose = true paths = ["myscript.py", "mydir"] ``` @@ -179,6 +180,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`): @@ -214,6 +263,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/deadcode/deadcode.py b/deadcode/deadcode.py new file mode 100644 index 00000000..fdb4a58a --- /dev/null +++ b/deadcode/deadcode.py @@ -0,0 +1,5 @@ +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 58e1da85..651e2516 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,5 +1,6 @@ import pathlib import subprocess +from subprocess import PIPE, STDOUT import sys import pytest @@ -11,6 +12,8 @@ str(path) for path in (REPO / "vulture" / "whitelists").glob("*.py") ] +CALL_TIMEOUT_SEC = 60 + def call_vulture(args, **kwargs): return subprocess.call( @@ -18,6 +21,40 @@ def call_vulture(args, **kwargs): ) +def run_vulture(args_list, **kwargs): + """ + Run vulture using subprocess.run(...) + + Returns a subprocess.CompletedProcess object + in order that a caller (i.e. tests) can examine + the output in more detail than with call_vulture(...) + + added to enable testing of absolute path generation + """ + check = kwargs.get("check", False) + if "check" in kwargs: + del kwargs["check"] + # WARNING - setting check = True may raise an Error/Exception + # on process failure + 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 f7d3e5cd..1aa1cbd7 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, + format="relative", sort_by_size=True, verbose=True, ) @@ -39,6 +40,7 @@ def test_cli_args(): "--min-confidence=10", "--sort-by-size", "--verbose", + "--format=relative", "path1", "path2", ] @@ -60,6 +62,7 @@ def test_toml_config(): min_confidence=10, sort_by_size=True, verbose=True, + format="relative", ) data = StringIO( dedent( @@ -73,6 +76,7 @@ def test_toml_config(): sort_by_size = true verbose = true paths = ["path1", "path2"] + format = "relative" """ ) ) @@ -97,6 +101,7 @@ def test_config_merging(): min_confidence = 10 sort_by_size = false verbose = false + format = "relative" paths = ["toml_path"] """ ) @@ -108,6 +113,7 @@ def test_config_merging(): "--make-whitelist", "--min-confidence=20", "--sort-by-size", + "--format=relative", "--verbose", "cli_path", ] @@ -120,6 +126,7 @@ def test_config_merging(): make_whitelist=True, min_confidence=20, sort_by_size=True, + format="relative", verbose=True, ) assert result == expected diff --git a/tests/test_script.py b/tests/test_script.py index b57b5d02..166df346 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -1,9 +1,10 @@ import glob -import os.path +import os 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,42 @@ def test_make_whitelist(): == 1 ) assert call_vulture(["vulture/", "--make-whitelist"]) == 0 + + +def test_absolute_paths(): + try: + completed_process = run_vulture( + ["--format", "absolute", "deadcode/"], check=False + ) + output_lines = completed_process.stdout.strip().split(os.linesep) + for line in output_lines: + 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: + 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 diff --git a/tests/test_utils.py b/tests/test_utils.py index 176dc0c7..2ecb251d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,28 @@ import ast +from pathlib import PurePath from vulture import utils +from vulture.utils import format_path +from vulture.config import ABSOLUTE_PATH_FORMAT, RELATIVE_PATH_FORMAT + + +def check_paths(filename, format_name="relative"): + assert format_name in (ABSOLUTE_PATH_FORMAT, RELATIVE_PATH_FORMAT) + pathstr = format_path(filename, None, format_id=format_name) + 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__, format_name="absolute") + + +def test_relative_path(): + check_paths(__file__, format_name="relative") def check_decorator_names(code, expected_names): diff --git a/tox.ini b/tox.ini index e620fb48..d5f77d00 100644 --- a/tox.ini +++ b/tox.ini @@ -31,8 +31,8 @@ whitelist_externals = commands = black --check --diff . # C408: unnecessary dict call - flake8 --extend-ignore=C408 setup.py tests/ vulture/ - bash -c "pyupgrade --py36-plus `find dev/ tests/ vulture/ -name '*.py'` setup.py" + flake8 --extend-ignore=C408 setup.py tests/ deadcode/ vulture/ + bash -c "pyupgrade --py36-plus `find dev/ tests/ deadcode/ vulture/ -name '*.py'` setup.py" [testenv:fix-style] basepython = python3 @@ -43,4 +43,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" diff --git a/vulture/config.py b/vulture/config.py index 8b72bca2..f0ab82f7 100644 --- a/vulture/config.py +++ b/vulture/config.py @@ -10,6 +10,9 @@ from .version import __version__ +RELATIVE_PATH_FORMAT = "relative" +ABSOLUTE_PATH_FORMAT = "absolute" + #: Possible configuration options and their respective defaults DEFAULTS = { "min_confidence": 0, @@ -20,6 +23,7 @@ "make_whitelist": False, "sort_by_size": False, "verbose": False, + "format": RELATIVE_PATH_FORMAT, } @@ -69,6 +73,7 @@ def _parse_toml(infile): make_whitelist = true min_confidence = 10 sort_by_size = true + format = relative verbose = true paths = ["path1", "path2"] """ @@ -150,6 +155,14 @@ def csv(exclude): default=missing, help="Sort unused functions and classes by their lines of code.", ) + parser.add_argument( + "--format", + type=str, + action="store", + default=RELATIVE_PATH_FORMAT, + required=False, + 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 3396ba84..54f89b2f 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -1,16 +1,20 @@ import ast from fnmatch import fnmatch, fnmatchcase -from pathlib import Path import pkgutil import re import string import sys +from pathlib import Path from vulture import lines from vulture import noqa from vulture import utils -from vulture.config import make_config - +from vulture.config import ( + make_config, + RELATIVE_PATH_FORMAT, + ABSOLUTE_PATH_FORMAT, +) +from vulture.utils import format_path DEFAULT_CONFIDENCE = 60 @@ -101,6 +105,7 @@ class Item: "last_lineno", "message", "confidence", + "_path_format", ) def __init__( @@ -112,6 +117,7 @@ def __init__( last_lineno, message="", confidence=DEFAULT_CONFIDENCE, + path_format="relative", ): self.name = name self.typ = typ @@ -120,12 +126,18 @@ def __init__( self.last_lineno = last_lineno self.message = message or f"unused {typ} '{name}'" self.confidence = confidence + self._path_format = path_format @property def size(self): 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): if add_size: line_format = "line" if self.size == 1 else "lines" @@ -133,7 +145,7 @@ def get_report(self, add_size=False): else: size_report = "" return "{}:{:d}: {} ({}% confidence{})".format( - utils.format_path(self.filename), + format_path(self.filename, None, format_id=self._path_format), self.first_lineno, self.message, self.confidence, @@ -141,16 +153,18 @@ def get_report(self, add_size=False): ) def get_whitelist_string(self): - filename = utils.format_path(self.filename) + filename = format_path( + self.filename, None, format_id=self._path_format + ) 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) @@ -169,9 +183,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, + path_format="relative", ): self.verbose = verbose + self._path_format = path_format def get_list(typ): return utils.LoggingList(typ, self.verbose) @@ -193,6 +212,7 @@ def get_list(typ): self.filename = Path() self.code = [] self.found_dead_code_or_error = False + self.noqa_lines = {} def scan(self, code, filename=""): filename = Path(filename) @@ -200,10 +220,13 @@ def scan(self, code, filename=""): self.noqa_lines = noqa.parse_noqa(self.code) self.filename = filename + _fpath = format_path(filename, None, format_id=self._path_format) + 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"{_fpath}:\ +{e.lineno}: {e.msg}{text}", file=sys.stderr, ) self.found_dead_code_or_error = True @@ -221,7 +244,8 @@ 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'{_fpath}: \ + invalid source code "{err}"', file=sys.stderr, ) self.found_dead_code_or_error = True @@ -347,6 +371,10 @@ def unused_methods(self): def unused_props(self): 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): return _get_unused_items(self.defined_vars, self.used_names) @@ -455,6 +483,7 @@ def ignored(lineno): last_lineno, message=message, confidence=confidence, + path_format=self._path_format, ) ) @@ -530,8 +559,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) @@ -668,7 +697,9 @@ def _handle_ast_list(self, ast_list): return def generic_visit(self, node): - """Called if no explicit visitor function exists for a node.""" + """ + 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) @@ -681,10 +712,22 @@ def generic_visit(self, node): def main(): config = make_config() + if config["format"] not in (RELATIVE_PATH_FORMAT, ABSOLUTE_PATH_FORMAT): + print( + "--format {} not recognized.".format(config["format"]), + file=sys.stderr, + flush=True, + ) + print("available formats are:", file=sys.stderr, flush=True) + for format_name in (RELATIVE_PATH_FORMAT, ABSOLUTE_PATH_FORMAT): + 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"], + path_format=config["format"], ) vulture.scavenge(config["paths"], exclude=config["exclude"]) sys.exit( 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 1e7bfa01..c8db4a11 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -1,7 +1,16 @@ +"""module: utils""" + +# standard imports import ast import os import sys import tokenize +from pathlib import Path +from typing import Union + +from .config import RELATIVE_PATH_FORMAT, ABSOLUTE_PATH_FORMAT + +# EMPTY_PATH = "" class VultureInputException(Exception): @@ -26,15 +35,14 @@ 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): @@ -45,12 +53,42 @@ def condition_is_always_true(condition): return _safe_eval(condition, False) -def format_path(path): - try: - return path.relative_to(os.curdir) - except ValueError: - # Path is not below the current directory. - return path +def format_path( + filename_path: Union[str, os.PathLike], + format_str: str, + *args, + format_id: str = RELATIVE_PATH_FORMAT, +) -> str: + 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} 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) + _relpath_str = os.path.relpath(_path_str, start=os.curdir) + _use_path_str = ( + _path_str if _relpath_str.startswith("..") else _relpath_str + ) + _formatted_path_str = ( + format_str.format(_use_path_str, *args) + if format_str + else _use_path_str + ) + return _formatted_path_str + if format_id == ABSOLUTE_PATH_FORMAT: + _abs_path = _path.resolve(strict=True) + if format_str: + return format_str.format(str(_abs_path), *args) + return str(_abs_path) + # should never get here + raise ValueError(f"path format {format_id} uknown.") def get_decorator_name(decorator): @@ -93,14 +131,15 @@ def read_file(filename): with tokenize.open(filename) as f: return f.read() except (SyntaxError, UnicodeDecodeError) as err: - raise VultureInputException(err) + raise VultureInputException(err) from err 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: @@ -112,7 +151,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: