Skip to content

Commit

Permalink
Better output on wrong argument given to Ruff
Browse files Browse the repository at this point in the history
  • Loading branch information
sbrunner committed Nov 14, 2024
1 parent 45015ad commit 9123570
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 24 deletions.
5 changes: 2 additions & 3 deletions prospector/blender.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,10 @@ def blend(messages: list[Message], blend_combos: Optional[list[list[tuple[str, s
blend_combos = blend_combos or BLEND_COMBOS

# group messages by file and then line number
msgs_grouped: dict[Path, dict[int, list[Message]]] = defaultdict(lambda: defaultdict(list))
msgs_grouped: dict[Optional[Path], dict[Optional[int], list[Message]]] = defaultdict(lambda: defaultdict(list))

for message in messages:
line = message.location.line
msgs_grouped[message.location.path][-1 if line is None else line].append(
msgs_grouped[message.location.path][message.location.line].append(
message,
)

Expand Down
3 changes: 2 additions & 1 deletion prospector/formatters/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ def render(self, summary: bool = True, messages: bool = True, profile: bool = Fa
raise NotImplementedError

def _make_path(self, location: Location) -> Path:
return location.relative_path(self.paths_relative_to)
path_ = location.relative_path(self.paths_relative_to)
return Path() if path_ is None else path_

def _message_to_dict(self, message: Message) -> dict[str, Any]:
loc = {
Expand Down
4 changes: 2 additions & 2 deletions prospector/formatters/grouped.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections import defaultdict
from pathlib import Path
from typing import Optional

from prospector.formatters.text import TextFormatter
from prospector.message import Message
Expand All @@ -15,10 +16,9 @@ def render_messages(self) -> str:
"",
]

groups: dict[Path, dict[int, list[Message]]] = defaultdict(lambda: defaultdict(list))
groups: dict[Path, dict[Optional[int], list[Message]]] = defaultdict(lambda: defaultdict(list))

for message in self.messages:
assert message.location.line is not None
groups[self._make_path(message.location)][message.location.line].append(message)

for filename in sorted(groups.keys()):
Expand Down
16 changes: 13 additions & 3 deletions prospector/formatters/pylint.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,26 @@ def render_messages(self) -> list[str]:
# Missing function docstring

template_location = (
"%(path)s"
""
if message.location.path is None
else "%(path)s"
if message.location.line is None
else "%(path)s:%(line)s"
if message.location.character is None
else "%(path)s:%(line)s:%(character)s"
)
template_code = (
"%(code)s(%(source)s)" if message.location.function is None else "[%(code)s(%(source)s), %(function)s]"
"(%(source)s)"
if message.code is None
else "%(code)s(%(source)s)"
if message.location.function is None
else "[%(code)s(%(source)s), %(function)s]"
)
template = (
f"{template_location}: {template_code}: %(message)s"
if template_location
else f"{template_code}: %(message)s"
)
template = f"{template_location}: {template_code}: %(message)s"

output.append(
template
Expand Down
21 changes: 17 additions & 4 deletions prospector/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@


class Location:
_path: Optional[Path]

def __init__(
self,
path: Union[Path, str],
path: Optional[Union[Path, str]],
module: Optional[str],
function: Optional[str],
line: Optional[int],
Expand All @@ -15,6 +17,8 @@ def __init__(
self._path = path.absolute()
elif isinstance(path, str):
self._path = Path(path).absolute()
elif path is None:
self._path = None
else:
raise ValueError
self.module = module or None
Expand All @@ -23,13 +27,15 @@ def __init__(
self.character = None if character == -1 else character

@property
def path(self) -> Path:
def path(self) -> Optional[Path]:
return self._path

def absolute_path(self) -> Path:
def absolute_path(self) -> Optional[Path]:
return self._path

def relative_path(self, root: Optional[Path]) -> Path:
def relative_path(self, root: Optional[Path]) -> Optional[Path]:
if self._path is None:
return None
return self._path.relative_to(root) if root else self._path

def __repr__(self) -> str:
Expand All @@ -46,6 +52,13 @@ def __eq__(self, other: object) -> bool:
def __lt__(self, other: "Location") -> bool:
if not isinstance(other, Location):
raise ValueError

if self._path is None and other._path is None:
return False
if self._path is None:
return True
if other._path is None:
return False
if self._path == other._path:
if self.line == other.line:
return (self.character or -1) < (other.character or -1)
Expand Down
2 changes: 1 addition & 1 deletion prospector/postfilter.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def filter_messages(filepaths: List[Path], messages: List[Message]) -> List[Mess
filtered = []
for message in messages:
# first get rid of the pylint informational messages
relative_message_path = Path(message.location.path)
relative_message_path = None if message.location.path is None else Path(message.location.path)

if message.source == "pylint" and message.code in (
"suppressed-message",
Expand Down
21 changes: 12 additions & 9 deletions prospector/suppression.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import warnings
from collections import defaultdict
from pathlib import Path
from typing import Optional

from prospector import encoding
from prospector.exceptions import FatalProspectorException
Expand Down Expand Up @@ -63,9 +64,11 @@ def get_noqa_suppressions(file_contents: list[str]) -> tuple[bool, set[int]]:
}


def _parse_pylint_informational(messages: list[Message]) -> tuple[set[Path], dict[Path, dict[int, list[str]]]]:
ignore_files: set[Path] = set()
ignore_messages: dict[Path, dict[int, list[str]]] = defaultdict(lambda: defaultdict(list))
def _parse_pylint_informational(
messages: list[Message],
) -> tuple[set[Optional[Path]], dict[Optional[Path], dict[int, list[str]]]]:
ignore_files: set[Optional[Path]] = set()
ignore_messages: dict[Optional[Path], dict[int, list[str]]] = defaultdict(lambda: defaultdict(list))

for message in messages:
if message.source == "pylint":
Expand All @@ -86,15 +89,15 @@ def _parse_pylint_informational(messages: list[Message]) -> tuple[set[Path], dic

def get_suppressions(
filepaths: list[Path], messages: list[Message]
) -> tuple[set[Path], dict[Path, set[int]], dict[Path, dict[int, set[tuple[str, str]]]]]:
) -> tuple[set[Optional[Path]], dict[Path, set[int]], dict[Optional[Path], dict[int, set[tuple[str, str]]]]]:
"""
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[Path] = set()
paths_to_ignore: set[Optional[Path]] = set()
lines_to_ignore: dict[Path, set[int]] = defaultdict(set)
messages_to_ignore: dict[Path, dict[int, set[tuple[str, str]]]] = defaultdict(lambda: defaultdict(set))
messages_to_ignore: dict[Optional[Path], dict[int, set[tuple[str, str]]]] = defaultdict(lambda: defaultdict(set))

# first deal with 'noqa' style messages
for filepath in filepaths:
Expand All @@ -113,12 +116,12 @@ def get_suppressions(
# 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 filepath, line in pylint_ignore_messages.items():
for pylint_filepath, line in pylint_ignore_messages.items():
for line_number, codes in line.items():
for code in codes:
messages_to_ignore[filepath][line_number].add(("pylint", code))
messages_to_ignore[pylint_filepath][line_number].add(("pylint", code))
if code in _PYLINT_EQUIVALENTS:
for equivalent in _PYLINT_EQUIVALENTS[code]:
messages_to_ignore[filepath][line_number].add(equivalent)
messages_to_ignore[pylint_filepath][line_number].add(equivalent)

return paths_to_ignore, lines_to_ignore, messages_to_ignore
10 changes: 10 additions & 0 deletions prospector/tools/ruff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ def run(self, found_files: FileFinder) -> list[Message]:
completed_process = subprocess.run( # noqa: S603
[self.ruff_bin, *self.ruff_args, *found_files.python_modules], capture_output=True
)
if not completed_process.stdout:
messages.append(
Message(
"ruff",
"",
Location(None, None, None, None, None),
completed_process.stderr.decode(),
)
)
return messages
for message in json.loads(completed_process.stdout):
sub_message = {}
if message.get("url"):
Expand Down
1 change: 0 additions & 1 deletion tests/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ def test_strings_or_paths(self):

def test_bad_path_input(self):
self.assertRaises(ValueError, Location, 3.2, "module", "func", 1, 2)
self.assertRaises(ValueError, Location, None, "module", "func", 1, 2)


class LocationOrderTest(TestCase):
Expand Down

0 comments on commit 9123570

Please sign in to comment.