diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 7f25d02a9..51bc042de 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -3840,6 +3840,14 @@ def _emit_java_parent_type(type_node, rel: str, at_line: int) -> None: for body_id, (method_node, class_nid) in csharp_method_scopes.items() } + # #1972: during the top-level root walk, suppress module-scope INDIRECT + # dispatch. Module-level indirect dispatch already has a dedicated pass below + # with correct module-scope shadow filtering, so a root-walk emission would be + # a duplicate carrying wrong (empty) shadow context. This is narrower than + # "only `calls` edges": relations walk_calls emits by other routes (e.g. a + # JS/TS dynamic `import()` producing `imports_from`) are NOT affected. + _toplevel_calls_only = False + def _emit_indirect_by_name(ident_name: str, loc_node, scope_nid: str, context: str) -> None: """Resolve a name that is referenced AS A VALUE to a real callable def and emit @@ -3851,6 +3859,8 @@ def _emit_indirect_by_name(ident_name: str, loc_node, scope_nid: str, string names an ATTRIBUTE and is never shadowed by a local, so that path passes the name straight through. ``loc_node`` supplies the source line. """ + if _toplevel_calls_only: + return ref_nid = label_to_nid.get(ident_name) # Defer to the cross-file resolver when the name is not defined in this file # (`from .h import fn`), or resolves to an import-surfaced FOREIGN symbol whose @@ -4401,7 +4411,7 @@ def walk_calls( _emit_indirect_ref(arg, caller_nid, enclosing_locals, "argument") # Helper function calls: config('foo.bar') → uses_config edge to "foo" - if (callee_name and callee_name in config.helper_fn_names): + if (not _toplevel_calls_only and callee_name and callee_name in config.helper_fn_names): args_node = node.child_by_field_name("arguments") first_key: str | None = None if args_node: @@ -4439,7 +4449,8 @@ def walk_calls( }) # Service container bindings: $this->app->bind(Foo::class, Bar::class) - if (node.type == "member_call_expression" + if (not _toplevel_calls_only + and node.type == "member_call_expression" and callee_name and callee_name in config.container_bind_methods): args_node = node.child_by_field_name("arguments") @@ -4477,7 +4488,7 @@ def walk_calls( }) # Static property access: Foo::$bar → uses_static_prop edge - if node.type in config.static_prop_types: + if node.type in config.static_prop_types and not _toplevel_calls_only: scope_node = node.child_by_field_name("scope") if scope_node is None: for child in node.children: @@ -4504,7 +4515,8 @@ def walk_calls( }) # PHP class constant access: Foo::BAR → references_constant edge - if config.ts_module == "tree_sitter_php" and node.type == "class_constant_access_expression": + if (config.ts_module == "tree_sitter_php" and not _toplevel_calls_only + and node.type == "class_constant_access_expression"): class_name = _php_class_const_scope(node) if class_name: tgt_nid = label_to_nid_ci.get(class_name.lower()) @@ -4595,6 +4607,77 @@ def walk_calls( receiver_types_by_body.get(id(body_node)), ) + # Top-level / script-context calls have no enclosing definition, so they + # never entered function_bodies and got no caller at all (#1972). walk_calls + # already returns without descending at every config.function_boundary_types + # node, so walking from the file's root picks up calls outside every tracked + # def — it cannot double-emit anything already walked via the function_bodies + # loop above. During this walk we emit ONLY direct `calls` (via + # _toplevel_calls_only), because module-level indirect dispatch and + # references have their own dedicated passes. + # + # The caller is a synthetic per-file entry node, NOT file_nid. The built + # graph is an undirected nx.Graph (build.py:673, watch.py's build_from_json + # call), so a node PAIR holds exactly one edge and the last write wins + # (build.py:836-838, :968). Edges are inserted sorted by relation, and every + # top-level callee is a symbol the file already `contains` (same file) or + # `imports` (cross-file) — both of which sort after `calls`. A + # file_nid -> callee `calls` edge is therefore overwritten by the structural + # edge on the identical pair and never reaches graph.json. The entry node + # has no such pre-existing edge to the callees, so the call survives. This + # mirrors the bash extractor, which has always attributed top-level calls to + # `__entry` for the same reason (bash.py:136-139, :443). + # + # Java is skipped: it has no top-level executable statements, its field + # initializers are already walked via initializer_nodes with the owning + # class as caller, and a root walk would only defer them as file-attributed + # raw_calls that resurrect the ambiguous phantom stubs #1744 removed. + if config.ts_module != "tree_sitter_java": + # file_nid is path-derived, so `file_nid + "__entry"` never equals a + # symbol id VERBATIM. That is not quite enough: normalize_id collapses + # runs of underscores (ids.py), so `_py__entry` and a real symbol + # named `py_entry` land on the same normalized key, and build.py's + # norm_to_id fallback table (build.py:776) would then resolve an + # inexactly-matched edge to whichever of the two it indexed last. Salt + # the suffix until the NORMALIZED id is unique among the nodes minted so + # far, so that table can never conflate the two. + # Salt with "x", never "_": normalize_id collapses underscore runs AND + # strips trailing ones, so appending "_" leaves the normalized form + # unchanged and the loop would never terminate. + entry_nid = file_nid + "__entry" + _taken = {normalize_id(_seen) for _seen in seen_ids} + while normalize_id(entry_nid) in _taken: + entry_nid += "x" + _edges_before = len(edges) + _raw_calls_before = len(raw_calls) + _toplevel_calls_only = True + try: + walk_calls(root, entry_nid, java_receiver_types.get(id(root))) + finally: + _toplevel_calls_only = False + # Materialize the entry node only when the walk actually attributed + # something to it, so files with no top-level call gain no node. Deferred + # cross-file raw_calls count too: extract() resolves them into edges + # sourced at entry_nid later, and build.py drops edges whose endpoint is + # not in the node set. + if len(edges) > _edges_before or len(raw_calls) > _raw_calls_before: + # The label deliberately does NOT embed the file name: the name is + # already carried by the containing file node, and baking it in here + # makes the same source yield a different node set under a different + # extension — exactly what tests/test_cjs_module_extension.py locks + # down for .cjs vs .js. + add_node(entry_nid, "module top-level", 1) + edges.append({ + "source": file_nid, + "target": entry_nid, + "relation": "contains", + "context": "file", + "confidence": "EXTRACTED", + "source_file": str_path, + "source_location": "L1", + "weight": 1.0, + }) + # #1356: walk property/field initializers (collected above). walk_calls # self-guards against re-entering function bodies and dedups via # seen_call_pairs, so a closure inside an initializer is not double-walked. diff --git a/tests/test_extract.py b/tests/test_extract.py index da7a615f8..9de460de1 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -2937,3 +2937,144 @@ def test_rewire_does_not_bind_supertype_stub_to_function(): "source_file": "store.py", "weight": 1.0}] _rewire_unique_stub_nodes(nodes, edges) assert edges[0]["target"] == "BookStore" # inherits stub not bound to function + + +def _file_node_ids(result): + return {n["id"] for n in result["nodes"] if str(n.get("label", "")).endswith((".py", ".js", ".rb", ".rake"))} + + +def _entry_node_ids(result): + # Substring, not endswith: the id carries a salt suffix when a real symbol + # would otherwise share its normalized form (see the collision test below). + return {n["id"] for n in result["nodes"] if "__entry" in str(n["id"])} + + +def _assert_toplevel_call(result, callee_label, *, lang): + """#1972: the top-level call must be sourced from the file's synthetic entry + node, and that node must be reachable from the file via `contains`. + + The caller is deliberately NOT the file node. The built graph is an + undirected nx.Graph, so one node pair holds one edge and the last write + wins; every top-level callee is a symbol the file already `contains` or + `imports`, and both sort after `calls`, so a file-sourced calls edge is + overwritten before it reaches graph.json. Asserting the entry node here is + what keeps this test honest about the shape the pipeline can actually carry + — see test_toplevel_call_survives_to_graph_json for the end-to-end proof. + """ + calls = [(e["source"], e["target"]) for e in result["edges"] if e["relation"] == "calls"] + callee_id = next(n["id"] for n in result["nodes"] if n.get("label") == callee_label) + entry_ids = _entry_node_ids(result) + file_ids = _file_node_ids(result) + + assert entry_ids, f"{lang}: no entry node was created for a file with a top-level call" + assert any(s in entry_ids and t == callee_id for s, t in calls), \ + f"{lang}: top-level {callee_label} not sourced from the entry node: {calls}" + assert not any(s in file_ids and t == callee_id for s, t in calls), \ + f"{lang}: call sourced from the file node — it would be overwritten by `contains`" + + contains = {(e["source"], e["target"]) for e in result["edges"] if e["relation"] == "contains"} + assert any(s in file_ids and t in entry_ids for s, t in contains), \ + f"{lang}: entry node is not contained by its file, so it would be an orphan" + + +def test_python_toplevel_call_gets_entry_caller(tmp_path): + """#1972: a module-level call with no enclosing def must still produce a + calls edge instead of being dropped.""" + f = tmp_path / "toplevel.py" + f.write_text("def tally():\n return 1\n\ntally()\n") + _assert_toplevel_call(extract([f], cache_root=tmp_path), "tally()", lang="python") + + +def test_js_toplevel_call_gets_entry_caller(tmp_path): + """#1972: same fix covers JS/TS via the shared engine.py walk.""" + f = tmp_path / "toplevel.js" + f.write_text("function tally(){ return 1; }\ntally();\n") + _assert_toplevel_call(extract([f], cache_root=tmp_path), "tally()", lang="js") + + +def test_ruby_toplevel_call_gets_entry_caller(tmp_path): + """#1972: a plain top-level Ruby call (no enclosing def). .rake extraction + (0.9.13) is the main consumer of this fix.""" + f = tmp_path / "toplevel.rb" + f.write_text("def tally\n 1\nend\n\ntally()\n") + _assert_toplevel_call(extract([f], cache_root=tmp_path), "tally()", lang="ruby") + + +def test_ruby_rake_task_block_call_gets_entry_caller(tmp_path): + """#1972: a call inside a ``task :name do ... end`` block. The block is not a + definition, so the root walk must attribute the inner call rather than + dropping it. This is the rake-task worst case from the issue.""" + f = tmp_path / "build.rake" + f.write_text("def compile\n 1\nend\n\ntask :build do\n compile()\nend\n") + _assert_toplevel_call(extract([f], cache_root=tmp_path), "compile()", lang="rake") + + +def test_crossfile_toplevel_call_gets_entry_caller(tmp_path): + """#1972: a top-level call into another file resolves through the cross-file + pass with the entry node as caller. + + This is the case that stayed broken while the file node was the caller: the + file already has an `imports` edge to the callee, so the resolved calls edge + collided with it on the same node pair. + """ + (tmp_path / "b.py").write_text("class B:\n @staticmethod\n def y():\n return 2\n") + uses = tmp_path / "uses.py" + uses.write_text("from b import B\n\nB.y()\n") + result = extract([tmp_path / "b.py", uses], cache_root=tmp_path) + calls = [(e["source"], e["target"]) for e in result["edges"] + if e["relation"] in ("calls", "indirect_call")] + entry_ids = _entry_node_ids(result) + assert any(s in entry_ids for s, _ in calls), \ + f"cross-file top-level call not sourced from the entry node: {calls}" + + +def test_no_entry_node_without_a_toplevel_call(tmp_path): + """#1972: the entry node is created lazily. A file whose calls all sit inside + definitions must not gain one, or every file in a corpus grows a node.""" + f = tmp_path / "incontext.py" + f.write_text("def tally():\n return 1\n\ndef run():\n tally()\n") + result = extract([f], cache_root=tmp_path) + assert not _entry_node_ids(result), \ + f"entry node created for a file with no top-level call: {_entry_node_ids(result)}" + + +def test_entry_node_id_survives_normalization_against_a_colliding_symbol(tmp_path): + """#1972: the entry id must stay distinct AFTER normalize_id, not just literally. + + ``normalize_id`` collapses runs of underscores, so ``_py__entry`` and a + real symbol named ``py_entry`` normalize to the same key — and build.py's + ``norm_to_id`` fallback table would then resolve an inexactly-matched edge to + whichever of the two it indexed last. The salt must also terminate: + normalize_id strips trailing underscores, so salting with "_" would spin + forever. + """ + from graphify.ids import normalize_id + + f = tmp_path / "toplevel.py" + f.write_text("def py_entry():\n return 1\n\ndef tally():\n return 2\n\ntally()\n") + result = extract([f], cache_root=tmp_path) + + normalized = [normalize_id(n["id"]) for n in result["nodes"]] + assert len(set(normalized)) == len(normalized), \ + f"two nodes share a normalized id: {sorted(normalized)}" + entry_ids = _entry_node_ids(result) + assert entry_ids, "entry node missing on the collision corpus" + tally_id = next(n["id"] for n in result["nodes"] if n.get("label") == "tally()") + calls = [(e["source"], e["target"]) for e in result["edges"] if e["relation"] == "calls"] + assert any(s in entry_ids and t == tally_id for s, t in calls), \ + f"top-level call lost on the collision corpus: {calls}" + + +def test_incontext_call_unaffected_by_toplevel_fix(tmp_path): + """#1972 regression guard: a call inside a function stays sourced from that + function, and the new root walk adds no duplicate edge.""" + f = tmp_path / "incontext.py" + f.write_text("def tally():\n return 1\n\ndef run():\n tally()\n") + result = extract([f], cache_root=tmp_path) + calls = [(e["source"], e["target"]) for e in result["edges"] if e["relation"] == "calls"] + file_ids = _file_node_ids(result) + run_id = next(n["id"] for n in result["nodes"] if n.get("label") == "run()") + tally_id = next(n["id"] for n in result["nodes"] if n.get("label") == "tally()") + assert (run_id, tally_id) in calls + assert calls.count((run_id, tally_id)) == 1 # no duplicate from the root walk + assert not any(s in file_ids and t == tally_id for s, t in calls) # file is not the caller diff --git a/tests/test_toplevel_calls_e2e.py b/tests/test_toplevel_calls_e2e.py new file mode 100644 index 000000000..d50ef2fc0 --- /dev/null +++ b/tests/test_toplevel_calls_e2e.py @@ -0,0 +1,114 @@ +"""#1972 end-to-end: top-level `calls` edges must survive into graph.json. + +The unit tests in test_extract.py assert on `extract()`'s return value. That is +not the surface users consume, and the gap mattered: the first cut of this fix +attributed top-level calls to the FILE node, every one of those tests passed, +and the edges still never reached graph.json. + +The built graph is an undirected ``nx.Graph`` (build.py:673), so a node pair +holds exactly one edge and the last write wins (build.py:836-838, :968). Edges +are inserted sorted by relation, and every top-level callee is a symbol the file +already ``contains`` (same file) or ``imports`` (cross-file) — both of which +sort after ``calls``. A file-sourced calls edge is therefore overwritten by the +structural edge on the identical pair. Attributing to a synthetic per-file entry +node avoids the collision, matching what the bash extractor has always done. + +These tests run the real CLI against a temp repo and read the exported file, so +a regression at any layer between the extractor and the export fails here. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +PYTHON = sys.executable +_KEY_VARS = ("GEMINI_API_KEY", "GOOGLE_API_KEY", "OPENAI_API_KEY", "OPENAI_BASE_URL", + "ANTHROPIC_API_KEY", "MOONSHOT_API_KEY", "DEEPSEEK_API_KEY") + + +def _run(repo: Path, *extra: str): + env = {k: v for k, v in os.environ.items() if k not in _KEY_VARS} + env["GRAPHIFY_OUT"] = str(repo / "graphify-out") + return subprocess.run( + [PYTHON, "-m", "graphify", "extract", ".", "--code-only", *extra], + cwd=repo, capture_output=True, text=True, env=env, + ) + + +def _graph(repo: Path) -> dict: + return json.loads((repo / "graphify-out" / "graph.json").read_text(encoding="utf-8")) + + +def _edges(graph: dict, relation: str) -> list[tuple[str, str]]: + links = graph.get("links", graph.get("edges", [])) + return [(e["source"], e["target"]) for e in links if e.get("relation") == relation] + + +def _repo(tmp_path: Path, name: str, body: str) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + (repo / name).write_text(body, encoding="utf-8") + return repo + + +def test_rake_toplevel_call_survives_to_graph_json(tmp_path): + """The reporter's exact repro from PR #2016: a call inside a rake task block. + + Before the entry-node change the exported graph held only the `contains` + edge; the `calls` edge was created by the extractor and then overwritten. + """ + repo = _repo( + tmp_path, "build.rake", + "def compile\n 1\nend\n\ntask :build do\n compile()\nend\n", + ) + r = _run(repo) + assert r.returncode == 0, f"extract failed: {r.stderr}" + + graph = _graph(repo) + calls = _edges(graph, "calls") + ids = {n["id"] for n in graph["nodes"]} + entry_ids = {i for i in ids if "__entry" in str(i)} + compile_id = next(n["id"] for n in graph["nodes"] if n.get("label") == "compile()") + + assert entry_ids, f"no entry node in the exported graph; nodes={sorted(ids)}" + assert any(s in entry_ids and t == compile_id for s, t in calls), ( + "the top-level call did not survive into graph.json — this is the exact " + f"failure reported on PR #2016. calls={calls}" + ) + + +def test_python_toplevel_call_survives_to_graph_json(tmp_path): + """Same guarantee for the shared engine path, not just the rake shape.""" + repo = _repo(tmp_path, "toplevel.py", "def tally():\n return 1\n\ntally()\n") + r = _run(repo) + assert r.returncode == 0, f"extract failed: {r.stderr}" + + graph = _graph(repo) + calls = _edges(graph, "calls") + entry_ids = {n["id"] for n in graph["nodes"] if "__entry" in str(n["id"])} + tally_id = next(n["id"] for n in graph["nodes"] if n.get("label") == "tally()") + + assert any(s in entry_ids and t == tally_id for s, t in calls), ( + f"top-level tally() missing from the exported graph. calls={calls}" + ) + + +def test_entry_node_is_reachable_from_its_file_in_graph_json(tmp_path): + """An entry node with no `contains` edge from its file would be an orphan in + the exported graph — present, but unreachable when walking from the file.""" + repo = _repo(tmp_path, "toplevel.py", "def tally():\n return 1\n\ntally()\n") + assert _run(repo).returncode == 0 + + graph = _graph(repo) + entry_ids = {n["id"] for n in graph["nodes"] if "__entry" in str(n["id"])} + file_ids = {n["id"] for n in graph["nodes"] + if str(n.get("label", "")).endswith((".py", ".rake", ".rb", ".js"))} + contains = _edges(graph, "contains") + + assert entry_ids, "expected an entry node" + assert any(s in file_ids and t in entry_ids for s, t in contains), ( + f"entry node not contained by its file: contains={contains}" + )