Avoid infinite loop in module_name_from_path (#89)

`Path(".").parent` can't move past the "." in a relative path and will
just return `Path(".")` again. This lead to `while True` never breaking.

To fix this, we make sure that absolute paths are used. We need to
resolve the `path` before `lru_cache` sees it. Otherwise,  `lru_cache`
might return a wrong cached result in case the current working directory
changes. That should never happen in docstub, but I think it's still
good to be defensive here.

This change also adds a few other defensive guards and asserts.

Long term, it might be the least error-prone to resolve all paths to
absolute ones as soon as possible. However, we'd have to do some
additional work to shorten paths that are within the current working
directory. Otherwise, users might see unnecessarily long paths in
their output.

Also: 
- Warn in case docstub is run on a subpackage only
- Appease mypy pointing out call to untyped function in tests
- Appease mypy pointing out a missing return type annotation

---------

Co-authored-by: Lars Grüter <lagru@mailbox.org>

Author: geographika

src/docstub/_cli.py

@@ -18,6 +18,7 @@
 from ._config import Config
 from ._path_utils import (
     STUB_HEADER_COMMENT,
+    find_package_root,
     walk_source_and_targets,
     walk_source_package,
 )
@@ -326,8 +327,13 @@ def run(
     root_path = Path(root_path)
     if root_path.is_file():
         logger.warning(
-            "Running docstub on a single file. Relative imports "
-            "or type references outside this file won't work."
+            "Running docstub on a single module. Relative imports "
+            "or type references pointing outside this module won't work."
+        )
+    elif find_package_root(root_path) != root_path.resolve():
+        logger.warning(
+            "Running docstub only on a subpackage. Relative imports "
+            "or type references pointing outside this subpackage won't work."
         )
 
     config = _load_configuration(config_paths)

src/docstub/_path_utils.py

@@ -107,7 +107,7 @@ def is_python_package_dir(path):
 
 
 def find_package_root(path):
-    """Determine the root a Python package from any path pointing inside it.
+    """Determine the root of a Python package from any path pointing inside it.
 
     Parameters
     ----------
@@ -121,18 +121,21 @@ def find_package_root(path):
     --------
     >>> from pathlib import Path
     >>> package_root = find_package_root(Path(__file__))
-    >>> (package_root / "docstub").is_dir()
+    >>> package_root.name
+    'docstub'
+
+    >>> find_package_root(package_root) == package_root
     True
     """
-    root = path
-    if root.is_file():
-        root = root.parent
+    root = path.resolve()  # `Path.parent` can't move past relative "." part
 
     for _ in range(2**16):
-        if not is_python_package_dir(root):
+        parent = root.parent
+        assert parent
+        if not is_python_package_dir(parent):
             logger.debug("Detected %s as the package root of %s", root, path)
             return root
-        root = root.parent
+        root = parent
 
     msg = f"exceeded iteration length while trying to find package root for {path}"
     raise RuntimeError(msg)

src/docstub/_utils.py

@@ -1,6 +1,6 @@
 import itertools
 import re
-from functools import lru_cache
+from functools import lru_cache, wraps
 from zlib import crc32
 
 
@@ -60,6 +60,33 @@ def escape_qualname(name):
     return qualname
 
 
+def _resolve_path_before_caching(func):
+    """Resolve relative paths passed to :func:`module_name_from_path`.
+
+    :func:`module_name_from_path` makes use of Python's :func:`lru_cache`
+    decorator. Caching results based on relative paths may return wrong results
+    if the current working directory changes.
+
+    Access the :func:`lru_cache` specific attributes with ``func.__wrapped__``.
+
+    Parameters
+    ----------
+    func : Callable
+
+    Returns
+    -------
+    wrapped : Callable
+    """
+
+    @wraps(func)
+    def wrapped(file_path):
+        file_path = file_path.resolve()
+        return func(file_path)
+
+    return wrapped
+
+
+@_resolve_path_before_caching
 @lru_cache(maxsize=100)
 def module_name_from_path(path):
     """Find the full name of a module within its package from its file path.
@@ -86,16 +113,25 @@ def module_name_from_path(path):
 
     name_parts = []
     if path.name != "__init__.py":
+        assert path.stem
         name_parts.insert(0, path.stem)
 
+    iter_limit = 10_000
     directory = path.parent
-    while True:
+    for _ in range(iter_limit):
         is_in_package = (directory / "__init__.py").is_file()
         if is_in_package:
+            assert directory.name
             name_parts.insert(0, directory.name)
             directory = directory.parent
         else:
             break
+    else:
+        msg = (
+            f"Reached iteration limit ({iter_limit}) "
+            f"while trying to find module name for {path!r}"
+        )
+        raise RuntimeError(msg)
 
     name = ".".join(name_parts)
     return name

tests/test_utils.py

@@ -1,6 +1,19 @@
+import os
+from pathlib import Path
+
 from docstub import _utils
 
 
+def _create_dummy_package(root: Path, structure: list[str]) -> None:
+    """Create a dummy Python package in `root` based on subpaths in `structure`."""
+    for item in structure:
+        path = root / item
+        if item.endswith(".py"):
+            path.touch()
+        else:
+            path.mkdir()
+
+
 class Test_module_name_from_path:
     def test_basic(self, tmp_path):
         # Package structure
@@ -12,12 +25,7 @@ def test_basic(self, tmp_path):
             "foo/baz/__init__.py",
             "foo/baz/qux.py",
         ]
-        for item in structure:
-            path = tmp_path / item
-            if item.endswith(".py"):
-                path.touch()
-            else:
-                path.mkdir()
+        _create_dummy_package(tmp_path, structure)
 
         assert _utils.module_name_from_path(tmp_path / "foo/__init__.py") == "foo"
         assert _utils.module_name_from_path(tmp_path / "foo/bar.py") == "foo.bar"
@@ -28,6 +36,28 @@ def test_basic(self, tmp_path):
             _utils.module_name_from_path(tmp_path / "foo/baz/qux.py") == "foo.baz.qux"
         )
 
+    def test_relative_path(self, tmp_path_cwd):
+        structure = [
+            "foo/",
+            "foo/__init__.py",
+            "foo/bar.py",
+            "foo/baz/",
+            "foo/baz/__init__.py",
+            "foo/baz/bar.py",
+        ]
+        _create_dummy_package(tmp_path_cwd, structure)
+        os.chdir(tmp_path_cwd / "foo")
+        cwd = Path()
+
+        assert _utils.module_name_from_path(cwd / "__init__.py") == "foo"
+        assert _utils.module_name_from_path(cwd / "bar.py") == "foo.bar"
+
+        # `./__init__.py` and `./bar.py` should return different results in
+        # different working directories
+        os.chdir(tmp_path_cwd / "foo/baz")
+        assert _utils.module_name_from_path(cwd / "__init__.py") == "foo.baz"
+        assert _utils.module_name_from_path(cwd / "bar.py") == "foo.baz.bar"
+
 
 def test_pyfile_checksum(tmp_path):
     # Create package