From bb467634c9bd7c595b7c61ec1fe43e632bf9e28a Mon Sep 17 00:00:00 2001 From: safishamsi Date: Tue, 21 Jul 2026 13:44:49 +0100 Subject: [PATCH] fix(extract): resolve Python imports regardless of scan root (src-layout) (#2072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Python absolute imports were resolved only against the scan root, and file-node ids are scan-root-relative, so a src-layout project (code under src/) lost most of its imports/imports_from edges when scanned from the repo root — the dangling edges were silently dropped, so the graph looked complete but wasn't. The chosen scan root thus silently changed the graph. Two fixes: (1) _resolve_python_module_path probes the scan root first, then walks up from the importing file toward the root so a nested package root (src/pkg) resolves (mirrors the Lua upward walk); (2) a Python post-pass detects each file's package root via its __init__.py chain and repoints absolute-import edge targets (dotted-module id -> real file-node id), guarded against shadowing an existing id and against ambiguous aliases claimed by >1 file. Result: byte- identical import edges whether scanned from the repo root or from src/. --- graphify/extract.py | 66 +++++++++++ graphify/extractors/resolution.py | 43 +++++-- tests/test_src_layout_import_resolution.py | 125 +++++++++++++++++++++ 3 files changed, 225 insertions(+), 9 deletions(-) create mode 100644 tests/test_src_layout_import_resolution.py diff --git a/graphify/extract.py b/graphify/extract.py index 401f983..ccc9c53 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -179,6 +179,68 @@ def _file_node_id(rel_path: Path) -> str: return _make_id(_file_stem(rel_path)) +def _repoint_python_package_imports(paths, all_nodes, all_edges, root) -> None: + """Repoint Python absolute-import edges to the real file node under a nested + (e.g. ``src/``) package root (#2072). + + Absolute imports target an id derived from the dotted module path + (``_make_id('pkg.mod')`` -> ``pkg_mod``), but file-node ids are + scan-root-relative (``src_pkg_mod`` when the code lives under ``src/``), so + the edge dangles and is silently dropped — the graph loses most ``imports`` + edges purely because of where the scan started. Build an alias map from the + dotted-module id to the real file-node id by detecting each ``.py`` file's + package root (the contiguous run of ancestor dirs carrying ``__init__.py``) + and rewrite matching ``imports``/``imports_from`` edge targets. Guards: never + shadow an existing node id, and drop an alias claimed by more than one file + (ambiguous -> leave dangling, as before). Files whose package root IS the + scan root are skipped (ids already coincide).""" + try: + root = Path(root).resolve() + except OSError: + root = Path(root) + node_ids = {n.get("id") for n in all_nodes if isinstance(n, dict)} + alias_to_files: dict[str, set[str]] = {} + for p in paths: + if p.suffix.lower() not in (".py", ".pyi"): + continue + try: + rel = Path(p).resolve().relative_to(root) + except (ValueError, OSError): + continue + parts = rel.parts + if len(parts) < 2: + continue # top-level file: scan-root-relative id already matches + d = Path(p).resolve().parent + levels = 0 + while (d / "__init__.py").is_file(): + levels += 1 + d = d.parent + if levels == 0: + continue # not inside a package (namespace pkg / loose module) + mod_parts = parts[-(levels + 1):] # package dirs + the file itself + if len(mod_parts) == len(parts): + continue # package root == scan root: file-node id already coincides + file_node = _file_node_id(rel) + alias = _make_id(str(Path(*mod_parts).with_suffix(""))) + alias_to_files.setdefault(alias, set()).add(file_node) + if p.name in ("__init__.py", "__init__.pyi") and len(mod_parts) > 1: + # `import pkg` / `from pkg import x` targets the package-dir id. + pkg_alias = _make_id(str(Path(*mod_parts[:-1]))) + alias_to_files.setdefault(pkg_alias, set()).add(file_node) + alias_map = { + a: next(iter(fs)) + for a, fs in alias_to_files.items() + if len(fs) == 1 and a not in node_ids + } + if not alias_map: + return + for e in all_edges: + if isinstance(e, dict) and e.get("relation") in ("imports", "imports_from"): + tgt = e.get("target") + if tgt in alias_map: + e["target"] = alias_map[tgt] + + SEMANTIC_RELATIONS = frozenset({ "inherits", "implements", "mixes_in", "embeds", "references", "calls", "imports", "imports_from", "re_exports", "contains", "method", @@ -4755,6 +4817,10 @@ def extract( if dec is not None: e["target"] = f"{dec[0]}_{dec[1]}" + # Repoint Python absolute imports onto the real file nodes under a nested + # (src/) package root before the resolver/import-evidence passes run, so the + # graph is identical regardless of scan root (#2072). + _repoint_python_package_imports(paths, all_nodes, all_edges, root) _merge_swift_extensions(per_file, all_nodes, all_edges) _disambiguate_colliding_node_ids(all_nodes, all_edges, all_raw_calls, root) _canonicalize_csharp_namespace_nodes(all_nodes, all_edges) diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index 31a2e79..7631f1a 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -1605,15 +1605,9 @@ def _python_imported_names(node, source: bytes) -> list[tuple[str, str]]: names.append((name, local)) return names -def _resolve_python_module_path(module_name: str, current_path: Path, root: Path, level: int) -> Path | None: - if level > 0: - base = current_path.parent - for _ in range(level - 1): - base = base.parent - candidate = base / module_name.replace(".", "/") if module_name else base - else: - candidate = root / module_name.replace(".", "/") - +def _probe_python_module_candidate(candidate: Path) -> Path | None: + """Resolve one module-path candidate to a .py file (dir+__init__, exact, or + with a .py suffix), or None.""" if candidate.is_dir(): init_path = candidate / "__init__.py" if init_path.is_file(): @@ -1625,6 +1619,37 @@ def _resolve_python_module_path(module_name: str, current_path: Path, root: Path return py_candidate return None + +def _resolve_python_module_path(module_name: str, current_path: Path, root: Path, level: int) -> Path | None: + if level > 0: + base = current_path.parent + for _ in range(level - 1): + base = base.parent + candidate = base / module_name.replace(".", "/") if module_name else base + return _probe_python_module_candidate(candidate) + + # Absolute import. Probe the scan root first (unchanged for the common + # root-is-package-root layout), then walk up from the importing file toward + # the root so a `src/` (or otherwise nested) package root resolves regardless + # of where the scan started — `import pkg.mod` from src/pkg/app.py must find + # src/pkg/mod.py whether the scan root is the repo or src/ (#2072). Mirrors + # the upward walk already used for Lua (_resolve_lua_import_target, #1075). + rel = module_name.replace(".", "/") + hit = _probe_python_module_candidate(root / rel) + if hit is not None: + return hit + for anc in current_path.parents: + try: + anc.relative_to(root) + except ValueError: + break # left the scan root; stop walking up + if anc == root: + continue # already probed root/rel above + cand = _probe_python_module_candidate(anc / rel) + if cand is not None: + return cand + return None + def _python_top_level_function_bodies(path: Path, root_node, source: bytes) -> list[tuple[str, object]]: bodies: list[tuple[str, object]] = [] stem = _file_stem(path) diff --git a/tests/test_src_layout_import_resolution.py b/tests/test_src_layout_import_resolution.py new file mode 100644 index 0000000..31d7587 --- /dev/null +++ b/tests/test_src_layout_import_resolution.py @@ -0,0 +1,125 @@ +"""#2072: Python import resolution must not depend on the scan root. + +A src-layout project (code under `src/`) used to lose most of its `imports` / +`imports_from` edges when scanned from the repo root, because absolute imports +were resolved only against the scan root while file-node ids are scan-root +relative. The same project scanned from `src/` resolved fine — so the chosen +scan root silently changed the graph. +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract +from graphify.extractors.resolution import _resolve_python_module_path +from graphify.build import build_from_json + + +_FILES = { + "mypkg/__init__.py": "from mypkg.core import Engine\n", + "mypkg/core.py": "class Engine:\n pass\n", + "mypkg/helpers.py": "def helper():\n return 1\n", + "mypkg/app.py": ( + "from mypkg.core import Engine\n" + "import mypkg.helpers\n\n" + "def run():\n return mypkg.helpers.helper()\n" + ), +} + + +def _write(base: Path, prefix: str = "") -> list[Path]: + written = [] + for rel, body in _FILES.items(): + p = base / prefix / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body, encoding="utf-8") + written.append(p) + return written + + +def _import_edges(G): + """(relation, source, target) for import edges, present-endpoints only.""" + return { + (d.get("relation"), u, v) + for u, v, d in G.edges(data=True) + if d.get("relation") in ("imports", "imports_from") + } + + +def test_resolve_python_module_path_walks_up_to_src_package_root(tmp_path): + (tmp_path / "src" / "mypkg").mkdir(parents=True) + core = tmp_path / "src" / "mypkg" / "core.py" + core.write_text("class Engine: pass\n") + app = tmp_path / "src" / "mypkg" / "app.py" + app.write_text("from mypkg.core import Engine\n") + # scan root is the repo, code is under src/: must still resolve. + resolved = _resolve_python_module_path("mypkg.core", app, tmp_path, level=0) + assert resolved == core + # flat layout (package at root) is unchanged. + (tmp_path / "flat").mkdir() + (tmp_path / "flat" / "mod.py").write_text("x = 1\n") + assert _resolve_python_module_path("flat.mod", tmp_path / "flat" / "a.py", tmp_path, 0) == ( + tmp_path / "flat" / "mod.py" + ) + + +def test_import_edges_identical_from_root_or_src(tmp_path): + """Headline (#2072): the same project yields the same import edges whether + scanned from the repo root or from src/ (modulo the `src_` id prefix).""" + direct = tmp_path / "direct" + nested = tmp_path / "nested" + _write(direct) # direct/mypkg/... + _write(nested, prefix="src") # nested/src/mypkg/... (byte-identical) + + dpaths = [direct / r for r in _FILES] + npaths = [nested / "src" / r for r in _FILES] + dG = build_from_json(extract(dpaths, cache_root=tmp_path / "cd", root=direct, parallel=False), root=str(direct)) + nG = build_from_json(extract(npaths, cache_root=tmp_path / "cn", root=nested, parallel=False), root=str(nested)) + + d_edges = _import_edges(dG) + # strip the `src_` prefix the nested layout adds to every id. + n_edges = { + (rel, u[4:] if u.startswith("src_") else u, v[4:] if v.startswith("src_") else v) + for rel, u, v in _import_edges(nG) + } + assert d_edges, "sanity: the flat layout must produce import edges" + assert n_edges == d_edges, ( + f"scan root changed the import graph (#2072)\n root-only: {d_edges - n_edges}\n src-only: {n_edges - d_edges}" + ) + # Concretely, in the src layout: app<->core are connected by an import edge + # (endpoint order is storage-dependent on an undirected graph), and no import + # endpoint is a bare, unresolved `mypkg_*` id — every target resolved to a + # real `src_mypkg_*` file/symbol node. + n_imports = _import_edges(nG) + assert any({"src_mypkg_app"} <= {u, v} and any(n.startswith("src_mypkg_core") for n in (u, v)) + for _, u, v in n_imports), f"app->core import not resolved: {n_imports}" + endpoints = {n for _, u, v in n_imports for n in (u, v)} + assert not any(n.startswith("mypkg_") for n in endpoints), ( + f"unresolved bare import id survived (scan-root-relative mismatch): {endpoints}" + ) + + +def test_ambiguous_package_alias_is_not_repointed(tmp_path): + """A dotted-module id claimed by two different files (two src roots with the + same package) must stay dangling rather than pick an arbitrary file.""" + for sub in ("a", "b"): + d = tmp_path / sub / "src" / "pkg" + d.mkdir(parents=True) + (d / "__init__.py").write_text("") + (d / "mod.py").write_text("def f():\n return 1\n") + (tmp_path / "a" / "src" / "pkg" / "app.py").write_text("import pkg.mod\n") + paths = [ + tmp_path / "a" / "src" / "pkg" / "app.py", + tmp_path / "a" / "src" / "pkg" / "mod.py", + tmp_path / "b" / "src" / "pkg" / "mod.py", + tmp_path / "a" / "src" / "pkg" / "__init__.py", + tmp_path / "b" / "src" / "pkg" / "__init__.py", + ] + G = build_from_json(extract(paths, cache_root=tmp_path / "c", root=tmp_path, parallel=False), root=str(tmp_path)) + # The ambiguous `pkg_mod` alias claimed by both a/ and b/ must not be + # repointed onto either file — no fabricated cross-tree import edge. + imports = _import_edges(G) + targets = {v for _, _, v in imports} + assert "a_src_pkg_mod" not in targets or "b_src_pkg_mod" not in targets, ( + f"ambiguous alias was repointed to a specific file: {imports}" + )