From 20945e9cc5102674447a3c0ae5587b952477f32f Mon Sep 17 00:00:00 2001 From: johnr Date: Thu, 11 Jun 2026 02:49:59 -0400 Subject: [PATCH] fix: honor pyproject-declared source roots in Python import resolution Projects that keep importable code in a subdirectory of the repo root (e.g. scripts/ run with PYTHONPATH=scripts) previously had every absolute import fail to resolve: the dependency graph reported 0 importers for every module and the test-coverage mapper flagged fully-tested modules as 'Untested module (N LOC, 0 importers)' across the board. resolve_absolute_import and the test-coverage import-spec mapper now also try source roots declared in pyproject.toml: - [tool.desloppify] python_source_roots (explicit override) - [tool.pytest.ini_options] pythonpath - [tool.mypy] mypy_path Existing resolution order is preserved (scan root, then project root, then declared roots); src/ layout support is unchanged. --- .../python/detectors/deps_resolution.py | 28 +-- desloppify/languages/python/source_roots.py | 80 ++++++++ desloppify/languages/python/test_coverage.py | 27 ++- .../python/tests/test_py_source_roots.py | 174 ++++++++++++++++++ 4 files changed, 295 insertions(+), 14 deletions(-) create mode 100644 desloppify/languages/python/source_roots.py create mode 100644 desloppify/languages/python/tests/test_py_source_roots.py diff --git a/desloppify/languages/python/detectors/deps_resolution.py b/desloppify/languages/python/detectors/deps_resolution.py index ef770c22c..8c3586a59 100644 --- a/desloppify/languages/python/detectors/deps_resolution.py +++ b/desloppify/languages/python/detectors/deps_resolution.py @@ -5,6 +5,7 @@ from pathlib import Path from desloppify.base.discovery.paths import get_project_root +from desloppify.languages.python.source_roots import declared_source_roots def resolve_python_from_import( @@ -107,19 +108,22 @@ def resolve_relative_import(module_path: str, source_dir: Path) -> str | None: def resolve_absolute_import(module_path: str, scan_root: Path) -> str | None: - """Resolve an absolute import within scan root first, then project root.""" + """Resolve an absolute import within scan root, project root, then any + pyproject-declared source roots (e.g. ``scripts/`` for projects run with + ``PYTHONPATH=scripts``).""" parts = module_path.split(".") - target_base = scan_root.resolve() - for part in parts: - target_base = target_base / part - resolved = try_resolve_path(target_base) - if resolved: - return resolved - - target_base = get_project_root() - for part in parts: - target_base = target_base / part - return try_resolve_path(target_base) + project_root = get_project_root() + bases = [scan_root.resolve(), project_root] + bases += [project_root / root for root in declared_source_roots(str(project_root))] + + for base in bases: + target_base = base + for part in parts: + target_base = target_base / part + resolved = try_resolve_path(target_base) + if resolved: + return resolved + return None def try_resolve_path(target_base: Path) -> str | None: diff --git a/desloppify/languages/python/source_roots.py b/desloppify/languages/python/source_roots.py new file mode 100644 index 000000000..90205a253 --- /dev/null +++ b/desloppify/languages/python/source_roots.py @@ -0,0 +1,80 @@ +"""Discover Python source roots declared by the project layout. + +Projects that keep importable code in a subdirectory of the repo root (e.g. +``scripts/`` run with ``PYTHONPATH=scripts``, or ``src/`` layouts) declare +those roots in ``pyproject.toml``. Import resolution honors the declared +roots so absolute imports resolve to files the same way they do at runtime. +Without this, every ``import mypkg`` in such a project fails to resolve: +the dependency graph reports 0 importers everywhere and the test-coverage +mapper marks fully-tested modules as untested. + +Recognized declarations (first match wins per root, duplicates dropped): + +- ``[tool.desloppify] python_source_roots = ["scripts"]`` (explicit override) +- ``[tool.pytest.ini_options] pythonpath = ["scripts", ...]`` +- ``[tool.mypy] mypy_path = "scripts"`` +""" + +from __future__ import annotations + +import tomllib +from functools import lru_cache +from pathlib import Path + + +def _as_list(value: object) -> list[str]: + """Normalize a TOML string (``:``/``,`` separated) or list into a list.""" + if isinstance(value, str): + parts = value.replace(",", ":").split(":") + return [part.strip() for part in parts if part.strip()] + if isinstance(value, (list, tuple)): + return [str(item).strip() for item in value if str(item).strip()] + return [] + + +@lru_cache(maxsize=None) +def declared_source_roots(project_root: str) -> tuple[str, ...]: + """Return source-root directories (relative to *project_root*) declared + in ``pyproject.toml``. + + Only safe relative roots are returned: absolute paths, parent traversal, + and ``.`` (the project root itself, already tried by resolvers) are + dropped. Returns ``()`` when no pyproject exists or nothing is declared. + """ + pyproject = Path(project_root) / "pyproject.toml" + if not pyproject.is_file(): + return () + try: + data = tomllib.loads(pyproject.read_text(encoding="utf-8")) + except (OSError, tomllib.TOMLDecodeError): + return () + tool = data.get("tool") + if not isinstance(tool, dict): + return () + + roots: list[str] = [] + explicit = tool.get("desloppify") + if isinstance(explicit, dict): + roots += _as_list(explicit.get("python_source_roots")) + pytest_tool = tool.get("pytest") + if isinstance(pytest_tool, dict): + ini_options = pytest_tool.get("ini_options") + if isinstance(ini_options, dict): + roots += _as_list(ini_options.get("pythonpath")) + mypy_tool = tool.get("mypy") + if isinstance(mypy_tool, dict): + roots += _as_list(mypy_tool.get("mypy_path")) + + cleaned: list[str] = [] + seen: set[str] = set() + for root in roots: + root = root.strip().rstrip("/") + if not root or root == "." or root.startswith(("/", "..", "~")): + continue + if root not in seen: + seen.add(root) + cleaned.append(root) + return tuple(cleaned) + + +__all__ = ["declared_source_roots"] diff --git a/desloppify/languages/python/test_coverage.py b/desloppify/languages/python/test_coverage.py index 8b7710cb0..30e682daf 100644 --- a/desloppify/languages/python/test_coverage.py +++ b/desloppify/languages/python/test_coverage.py @@ -48,6 +48,29 @@ _SRC_PREFIXES = ("src/",) +def _layout_prefixes() -> tuple[str, ...]: + """Source-layout prefixes: ``src/`` plus any pyproject-declared roots. + + Projects that run with ``PYTHONPATH=`` (declared via + ``[tool.pytest.ini_options] pythonpath``, ``[tool.mypy] mypy_path`` or + ``[tool.desloppify] python_source_roots``) keep production files under + ``/`` while tests import them root-relatively; without these + prefixes every import-based test->source mapping misses. + """ + prefixes = list(_SRC_PREFIXES) + try: + from desloppify.base.discovery.paths import get_project_root + from desloppify.languages.python.source_roots import declared_source_roots + + for root in declared_source_roots(str(get_project_root())): + prefix = f"{root}/" + if prefix not in prefixes: + prefixes.append(prefix) + except Exception: # pragma: no cover - project root unset in bare unit use + pass + return tuple(prefixes) + + def has_testable_logic(filepath: str, content: str) -> bool: """Return True if the file contains runtime logic worth testing.""" del filepath @@ -69,8 +92,8 @@ def resolve_import_spec( for candidate in candidates: if candidate in production_files: return candidate - # Try src/-prefixed variants for src-layout projects - for prefix in _SRC_PREFIXES: + # Try layout-prefixed variants (src/ and pyproject-declared roots). + for prefix in _layout_prefixes(): prefixed = f"{prefix}{candidate}" if prefixed in production_files: return prefixed diff --git a/desloppify/languages/python/tests/test_py_source_roots.py b/desloppify/languages/python/tests/test_py_source_roots.py new file mode 100644 index 000000000..d65b75aba --- /dev/null +++ b/desloppify/languages/python/tests/test_py_source_roots.py @@ -0,0 +1,174 @@ +"""Tests for pyproject-declared Python source roots in import resolution. + +Covers projects whose importable code lives in a subdirectory of the repo +root (e.g. ``scripts/`` with ``PYTHONPATH=scripts``): declared roots must be +honored by ``resolve_absolute_import`` and by the test-coverage import-spec +mapper, otherwise the dependency graph reports 0 importers everywhere and +fully-tested modules are flagged untested. +""" + +import textwrap +from pathlib import Path + +from desloppify.languages.python.detectors.deps_resolution import ( + resolve_absolute_import, +) +from desloppify.languages.python.source_roots import declared_source_roots +from desloppify.languages.python.test_coverage import resolve_import_spec + +# ── Helpers ──────────────────────────────────────────────── + + +def _project(tmp_path: Path, pyproject: str, files: dict[str, str]) -> Path: + (tmp_path / "pyproject.toml").write_text(textwrap.dedent(pyproject)) + for rel_path, content in files.items(): + fp = tmp_path / rel_path + fp.parent.mkdir(parents=True, exist_ok=True) + fp.write_text(content) + declared_source_roots.cache_clear() + return tmp_path + + +def _use_root(monkeypatch, root: Path) -> None: + monkeypatch.setenv("DESLOPPIFY_ROOT", str(root)) + + +# ── declared_source_roots ───────────────────────────────── + + +class TestDeclaredSourceRoots: + def test_no_pyproject_returns_empty(self, tmp_path): + declared_source_roots.cache_clear() + assert declared_source_roots(str(tmp_path)) == () + + def test_pytest_pythonpath_list(self, tmp_path): + _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = ["scripts", "tools"] + """, + {}, + ) + assert declared_source_roots(str(tmp_path)) == ("scripts", "tools") + + def test_pytest_pythonpath_string(self, tmp_path): + _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = "scripts" + """, + {}, + ) + assert declared_source_roots(str(tmp_path)) == ("scripts",) + + def test_mypy_path_and_explicit_override_deduped(self, tmp_path): + _project( + tmp_path, + """ + [tool.desloppify] + python_source_roots = ["scripts"] + + [tool.mypy] + mypy_path = "scripts" + """, + {}, + ) + assert declared_source_roots(str(tmp_path)) == ("scripts",) + + def test_unsafe_roots_dropped(self, tmp_path): + _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = [".", "/abs", "../up", "scripts/"] + """, + {}, + ) + assert declared_source_roots(str(tmp_path)) == ("scripts",) + + def test_invalid_toml_returns_empty(self, tmp_path): + (tmp_path / "pyproject.toml").write_text("not [ valid toml") + declared_source_roots.cache_clear() + assert declared_source_roots(str(tmp_path)) == () + + +# ── resolve_absolute_import with declared roots ─────────── + + +class TestResolveAbsoluteImportSourceRoots: + def test_resolves_module_under_declared_root(self, tmp_path, monkeypatch): + root = _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = ["scripts"] + """, + {"scripts/mypkg/__init__.py": "", "scripts/mypkg/store.py": "X = 1\n"}, + ) + _use_root(monkeypatch, root) + resolved = resolve_absolute_import("mypkg.store", root) + assert resolved == str((root / "scripts/mypkg/store.py").resolve()) + + def test_scan_root_still_wins_over_declared_root(self, tmp_path, monkeypatch): + root = _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = ["scripts"] + """, + { + "mypkg/store.py": "ROOT = 1\n", + "scripts/mypkg/store.py": "SCRIPTS = 1\n", + }, + ) + _use_root(monkeypatch, root) + resolved = resolve_absolute_import("mypkg.store", root) + assert resolved == str((root / "mypkg/store.py").resolve()) + + def test_unresolvable_returns_none(self, tmp_path, monkeypatch): + root = _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = ["scripts"] + """, + {}, + ) + _use_root(monkeypatch, root) + assert resolve_absolute_import("missing.module", root) is None + + +# ── test-coverage import-spec mapping with declared roots ─ + + +class TestResolveImportSpecSourceRoots: + def test_spec_resolves_via_declared_root_prefix(self, tmp_path, monkeypatch): + root = _project( + tmp_path, + """ + [tool.pytest.ini_options] + pythonpath = ["scripts"] + """, + {}, + ) + _use_root(monkeypatch, root) + production = {"scripts/mypkg/store.py", "scripts/mypkg/__init__.py"} + assert ( + resolve_import_spec("mypkg.store", "tests/unit/test_store.py", production) + == "scripts/mypkg/store.py" + ) + assert ( + resolve_import_spec("mypkg", "tests/unit/test_store.py", production) + == "scripts/mypkg/__init__.py" + ) + + def test_src_prefix_still_supported(self, tmp_path, monkeypatch): + root = _project(tmp_path, "", {}) + _use_root(monkeypatch, root) + production = {"src/mypkg/store.py"} + assert ( + resolve_import_spec("mypkg.store", "tests/test_store.py", production) + == "src/mypkg/store.py" + )