Skip to content

Commit

Permalink
Better noqa interpretation
Browse files Browse the repository at this point in the history
- To completely ignore a line the `# noqa` should be alone
- To ignore a code we can use `# noqa: <code>` ad ruff do
- To ignore a code from a certain source use `# noqa: <source>.<code>`,
  this will also create an error if the ignore is not used.
  • Loading branch information
sbrunner committed Jan 21, 2025
1 parent f9b703e commit cb93607
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 28 deletions.
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
9 changes: 8 additions & 1 deletion docs/suppression.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,12 @@ 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.

If Prospector encounters a ``# noqa: <source>.<code>`` comment it will suppress the error with the given code from the given source.
This will also create an `useless-suppression` warning to inform you that the suppression is not needed.
4 changes: 2 additions & 2 deletions prospector/formatters/pylint.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ def render_messages(self) -> list[str]:
template_code = (
"(%(source)s)"
if message.code is None
else "%(code)s(%(source)s)"
else "%(source)s.%(code)s"
if message.location.function is None
else "[%(code)s(%(source)s), %(function)s]"
else "[%(source)s.%(code)s, %(function)s]"
)
template = (
f"{template_location}: {template_code}: %(message)s"
Expand Down
33 changes: 30 additions & 3 deletions prospector/postfilter.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from pathlib import Path

from prospector.message import Message
from prospector.message import Location, Message
from prospector.suppression import get_suppressions


Expand Down Expand Up @@ -48,11 +48,38 @@ 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:
ignore.used = True
matched = True
continue
if matched:
continue

# otherwise this message was not filtered
filtered.append(message)

for path, lines_ignores in messages_to_ignore.items():
for line, ignores in lines_ignores.items():
for ignore in ignores:
if ignore.should_be_used and not ignore.used:
formatted_code = ignore.code if ignore.source is None else f"{ignore.source}.{ignore.code}"
filtered.append(
Message(
source="prospector",
code="useless-suppression",
location=Location(
path=path,
module=None,
function=None,
line=line,
character=ignore.character_start,
character_end=ignore.character_end,
),
message=f"This comment didn't suppress any message of {formatted_code}.",
)
)

return filtered
81 changes: 68 additions & 13 deletions prospector/suppression.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,40 @@
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
# The not used create an error message
should_be_used: bool
used: bool
character_start: Optional[int]
character_end: Optional[int]

def __init__(
self,
source: Optional[str],
code: str,
character_start: Optional[int] = None,
character_end: Optional[int] = None,
should_be_used: bool = False,
) -> None:
self.source = source
self.code = code
self.should_be_used = should_be_used
self.used = False
self.character_start = character_start
self.character_end = character_end

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 +76,30 @@ 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 full_code in prospector_ignore:
if "." in full_code:
tool, code = full_code.split(".", 1)
to_be_used = True
else:
tool = None
code = full_code
to_be_used = False
messages_to_ignore[line_number + 1].add(
Ignore(tool, code, noqa_match.start(), noqa_match.start(2) - 1, to_be_used)
)

return ignore_whole_file, ignore_lines, messages_to_ignore


_PYLINT_EQUIVALENTS = {
Expand Down Expand Up @@ -89,17 +136,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 +155,28 @@ 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)
# Never display a unused error
ignore.used = True
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)
# Never display a unused error
ignore.used = True
messages_to_ignore[pylint_filepath][line_number].add(ignore)

return paths_to_ignore, lines_to_ignore, messages_to_ignore
29 changes: 25 additions & 4 deletions tests/suppression/test_suppression.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,38 @@ 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, 9}
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"
l8a = messages_to_ignore[8].pop()
assert l8a.source is None
assert l8a.code == "code1"
l8a = messages_to_ignore[8].pop()
assert l8a.source is None
assert l8a.code == "code2"
assert l8a.character_start == 14
assert l8a.character_end == 32
l9 = messages_to_ignore[9].pop()
assert l9.source == "source"
assert l9.code == "code"
assert l9.character_start == 14
assert l9.character_end == 32

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
6 changes: 6 additions & 0 deletions tests/suppression/testdata/test_ignore_lines/test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
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
import test # noqa: source.code# test

0 comments on commit cb93607

Please sign in to comment.