Skip to content

Commit

Permalink
Enhanced line number reporting and log all errors in Python files
Browse files Browse the repository at this point in the history
  • Loading branch information
LilSpazJoekp committed Aug 28, 2024
1 parent 5b46a96 commit 7bd2c8a
Show file tree
Hide file tree
Showing 11 changed files with 295 additions and 135 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ jobs:
matrix:
os: [ macOS-latest, ubuntu-latest, windows-latest ]
test-multi-os:
env:
PYTHONUTF8: 1
name: Test ${{ matrix.os }}
runs-on: ${{ matrix.os }}
steps:
Expand Down
9 changes: 9 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ Change Log
Unreleased
----------

**Changed**

- Better line number reporting for syntax errors in Python code blocks.

**Fixed**

- When processing Python files, the file is now fully processed and all errors are
reported at once.

1.9.0 (2024/08/26)
------------------

Expand Down
3 changes: 2 additions & 1 deletion docstrfmt/__main__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from docstrfmt.main import main # pragma: no cover

main() # pragma: no cover
if __name__ == "__main__": # pragma: no cover
main()
21 changes: 15 additions & 6 deletions docstrfmt/docstrfmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,9 @@ def directive(self, node: Directive, context: FormatContext) -> line_iterator:
)
else:
sub_doc = self.manager.parse_string(
context.current_file, "\n".join(directive.content)
context.current_file,
"\n".join(directive.content),
self.manager.current_offset + directive.content_offset,
)
if sub_doc.children:
yield ""
Expand Down Expand Up @@ -1219,6 +1221,7 @@ class Manager:
def __init__(self, reporter, black_config=None, docstring_trailing_line=True):
rst_extras.register()
self.black_config = black_config
self.current_offset = 0
self.error_count = 0
self.reporter = reporter
self.settings = OptionParser(components=[rst.Parser]).get_default_values()
Expand All @@ -1231,7 +1234,9 @@ def __init__(self, reporter, black_config=None, docstring_trailing_line=True):
self.current_file = None
self.docstring_trailing_line = docstring_trailing_line

