From a9959210264db8868db7196e9cfccb5e62171e5d Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Thu, 13 Aug 2026 13:30:24 +0100 Subject: [PATCH] fix(js): shadow for...of / for...in loop bindings from indirect_call args (#2568 family) --- graphify/extractors/engine.py | 10 ++ ...est_indirect_call_for_of_binding_shadow.py | 97 +++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 tests/test_indirect_call_for_of_binding_shadow.py diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 2b3433cf..c3b13bb7 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1246,6 +1246,16 @@ def _js_local_bound_names(func_node, source: bytes) -> set[str]: name = c.child_by_field_name("name") if name is not None: _js_collect_pattern_idents(name, source, bound) + elif c.type == "for_in_statement": + # `for (const entry of xs)` / `for (const {k} of xs)`: the loop + # binding is the `left` pattern, NOT wrapped in a + # variable_declarator, so the branch above misses it and `entry` + # read as a by-name reference to any same-named module callable + # (#2606). C-style `for (let i = 0; ...)` uses a lexical_declaration + # with real declarators, already covered by the recursion below. + left = c.child_by_field_name("left") + if left is not None: + _js_collect_pattern_idents(left, source, bound) walk(c) body = func_node.child_by_field_name("body") diff --git a/tests/test_indirect_call_for_of_binding_shadow.py b/tests/test_indirect_call_for_of_binding_shadow.py new file mode 100644 index 00000000..58aee4b4 --- /dev/null +++ b/tests/test_indirect_call_for_of_binding_shadow.py @@ -0,0 +1,97 @@ +"""A `for...of` / `for...in` loop binding must shadow indirect_call references. + +`_js_local_bound_names` collected parameters and `variable_declarator` targets, +but the loop variable of `for (const entry of xs)` is the statement's `left` +pattern, NOT wrapped in a `variable_declarator`. So `entry` contributed nothing +to the shadow set: used in an object-shorthand argument (`push({ entry })`) it +read as an unresolved by-name reference, resolved against the corpus-wide label +index, and fabricated an INFERRED `indirect_call` edge to an unrelated same-named +module callable (#2606). A generic fixture/helper name then became a false +high-betweenness hub, distorting god-node and community analysis. + +C-style `for (let i = 0; ...)` uses a `lexical_declaration` with real +declarators, which was always handled — the same declared/undeclared trap as the +single-arrow-parameter and catch-binding cases. +""" +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(): + (base / name).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 _indirect(r): + return {(e["source"], e["target"]) for e in r["edges"] if e["relation"] == "indirect_call"} + + +def test_for_of_binding_does_not_fabricate_indirect_call(tmp_path): + """The reported shape: a `for...of` binding `entry` used in an + object-shorthand argument must not resolve to an unrelated `entry()`.""" + r, nid = _extract_js_dir(tmp_path, { + "declaration.mjs": ( + "export function entry(signature) {\n" + " return { signature };\n" + "}\n" + "export const fixture = entry(\"public.entry()\");\n" + ), + "consumer.mjs": ( + "export function findNamed(entries, lookup) {\n" + " const resolved = [];\n" + " for (const entry of entries) {\n" + " if (lookup(entry.name)) resolved.push({ entry });\n" + " }\n" + " return resolved;\n" + "}\n" + ), + }) + assert (nid["findNamed"], nid["entry"]) not in _indirect(r) + + +def test_for_of_destructuring_binding_shadows(tmp_path): + """A destructured loop binding (`for (const { entry } of xs)`) must shadow + the same way — `_js_collect_pattern_idents` walks the pattern.""" + r, nid = _extract_js_dir(tmp_path, { + "declaration.mjs": "export function entry(x) { return x; }\n", + "consumer.mjs": ( + "export function findNamed(rows) {\n" + " const out = [];\n" + " for (const { entry } of rows) {\n" + " out.push({ entry });\n" + " }\n" + " return out;\n" + "}\n" + ), + }) + assert (nid["findNamed"], nid["entry"]) not in _indirect(r) + + +def test_genuine_reference_outside_the_loop_still_emits(tmp_path): + """Widening the shadow set must not blanket-suppress: a same-named callable + referenced from a function that does NOT bind it in a loop still resolves.""" + r, nid = _extract_js_dir(tmp_path, {"a.js": ( + "function entry(x){ return x; }\n" + "export function findNamed(rows) {\n" + " const out = [];\n" + " for (const entry of rows) { out.push({ entry }); }\n" + " return out;\n" + "}\n" + "export function elsewhere(pool) { pool.submit(entry); }\n" + )}) + assert (nid["elsewhere"], nid["entry"]) in _indirect(r)