Merge pull request #39 from analog-garage/38-better-location

Improved line/col information (38)

Author: analog-cbarber

.idea/garpy.mkdocstrings.iml

@@ -1,13 +1,21 @@
 # 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 numbers
+  (starting at 1).
+
 ## 1.16.1
 
 * Fix sdist distributions (should enable conda-forge to build)
 
 ## 1.16.0
 
 * Compatibility with mkdocstrings-python 1.16.*
-* Removed some deprecated imports from mkdoctrings
+* Removed some deprecated imports from mkdocstrings
 
 ## 1.14.1
 

CHANGELOG.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

docs/index.md

@@ -1 +1 @@
-1.16.1
+1.16.2

src/mkdocstrings_handlers/python_xref/VERSION

@@ -15,8 +15,10 @@
 
 from __future__ import annotations
 
+import ast
 import re
-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
@@ -303,14 +305,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 +334,68 @@ 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 = -2
+
+    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(safe_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*['\"]{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
+                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 + 1
+
+
+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
+
+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
+

src/mkdocstrings_handlers/python_xref/crossref.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
+        """

tests/project/src/myproj/bar.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,90 @@ 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(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"),
+    )
+
+    # 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(
+            '''   
+               """   first
+                  second
+                   third
+               """  # a comment
+               
+            # another comment
+            '''
+        ).lstrip("\n"),
+        lineno=3,
+    )
+
+    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
+    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, 5)
+    assert doc_value_offset_to_location(doc3, 6) == (3, 3)

tests/test_crossref.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]) - 1 # 1-based indexing
     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')