def _pre_process(self, node: docutils.nodes.Node) -> None:
def _pre_process(
self, node: docutils.nodes.Node, line_offset: int, block_length: int
) -> None:
"""Preprocess nodes.
This does some preprocessing to all nodes that is generic across node types and
Expand All @@ -1254,7 +1259,8 @@ def _pre_process(self, node: docutils.nodes.Node) -> None:
InvalidRstError(
self.current_file,
error.attributes["type"],
error.line,
(block_length - 1 if error.line is None else error.line)
+ line_offset,
error.children[0].children[0],
)
for error in errors
Expand Down Expand Up @@ -1288,7 +1294,7 @@ def _pre_process(self, node: docutils.nodes.Node) -> None:

# Recurse.
for child in node.children:
self._pre_process(child)
self._pre_process(child, line_offset, block_length)

def format_node(self, width, node: docutils.nodes.Node, is_docstring=False) -> str:
formatted_node = "\n".join(
Expand All @@ -1305,16 +1311,19 @@ def format_node(self, width, node: docutils.nodes.Node, is_docstring=False) -> s
)
return f"{formatted_node}\n"

def parse_string(self, file_name: str, text: str) -> docutils.nodes.document:
def parse_string(
self, file_name: str, text: str, line_offset: int = 0
) -> docutils.nodes.document:
self.current_file = file_name
self.current_offset = line_offset
self._patch_unknown_directives(text)
doc = new_document(str(self.current_file), self.settings)
parser = rst.Parser()
parser.parse(text, doc)
doc.reporter = IgnoreMessagesReporter(
"", self.settings.report_level, self.settings.halt_level
)
self._pre_process(doc)
self._pre_process(doc, line_offset, len(text.splitlines()))
return doc

def perform_format(
Expand Down
47 changes: 33 additions & 14 deletions docstrfmt/main.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import asyncio
import glob
import logging
Expand Down Expand Up @@ -33,7 +35,7 @@
from .debug import dump_node
from .docstrfmt import Manager
from .exceptions import InvalidRstErrors
from .util import FileCache, plural
from .util import FileCache, LineResolver, plural

if TYPE_CHECKING: # pragma: no cover
from libcst import AssignTarget, ClassDef, FunctionDef, Module, SimpleString
Expand Down Expand Up @@ -124,7 +126,14 @@ def print(self, message, level=0, **formatting_kwargs):
class Visitor(CSTTransformer):
METADATA_DEPENDENCIES = (PositionProvider, ParentNodeProvider)

def __init__(self, object_name, file, line_length, manager):
def __init__(
self,
file: Path | str,
input_string: str,
line_length: int,
manager: Manager,
object_name: str,
):
super().__init__()
self._last_assign = None
self._object_names = [object_name]
Expand All @@ -135,6 +144,7 @@ def __init__(self, object_name, file, line_length, manager):
self.manager = manager
self.misformatted = False
self.error_count = 0
self.line_resolver = LineResolver(self.file, input_string)

def _is_docstring(self, node: "SimpleString") -> bool:
return node.quote.startswith(('"""', "'''")) and isinstance(
Expand Down Expand Up @@ -165,15 +175,22 @@ def leave_SimpleString(
source = dedent(
(" " * indent_level) + original_node.evaluated_value
).rstrip()
doc = self.manager.parse_string(self.file, source)
doc = self.manager.parse_string(
self.file, source, self.line_resolver.offset(original_node.value)
)
if reporter.level >= 3:
reporter.debug("=" * 60)
reporter.debug(dump_node(doc))
width = self.line_length - indent_level
if width < 1:
self.error_count += 1
raise ValueError(f"Invalid starting width {self.line_length}")
output = self.manager.format_node(width, doc, True).rstrip()
try:
output = self.manager.format_node(width, doc, True).rstrip()
except InvalidRstErrors as errors:
self.error_count += 1
reporter.error(str(errors))
return updated_node
self.error_count += self.manager.error_count
self.manager.error_count = 0
object_display_name = (
Expand Down Expand Up @@ -359,11 +376,11 @@ def _format_file(
except InvalidRstErrors as errors:
reporter.error(str(errors))
error_count += 1
reporter.print(f"Failed to format {str(file)!r}")
reporter.print(f"Failed to format '{str(file)}'")
except Exception as error:
reporter.error(f"{error.__class__.__name__}: {error}")
error_count += 1
reporter.print(f"Failed to format {str(file)!r}")
reporter.print(f"Failed to format '{str(file)}'")
return misformatted, error_count


Expand Down Expand Up @@ -460,7 +477,7 @@ def _process_python(
):
filename = basename(file)
object_name = filename.split(".")[0]
visitor = Visitor(object_name, file, line_length, manager)
visitor = Visitor(file, input_string, line_length, manager, object_name)
module = cst.parse_module(input_string)
wrapper = cst.MetadataWrapper(module)
result = wrapper.visit(visitor)
Expand All @@ -469,7 +486,7 @@ def _process_python(
if visitor.misformatted:
misformatted = True
if check and not raw_output:
reporter.print(f"File {str(file)!r} could be reformatted.")
reporter.print(f"File '{str(file)}' could be reformatted.")
else:
if file == "-" or raw_output:
with lock or nullcontext():
Expand Down Expand Up @@ -507,14 +524,14 @@ def _process_rst(
error_count = manager.error_count
misformatted = False
if output == input_string:
reporter.print(f"File {str(file)!r} is formatted correctly. Nice!", 1)
reporter.print(f"File '{str(file)}' is formatted correctly. Nice!", 1)
if raw_output:
with lock or nullcontext():
_write_output(file, input_string, nullcontext(sys.stdout), raw_output)
else:
misformatted = True
if check and not raw_output:
reporter.print(f"File {str(file)!r} could be reformatted.")
reporter.print(f"File '{str(file)}' could be reformatted.")
else:
if file == "-" or raw_output:
with lock or nullcontext():
Expand All @@ -537,7 +554,7 @@ def _write_output(file, output, output_manager, raw):
with output_manager as f:
f.write(output)
if not raw:
reporter.print(f"Reformatted {str(file)!r}.")
reporter.print(f"Reformatted '{str(file)}'.")


@click.command(context_settings=dict(help_option_names=["-h", "--help"]))
Expand Down Expand Up @@ -694,18 +711,20 @@ def main(
try:
misformatted = False
if file_type == "py":
misformatted, _ = _process_python(
misformatted, error_count = _process_python(
check, file, raw_input, line_length, manager, True
)
elif file_type == "rst":
misformatted, _ = _process_rst(
misformatted, error_count = _process_rst(
check, file, raw_input, line_length, manager, True
)
if misformatted:
misformatted_files.add(file)
except InvalidRstErrors as errors:
reporter.error(str(errors))
context.exit(1)
if error_count > 0:
context.exit(1)
context.exit(0)

cache = FileCache(context, ignore_cache)
Expand Down Expand Up @@ -773,7 +792,7 @@ def main(
" reformatted."
)
elif not raw_output:
reporter.print(f"{plural('file', len(files))} were checked.")
reporter.print(f"{plural('file', len(files))} was checked.")
if error_count > 0:
reporter.print(f"Done, but {plural('error', error_count)} occurred ❌💥❌")
elif not raw_output:
Expand Down
23 changes: 23 additions & 0 deletions docstrfmt/util.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from __future__ import annotations

import os
import pickle
import sys
import tempfile
from collections import defaultdict
from contextlib import nullcontext
from pathlib import Path

Expand All @@ -12,6 +15,26 @@
from .const import __version__


class LineResolver:
def __init__(self, file, source):
self.file = file
self.source = source
self._results = defaultdict(list)
self._searches = set()

def offset(self, code) -> int:
if code not in self._searches:
if code not in self.source: # pragma: no cover should be impossible
raise ValueError(f"Code not found in {self.file}")
self._searches.add(code)
split = self.source.split(code)
for i, block in enumerate(split[:-1]):
self._results[code].append(code.join(split[: i + 1]).count("\n") + 1)
if not self._results[code]: # pragma: no cover should be impossible
raise ValueError(f"Code not found in {self.file}")
return self._results[code].pop(0)


class FileCache:
def __init__(self, context, ignore_cache=False):
self.cache_dir = user_cache_path("docstrfmt", version=__version__)
Expand Down
12 changes: 1 addition & 11 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@

from docstrfmt import Manager
from docstrfmt.server import handler
from tests import node_eq


def run_test(manager, file, width, test_string):
doc = manager.parse_string(file, test_string)
output = manager.format_node(width, doc)
doc2 = manager.parse_string(file, output)
output2 = manager.format_node(width, doc2)
assert node_eq(doc, doc2)
assert output == output2


@pytest.fixture
Expand All @@ -36,7 +26,7 @@ def manager():


@pytest.fixture
def runner(loop):
def runner():
runner = CliRunner()
files_to_copy = os.path.abspath("tests/test_files")
with runner.isolated_filesystem() as temp_dir:
Expand Down
9 changes: 7 additions & 2 deletions tests/test_docstrfmt.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from tests.conftest import run_test
from tests import node_eq

test_lengths = [8, 13, 34, 55, 89, 144, 72]

Expand All @@ -10,4 +10,9 @@ def test_formatting(manager, length):
file = "tests/test_files/test_file.rst"
with open(file, encoding="utf-8") as f:
test_string = f.read()
run_test(manager, file, length, test_string)
doc = manager.parse_string(file, test_string)
output = manager.format_node(length, doc)
doc2 = manager.parse_string(file, output)
output2 = manager.format_node(length, doc2)
assert node_eq(doc, doc2)
assert output == output2
33 changes: 33 additions & 0 deletions tests/test_files/error_files/py_file_error_duplicate_docstring.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""This is an example python file"""


class ExampleClass:
def _get_auto_context_keys_1(self):
"""
.. figure:: ../images/biohazard.png
:width: 200px
:align: center
:height: 100px
:alt: alternate text
:figclass: align-center
returns a dictionary which indicates the names of
the configuration variables needed to access:
* path to CA file
"""

def _get_auto_context_keys_2(self):
"""
.. figure:: ../images/biohazard.png
:width: 200px
:align: center
:height: 100px
:alt: alternate text
:figclass: align-center
returns a dictionary which indicates the names of
the configuration variables needed to access:
* path to CA file
"""
9 changes: 6 additions & 3 deletions tests/test_files/error_files/py_file_error_invalid_rst.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ def __init__(self, *args, **kwargs):
self.test = "test"
"""Test attr docstring
= ===
1 222
== ==
.. figure:: /_static/figure.png
:scale: 50%
= ===
1 222
== ==
..
Expand Down
Loading

0 comments on commit 7bd2c8a

Please sign in to comment.