From ceeafb05ce2c864bc46b4f8085585ea05030dc64 Mon Sep 17 00:00:00 2001 From: phudayyy <191508896+phudayyy@users.noreply.github.com> Date: Sat, 15 Aug 2026 16:42:12 +0100 Subject: [PATCH] fix(js): an import from outside the corpus shadows indirect_call resolution (#2757) An identifier bound by an import whose target resolves OUTSIDE the scanned corpus (e.g. a lucide-react icon) is now shadowed within the file, so using it as a value no longer fabricates an INFERRED indirect_call onto an unrelated same-named callable elsewhere in the corpus. The internal-vs-external decision is delegated to the existing import resolver, so a relative/in-corpus import still resolves to its real target. Same shadow family as #2241/#2568/#2685. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/extractors/engine.py | 88 ++++++++++ ...st_indirect_call_external_import_shadow.py | 154 ++++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 tests/test_indirect_call_external_import_shadow.py diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index b91463c3..f7247854 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1287,6 +1287,81 @@ def _js_module_bound_names(root, source: bytes) -> set[str]: walk(root) return bound +def _js_import_binds_external(raw: str, str_path: str) -> bool: + """True when a JS/TS import specifier names a module outside the scanned corpus. + + Reuses `_resolve_js_import_target`, so this is graphify's own verdict rather + than a second opinion: a specifier it cannot resolve is an external package + (the `ref`-namespaced branch). The extra `node_modules` test covers the case + where resolution *succeeds* but lands in a dependency tree — a `tsconfig` + `paths` entry mapping a package to its own installed copy + (`"lucide-react": ["./node_modules/lucide-react"]`) is common, and + `node_modules` is pruned from every scan, so the target is never a node. + """ + resolved = _resolve_js_import_target(raw, str_path) + if resolved is None: + return False # empty specifier — binds nothing + _target_nid, resolved_path = resolved + if resolved_path is None: + return True # unresolved after relative / alias / workspace lookup + return "node_modules" in resolved_path.parts + + +def _js_external_import_names(root, source: bytes, str_path: str) -> set[str]: + """Names an `import` binds to a module OUTSIDE the corpus. + + An imported name is a module-scoped binding: within this file it denotes the + imported symbol and nothing else. Neither shadow set collects it — + `_js_local_bound_names` reads parameters and `variable_declarator`s and + `_js_module_bound_names` only the latter — so the name reaches + `_emit_indirect_ref` as an unresolved by-name reference, gets resolved against + the corpus-wide label index, and fabricates an `indirect_call` (INFERRED, 0.8) + to any unique same-named callable elsewhere in the corpus. That is the symptom + already fixed for `catch` bindings, single-parameter arrows and untracked + closures; an import binding is the same class of shadow, and a UI kit makes it + land constantly because icon names (`Palette`, `Search`, `Filter`) collide with + ordinary component names. + + Only imports the corpus cannot contain are collected. A relative specifier + resolves to a real file and that edge is the graph's whole point, so those + names stay resolvable. + """ + bound: set[str] = set() + + def _clause_names(clause) -> None: + for c in clause.children: + if c.type == "identifier": # import Default from "pkg" + bound.add(_read_text(c, source)) + elif c.type == "namespace_import": # import * as NS from "pkg" + for ident in c.children: + if ident.type == "identifier": + bound.add(_read_text(ident, source)) + elif c.type == "named_imports": # import { A, B as C } from "pkg" + for spec in c.children: + if spec.type != "import_specifier": + continue + idents = [g for g in spec.children if g.type == "identifier"] + # `B as C` exposes both names; only the LAST one is bound here. + if idents: + bound.add(_read_text(idents[-1], source)) + + def walk(n) -> None: + for c in n.children: + if c.type == "import_statement": + src_node = c.child_by_field_name("source") + if src_node is not None: + raw = _read_text(src_node, source).strip("\"'`") + if _js_import_binds_external(raw, str_path): + for child in c.children: + if child.type == "import_clause": + _clause_names(child) + continue + walk(c) + + walk(root) + return bound + + def _js_dispatch_value_idents(coll_node): """Yield identifier value-nodes of a JS/TS object/array literal that are function-reference candidates: object property VALUES and shorthand properties @@ -2688,6 +2763,14 @@ def _extract_generic( stem = _file_stem(path) str_path = str(path) + # Names bound by an import of a module outside the corpus. Module-scoped, so it + # is computed once per file and consulted from every scope — see + # `_js_external_import_names`. + js_external_imports: set[str] = ( + _js_external_import_names(root, source, str_path) + if config.ts_module in ("tree_sitter_javascript", "tree_sitter_typescript") + else set() + ) nodes: list[dict] = [] edges: list[dict] = [] seen_ids: set[str] = set() @@ -4540,6 +4623,11 @@ def _extract_generic( # shadowing: a param / local binding names a local value, not the module fn if ident_name in enclosing_locals or ident_name in ("self", "cls"): return + # An import from outside the corpus binds the name for the whole module, so + # it shadows in every scope — no unique same-named definition elsewhere in + # the corpus is what this identifier refers to. + if ident_name in js_external_imports: + return _emit_indirect_by_name(ident_name, ident, scope_nid, context) def _python_dispatch_value_idents(coll_node): diff --git a/tests/test_indirect_call_external_import_shadow.py b/tests/test_indirect_call_external_import_shadow.py new file mode 100644 index 00000000..11fdfd76 --- /dev/null +++ b/tests/test_indirect_call_external_import_shadow.py @@ -0,0 +1,154 @@ +"""An import from outside the corpus must shadow indirect_call resolution. + +`_js_local_bound_names` collects a function's locals from parameters and +`variable_declarator` nodes, and `_js_module_bound_names` only the latter. A name +introduced by `import { X } from "pkg"` is neither, so it was absent from both +shadow sets: listing it in a dispatch table (`{ icon: X }`) or passing it on as a +call argument read as an unresolved by-name reference, resolved against the +corpus-wide label index, and fabricated an `indirect_call` edge (INFERRED, 0.8) to +an unrelated same-named callable elsewhere in the corpus. + +Same class as the `catch`-binding, single-parameter-arrow and untracked-closure +shadows already fixed — an import is simply a module-scoped binding. A UI icon kit +makes it land constantly: `Palette`, `Search`, `Filter` and `Menu` are icon exports +*and* ordinary component names, so any repo with both grows cross-package edges +between files that never referenced one another. + +The guard asks `_resolve_js_import_target` rather than second-guessing it, so a +relative import — which resolves to a real file, and whose edge is the whole point +of the graph — keeps resolving. `node_modules` is tested separately because a +`tsconfig` `paths` entry pointing a package at its own installed copy +(`"lucide-react": ["./node_modules/lucide-react"]`) *does* resolve, to a tree every +scan prunes. +""" +import os +from pathlib import Path + +from graphify.extract import extract + + +def _extract_js_dir(tmp_path, files: dict[str, str]): + base = tmp_path / "src" + base.mkdir() + for name, body in files.items(): + target = base / name + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(body) + old = os.getcwd() + try: + os.chdir(tmp_path) + r = extract( + [Path("src") / name for name in files], + cache_root=Path(".cache"), parallel=False, + ) + finally: + os.chdir(old) + nid = {n["label"].rstrip("()"): n["id"] for n in r["nodes"]} + return r, nid + + +def _rels(r, relation): + return {(e["source"], e["target"]) for e in r["edges"] if e["relation"] == relation} + + +def test_external_named_import_emits_no_indirect_call(tmp_path): + """Reported shape: an icon imported from a UI kit must not become a fabricated + indirect_call target because an unrelated component shares its name.""" + r, nid = _extract_js_dir(tmp_path, { + "Palette.tsx": "export function Palette() { return null; }\n", + "Sidebar.tsx": ( + "import { Palette } from 'lucide-react';\n" + "export function Sidebar() {\n" + " return [{ label: 'personalise', icon: Palette }];\n" + "}\n" + ), + }) + indirect = _rels(r, "indirect_call") + assert all(t != nid["Palette"] for _s, t in indirect) + + +def test_external_default_import_emits_no_indirect_call(tmp_path): + r, nid = _extract_js_dir(tmp_path, { + "Chart.tsx": "export function Chart() { return null; }\n", + "Panel.tsx": ( + "import Chart from 'some-chart-lib';\n" + "export function Panel(sink) { sink.register(Chart); }\n" + ), + }) + indirect = _rels(r, "indirect_call") + assert all(t != nid["Chart"] for _s, t in indirect) + + +def test_external_namespace_import_emits_no_indirect_call(tmp_path): + r, nid = _extract_js_dir(tmp_path, { + "Utils.ts": "export function Utils() { return 1; }\n", + "run.ts": ( + "import * as Utils from 'vendor-utils';\n" + "export function run(sink) { sink.push(Utils); }\n" + ), + }) + indirect = _rels(r, "indirect_call") + assert all(t != nid["Utils"] for _s, t in indirect) + + +def test_aliased_import_shadows_the_local_name_only(tmp_path): + """`import { Search as Find }` binds `Find` in this file, not `Search`. The + shadow must follow the binding: a same-named local `Find` is not referenced + here, while an unrelated `Search` definition stays reachable by its own name.""" + r, nid = _extract_js_dir(tmp_path, { + "Find.ts": "export function Find() { return 1; }\n", + "app.ts": ( + "import { Search as Find } from 'icon-pack';\n" + "export function app(sink) { sink.push(Find); }\n" + ), + }) + indirect = _rels(r, "indirect_call") + assert all(t != nid["Find"] for _s, t in indirect) + + +def test_tsconfig_alias_into_node_modules_still_counts_as_external(tmp_path): + """A `paths` entry pointing a package at its own installed copy resolves to a + real path — inside `node_modules`, which every scan prunes, so no node is ever + created for it. Resolution succeeding must not read as 'internal'.""" + (tmp_path / "tsconfig.json").write_text( + '{"compilerOptions": {"paths": {"icon-kit": ["./node_modules/icon-kit"]}}}\n' + ) + nm = tmp_path / "node_modules" / "icon-kit" + nm.mkdir(parents=True) + (nm / "index.js").write_text("export function Palette(){}\n") + r, nid = _extract_js_dir(tmp_path, { + "Palette.tsx": "export function Palette() { return null; }\n", + "Bar.tsx": ( + "import { Palette } from 'icon-kit';\n" + "export function Bar() { return [{ icon: Palette }]; }\n" + ), + }) + indirect = _rels(r, "indirect_call") + assert all(t != nid["Palette"] for _s, t in indirect) + + +def test_relative_import_still_resolves(tmp_path): + """The counter-test that bounds the fix: an import of a file INSIDE the corpus + is exactly the relationship the graph exists to record, so its name must stay + resolvable. Shadowing every import would delete real edges.""" + r, nid = _extract_js_dir(tmp_path, { + "widgets.ts": "export function Widget() { return null; }\n", + "host.ts": ( + "import { Widget } from './widgets';\n" + "export function host(sink) { sink.push(Widget); }\n" + ), + }) + indirect = _rels(r, "indirect_call") + assert (nid["host"], nid["Widget"]) in indirect + + +def test_unimported_same_file_callable_still_emits(tmp_path): + """Widening the shadow set must not blanket-suppress a file that also happens to + import something external: an unshadowed by-name reference still emits.""" + r, nid = _extract_js_dir(tmp_path, {"a.ts": ( + "import { Icon } from 'icon-pack';\n" + "function handler(x) { return x; }\n" + "export function run(pool) { pool.submit(handler); }\n" + )}) + indirect = _rels(r, "indirect_call") + assert (nid["run"], nid["handler"]) in indirect