diff --git a/graphify/dedup.py b/graphify/dedup.py index 4816f103b..386bf1f46 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -444,6 +444,13 @@ def _report_id_collision(nid: str, survivor: dict, losers: list[dict]) -> None: f"dropping '{lose_label}'.", file=sys.stderr, ) + elif (survivor.get("type") == "module" and loser.get("type") == "module" + and _norm(lose_label) == _norm(keep_label)): + # Module anchors (#1327) and registry-package refs (#3237) are + # shared nodes by design: every file that imports or declares the + # same module mints the same id on purpose, so collapsing the + # copies loses nothing — not a collision worth warning about. + continue elif _defines_id(survivor) and not _defines_id(loser): continue # the loser only references the entity the survivor defines else: diff --git a/graphify/extract.py b/graphify/extract.py index e015c9d71..d40bdf883 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -535,6 +535,23 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p }) +# Package roots that live in the JVM/Android platform or the Kotlin stdlib — +# never in the scanned repo. An import under these roots must not emit a +# repo-collidable bare-stem target: the bare id can byte-collide with an +# unrelated node that collapses to the same _make_id (e.g. `java.util.UUID` +# hitting a package.json dependency entry for npm's `uuid`), and it can ride +# build.py's pre-migration alias index onto whichever unrelated same-stem file +# uniquely claims that stem (`android.graphics.Color` binding to another +# project's `ui/theme/Color.kt`) — confident cross-project phantom edges in a +# monorepo (#3237). Retargeting to the "ref" namespace is the same cure +# _resolve_js_import_target applies to unresolvable JS imports (#1638): the +# ref id matches no repo node, so build drops the edge as an external import. +_JVM_PLATFORM_PACKAGE_ROOTS = ( + "java.", "javax.", "jakarta.", "kotlin.", "kotlinx.", + "android.", "androidx.", "dalvik.", +) + + def _import_java(node, source: bytes, file_nid: str, stem: str, edges: list, str_path: str, scope_stack: list[str] | None = None) -> None: def _walk_scoped(n) -> str: parts: list[str] = [] @@ -560,7 +577,10 @@ def _walk_scoped(n) -> str: path_str.split(".")[-2] if len(path_str.split(".")) > 1 else path_str ) if module_name: - tgt_nid = _make_id(module_name) + # Platform imports get a non-collidable external target (#3237). + tgt_nid = (_make_id("ref", path_str) + if path_str.startswith(_JVM_PLATFORM_PACKAGE_ROOTS) + else _make_id(module_name)) edges.append({ "source": file_nid, "target": tgt_nid, @@ -690,10 +710,15 @@ def _import_kotlin(node, source: bytes, file_nid: str, stem: str, edges: list, s # Target is the bare last segment for now; _resolve_kotlin_import_targets # rewrites it to the real node id via the target_fqn stamped here, once the # per-file package index exists. Unresolved targets stay dangling like other - # languages' external imports. + # languages' external imports — except platform-namespace imports, whose + # bare stem must never be collidable in the first place (#3237): the + # corpus resolver could not rewrite them anyway (their package is never a + # repo package), so they go straight to the external "ref" namespace. edges.append({ "source": file_nid, - "target": _make_id(module_name), + "target": (_make_id("ref", raw) + if raw.startswith(_JVM_PLATFORM_PACKAGE_ROOTS) + else _make_id(module_name)), "relation": "imports", "context": "import", "confidence": "EXTRACTED", diff --git a/graphify/extractors/json_config.py b/graphify/extractors/json_config.py index 6a9b641a9..71bacc695 100644 --- a/graphify/extractors/json_config.py +++ b/graphify/extractors/json_config.py @@ -91,11 +91,15 @@ def extract_json(path: Path) -> dict: "optionalDependencies", "bundleDependencies", "bundledDependencies", }) - def add_node(nid: str, label: str, line: int, file_type: str = "code") -> None: + def add_node(nid: str, label: str, line: int, file_type: str = "code", + node_type: str | None = None) -> None: if nid and nid not in seen_ids: seen_ids.add(nid) - nodes.append({"id": nid, "label": label, "file_type": file_type, - "source_file": str_path, "source_location": f"L{line}"}) + node = {"id": nid, "label": label, "file_type": file_type, + "source_file": str_path, "source_location": f"L{line}"} + if node_type: + node["type"] = node_type + nodes.append(node) def add_edge(src: str, tgt: str, relation: str, line: int, context: str | None = None) -> None: @@ -193,9 +197,23 @@ def walk_object(obj_node, parent_nid: str, parent_key: str | None, add_edge(parent_nid, ref_nid, "references", line) elif parent_key in _DEP_KEYS and val_text: - dep_nid = _make_id(key) + # Namespace the registry-package target with the "ref" + # prefix, like `extends`/`$ref` above (J-4) and unresolved + # JS bare specifiers (#1638). A bare _make_id(key) id + # byte-collides with any unrelated code node that collapses + # to the same id (e.g. a JVM `import java.util.UUID` edge + # targeting `uuid`), and two manifests naming the same + # package would join unrelated projects through it (#3237). + # The shared ref node models the registry package itself — + # `type="module"` so _disambiguate_colliding_node_ids keeps + # one node when several manifests declare the same package + # (the #1327 module-anchor exemption), and a JS + # `import ... from ""` (which already targets + # _make_id("ref", raw)) now lands on it instead of dangling. + dep_nid = _make_id("ref", key) if dep_nid: - add_node(dep_nid, key, line, file_type="concept") + add_node(dep_nid, key, line, file_type="concept", + node_type="module") add_edge(key_nid, dep_nid, "imports", line, context="import") # Entry: find root document → object diff --git a/tests/test_monorepo_import_collisions.py b/tests/test_monorepo_import_collisions.py new file mode 100644 index 000000000..fb9c6c361 --- /dev/null +++ b/tests/test_monorepo_import_collisions.py @@ -0,0 +1,259 @@ +"""Cross-project name-collision fixes for monorepos (#3237). + +Two mechanisms produced confident (EXTRACTED) edges between unrelated projects +that merely share a name: + +1. JVM/Android platform imports (``import java.util.UUID``, + ``import android.graphics.Color``) emitted a bare last-segment target id. + That id could byte-collide with an unrelated node that collapses to the + same ``_make_id`` (npm's ``uuid`` dependency entry in another project's + package.json), or ride build.py's pre-migration alias index onto whichever + unrelated same-stem file uniquely claims the stem (another project's + ``ui/theme/Color.kt``). + +2. package.json dependency entries minted a global bare target node per + package name, so two projects that independently install the same npm + package were joined through it. + +The fix applies the "ref" external namespace (J-4 / #1638 convention) to both +sites, and models a registry package as a single shared ``type="module"`` +anchor node. +""" + +import json + +import pytest + +from graphify.build import build +from graphify.extract import extract + + +def _extract_and_build(td, files): + r = extract(files, cache_root=td) + G = build([{"nodes": r["nodes"], "edges": r["edges"]}], root=str(td)) + return r, G + + +def _write(base, rel, text): + p = base / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(text, encoding="utf-8") + return p + + +KOTLIN_PLATFORM_UUID = ( + "package com.b.service\n\n" + "import java.util.UUID\n\n" + "class Service {\n" + " fun newId(): String = UUID.randomUUID().toString()\n" + "}\n" +) + +PACKAGE_JSON_WITH_UUID = json.dumps( + {"name": "proj-a", "version": "1.0.0", "dependencies": {"uuid": "^9.0.0"}} +) + + +def test_platform_import_does_not_bind_to_npm_dependency_node(tmp_path, monkeypatch): + """`import java.util.UUID` must not edge into another project's package.json. + + The Kotlin import's bare target id (`uuid`) used to byte-collide with the + npm dependency node minted from an unrelated project's manifest. + """ + monkeypatch.chdir(tmp_path) + _write(tmp_path, "projA/package.json", PACKAGE_JSON_WITH_UUID) + _write(tmp_path, "projB/src/Service.kt", KOTLIN_PLATFORM_UUID) + + _, G = _extract_and_build( + tmp_path, + [tmp_path / "projA/package.json", tmp_path / "projB/src/Service.kt"], + ) + + kotlin_sourced = { + n for n, a in G.nodes(data=True) + if str(a.get("source_file", "")).endswith(".kt") + } + manifest_sourced = { + n for n, a in G.nodes(data=True) + if str(a.get("source_file", "")).endswith("package.json") + } + crossing = [ + (u, v) for u, v in G.edges() + if (u in kotlin_sourced and v in manifest_sourced) + or (v in kotlin_sourced and u in manifest_sourced) + ] + assert crossing == [], ( + f"platform import bound across projects: {crossing}" + ) + + +def test_platform_import_does_not_ride_alias_onto_unrelated_file(tmp_path, monkeypatch): + """`import android.graphics.Color` must not bind to another project's Color.kt. + + The bare `color` target used to ride build.py's pre-migration alias index + onto the unrelated file node that uniquely claims the `color` stem. + """ + monkeypatch.chdir(tmp_path) + _write( + tmp_path, "projA/app/Painter.kt", + "package com.a.app\n\n" + "import android.graphics.Color\n\n" + "class Painter {\n" + " fun tint(): Int = Color.parseColor(\"#ff0000\")\n" + "}\n", + ) + _write( + tmp_path, "projB/ui/theme/Color.kt", + "package com.b.theme\n\n" + "object Color {\n" + " val Primary: Long = 0xFF6200EE\n" + "}\n", + ) + + _, G = _extract_and_build( + tmp_path, + [tmp_path / "projA/app/Painter.kt", tmp_path / "projB/ui/theme/Color.kt"], + ) + + painter_edges = [ + (u, v, a) for u, v, a in G.edges(data=True) + if str(a.get("source_file", "")).endswith("Painter.kt") + and a.get("relation") == "imports" + ] + assert painter_edges == [], ( + f"platform import survived into the built graph: {painter_edges}" + ) + + +def test_repo_local_kotlin_import_still_resolves(tmp_path, monkeypatch): + """Control: a repo-local FQN import keeps resolving to the real node (#2526).""" + monkeypatch.chdir(tmp_path) + _write( + tmp_path, "projA/app/Painter.kt", + "package com.a.app\n\n" + "import com.b.theme.Color\n\n" + "class Painter {\n" + " fun tint(): Long = 1L\n" + "}\n", + ) + _write( + tmp_path, "projB/ui/theme/Color.kt", + "package com.b.theme\n\n" + "object Color {\n" + " val Primary: Long = 0xFF6200EE\n" + "}\n", + ) + + _, G = _extract_and_build( + tmp_path, + [tmp_path / "projA/app/Painter.kt", tmp_path / "projB/ui/theme/Color.kt"], + ) + + resolved = [ + (u, v) for u, v, a in G.edges(data=True) + if a.get("relation") == "imports" + and str(a.get("source_file", "")).endswith("Painter.kt") + and any(n.startswith("projb_ui_theme_color") for n in (u, v)) + ] + assert resolved, "repo-local Kotlin import no longer resolves (#2526 regression)" + + +def test_java_platform_import_emits_ref_namespaced_target(tmp_path, monkeypatch): + """A .java platform import gets a non-collidable `ref_` target, a repo-shaped + one keeps the bare stem for downstream resolution.""" + monkeypatch.chdir(tmp_path) + f = _write( + tmp_path, "src/Service.java", + "package com.b.service;\n\n" + "import java.util.UUID;\n" + "import com.b.util.Helper;\n\n" + "public class Service {\n" + " public String newId() { return UUID.randomUUID().toString(); }\n" + "}\n", + ) + r = extract([f], cache_root=tmp_path) + targets = { + e["target"] for e in r["edges"] if e.get("relation") == "imports" + } + assert "uuid" not in targets, "platform import still emits a bare collidable id" + assert any(t.startswith("ref_java_util_uuid") for t in targets), targets + assert "helper" in targets, "repo-shaped import lost its resolvable bare stem" + + +def test_kotlin_platform_import_emits_ref_namespaced_target(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + f = _write( + tmp_path, "src/Service.kt", + "package com.b.service\n\n" + "import java.util.UUID\n" + "import androidx.compose.material3.Typography\n" + "import com.b.util.Helper\n\n" + "class Service {\n" + " fun newId(): String = UUID.randomUUID().toString()\n" + "}\n", + ) + r = extract([f], cache_root=tmp_path) + targets = { + e["target"] for e in r["edges"] if e.get("relation") == "imports" + } + assert "uuid" not in targets and "typography" not in targets, targets + assert any(t.startswith("ref_java_util_uuid") for t in targets), targets + assert any(t.startswith("ref_androidx_compose") for t in targets), targets + assert "helper" in targets, "repo-shaped import lost its resolvable bare stem" + + +def test_shared_dependency_joins_manifests_through_one_module_node(tmp_path, monkeypatch): + """Two projects installing the same npm package share ONE `type=module` ref + node instead of edging one project's declaration into the other's.""" + monkeypatch.chdir(tmp_path) + _write(tmp_path, "projA/package.json", json.dumps( + {"name": "proj-a", "dependencies": {"typescript": "^5.2.4"}})) + _write(tmp_path, "projC/package.json", json.dumps( + {"name": "proj-c", "dependencies": {"typescript": "6.0.8"}})) + + r, G = _extract_and_build( + tmp_path, + [tmp_path / "projA/package.json", tmp_path / "projC/package.json"], + ) + + ref_nodes = [n for n in G.nodes() if n.startswith("ref_typescript")] + assert len(ref_nodes) == 1, f"expected one shared registry node, got {ref_nodes}" + ref = ref_nodes[0] + assert G.nodes[ref].get("type") == "module" + + entry_nodes = { + n for n, a in G.nodes(data=True) + if a.get("label") == "typescript" and n != ref + } + # Both manifests' entries reach the shared node... + for entry in entry_nodes: + assert G.has_edge(entry, ref), f"{entry} not wired to the shared node" + # ...and no edge joins the two projects' entries directly. + proj_of = { + n: str(G.nodes[n].get("source_file", "")).split("/")[0] for n in entry_nodes + } + direct = [ + (u, v) for u, v in G.edges() + if u in entry_nodes and v in entry_nodes and proj_of[u] != proj_of[v] + ] + assert direct == [], f"projects still joined directly: {direct}" + + +def test_js_bare_specifier_binds_to_manifest_dependency(tmp_path, monkeypatch): + """`import ... from "uuid"` already targets _make_id("ref", "uuid") (#2457); + with the dependency node in the same namespace the code→manifest link binds.""" + monkeypatch.chdir(tmp_path) + _write(tmp_path, "package.json", PACKAGE_JSON_WITH_UUID) + _write(tmp_path, "main.ts", 'import { v4 } from "uuid";\nexport const x = v4();\n') + + _, G = _extract_and_build( + tmp_path, [tmp_path / "package.json", tmp_path / "main.ts"] + ) + + bind = [ + (u, v) for u, v, a in G.edges(data=True) + if a.get("relation") == "imports_from" + and str(a.get("source_file", "")).endswith("main.ts") + and ("ref_uuid" in (u, v)) + ] + assert bind, "TS npm import no longer binds to the manifest dependency node"