Skip to content

Commit

Permalink
Make linter output parsing stricter
Browse files Browse the repository at this point in the history
- no leading/trailing spaces in file path
- nothing but digits in line and column number (not even +/-)
- strip trailing whitespace from description
  • Loading branch information
akaihola committed Aug 30, 2022
1 parent a291401 commit a2c8941
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 18 deletions.
62 changes: 53 additions & 9 deletions src/darker/linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,27 +31,71 @@
logger = logging.getLogger(__name__)


def _strict_nonneg_int(text: str) -> int:
"""Strict parsing of strings to non-negative integers
Allow no leading or trailing whitespace, nor plus or minus signs.
:param text: The string to convert
:raises ValueError: Raises if the string has any non-numeric characters
:return: [description]
:rtype: [type]
"""
if text.strip("+-\t ") != text:
raise ValueError(r"invalid literal for int() with base 10: {text}")
return int(text)


def _parse_linter_line(line: str, root: Path) -> Tuple[Path, int, str, str]:
# Parse an error/note line.
# Given: line == "dir/file.py:123: error: Foo\n"
# Sets: path = Path("abs/path/to/dir/file.py:123"
# linenum = 123
# description = "error: Foo"
"""Parse one line of linter output
Only parses lines with
- a file path (without leading-trailing whitespace),
- a non-negative line number (without leading/trailing whitespace),
- optionally a column number (without leading/trailing whitespace), and
- a description.
Examples of successfully parsed lines::
path/to/file.py:42: Description
path/to/file.py:42:5: Description
Given a root of ``Path("path/")``, these would be parsed into::
(Path("to/file.py"), 42, "path/to/file.py:42:", "Description")
(Path("to/file.py"), 42, "path/to/file.py:42:5:", "Description")
For all other lines, a dummy entry is returned: an empty path, zero as the line
number, an empty location string and an empty description. Such lines should be
simply ignored, since many linters display supplementary information insterspersed
with the actual linting notifications.
:param line: The linter output line to parse. May have a trailing newline.
:param root: The root directory to resolve full file paths against
:return: A 4-tuple of
- a ``root``-relative file path,
- the line number,
- the path and location string, and
- the description.
"""
try:
location, description = line[:-1].split(": ", 1)
location, description = line.rstrip().split(": ", 1)
if location[1:3] == ":\\":
# Absolute Windows paths need special handling. Separate out the ``C:`` (or
# similar), then split by colons, and finally re-insert the ``C:``.
path_in_drive, linenum_str, *rest = location[2:].split(":")
path_str = f"{location[:2]}{path_in_drive}"
else:
path_str, linenum_str, *rest = location.split(":")
linenum = int(linenum_str)
if path_str.strip() != path_str:
raise ValueError(r"Filename {path_str!r} has leading/trailing whitespace")
linenum = _strict_nonneg_int(linenum_str)
if len(rest) > 1:
raise ValueError("Too many colon-separated tokens")
raise ValueError("Too many colon-separated tokens in {location!r}")
if len(rest) == 1:
# Make sure it column looks like an int on "<path>:<linenum>:<column>"
_column = int(rest[0]) # noqa: F841
_column = _strict_nonneg_int(rest[0]) # noqa: F841
except ValueError:
# Encountered a non-parsable line which doesn't express a linting error.
# For example, on Mypy:
Expand Down
40 changes: 31 additions & 9 deletions src/darker/tests/test_linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@

@pytest.mark.kwparametrize(
dict(
line="module.py:42: Description\n",
expect=(Path("module.py"), 42, "module.py:42:", "Description"),
line="module.py:42: Just a line number\n",
expect=(Path("module.py"), 42, "module.py:42:", "Just a line number"),
),
dict(
line="module.py:42:5: Description\n",
expect=(Path("module.py"), 42, "module.py:42:5:", "Description"),
line="module.py:42:5: With column \n",
expect=(Path("module.py"), 42, "module.py:42:5:", "With column"),
),
dict(line="no-linenum.py: Description\n", expect=(Path(), 0, "", "")),
dict(line="mod.py:invalid-linenum:5: Description\n", expect=(Path(), 0, "", "")),
dict(line="invalid linter output\n", expect=(Path(), 0, "", "")),
dict(
line="{git_root_absolute}{sep}mod.py:42: Full path\n",
expect=(
Expand All @@ -37,14 +34,39 @@
),
),
dict(
line="{git_root_absolute}{sep}mod.py:42:5: Full path\n",
line="{git_root_absolute}{sep}mod.py:42:5: Full path with column\n",
expect=(
Path("mod.py"),
42,
"{git_root_absolute}{sep}mod.py:42:5:",
"Full path",
"Full path with column",
),
),
dict(
line="mod.py:42: 123 digits start the description\n",
expect=(Path("mod.py"), 42, "mod.py:42:", "123 digits start the description"),
),
dict(
line="mod.py:42: indented description\n",
expect=(Path("mod.py"), 42, "mod.py:42:", " indented description"),
),
dict(
line="mod.py:42:5: indented description\n",
expect=(Path("mod.py"), 42, "mod.py:42:5:", " indented description"),
),
dict(line="mod.py: No line number\n", expect=(Path(), 0, "", "")),
dict(line="mod.py:foo:5: Invalid line number\n", expect=(Path(), 0, "", "")),
dict(line="mod.py:42:bar: Invalid column\n", expect=(Path(), 0, "", "")),
dict(line="invalid linter output\n", expect=(Path(), 0, "", "")),
dict(line=" leading:42: whitespace\n", expect=(Path(), 0, "", "")),
dict(line=" leading:42:5 whitespace and column\n", expect=(Path(), 0, "", "")),
dict(line="trailing :42: filepath whitespace\n", expect=(Path(), 0, "", "")),
dict(line="leading: 42: linenum whitespace\n", expect=(Path(), 0, "", "")),
dict(line="trailing:42 : linenum whitespace\n", expect=(Path(), 0, "", "")),
dict(line="plus:+42: before linenum\n", expect=(Path(), 0, "", "")),
dict(line="minus:-42: before linenum\n", expect=(Path(), 0, "", "")),
dict(line="plus:42:+5 before column\n", expect=(Path(), 0, "", "")),
dict(line="minus:42:-5 before column\n", expect=(Path(), 0, "", "")),
)
def test_parse_linter_line(git_repo, monkeypatch, line, expect):
"""Linter output is parsed correctly"""
Expand Down

0 comments on commit a2c8941

Please sign in to comment.