From 219653783572930c5db936546ce1d07b65411470 Mon Sep 17 00:00:00 2001 From: Rahul Jha Date: Tue, 31 Mar 2020 15:06:35 +0530 Subject: [PATCH] Ignore results annotated with '# noqa' (#195) * Ignore results annotated with '# noqa' * Report issue codes --- CHANGELOG.md | 6 + README.md | 44 ++++++- tests/test_noqa.py | 292 +++++++++++++++++++++++++++++++++++++++++++ tests/test_report.py | 14 +-- tests/test_script.py | 3 +- vulture/core.py | 31 ++++- vulture/lines.py | 25 ++++ vulture/noqa.py | 61 +++++++++ vulture/utils.py | 25 ++++ 9 files changed, 482 insertions(+), 19 deletions(-) create mode 100644 tests/test_noqa.py create mode 100644 vulture/noqa.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d4333cf6..9019115b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Unreleased + +* Report issue codes in output (e.g., `code.py:1: V104 unused import ...`) + (RJ722, #195) +* Support `# noqa` comments to suppress results on that line. (RJ722, #195). + # 1.4 (2020-03-30) * Ignore unused import statements in `__init__.py` (RJ722, #192). diff --git a/README.md b/README.md index a2ead129..3581c764 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ tool for higher code quality. * tested: tests itself and has complete test coverage * complements pyflakes and has the same output syntax * sorts unused classes and functions by size with `--sort-by-size` +* respects `# noqa` comments * supports Python 2.7 and Python \>= 3.5 ## Installation @@ -58,6 +59,17 @@ We collect whitelists for common Python modules and packages in a whole file or directory, use the `--exclude` parameter (e.g., `--exclude *settings.py,docs/`). +Another way of ignoring errors is to annotate the line causing the false +positive with `# noqa: ` in a trailing comment (e.g., +`# noqa: V103`). + +The `ERROR_CODE` specifies what kind of dead code to ignore (see the table +below for the list of error codes). In case no error code is specified, +Vulture ignores all results for the line. + +Note that the line number for any decorated object is the same as the line +number of the first decorator. + **Ignoring names** You can use `--ignore-names foo*,ba[rz]` to let Vulture ignore all names @@ -130,9 +142,9 @@ Calling : results in the following output: - dead_code.py:1: unused import 'os' (90% confidence) - dead_code.py:4: unused function 'greet' (60% confidence) - dead_code.py:8: unused variable 'message' (60% confidence) + dead_code.py:1: V104 unused import 'os' (90% confidence) + dead_code.py:4: V103 unused function 'greet' (60% confidence) + dead_code.py:8: V106 unused variable 'message' (60% confidence) Vulture correctly reports "os" and "message" as unused, but it fails to detect that "greet" is actually used. The recommended method to deal @@ -158,8 +170,30 @@ Passing both the original program and the whitelist to Vulture makes Vulture ignore the `greet` method: - dead_code.py:1: unused import 'os' (90% confidence) - dead_code.py:8: unused variable 'message' (60% confidence) + dead_code.py:1: V104 unused import 'os' (90% confidence) + dead_code.py:8: V106 unused variable 'message' (60% confidence) + +**Using "# noqa"** + +```python +import os # noqa + +class Greeter: # noqa: 102 + def greet(self): # noqa: V103 + print("Hi") +``` + +## Error codes + +| Error code | Description | +| ---------- | ----------------- | +| V101 | Unused attribute | +| V102 | Unused class | +| V103 | Unused function | +| V104 | Unused import | +| V105 | Unused property | +| V106 | Unused variable | +| V201 | Unreachable code | ## Exit codes diff --git a/tests/test_noqa.py b/tests/test_noqa.py new file mode 100644 index 00000000..2c6a6db6 --- /dev/null +++ b/tests/test_noqa.py @@ -0,0 +1,292 @@ +import pytest + +from vulture.noqa import NOQA_REGEXP, _parse_error_codes +from . import check, v + +assert v # Silence pyflakes. + + +@pytest.mark.parametrize( + "line, codes", + [ + ("# noqa", ["all"]), + ("## noqa", ["all"]), + ("# noqa Hi, go on.", ["all"]), + ("# noqa: V101", ["V101"]), + ("# noqa: V101, V106", ["V101", "V106"]), + ("# NoQA: V101, V103, \t V104", ["V101", "V103", "V104"]), + ], +) +def test_noqa_regex_present(line, codes): + match = NOQA_REGEXP.search(line) + parsed = _parse_error_codes(match) + assert parsed == codes + + +@pytest.mark.parametrize( + "line", + [ + ("# noqa: 123V"), + ("# noqa explanation: V012"), + ("# noqa: ,V101"), + ("# noqa: #noqa: V102"), + ("# noqa: # noqa: V102"), + ], +) +def test_noqa_regex_no_groups(line): + assert NOQA_REGEXP.search(line).groupdict()["codes"] is None + + +@pytest.mark.parametrize( + "line", + [("#noqa"), ("##noqa"), ("# n o q a"), ("#NOQA"), ("# Hello, noqa")], +) +def test_noqa_regex_not_present(line): + assert not NOQA_REGEXP.search(line) + + +def test_noqa_without_codes(v): + v.scan( + """\ +import this # noqa + +@underground # noqa +class Cellar: + @property # noqa + def wine(self): + grapes = True # noqa + + @without_ice # noqa + def serve(self, quantity=50): + self.quantity_served = quantity # noqa + return + self.pour() # noqa +""" + ) + check(v.unused_attrs, []) + check(v.unused_classes, []) + check(v.unused_funcs, []) + check(v.unused_imports, []) + check(v.unused_props, []) + check(v.unreachable_code, []) + check(v.unused_vars, []) + + +def test_noqa_specific_issue_codes(v): + v.scan( + """\ +import this # noqa: V104 + +@underground # noqa: V102 +class Cellar: + @property # noqa: V105 + def wine(self): + grapes = True # noqa: V106 + + @without_ice # noqa: V103 + def serve(self, quantity=50): + self.quantity_served = quantity # noqa: V101 + return + self.pour() # noqa: V201 +""" + ) + check(v.unused_attrs, []) + check(v.unused_classes, []) + check(v.unused_funcs, []) + check(v.unused_imports, []) + check(v.unused_props, []) + check(v.unreachable_code, []) + check(v.unused_vars, []) + + +def test_noqa_attributes(v): + v.scan( + """\ +something.x = 'x' # noqa: V101 +something.z = 'z' # noqa: V106 (code for unused variable) +something.u = 'u' # noqa +""" + ) + check(v.unused_attrs, ["z"]) + + +def test_noqa_classes(v): + v.scan( + """\ +class QtWidget: # noqa: V102 + pass + +class ABC(QtWidget): + pass # noqa: V102 (should not ignore) + +class DEF: # noqa + pass +""" + ) + check(v.unused_classes, ["ABC"]) + + +def test_noqa_functions(v): + v.scan( + """\ +def play(tune, instrument='bongs', _hz='50'): # noqa: V103 + pass + + +# noqa +def problems(): # noqa: V104 + pass # noqa: V103 + +def hello(name): # noqa + print("Hello") +""" + ) + check(v.unused_funcs, ["problems"]) + check(v.unused_vars, ["instrument", "tune"]) + + +def test_noqa_imports(v): + v.scan( + """\ +import foo +import this # noqa: V104 +import zoo +from koo import boo # noqa +from me import * +import dis # noqa: V101 (code for unused attr) +""" + ) + check(v.unused_imports, ["foo", "zoo", "dis"]) + + +def test_noqa_properties(v): + v.scan( + """\ +class Zoo: + @property + def no_of_koalas(self): # noqa + pass + + @property + def area(self, width, depth): # noqa: V105 + pass + + @property # noqa + def entry_gates(self): + pass + + @property # noqa: V103 (code for unused function) + def tickets(self): + pass +""" + ) + check(v.unused_props, ["no_of_koalas", "area", "tickets"]) + check(v.unused_classes, ["Zoo"]) + check(v.unused_vars, ["width", "depth"]) + + +def test_noqa_multiple_decorators(v): + v.scan( + """\ +@bar # noqa: V102 +class Foo: + @property # noqa: V105 + @make_it_cool + @log + def something(self): + pass + + @coolify + @property + def something_else(self): # noqa: V105 + pass + + @a + @property + @b # noqa + def abcd(self): + pass +""" + ) + check(v.unused_props, ["something_else", "abcd"]) + check(v.unused_classes, []) + + +def test_noqa_unreacahble_code(v): + v.scan( + """\ +def shave_sheep(sheep): + for a_sheep in sheep: + if a_sheep.is_bald: + continue + a_sheep.grow_hair() # noqa: V201 + a_sheep.shave() + return + for a_sheep in sheep: # noqa: V201 + if a_sheep.still_has_hair: + a_sheep.shave_again() +""" + ) + check(v.unreachable_code, []) + check(v.unused_funcs, ["shave_sheep"]) + + +def test_noqa_variables(v): + v.scan( + """\ +mitsi = "Mother" # noqa: V106 +harry = "Father" # noqa +shero = "doggy" # noqa: V101, V104 (code for unused import, attr) +shinchan.friend = ['masao'] # noqa: V106 (code for unused variable) +""" + ) + check(v.unused_vars, ["shero"]) + check(v.unused_attrs, ["friend"]) + + +def test_noqa_with_multiple_issue_codes(v): + v.scan( + """\ +def world(axis): # noqa: V103, V201 + pass + + +for _ in range(3): + continue + xyz = hello(something, else): # noqa: V201, V106 +""" + ) + check(v.get_unused_code(), []) + + +def test_noqa_on_empty_line(v): + v.scan( + """\ +# noqa +import this +# noqa +""" + ) + check(v.unused_imports, ["this"]) + + +def test_noqa_with_invalid_codes(v): + v.scan( + """\ +import this # V098, A123, F876 +""" + ) + check(v.unused_imports, ["this"]) + + +@pytest.mark.parametrize( + "first_file, second_file", + [ + ("foo = None", "bar = None # noqa"), + ("bar = None # noqa", "foo = None"), + ], +) +def test_noqa_multiple_files(first_file, second_file, v): + v.scan(first_file, filename="first_file.py") + v.scan(second_file, filename="second_file.py") + check(v.unused_vars, ["foo"]) diff --git a/tests/test_report.py b/tests/test_report.py index f4168ffc..84cbf342 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -37,13 +37,13 @@ def test_report(code, expected, make_whitelist=False): def test_item_report(check_report): expected = """\ -{filename}:1: unused import 'foo' (90% confidence) -{filename}:3: unused class 'Foo' (60% confidence) -{filename}:7: unused function 'bar' (60% confidence) -{filename}:8: unused attribute 'foobar' (60% confidence) -{filename}:9: unused variable 'foobar' (60% confidence) -{filename}:11: unreachable code after 'return' (100% confidence) -{filename}:13: unused property 'myprop' (60% confidence) +{filename}:1: V104 unused import 'foo' (90% confidence) +{filename}:3: V102 unused class 'Foo' (60% confidence) +{filename}:7: V103 unused function 'bar' (60% confidence) +{filename}:8: V101 unused attribute 'foobar' (60% confidence) +{filename}:9: V106 unused variable 'foobar' (60% confidence) +{filename}:11: V201 unreachable code after 'return' (100% confidence) +{filename}:13: V105 unused property 'myprop' (60% confidence) """ check_report(mock_code, expected) diff --git a/tests/test_script.py b/tests/test_script.py index ddaf76c8..b57b5d02 100644 --- a/tests/test_script.py +++ b/tests/test_script.py @@ -1,3 +1,4 @@ +import glob import os.path import subprocess import sys @@ -61,7 +62,7 @@ def call_vulture_with_excludes(excludes): return call_vulture(["vulture/", "--exclude", get_csv(excludes)]) assert call_vulture_with_excludes(["core.py", "utils.py"]) == 1 - assert call_vulture_with_excludes(["core.py", "utils.py", "lines.py"]) == 0 + assert call_vulture_with_excludes(glob.glob("vulture/*.py")) == 0 def test_make_whitelist(): diff --git a/vulture/core.py b/vulture/core.py index 827eb6a9..523022e7 100644 --- a/vulture/core.py +++ b/vulture/core.py @@ -2,7 +2,7 @@ # # vulture - Find dead code. # -# Copyright (c) 2012-2019 Jendrik Seipp (jendrikseipp@gmail.com) +# Copyright (c) 2012-2020 Jendrik Seipp (jendrikseipp@gmail.com) # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the @@ -35,6 +35,7 @@ import sys from vulture import lines +from vulture import noqa from vulture import utils __version__ = "1.4" @@ -46,6 +47,16 @@ if sys.version_info < (3, 4): IGNORED_VARIABLE_NAMES |= {"True", "False"} +ERROR_CODES = { + "attribute": "V101", + "class": "V102", + "function": "V103", + "import": "V104", + "property": "V105", + "unreachable_code": "V201", + "variable": "V106", +} + def _get_unused_items(defined_items, used_names): unused_items = [ @@ -145,9 +156,10 @@ def get_report(self, add_size=False): size_report = ", {:d} {}".format(self.size, line_format) else: size_report = "" - return "{}:{:d}: {} ({}% confidence{})".format( + return "{}:{:d}: {} {} ({}% confidence{})".format( utils.format_path(self.filename), self.first_lineno, + ERROR_CODES[self.typ], self.message, self.confidence, size_report, @@ -213,6 +225,7 @@ def get_set(typ): def scan(self, code, filename=""): code = utils.sanitize_code(code) self.code = code.splitlines() + self.noqa_lines = noqa.parse_noqa(self.code) self.filename = filename try: node = ast.parse(code, filename=self.filename) @@ -461,14 +474,20 @@ def _define( confidence=DEFAULT_CONFIDENCE, ignore=None, ): + def ignored(lineno): + return ( + (ignore and ignore(self.filename, name)) + or _match(name, self.ignore_names) + or noqa.ignore_line(self.noqa_lines, lineno, ERROR_CODES[typ]) + ) + last_node = last_node or first_node typ = collection.typ - if (ignore and ignore(self.filename, name)) or _match( - name, self.ignore_names - ): + first_lineno = lines.get_first_line_number(first_node) + + if ignored(first_lineno): self._log('Ignoring {typ} "{name}"'.format(**locals())) else: - first_lineno = lines.get_first_line_number(first_node) last_lineno = lines.get_last_line_number(last_node) collection.append( Item( diff --git a/vulture/lines.py b/vulture/lines.py index c7d0bc4c..f51331d7 100644 --- a/vulture/lines.py +++ b/vulture/lines.py @@ -1,3 +1,28 @@ +# -*- coding: utf-8 -*- +# +# vulture - Find dead code. +# +# Copyright (c) 2012-2020 Jendrik Seipp (jendrikseipp@gmail.com) +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +# CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + import ast import sys diff --git a/vulture/noqa.py b/vulture/noqa.py new file mode 100644 index 00000000..7631b435 --- /dev/null +++ b/vulture/noqa.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +# +# vulture - Find dead code. +# +# Copyright (c) 2012-2020 Jendrik Seipp (jendrikseipp@gmail.com) +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +# CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +from collections import defaultdict +import re + +NOQA_REGEXP = re.compile( + # Use the same regex as flake8 does. + # https://gitlab.com/pycqa/flake8/-/tree/master/src/flake8/defaults.py + # We're looking for items that look like this: + # `# noqa` + # `# noqa: E123` + # `# noqa: E123,W451,F921` + # `# NoQA: E123,W451,F921` + r"# noqa(?::[\s]?(?P([A-Z]+[0-9]+(?:[,\s]+)?)+))?", + re.IGNORECASE, +) + + +def _parse_error_codes(match): + # If no error code is specified, add the line to the "all" category. + return [ + c.strip() for c in (match.groupdict()["codes"] or "all").split(",") + ] + + +def parse_noqa(code): + noqa_lines = defaultdict(set) + for lineno, line in enumerate(code, start=1): + match = NOQA_REGEXP.search(line) + if match: + for error_code in _parse_error_codes(match): + noqa_lines[error_code].add(lineno) + return noqa_lines + + +def ignore_line(noqa_lines, lineno, error_code): + """Check if the reported line is annotated with "# noqa".""" + return lineno in noqa_lines[error_code] or lineno in noqa_lines["all"] diff --git a/vulture/utils.py b/vulture/utils.py index 453fcb94..2c3cc8c4 100644 --- a/vulture/utils.py +++ b/vulture/utils.py @@ -1,3 +1,28 @@ +# -*- coding: utf-8 -*- +# +# vulture - Find dead code. +# +# Copyright (c) 2012-2020 Jendrik Seipp (jendrikseipp@gmail.com) +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +# CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + import ast import codecs import os