fix(extract): resolve Python imports regardless of scan root (src-layout) (#2072)
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/.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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}"
|
||||
)
|
||||
Reference in New Issue
Block a user