diff --git a/CHANGELOG.md b/CHANGELOG.md index a5b7765..84db32d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) +## Unreleased + +- Fix: a JS/TS call with no local definition and no import no longer binds to a same-named export in an unrelated package (#1659, thanks @leonaburime-ucla). When a callee had exactly one same-named definition repo-wide, the cross-file resolver emitted a `calls` edge at INFERRED/0.8 even with no import path between the two files. On a monorepo this fabricated dependencies: a 14-package repo showed `platform` and `sidecar` depending on `registry-protocol` purely because it exported generically-named symbols (`*Schema`, etc.) that unresolved calls collapsed onto. JS/TS modules have no implicit cross-module scope, so a cross-file call is real only if the caller imported it — direct JS/TS cross-file `calls` attribution is now gated on import evidence and left unresolved otherwise. Other languages keep the single-candidate resolution (C/C++ headers, Ruby autoload, same-package implicit scope legitimately call across files without an explicit import), and the `indirect_call` path (already INFERRED and callable-gated) is unchanged. As part of the fix, caller→file mapping for import-evidence now uses the raw call's `source_file` string, so a path-resolution/symlink mismatch can no longer spuriously fail evidence and mislabel a real cross-file call. + ## 0.9.6 (2026-07-04) - Fix: Ruby plain modules and `Struct.new` / `Class.new` / `Data.define` constant assignments now get container nodes (#1640, thanks @krishnateja7). The extractor only created nodes for `class Foo`, so `module Foo` (utility/`module_function` modules), `Foo = Struct.new(...) do ... end`, `Foo = Class.new(StandardError)`, and `Result = Data.define(...)` produced no node at all — their methods hung off the file via `contains` with dot-less labels, and no edge could ever target them. `module` is now a container type (methods attach via `method` like a class, nested modules included), and a constant assignment whose RHS is one of those factories synthesizes a class node named after the constant, attaches block-defined methods to it, and emits an `inherits` edge for `Class.new(Super)`. Plain constant assignments (`MAX = 100`, `X = Foo.new`) are untouched. diff --git a/graphify/extract.py b/graphify/extract.py index 3f943e4..ab67154 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -16344,9 +16344,20 @@ def extract( elif e.get("relation") == "imports_from": file_to_module_imports.setdefault(e["source"], set()).add(e["target"]) - # Map each node back to its containing file_id so we can ask + # Map each node back to its containing file node id so we can ask # "did the caller's file import the callee's file?" - # Use relativized paths to match how file node IDs were remapped above (#502). + # A node and its file node share the exact same ``source_file`` string, and a + # file node is the one whose label is the basename (``add_node(file_nid, + # path.name)``). Resolving file membership by that shared string is robust + # against the path-resolution/symlink mismatch that makes + # ``relative_to(root.resolve())`` throw and fall back to a non-matching + # absolute-derived id — which would spuriously fail import evidence and (with + # the #1659 JS/TS gate below) drop a legitimately-imported call. + sf_to_file_nid: dict[str, str] = {} + for n in all_nodes: + sf = n.get("source_file") + if sf and n.get("label") == Path(str(sf)).name: + sf_to_file_nid.setdefault(str(sf), n["id"]) nid_to_file_nid: dict[str, str] = {} # nid -> raw source_file string, for the ambiguous-name tie-breakers below # (test/non-test classification + path proximity). Kept separate from the @@ -16357,6 +16368,12 @@ def extract( if not sf: continue nid_to_source_file[n["id"]] = str(sf) + fnid = sf_to_file_nid.get(str(sf)) + if fnid is not None: + nid_to_file_nid[n["id"]] = fnid + continue + # Fallback (no file node found for this source_file): derive it the old + # way from the relativized path. sf_path = Path(sf) try: sf_rel = sf_path.relative_to(root) if sf_path.is_absolute() else sf_path @@ -16372,6 +16389,10 @@ def extract( (e["source"], e["target"]) for e in all_edges if e.get("relation") in ("calls", "indirect_call") } + # JS/TS/JSX modules have no implicit cross-module scope: a call into another + # file is real ONLY if the caller imported it. So a cross-file call from one + # of these files with no import evidence is gated below (#1659). + _JS_TS_CALL_SUFFIXES = (".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs") for rc in all_raw_calls: callee = rc.get("callee", "") if not callee: @@ -16392,7 +16413,15 @@ def extract( if not candidates: continue caller = rc["caller_nid"] - caller_file_nid = nid_to_file_nid.get(caller) + # Resolve the caller's file via the raw_call's own source_file string, + # which is stable regardless of any caller_nid remap. An indirect + # callback's caller_nid is the file node, whose id may have been + # relativized after the raw_call was recorded, so a caller_nid lookup can + # miss and (with the #1659 gate) drop a legitimately-imported callback. + caller_file_nid = ( + sf_to_file_nid.get(str(rc.get("source_file", ""))) + or nid_to_file_nid.get(caller) + ) imported_symbols = file_to_symbol_imports.get(caller_file_nid, set()) imported_modules = file_to_module_imports.get(caller_file_nid, set()) @@ -16469,6 +16498,19 @@ def extract( "weight": 1.0, }) continue + # #1659: a JS/TS DIRECT call with no import evidence is almost always an + # unrelated same-named export in a package that was never imported — a + # phantom cross-package edge (a 14-package monorepo had `platform` and + # `sidecar` shown as depending on `registry-protocol` purely because it + # exported generically-named symbols). JS/TS modules have no implicit + # cross-module scope, so leave it unresolved rather than binding by name + # alone. Other languages keep the #1553 single-candidate resolution: + # C/C++ headers, Ruby autoload, and same-package implicit scope + # legitimately call across files without an explicit import. Scoped to + # direct calls: the indirect_call path above is already conservative + # (INFERRED, callable-target-gated) and independent of import evidence. + if not has_import_evidence and str(rc.get("source_file", "")).endswith(_JS_TS_CALL_SUFFIXES): + continue if tgt != caller and (caller, tgt) not in existing_pairs: existing_pairs.add((caller, tgt)) # Promote to EXTRACTED when there's a direct import edge from the diff --git a/tests/test_extract.py b/tests/test_extract.py index 528b6db..38be9f6 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -879,9 +879,11 @@ def test_cross_file_call_promoted_to_extracted_with_import_evidence(tmp_path): assert call_edges[0]["confidence_score"] == 1.0 -def test_cross_file_call_remains_inferred_without_import_evidence(tmp_path): - """A cross-file `calls` edge must stay INFERRED when there is no import - edge — name collision alone is insufficient evidence.""" +def test_js_cross_file_call_without_import_emits_no_edge(tmp_path): + """A JS/TS call with no local definition and no import must NOT bind to a + same-named export in another file (#1659). JS/TS modules have no implicit + cross-module scope, so name collision alone is not a real call — it used to + produce a phantom INFERRED edge that fabricated cross-package dependencies.""" caller = tmp_path / "caller.js" callee = tmp_path / "lib.js" # Caller does NOT require lib — same-name function happens to exist elsewhere @@ -898,8 +900,7 @@ def test_cross_file_call_remains_inferred_without_import_evidence(tmp_path): and nodes[e["source"]]["label"] == "run()" and nodes[e["target"]]["label"] == "doUnique()" ] - assert len(call_edges) == 1 - assert call_edges[0]["confidence"] == "INFERRED" + assert call_edges == [], f"unimported cross-file JS call should not resolve: {call_edges}" def test_python_qualified_class_method_call_resolves_extracted(tmp_path): diff --git a/tests/test_phantom_cross_package_call.py b/tests/test_phantom_cross_package_call.py new file mode 100644 index 0000000..07fc89d --- /dev/null +++ b/tests/test_phantom_cross_package_call.py @@ -0,0 +1,87 @@ +"""#1659 — a JS/TS call with no local definition and no import must not bind to +a same-named export in an unrelated package that was never imported. + +JS/TS modules have no implicit cross-module scope: a call into another file is +real only if the caller imported it. The cross-file resolver used to fall back +to any lone same-named export repo-wide and emit a `calls` edge at INFERRED/0.8, +so on a monorepo a package that exports generically-named symbols (`*Schema`, +`validate`, ...) appeared depended-on by packages that import nothing from it. + +The fix gates JS/TS cross-file call attribution on import evidence; other +languages keep the #1553 single-candidate resolution (headers, autoload, +same-package implicit scope legitimately call across files with no import). +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract + + +def _write(path: Path, text: str) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def _calls(files: list[Path], base: Path) -> set[tuple[str, str, str]]: + r = extract(files, cache_root=base, parallel=False) + lbl = {n["id"]: n["label"] for n in r["nodes"]} + return { + (lbl.get(e["source"], ""), lbl.get(e["target"], ""), e.get("confidence")) + for e in r["edges"] if e["relation"] == "calls" + } + + +def test_unimported_cross_package_call_emits_no_edge(tmp_path: Path) -> None: + _write(tmp_path / "pkg-a/src/index.ts", + "declare function validate(x: number): boolean;\n" + "export function run(x: number): boolean { return validate(x); }\n") + _write(tmp_path / "pkg-b/src/index.ts", + "export function validate(name: string): boolean { return name.length > 0; }\n") + calls = _calls(sorted(tmp_path.rglob("*.ts")), tmp_path) + assert not any("run" in s and "validate" in t for s, t, _ in calls), calls + + +def test_many_files_do_not_collapse_onto_one_export(tmp_path: Path) -> None: + # The real-world symptom: N packages importing nothing all showed edges to a + # single package that exported a generically-named symbol. + _write(tmp_path / "proto/index.ts", + "export function encode(x: string): string { return x; }\n") + for i in range(4): + _write(tmp_path / f"svc{i}/index.ts", + "declare function encode(x: string): string;\n" + f"export function use{i}(x: string) {{ return encode(x); }}\n") + calls = _calls(sorted(tmp_path.rglob("*.ts")), tmp_path) + assert not any("encode" in t for _s, t, _ in calls), calls + + +def test_imported_cross_file_call_still_resolves(tmp_path: Path) -> None: + # A real import must still resolve (and be promoted to EXTRACTED). + _write(tmp_path / "a.ts", + 'import { validate } from "./b";\n' + "export function run(x: number) { return validate(x); }\n") + _write(tmp_path / "b.ts", + "export function validate(name: string): boolean { return name.length > 0; }\n") + calls = _calls([tmp_path / "a.ts", tmp_path / "b.ts"], tmp_path) + resolved = [c for c in calls if "run" in c[0] and "validate" in c[1]] + assert resolved, calls + assert resolved[0][2] == "EXTRACTED" + + +def test_same_file_call_unaffected(tmp_path: Path) -> None: + _write(tmp_path / "s.ts", + "function helper() { return 1; }\n" + "export function main() { return helper(); }\n") + calls = _calls([tmp_path / "s.ts"], tmp_path) + assert any("main" in s and "helper" in t for s, t, _ in calls), calls + + +def test_non_js_single_candidate_cross_file_still_resolves(tmp_path: Path) -> None: + # The gate is JS/TS-only. Ruby (autoload, no require) legitimately calls a + # lone same-named function across files without an import — keep the #1553 + # single-candidate resolution for it. + _write(tmp_path / "helper.rb", "def transform(data)\n data.upcase\nend\n") + _write(tmp_path / "main.rb", "def handle(v)\n transform(v)\nend\n") + calls = _calls([tmp_path / "main.rb", tmp_path / "helper.rb"], tmp_path) + assert any("handle" in s and "transform" in t for s, t, _ in calls), calls