From e5c78c0c8c5c4a4320f19fb811ecfdcf2da20871 Mon Sep 17 00:00:00 2001 From: Christopher Barber Date: Sun, 23 Mar 2025 12:34:45 -0400 Subject: [PATCH 1/3] Improved line/col information (38) --- .idea/garpy.mkdocstrings.iml | 7 ++ CHANGELOG.md | 9 +- docs/index.md | 6 +- src/mkdocstrings_handlers/python_xref/VERSION | 2 +- .../python_xref/crossref.py | 68 ++++++++++++-- tests/project/src/myproj/bar.py | 10 +- tests/test_crossref.py | 93 ++++++++++++++++++- tests/test_integration.py | 13 ++- 8 files changed, 186 insertions(+), 22 deletions(-) diff --git a/.idea/garpy.mkdocstrings.iml b/.idea/garpy.mkdocstrings.iml index 288b948..2edac60 100644 --- a/.idea/garpy.mkdocstrings.iml +++ b/.idea/garpy.mkdocstrings.iml @@ -10,4 +10,11 @@ + + + + \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a01fd8..0760c9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # mkdocstring-python-xref changes +*Note that versions roughly correspond to the version of mkdocstrings-python that they +are compatible with.* + +## 1.16.2 + +* Improved source locations for errors in docstrings now including column offset. + ## 1.16.1 * Fix sdist distributions (should enable conda-forge to build) @@ -7,7 +14,7 @@ ## 1.16.0 * Compatibility with mkdocstrings-python 1.16.* -* Removed some deprecated imports from mkdoctrings +* Removed some deprecated imports from mkdocstrings ## 1.14.1 diff --git a/docs/index.md b/docs/index.md index eb03413..895569e 100644 --- a/docs/index.md +++ b/docs/index.md @@ -107,11 +107,7 @@ If `relative_crossrefs` and `check_crossrefs` are both enabled (the latter is tr then all cross-reference expressions will be checked to ensure that they exist and failures will be reported with the source location. Otherwise, missing cross-references will be reported by mkdocstrings without the source location, in which case it is often difficult to locate the source -of the error. Note that the errors generatoed by this feat[.gitignore](..%2F.gitignore) - - - -ure are in addition to the errors +of the error. Note that the errors generated by this feature are in addition to the errors from mkdocstrings. The current implementation of this feature can produce false errors for definitions from the diff --git a/src/mkdocstrings_handlers/python_xref/VERSION b/src/mkdocstrings_handlers/python_xref/VERSION index 41c11ff..4a02d2c 100644 --- a/src/mkdocstrings_handlers/python_xref/VERSION +++ b/src/mkdocstrings_handlers/python_xref/VERSION @@ -1 +1 @@ -1.16.1 +1.16.2 diff --git a/src/mkdocstrings_handlers/python_xref/crossref.py b/src/mkdocstrings_handlers/python_xref/crossref.py index 4dd63c1..24fb89f 100644 --- a/src/mkdocstrings_handlers/python_xref/crossref.py +++ b/src/mkdocstrings_handlers/python_xref/crossref.py @@ -16,6 +16,7 @@ from __future__ import annotations import re +from ast import literal_eval from typing import Callable, List, Optional, cast from griffe import Docstring, Object @@ -303,14 +304,12 @@ def _error(self, msg: str, just_warn: bool = False) -> None: # We include the file:// prefix because it helps IDEs such as PyCharm # recognize that this is a navigable location it can highlight. prefix = f"file://{parent.filepath}:" - line = doc.lineno - if line is not None: # pragma: no branch - # Add line offset to match in docstring. This can still be - # short if the doc string has leading newlines. - line += doc.value.count("\n", 0, self._cur_offset) + line, col = doc_value_offset_to_location(doc, self._cur_offset) + if line >= 0: prefix += f"{line}:" - # It would be nice to add the column as well, but we cannot determine - # that without knowing how much the doc string was unindented. + if col >= 0: + prefix += f"{col}:" + prefix += " \n" logger.warning(prefix + msg) @@ -334,3 +333,58 @@ def substitute_relative_crossrefs(obj: Object, checkref: Optional[Callable[[str] for member in obj.members.values(): if isinstance(member, Object): # pragma: no branch substitute_relative_crossrefs(member, checkref=checkref) + +def doc_value_offset_to_location(doc: Docstring, offset: int) -> tuple[int,int]: + """ + Converts offset into doc.value to line and column in source file. + + Returns: + line and column or else (-1,-1) if it cannot be computed + """ + linenum = -1 + colnum = -1 + + if doc.lineno is not None: + linenum = doc.lineno # start of the docstring source + # line offset with respect to start of cleaned up docstring + lineoffset = clean_lineoffset = doc.value.count("\n", 0, offset) + + # look at original doc source, if available + try: + source = doc.source + # compute docstring without cleaning up spaces and indentation + rawvalue = str(literal_eval(source)) + + # adjust line offset by number of lines removed from front of docstring + lineoffset += leading_space(rawvalue).count("\n") + + if lineoffset == 0 and (m := re.match(r"(\s*['\"]{3}\s*)\S", source)): + # is on the same line as opening triple quote + colnum = offset + len(m.group(1)) + else: + # indentation of first non-empty line in raw and cleaned up strings + raw_line = rawvalue.splitlines()[lineoffset] + clean_line = doc.value.splitlines()[clean_lineoffset] + raw_indent = len(leading_space(raw_line)) + clean_indent = len(leading_space(clean_line)) + try: + linestart = doc.value.rindex("\n", 0, offset) + 1 + except ValueError: # pragma: no cover + linestart = 0 # paranoid check, should not really happen + colnum = offset - linestart + raw_indent - clean_indent + + except Exception: + # Don't expect to get here, but just in case, it is better to + # not fix up the line/column than to die. + pass + + linenum += lineoffset + + return linenum, colnum + + +def leading_space(s: str) -> str: + """Returns whitespace at the front of string.""" + if m := re.match(r"\s*", s): + return m[0] + return "" # pragma: no cover diff --git a/tests/project/src/myproj/bar.py b/tests/project/src/myproj/bar.py index fc75c9d..9bb80b7 100644 --- a/tests/project/src/myproj/bar.py +++ b/tests/project/src/myproj/bar.py @@ -1,4 +1,4 @@ -# Copyright (c) 2022. Analog Devices Inc. +# Copyright (c) 2022-2025. Analog Devices Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -33,3 +33,11 @@ def foo(self) -> None: def func() -> None: """This is a function in the [bar][(m)] module.""" + +class Bad: + """More bad references""" + def bad_ref_leading_space(self) -> None: + """ + + This is a [bad][.] reference with leading space + """ diff --git a/tests/test_crossref.py b/tests/test_crossref.py index d0908fd..932fc45 100644 --- a/tests/test_crossref.py +++ b/tests/test_crossref.py @@ -18,18 +18,20 @@ import inspect import logging import re +from ast import literal_eval from pathlib import Path +from textwrap import dedent from typing import Callable, Optional import pytest -from griffe import Class, Docstring, Function, Module, Object +from griffe import Class, Docstring, Function, Module, Object, LinesCollection # noinspection PyProtectedMember from mkdocstrings_handlers.python_xref.crossref import ( _RE_CROSSREF, _RE_REL_CROSSREF, _RelativeCrossrefProcessor, - substitute_relative_crossrefs, + substitute_relative_crossrefs, doc_value_offset_to_location, ) def test_RelativeCrossrefProcessor(caplog: pytest.LogCaptureFixture) -> None: @@ -153,6 +155,7 @@ def test_substitute_relative_crossrefs(caplog: pytest.LogCaptureFixture) -> None """, parent=meth1, lineno=42, + endlineno=45, ) mod1.docstring = Docstring( @@ -161,6 +164,7 @@ def test_substitute_relative_crossrefs(caplog: pytest.LogCaptureFixture) -> None """, parent=mod1, lineno=23, + endlineno=25, ) substitute_relative_crossrefs(mod1) @@ -173,3 +177,88 @@ def test_substitute_relative_crossrefs(caplog: pytest.LogCaptureFixture) -> None ) assert len(caplog.records) == 0 + +def make_docstring_from_source( + source: str, + *, + lineno: int = 1, + mod_name: str = "mod", + mod_dir: Path = Path(""), +) -> Docstring: + """ + Create a docstring object from source code. + + Args: + source: raw source code containing docstring source lines + lineno: line number of docstring starting quotes + mod_name: name of module + mod_dir: module directory + """ + filepath = mod_dir.joinpath(mod_name).with_suffix(".py") + parent = Object("", lines_collection=LinesCollection()) + mod = Module(name=mod_name, filepath=filepath, parent=parent) + lines = source.splitlines(keepends=False) + if lineno > 1: + # Insert empty lines to pad to the desired line number + lines = [""] * (lineno - 1) + lines + mod.lines_collection[filepath] = lines + doc = Docstring( + parent=mod, + value=inspect.cleandoc(literal_eval(source)), + lineno=lineno, + endlineno=len(lines) + ) + return doc + +def test_doc_value_offset_to_location() -> None: + """Unit test for _doc_value_offset_to_location.""" + doc1 = make_docstring_from_source( + dedent( + ''' + """first + second + third + """ + ''' + ).lstrip("\n"), + ) + + assert doc_value_offset_to_location(doc1, 0) == (1, 3) + assert doc_value_offset_to_location(doc1, 3) == (1, 6) + assert doc_value_offset_to_location(doc1, 7) == (2, 1) + assert doc_value_offset_to_location(doc1, 15) == (3, 2) + + doc2 = make_docstring_from_source( + dedent( + ''' + """ first + second + third + """ # a comment + # another comment + ''' + ).lstrip("\n"), + lineno=3, + ) + + assert doc_value_offset_to_location(doc2, 0) == (3, 9) + assert doc_value_offset_to_location(doc2, 6) == (4, 6) + assert doc_value_offset_to_location(doc2, 15) == (5, 8) + + # Remove parent so that source is not available + doc2.parent = None + assert doc_value_offset_to_location(doc2, 0) == (3, -1) + + doc3 = make_docstring_from_source( + dedent( + """ + ''' + first + second + ''' + """ + ).lstrip("\n"), + ) + + assert doc_value_offset_to_location(doc3, 0) == (2, 4) + assert doc_value_offset_to_location(doc3, 6) == (3, 2) diff --git a/tests/test_integration.py b/tests/test_integration.py index 472dd22..60f70d1 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,4 +1,4 @@ -# Copyright (c) 2022-2024. Analog Devices Inc. +# Copyright (c) 2022-2025. Analog Devices Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -77,17 +77,20 @@ def test_integration(tmpdir: PathLike) -> None: assert result.returncode == 0 m = re.search( - r"WARNING.*file://(/.*/myproj/bar.py):(\d+):\s*\n\s*Cannot load reference '(.*)'", + r"WARNING.*file://(/.*/myproj/bar.py):(\d+):(\d+):\s*\n\s*Cannot load reference '(.*)'", result.stderr ) assert m is not None if os.path.sep == '/': assert m[1] == str(bar_src_file) - assert m[3] == 'myproj.bar.bad' + assert m[4] == 'myproj.bar.bad' # Source location not accurate in python 3.7 - bad_line = int(m[2]) + bad_linenum = int(m[2]) + bad_col = int(m[3]) bar_lines = bar_src_file.read_text().splitlines() - assert '[bad]' in bar_lines[bad_line - 1] + bad_line = bar_lines[bad_linenum - 1] + assert '[bad]' in bad_line + assert bad_line[bad_col:].startswith('[bad]') bar_html = site_dir.joinpath('bar', 'index.html').read_text() bar_bs = bs4.BeautifulSoup(bar_html, 'html.parser') From 20b906281d975792a63a3f4cbe91e6d7ad417943 Mon Sep 17 00:00:00 2001 From: Christopher Barber Date: Sun, 23 Mar 2025 20:51:18 -0400 Subject: [PATCH 2/3] Fix issue with literal_eval --- .../python_xref/crossref.py | 17 ++++++++++++++--- tests/test_crossref.py | 3 ++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/mkdocstrings_handlers/python_xref/crossref.py b/src/mkdocstrings_handlers/python_xref/crossref.py index 24fb89f..687ac01 100644 --- a/src/mkdocstrings_handlers/python_xref/crossref.py +++ b/src/mkdocstrings_handlers/python_xref/crossref.py @@ -15,9 +15,10 @@ from __future__ import annotations +import ast import re -from ast import literal_eval -from typing import Callable, List, Optional, cast +import sys +from typing import Any, Callable, List, Optional, cast from griffe import Docstring, Object from mkdocstrings import get_logger @@ -353,7 +354,7 @@ def doc_value_offset_to_location(doc: Docstring, offset: int) -> tuple[int,int]: try: source = doc.source # compute docstring without cleaning up spaces and indentation - rawvalue = str(literal_eval(source)) + rawvalue = str(safe_eval(source)) # adjust line offset by number of lines removed from front of docstring lineoffset += leading_space(rawvalue).count("\n") @@ -388,3 +389,13 @@ def leading_space(s: str) -> str: if m := re.match(r"\s*", s): return m[0] return "" # pragma: no cover + +if sys.version_info < (3, 10) or True: + # TODO: remove when 3.9 support is dropped + # In 3.9, literal_eval cannot handle comments in input + def safe_eval(s: str) -> Any: + """Safely evaluate a string expression.""" + return eval(s) #eval(s, dict(__builtins__={}), {}) +else: + save_eval = ast.literal_eval + diff --git a/tests/test_crossref.py b/tests/test_crossref.py index 932fc45..df55c8e 100644 --- a/tests/test_crossref.py +++ b/tests/test_crossref.py @@ -204,7 +204,7 @@ def make_docstring_from_source( mod.lines_collection[filepath] = lines doc = Docstring( parent=mod, - value=inspect.cleandoc(literal_eval(source)), + value=inspect.cleandoc(eval(source)), lineno=lineno, endlineno=len(lines) ) @@ -235,6 +235,7 @@ def test_doc_value_offset_to_location() -> None: second third """ # a comment + # another comment ''' ).lstrip("\n"), From 0730a23acab30591fa7945ee86ab14af1c72cddc Mon Sep 17 00:00:00 2001 From: Christopher Barber Date: Sun, 30 Mar 2025 13:31:23 -0400 Subject: [PATCH 3/3] Report using column numbers starting with 1 --- CHANGELOG.md | 3 ++- .../python_xref/crossref.py | 8 ++++---- tests/test_crossref.py | 19 ++++++++++--------- tests/test_integration.py | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0760c9d..a72bf44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ are compatible with.* ## 1.16.2 -* Improved source locations for errors in docstrings now including column offset. +* Improved source locations for errors in docstrings now including column numbers + (starting at 1). ## 1.16.1 diff --git a/src/mkdocstrings_handlers/python_xref/crossref.py b/src/mkdocstrings_handlers/python_xref/crossref.py index 687ac01..2fb3084 100644 --- a/src/mkdocstrings_handlers/python_xref/crossref.py +++ b/src/mkdocstrings_handlers/python_xref/crossref.py @@ -343,7 +343,7 @@ def doc_value_offset_to_location(doc: Docstring, offset: int) -> tuple[int,int]: line and column or else (-1,-1) if it cannot be computed """ linenum = -1 - colnum = -1 + colnum = -2 if doc.lineno is not None: linenum = doc.lineno # start of the docstring source @@ -359,8 +359,8 @@ def doc_value_offset_to_location(doc: Docstring, offset: int) -> tuple[int,int]: # adjust line offset by number of lines removed from front of docstring lineoffset += leading_space(rawvalue).count("\n") - if lineoffset == 0 and (m := re.match(r"(\s*['\"]{3}\s*)\S", source)): - # is on the same line as opening triple quote + if lineoffset == 0 and (m := re.match(r"(\s*['\"]{1,3}\s*)\S", source)): + # is on the same line as opening quote colnum = offset + len(m.group(1)) else: # indentation of first non-empty line in raw and cleaned up strings @@ -381,7 +381,7 @@ def doc_value_offset_to_location(doc: Docstring, offset: int) -> tuple[int,int]: linenum += lineoffset - return linenum, colnum + return linenum, colnum + 1 def leading_space(s: str) -> str: diff --git a/tests/test_crossref.py b/tests/test_crossref.py index df55c8e..501ef0e 100644 --- a/tests/test_crossref.py +++ b/tests/test_crossref.py @@ -223,10 +223,11 @@ def test_doc_value_offset_to_location() -> None: ).lstrip("\n"), ) - assert doc_value_offset_to_location(doc1, 0) == (1, 3) - assert doc_value_offset_to_location(doc1, 3) == (1, 6) - assert doc_value_offset_to_location(doc1, 7) == (2, 1) - assert doc_value_offset_to_location(doc1, 15) == (3, 2) + # note columns start with 1 + assert doc_value_offset_to_location(doc1, 0) == (1, 4) + assert doc_value_offset_to_location(doc1, 3) == (1, 7) + assert doc_value_offset_to_location(doc1, 7) == (2, 2) + assert doc_value_offset_to_location(doc1, 15) == (3, 3) doc2 = make_docstring_from_source( dedent( @@ -242,9 +243,9 @@ def test_doc_value_offset_to_location() -> None: lineno=3, ) - assert doc_value_offset_to_location(doc2, 0) == (3, 9) - assert doc_value_offset_to_location(doc2, 6) == (4, 6) - assert doc_value_offset_to_location(doc2, 15) == (5, 8) + assert doc_value_offset_to_location(doc2, 0) == (3, 10) + assert doc_value_offset_to_location(doc2, 6) == (4, 7) + assert doc_value_offset_to_location(doc2, 15) == (5, 9) # Remove parent so that source is not available doc2.parent = None @@ -261,5 +262,5 @@ def test_doc_value_offset_to_location() -> None: ).lstrip("\n"), ) - assert doc_value_offset_to_location(doc3, 0) == (2, 4) - assert doc_value_offset_to_location(doc3, 6) == (3, 2) + assert doc_value_offset_to_location(doc3, 0) == (2, 5) + assert doc_value_offset_to_location(doc3, 6) == (3, 3) diff --git a/tests/test_integration.py b/tests/test_integration.py index 60f70d1..42661ac 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -86,7 +86,7 @@ def test_integration(tmpdir: PathLike) -> None: assert m[4] == 'myproj.bar.bad' # Source location not accurate in python 3.7 bad_linenum = int(m[2]) - bad_col = int(m[3]) + bad_col = int(m[3]) - 1 # 1-based indexing bar_lines = bar_src_file.read_text().splitlines() bad_line = bar_lines[bad_linenum - 1] assert '[bad]' in bad_line