From 8930755a885cfff77ebbd0e0863fd96bb0517c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Brunner?= <stephane.brunner@camptocamp.com> Date: Thu, 30 Jan 2025 14:25:28 +0100 Subject: [PATCH] Ignore the blended associated code --- prospector/postfilter.py | 14 +++- prospector/run.py | 13 +++- prospector/suppression.py | 68 ++++++++++++++----- prospector/tools/base.py | 10 +++ prospector/tools/mypy/__init__.py | 9 +++ prospector/tools/pylint/__init__.py | 8 +++ prospector/tools/ruff/__init__.py | 8 ++- tests/suppression/test_blender_suppression.py | 48 +++++++++++++ .../test_blender_suppressions/test.py | 3 + tests/tools/mypy/test_mypy_tool.py | 10 ++- tests/tools/pylint/test_pylint_tool.py | 14 ++++ 11 files changed, 182 insertions(+), 23 deletions(-) create mode 100644 tests/suppression/test_blender_suppression.py create mode 100644 tests/suppression/testdata/test_blender_suppressions/test.py diff --git a/prospector/postfilter.py b/prospector/postfilter.py index 35ce4272..c4c36961 100644 --- a/prospector/postfilter.py +++ b/prospector/postfilter.py @@ -1,10 +1,18 @@ from pathlib import Path +from typing import Optional from prospector.message import Message from prospector.suppression import get_suppressions +from prospector.tools.base import ToolBase -def filter_messages(filepaths: list[Path], messages: list[Message]) -> list[Message]: +def filter_messages( + filepaths: list[Path], + messages: list[Message], + tools: Optional[dict[str, ToolBase]] = None, + blending: bool = False, + blend_combos: Optional[list[list[tuple[str, str]]]] = None, +) -> list[Message]: """ This method post-processes all messages output by all tools, in order to filter out any based on the overall output. @@ -23,7 +31,9 @@ def filter_messages(filepaths: list[Path], messages: list[Message]) -> list[Mess This method uses the information about suppressed messages from pylint to squash the unwanted redundant error from pyflakes and frosted. """ - paths_to_ignore, lines_to_ignore, messages_to_ignore = get_suppressions(filepaths, messages) + paths_to_ignore, lines_to_ignore, messages_to_ignore = get_suppressions( + filepaths, messages, tools, blending, blend_combos + ) filtered = [] for message in messages: diff --git a/prospector/run.py b/prospector/run.py index 31b0f385..b2339788 100644 --- a/prospector/run.py +++ b/prospector/run.py @@ -16,6 +16,7 @@ from prospector.formatters import FORMATTERS, Formatter from prospector.message import Location, Message from prospector.tools import DEPRECATED_TOOL_NAMES +from prospector.tools.base import ToolBase from prospector.tools.utils import CaptureOutput @@ -25,7 +26,9 @@ def __init__(self, config: ProspectorConfig) -> None: self.summary: Optional[dict[str, Any]] = None self.messages = config.messages - def process_messages(self, found_files: FileFinder, messages: list[Message]) -> list[Message]: + def process_messages( + self, found_files: FileFinder, messages: list[Message], tools: dict[str, tools.ToolBase] + ) -> list[Message]: if self.config.blending: messages = blender.blend(messages) @@ -37,7 +40,7 @@ def process_messages(self, found_files: FileFinder, messages: list[Message]) -> updated.append(msg) messages = updated - return postfilter.filter_messages(found_files.python_modules, messages) + return postfilter.filter_messages(found_files.python_modules, messages, tools, self.config.blending) def execute(self) -> None: deprecated_names = self.config.replace_deprecated_tool_names() @@ -70,6 +73,8 @@ def execute(self) -> None: messages.append(message) warnings.warn(msg, category=DeprecationWarning, stacklevel=0) + running_tools: dict[str, ToolBase] = {} + # Run the tools for tool in self.config.get_tools(found_files): for name, cls in tools.TOOLS.items(): @@ -79,6 +84,8 @@ def execute(self) -> None: else: toolname = "Unknown" + running_tools[toolname] = tool + try: # Tools can output to stdout/stderr in unexpected places, for example, # pydocstyle emits warnings about __all__ and as pyroma exec's the setup.py @@ -116,7 +123,7 @@ def execute(self) -> None: ) messages.append(message) - messages = self.process_messages(found_files, messages) + messages = self.process_messages(found_files, messages, running_tools) summary["message_count"] = len(messages) summary["completed"] = datetime.now() diff --git a/prospector/suppression.py b/prospector/suppression.py index c958d3cf..a44ec74e 100644 --- a/prospector/suppression.py +++ b/prospector/suppression.py @@ -27,12 +27,13 @@ from typing import Optional from prospector import encoding +from prospector.blender import BLEND_COMBOS from prospector.exceptions import FatalProspectorException from prospector.message import Message +from prospector.tools.base import PEP8_IGNORE_LINE_CODE, ToolBase _FLAKE8_IGNORE_FILE = re.compile(r"flake8[:=]\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+\)$") @@ -51,6 +52,17 @@ def __init__( def __str__(self) -> str: return self.code if self.source is None else f"{self.source}.{self.code}" + def __repr__(self) -> str: + return f"<{type(self).__name__} {self}>" + + def __eq__(self, value: object) -> bool: + if not isinstance(value, Ignore): + return False + return self.code == value.code and self.source == value.source + + def __hash__(self) -> int: + return hash((self.source, self.code)) + def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int], dict[int, set[Ignore]]]: """ @@ -71,7 +83,7 @@ def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int], dic if _PEP8_IGNORE_LINE.search(line): ignore_lines.add(line_number + 1) else: - noqa_match = _PEP8_IGNORE_LINE_CODE.search(line) + 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] @@ -81,15 +93,6 @@ def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int], dic return ignore_whole_file, ignore_lines, messages_to_ignore -_PYLINT_EQUIVALENTS = { - # TODO: blending has this info already? - "unused-import": ( - ("pyflakes", "FL0001"), - ("frosted", "E101"), - ) -} - - def _parse_pylint_informational( messages: list[Message], ) -> tuple[set[Optional[Path]], dict[Optional[Path], dict[int, list[str]]]]: @@ -113,17 +116,43 @@ def _parse_pylint_informational( return ignore_files, ignore_messages +def _process_tool_ignores( + tools_ignore: dict[Path, dict[int, set[Ignore]]], + blend_combos_dict: dict[Ignore, set[Ignore]], + messages_to_ignore: dict[Optional[Path], dict[int, set[Ignore]]], +) -> None: + for path, lines_ignore in tools_ignore.items(): + for line, ignores in lines_ignore.items(): + for ignore in ignores: + if ignore in blend_combos_dict: + messages_to_ignore[path][line].update(blend_combos_dict[ignore]) + + def get_suppressions( - filepaths: list[Path], messages: list[Message] + filepaths: list[Path], + messages: list[Message], + tools: Optional[dict[str, ToolBase]] = None, + blending: bool = False, + blend_combos: Optional[list[list[tuple[str, str]]]] = None, ) -> 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 """ + tools = tools or {} + blend_combos = blend_combos or BLEND_COMBOS + blend_combos_dict: dict[Ignore, set[Ignore]] = defaultdict(set) + if blending: + for combo in blend_combos: + ignore_combos = {Ignore(tool, code) for tool, code in combo} + for ignore in ignore_combos: + blend_combos_dict[ignore] |= ignore_combos + 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[Ignore]]] = defaultdict(lambda: defaultdict(set)) + tools_ignore: dict[Path, dict[int, set[Ignore]]] = defaultdict(lambda: defaultdict(set)) # First deal with 'noqa' style messages for filepath in filepaths: @@ -141,6 +170,17 @@ def get_suppressions( for line, codes_ignore in file_messages_to_ignore.items(): messages_to_ignore[filepath][line] |= codes_ignore + if blending: + for line_number, line_content in enumerate(file_contents): + for tool_name, tool in tools.items(): + tool_ignores = tool.get_ignored_codes(line_content) + for tool_ignore in tool_ignores: + tools_ignore[filepath][line_number + 1].add(Ignore(tool_name, tool_ignore)) + + # Ignore the blending messages + if blending: + _process_tool_ignores(tools_ignore, blend_combos_dict, messages_to_ignore) + # 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 @@ -149,9 +189,5 @@ def get_suppressions( for code in codes: ignore = Ignore("pylint", code) messages_to_ignore[pylint_filepath][line_number].add(ignore) - if code in _PYLINT_EQUIVALENTS: - 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 diff --git a/prospector/tools/base.py b/prospector/tools/base.py index 16b17f8a..d8876ec7 100644 --- a/prospector/tools/base.py +++ b/prospector/tools/base.py @@ -1,3 +1,4 @@ +import re from abc import ABC, abstractmethod from collections.abc import Iterable from pathlib import Path @@ -9,6 +10,8 @@ if TYPE_CHECKING: from prospector.config import ProspectorConfig +PEP8_IGNORE_LINE_CODE = re.compile(r"#\s*noqa:([^#]*[^# ])(\s*#.*)?$", re.IGNORECASE) + class ToolBase(ABC): @abstractmethod @@ -40,3 +43,10 @@ def run(self, found_files: FileFinder) -> list[Message]: standard prospector Message and Location objects. """ raise NotImplementedError + + def get_ignored_codes(self, line: str) -> list[str]: + """ + Return a list of error codes that the tool will ignore from a line of code. + """ + del line # unused + return [] diff --git a/prospector/tools/mypy/__init__.py b/prospector/tools/mypy/__init__.py index 45275e45..4ff1d16b 100644 --- a/prospector/tools/mypy/__init__.py +++ b/prospector/tools/mypy/__init__.py @@ -1,3 +1,4 @@ +import re from multiprocessing import Process, Queue from typing import TYPE_CHECKING, Any, Callable, Optional @@ -14,6 +15,8 @@ if TYPE_CHECKING: from prospector.config import ProspectorConfig +_IGNORE_RE = re.compile(r"#\s*type:\s*ignore\[([^#]*[^# ])\](\s*#.*)?$", re.IGNORECASE) + def format_message(message: str) -> Message: character: Optional[int] @@ -105,3 +108,9 @@ def run(self, found_files: FileFinder) -> list[Message]: report, _ = result[0], result[1:] # noqa return [format_message(message) for message in report.splitlines()] + + def get_ignored_codes(self, line: str) -> list[str]: + match = _IGNORE_RE.search(line) + if match: + return [e.strip() for e in match.group(1).split(",")] + return [] diff --git a/prospector/tools/pylint/__init__.py b/prospector/tools/pylint/__init__.py index 6af3122d..fa8078a5 100644 --- a/prospector/tools/pylint/__init__.py +++ b/prospector/tools/pylint/__init__.py @@ -21,6 +21,8 @@ _UNUSED_WILDCARD_IMPORT_RE = re.compile(r"^Unused import(\(s\))? (.*) from wildcard import") +_IGNORE_RE = re.compile(r"#\s*pylint:\s*disable=([^#]*[^#\s])(\s*#.*)?$", re.IGNORECASE) + def _is_in_dir(subpath: Path, path: Path) -> bool: return subpath.parent == path @@ -266,3 +268,9 @@ def run(self, found_files: FileFinder) -> list[Message]: messages = self._collector.get_messages() return self.combine(messages) + + def get_ignored_codes(self, line: str) -> list[str]: + match = _IGNORE_RE.search(line) + if match: + return [e.strip() for e in match.group(1).split(",")] + return [] diff --git a/prospector/tools/ruff/__init__.py b/prospector/tools/ruff/__init__.py index 3add2ea8..02018379 100644 --- a/prospector/tools/ruff/__init__.py +++ b/prospector/tools/ruff/__init__.py @@ -6,7 +6,7 @@ from prospector.finder import FileFinder from prospector.message import Location, Message -from prospector.tools.base import ToolBase +from prospector.tools.base import PEP8_IGNORE_LINE_CODE, ToolBase if TYPE_CHECKING: from prospector.config import ProspectorConfig @@ -84,3 +84,9 @@ def run(self, found_files: FileFinder) -> list[Message]: ) ) return messages + + def get_ignored_codes(self, line: str) -> list[str]: + match = PEP8_IGNORE_LINE_CODE.search(line) + if match: + return [e.strip() for e in match.group(1).split(",")] + return [] diff --git a/tests/suppression/test_blender_suppression.py b/tests/suppression/test_blender_suppression.py new file mode 100644 index 00000000..2e1696ac --- /dev/null +++ b/tests/suppression/test_blender_suppression.py @@ -0,0 +1,48 @@ +import unittest +from pathlib import Path + +from prospector.suppression import Ignore, get_suppressions +from prospector.tools.mypy import MypyTool +from prospector.tools.pylint import PylintTool +from prospector.tools.ruff import RuffTool + + +class BlenderSuppressionsTest(unittest.TestCase): + def test_blender_suppressions_pylint(self): + path = Path(__file__).parent / "testdata" / "test_blender_suppressions" / "test.py" + tools = {"pylint": PylintTool()} + blend_combos = [[("pylint", "n2"), ("other", "o2")]] + + _, _, messages_to_ignore = get_suppressions([path], [], tools, blending=False, blend_combos=blend_combos) + assert messages_to_ignore == {path: {1: {Ignore(None, "n1")}}} + + _, _, messages_to_ignore = get_suppressions([path], [], tools, blending=True, blend_combos=blend_combos) + assert path in messages_to_ignore + assert 2 in messages_to_ignore[path] + assert messages_to_ignore[path][2] == {Ignore("pylint", "n2"), Ignore("other", "o2")} + + def test_blender_suppressions_mypy(self): + path = Path(__file__).parent / "testdata" / "test_blender_suppressions" / "test.py" + tools = {"mypy": MypyTool()} + blend_combos = [[("mypy", "n3"), ("other", "o3")]] + + _, _, messages_to_ignore = get_suppressions([path], [], tools, blending=False, blend_combos=blend_combos) + assert messages_to_ignore == {path: {1: {Ignore(None, "n1")}}} + + _, _, messages_to_ignore = get_suppressions([path], [], tools, blending=True, blend_combos=blend_combos) + assert path in messages_to_ignore + assert 3 in messages_to_ignore[path] + assert messages_to_ignore[path][3] == {Ignore("mypy", "n3"), Ignore("other", "o3")} + + def test_blender_suppressions_ruff(self): + path = Path(__file__).parent / "testdata" / "test_blender_suppressions" / "test.py" + tools = {"ruff": RuffTool()} + blend_combos = [[("ruff", "n1"), ("other", "o1")]] + + _, _, messages_to_ignore = get_suppressions([path], [], tools, blending=False, blend_combos=blend_combos) + assert messages_to_ignore == {path: {1: {Ignore(None, "n1")}}} + + _, _, messages_to_ignore = get_suppressions([path], [], tools, blending=True, blend_combos=blend_combos) + assert path in messages_to_ignore + assert 1 in messages_to_ignore[path] + assert messages_to_ignore[path][1] == {Ignore("ruff", "n1"), Ignore("other", "o1"), Ignore(None, "n1")} diff --git a/tests/suppression/testdata/test_blender_suppressions/test.py b/tests/suppression/testdata/test_blender_suppressions/test.py new file mode 100644 index 00000000..79ef3361 --- /dev/null +++ b/tests/suppression/testdata/test_blender_suppressions/test.py @@ -0,0 +1,3 @@ +import test # noqa: n1 +import test # pylint: disable=n2 +import test # type: ignore[n3] diff --git a/tests/tools/mypy/test_mypy_tool.py b/tests/tools/mypy/test_mypy_tool.py index 325ef014..f390ac86 100644 --- a/tests/tools/mypy/test_mypy_tool.py +++ b/tests/tools/mypy/test_mypy_tool.py @@ -8,7 +8,7 @@ from prospector.tools.exceptions import BadToolConfig try: - from prospector.tools.mypy import format_message + from prospector.tools.mypy import MypyTool, format_message except ImportError: raise SkipTest # noqa: B904 @@ -28,6 +28,14 @@ def test_good_options(self): finder = FileFinder(Path(__file__).parent) self._get_config("mypy_good_options").get_tools(finder) + def test_ignore_code(self): + mypy_tool = MypyTool() + assert mypy_tool.get_ignored_codes("toto # type: ignore[misc]") == ["misc"] + assert mypy_tool.get_ignored_codes("toto # type: ignore[misc] # toto") == ["misc"] + assert mypy_tool.get_ignored_codes("toto # Type: Ignore[misc] # toto") == ["misc"] + assert mypy_tool.get_ignored_codes("toto # type: ignore[misc,misc2]") == ["misc", "misc2"] + assert mypy_tool.get_ignored_codes("toto # type: ignore[misc, misc2]") == ["misc", "misc2"] + class TestMypyMessageFormat(TestCase): def test_format_message_with_character(self): diff --git a/tests/tools/pylint/test_pylint_tool.py b/tests/tools/pylint/test_pylint_tool.py index ca23e52f..d3ac8969 100644 --- a/tests/tools/pylint/test_pylint_tool.py +++ b/tests/tools/pylint/test_pylint_tool.py @@ -111,3 +111,17 @@ def test_parallel_execution(self): messages = pylint_tool.run(found_files) assert "line-too-long" in [msg.code for msg in messages if msg.source == "pylint"] + + def test_ignore_code(self): + pylint_tool, _ = _get_pylint_tool_and_prospector_config() + assert pylint_tool.get_ignored_codes("toto # pylint: disable=missing-docstring") == ["missing-docstring"] + assert pylint_tool.get_ignored_codes("toto # pylint: disable=missing-docstring # titi") == ["missing-docstring"] + assert pylint_tool.get_ignored_codes("toto # Pylint: Disable=missing-docstring") == ["missing-docstring"] + assert pylint_tool.get_ignored_codes("toto # pylint: disable=missing-docstring,invalid-name") == [ + "missing-docstring", + "invalid-name", + ] + assert pylint_tool.get_ignored_codes("toto # pylint: disable=missing-docstring, invalid-name") == [ + "missing-docstring", + "invalid-name", + ]