Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better noqa interpretation #717

Merged
merged 1 commit into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions docs/profiles.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Profiles / Configuration
========================

The behaviour of prospector can be configured by creating a profile. A profile is
The behavior of prospector can be configured by creating a profile. A profile is
a YAML file containing several sections as described below.

Prospector will search for a ``.prospector.yaml`` file (and `several others`_) in the path that it is checking.
Expand Down Expand Up @@ -120,8 +120,8 @@ If creating your own profile, you can use the ``strictness`` like so::

strictness: medium

Valid values are 'verylow', 'low', 'medium' (the default), 'high' and 'veryhigh'. If you don't specify a
strictness value, then the default of 'medium' will be used. To avoid using any of Prospector's default
Valid values are ``verylow``, ``low``, ``medium`` (the default), ``high`` and ``veryhigh``. If you don't specify a
strictness value, then the default of ``medium`` will be used. To avoid using any of Prospector's default
strictness profiles, set ``strictness: none``.


Expand Down Expand Up @@ -172,7 +172,7 @@ but you can turn it on using the ``--member-warnings`` flag or in a profile::
Libraries Used and Autodetect
.............................

Prospector will adjust the behaviour of the underlying tools based on the libraries that your project
Prospector will adjust the behavior of the underlying tools based on the libraries that your project
uses. If you use Django, for example, the `pylint-django <https://github.com/PyCQA/pylint-django>`_ plugin
will be loaded. This will happen automatically.

Expand Down Expand Up @@ -276,7 +276,6 @@ This general option, provides a way to select maximum line length allowed.
.. Note:: This general option overrides and takes precedence over same option in a particular tool (pycodestyle or pylint)



Individual Configuration Options
--------------------------------

Expand Down
6 changes: 5 additions & 1 deletion docs/suppression.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,9 @@ prospector::
----------

A comment of ``noqa`` is used by `pycodestyle` and `pyflakes` when ignoring all errors on a certain
line. If Prospector encounters a ``# noqa`` comment it will suppress any error from any tool
line.

If Prospector encounters a ``# noqa`` comment it will suppress any error from any tool
including ``pylint`` and others such as ``dodgy``.

If Prospector encounters a ``# noqa: <code>`` comment it will suppress the error with the given code.
5 changes: 5 additions & 0 deletions prospector/blender_combinations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,11 @@ combinations:
- - pep257: D103
- pydocstyle: D103
- pylint: missing-docstring

- - pylint: singleton-comparison
- pep8: E711
- pycodestyle: E711

- - pylint: subprocess-run-check
- bandit: B603
- ruff: S603
9 changes: 7 additions & 2 deletions prospector/postfilter.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,14 @@ def filter_messages(filepaths: list[Path], messages: list[Message]) -> list[Mess
if (
relative_message_path in messages_to_ignore
and message.location.line in messages_to_ignore[relative_message_path]
and message.code in messages_to_ignore[relative_message_path][message.location.line]
):
continue
matched = False
for ignore in messages_to_ignore[relative_message_path][message.location.line]:
if (ignore.source is None or message.source == ignore.source) and message.code in ignore.code:
matched = True
continue
if matched:
continue

# otherwise this message was not filtered
filtered.append(message)
Expand Down
56 changes: 43 additions & 13 deletions prospector/suppression.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,28 @@
from prospector.message import Message

_FLAKE8_IGNORE_FILE = re.compile(r"flake8[:=]\s*noqa", re.IGNORECASE)
_PEP8_IGNORE_LINE = re.compile(r"#\s+noqa", re.IGNORECASE)
_PEP8_IGNORE_LINE = re.compile(r"#\s*noqa(\s*#.*)?$", re.IGNORECASE)
_PEP8_IGNORE_LINE_CODE = re.compile(r"#\s*noqa:([^#]*[^# ])(\s*#.*)?$", re.IGNORECASE)
_PYLINT_SUPPRESSED_MESSAGE = re.compile(r"^Suppressed \'([a-z0-9-]+)\' \(from line \d+\)$")


def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int]]:
class Ignore:
source: Optional[str]
code: str

def __init__(
self,
source: Optional[str],
code: str,
) -> None:
self.source = source
self.code = code

def __str__(self) -> str:
return self.code if self.source is None else f"{self.source}.{self.code}"


def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int], dict[int, set[Ignore]]]:
"""
Finds all pep8/flake8 suppression messages

Expand All @@ -47,12 +64,21 @@ def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int]]:
"""
ignore_whole_file = False
ignore_lines = set()
messages_to_ignore: dict[int, set[Ignore]] = defaultdict(set)
for line_number, line in enumerate(file_contents):
if _FLAKE8_IGNORE_FILE.search(line):
ignore_whole_file = True
if _PEP8_IGNORE_LINE.search(line):
ignore_lines.add(line_number + 1)
return ignore_whole_file, ignore_lines
else:
noqa_match = _PEP8_IGNORE_LINE_CODE.search(line)
if noqa_match:
prospector_ignore = noqa_match.group(1).strip().split(",")
prospector_ignore = [elem.strip() for elem in prospector_ignore]
for code in prospector_ignore:
messages_to_ignore[line_number + 1].add(Ignore(None, code))

return ignore_whole_file, ignore_lines, messages_to_ignore


_PYLINT_EQUIVALENTS = {
Expand Down Expand Up @@ -89,17 +115,17 @@ def _parse_pylint_informational(

def get_suppressions(
filepaths: list[Path], messages: list[Message]
) -> tuple[set[Optional[Path]], dict[Path, set[int]], dict[Optional[Path], dict[int, set[tuple[str, str]]]]]:
) -> tuple[set[Optional[Path]], dict[Path, set[int]], dict[Optional[Path], dict[int, set[Ignore]]]]:
"""
Given every message which was emitted by the tools, and the
list of files to inspect, create a list of files to ignore,
and a map of filepath -> line-number -> codes to ignore
"""
paths_to_ignore: set[Optional[Path]] = set()
lines_to_ignore: dict[Path, set[int]] = defaultdict(set)
messages_to_ignore: dict[Optional[Path], dict[int, set[tuple[str, str]]]] = defaultdict(lambda: defaultdict(set))
messages_to_ignore: dict[Optional[Path], dict[int, set[Ignore]]] = defaultdict(lambda: defaultdict(set))

# first deal with 'noqa' style messages
# First deal with 'noqa' style messages
for filepath in filepaths:
try:
file_contents = encoding.read_py_file(filepath).split("\n")
Expand All @@ -108,20 +134,24 @@ def get_suppressions(
warnings.warn(f"{err.path}: {err.__cause__}", ImportWarning, stacklevel=2)
continue

ignore_file, ignore_lines = get_noqa_suppressions(file_contents)
ignore_file, ignore_lines, file_messages_to_ignore = get_noqa_suppressions(file_contents)
if ignore_file:
paths_to_ignore.add(filepath)
lines_to_ignore[filepath] |= ignore_lines
for line, codes_ignore in file_messages_to_ignore.items():
messages_to_ignore[filepath][line] |= codes_ignore

# now figure out which messages were suppressed by pylint
# Now figure out which messages were suppressed by pylint
pylint_ignore_files, pylint_ignore_messages = _parse_pylint_informational(messages)
paths_to_ignore |= pylint_ignore_files
for pylint_filepath, line in pylint_ignore_messages.items():
for line_number, codes in line.items():
for pylint_filepath, line_codes in pylint_ignore_messages.items():
for line_number, codes in line_codes.items():
for code in codes:
messages_to_ignore[pylint_filepath][line_number].add(("pylint", code))
ignore = Ignore("pylint", code)
messages_to_ignore[pylint_filepath][line_number].add(ignore)
if code in _PYLINT_EQUIVALENTS:
for equivalent in _PYLINT_EQUIVALENTS[code]:
messages_to_ignore[pylint_filepath][line_number].add(equivalent)
for ignore_source, ignore_code in _PYLINT_EQUIVALENTS[code]:
ignore = Ignore(ignore_source, ignore_code)
messages_to_ignore[pylint_filepath][line_number].add(ignore)

return paths_to_ignore, lines_to_ignore, messages_to_ignore
2 changes: 1 addition & 1 deletion prospector/tools/ruff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def configure(self, prospector_config: "ProspectorConfig", _: Any) -> None:

def run(self, found_files: FileFinder) -> list[Message]:
messages = []
completed_process = subprocess.run( # noqa: S603
completed_process = subprocess.run( # noqa
[self.ruff_bin, *self.ruff_args, *found_files.python_modules], capture_output=True
)
if not completed_process.stdout:
Expand Down
23 changes: 19 additions & 4 deletions tests/suppression/test_suppression.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,32 @@ def _get_file_contents(self, name):

def test_ignore_file(self):
file_contents = self._get_file_contents("test_ignore_file/test.py")
whole_file, _ = get_noqa_suppressions(file_contents)
whole_file, _, _ = get_noqa_suppressions(file_contents)
self.assertTrue(whole_file)

def test_ignore_lines(self):
file_contents = self._get_file_contents("test_ignore_lines/test.py")
_, lines = get_noqa_suppressions(file_contents)
self.assertSetEqual({1, 2, 3}, lines)
_, lines, messages_to_ignore = get_noqa_suppressions(file_contents)
self.assertSetEqual({1, 2, 3, 4}, lines)

assert set(messages_to_ignore.keys()) == {6, 7, 8}
l6 = messages_to_ignore[6].pop()
assert l6.source is None
assert l6.code == "code"
l7 = messages_to_ignore[7].pop()
assert l7.source is None
assert l7.code == "code"
l8_sorted = sorted(messages_to_ignore[8], key=lambda x: x.code)
l8a = l8_sorted.pop()
assert l8a.source is None
assert l8a.code == "code2"
l8a = l8_sorted.pop()
assert l8a.source is None
assert l8a.code == "code1"

def test_ignore_enum_error(self):
file_contents = self._get_file_contents("test_ignore_enum/test.py")
_, lines = get_noqa_suppressions(file_contents)
_, lines, _ = get_noqa_suppressions(file_contents)
self.assertSetEqual({5}, lines)

def test_filter_messages(self):
Expand Down
5 changes: 5 additions & 0 deletions tests/suppression/testdata/test_ignore_lines/test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import collections # NOQA
import os # noqa
import tempfile # noqa
import test # noqa # test
import test # noqa test # test
import test # noqa: code
import test # noqa: code # test
import test # noqa: code1,code2 # test
Loading