diff --git a/code_review_graph/flows.py b/code_review_graph/flows.py index 193171e4..29886db4 100644 --- a/code_review_graph/flows.py +++ b/code_review_graph/flows.py @@ -161,8 +161,11 @@ def detect_entry_points( When *include_tests* is False (the default), Test nodes are excluded so that flow analysis focuses on production entry points. """ - # Build a set of all qualified names that are CALLS targets. - called_qnames = store.get_all_call_targets() + # Build a set of all qualified names that are CALLS targets. Exclude + # edges sourced at File nodes so that script-/notebook-/top-level-only + # callees (e.g. ``run_job()`` invoked from module scope, a top-level + # ```` render) remain detectable as entry points. + called_qnames = store.get_all_call_targets(include_file_sources=False) # Scan all nodes for entry-point candidates. candidate_nodes = store.get_nodes_by_kind(["Function", "Test"]) diff --git a/code_review_graph/graph.py b/code_review_graph/graph.py index 89442c98..00f9b50e 100644 --- a/code_review_graph/graph.py +++ b/code_review_graph/graph.py @@ -1075,12 +1075,33 @@ def get_node_kind_by_id(self, node_id: int) -> str | None: ).fetchone() return row["kind"] if row else None - def get_all_call_targets(self) -> set[str]: - """Return the set of all CALLS-edge target qualified names.""" - rows = self._conn.execute( - "SELECT DISTINCT target_qualified FROM edges " - "WHERE kind = 'CALLS'" - ).fetchall() + def get_all_call_targets(self, include_file_sources: bool = True) -> set[str]: + """Return the set of all CALLS-edge target qualified names. + + When ``include_file_sources`` is False, CALLS edges whose source is a + File node (module-scope calls from top-level script glue, CLI + entrypoints, or notebook cells) are excluded. Callers that treat "has + an incoming call" as "is not a root" (e.g. entry-point detection) + should pass ``include_file_sources=False`` — otherwise a script-only + callee looks called and is hidden from flow analysis. + + The File-node filter joins against ``nodes.kind`` rather than pattern- + matching ``source_qualified`` so that file paths containing ``::`` or + any future change to the File-node naming convention cannot silently + miscategorize edges. + """ + if include_file_sources: + rows = self._conn.execute( + "SELECT DISTINCT target_qualified FROM edges " + "WHERE kind = 'CALLS'" + ).fetchall() + else: + rows = self._conn.execute( + "SELECT DISTINCT e.target_qualified FROM edges e " + "LEFT JOIN nodes n ON n.qualified_name = e.source_qualified " + "WHERE e.kind = 'CALLS' " + "AND (n.kind IS NULL OR n.kind != 'File')" + ).fetchall() return {r["target_qualified"] for r in rows} def get_communities_list( diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 31af17f7..c4ac55be 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -1450,26 +1450,27 @@ def _extract_elixir_constructs( return True # ---- Everything else = a regular function/method call ---------- - # Emit a CALLS edge when we're inside a function (same rule as - # the generic _extract_calls path). - if enclosing_func: - # For dotted calls like `IO.puts(msg)`, prefer the dotted - # identifier; for bare calls use the first identifier. - call_name = ident - caller = self._qualify( - enclosing_func, file_path, enclosing_class, - ) - target = self._resolve_call_target( - call_name, file_path, language, - import_map or {}, defined_names or set(), - ) - edges.append(EdgeInfo( - kind="CALLS", - source=caller, - target=target, - file_path=file_path, - line=node.start_point[0] + 1, - )) + # Module-scope calls attribute to the File node (same rule as the + # generic _extract_calls path). + # For dotted calls like `IO.puts(msg)`, prefer the dotted + # identifier; for bare calls use the first identifier. + call_name = ident + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path + ) + target = self._resolve_call_target( + call_name, file_path, language, + import_map or {}, defined_names or set(), + ) + edges.append(EdgeInfo( + kind="CALLS", + source=caller, + target=target, + file_path=file_path, + line=node.start_point[0] + 1, + )) # Recurse into arguments + do_block so nested calls are caught. for sub in node.children: if sub.type in ("arguments", "do_block"): @@ -2376,9 +2377,17 @@ def _extract_calls( ) return True - if call_name and enclosing_func: - caller = self._qualify( - enclosing_func, file_path, enclosing_class, + if call_name: + # Module-scope calls (no enclosing function) are attributed to + # the File node. Matches the existing convention for CONTAINS + # edges and _extract_value_references. Without this fallback, + # any function called only from top-level script glue, CLI + # entrypoints, or Jupyter/Databricks notebook cells is flagged + # as dead by find_dead_code. + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path ) target = self._resolve_call_target( call_name, file_path, language, @@ -2411,17 +2420,21 @@ def _extract_jsx_component_call( Treat uppercase component tags such as ```` as call-like edges so caller/impact queries can cross the JSX boundary. Intrinsic DOM tags (``
``) are ignored. - """ - if not enclosing_func: - return + Module-scope JSX (e.g. a top-level ```` render call) attributes + to the File node. + """ target = self._resolve_jsx_component_target( child, language, file_path, import_map or {}, defined_names or set(), ) if not target: return - caller = self._qualify(enclosing_func, file_path, enclosing_class) + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path + ) edges.append(EdgeInfo( kind="CALLS", source=caller, @@ -2702,15 +2715,20 @@ def _extract_solidity_constructs( Returns True if the child was fully handled and should skip default recursion. """ - # Emit statements: emit EventName(...) -> CALLS edge - if node_type == "emit_statement" and enclosing_func: + # Emit statements: emit EventName(...) -> CALLS edge. + # Module-scope emits attribute to the File node. + if node_type == "emit_statement": for sub in child.children: if sub.type == "expression": for ident in sub.children: if ident.type == "identifier": - caller = self._qualify( - enclosing_func, file_path, - enclosing_class, + caller = ( + self._qualify( + enclosing_func, file_path, + enclosing_class, + ) + if enclosing_func + else file_path ) edges.append(EdgeInfo( kind="CALLS", @@ -3999,21 +4017,25 @@ def _handle_r_call( import_map, defined_names, ) - if enclosing_func: - call_name = self._get_call_name(node, language, source) - if call_name: - caller = self._qualify(enclosing_func, file_path, enclosing_class) - target = self._resolve_call_target( - call_name, file_path, language, - import_map or {}, defined_names or set(), - ) - edges.append(EdgeInfo( - kind="CALLS", - source=caller, - target=target, - file_path=file_path, - line=node.start_point[0] + 1, - )) + # Module-scope R calls attribute to the File node. + call_name = self._get_call_name(node, language, source) + if call_name: + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path + ) + target = self._resolve_call_target( + call_name, file_path, language, + import_map or {}, defined_names or set(), + ) + edges.append(EdgeInfo( + kind="CALLS", + source=caller, + target=target, + file_path=file_path, + line=node.start_point[0] + 1, + )) self._extract_from_tree( node, source, language, file_path, nodes, edges, diff --git a/tests/test_flows.py b/tests/test_flows.py index 34cfd05d..523e7d22 100644 --- a/tests/test_flows.py +++ b/tests/test_flows.py @@ -202,6 +202,30 @@ def test_detect_entry_points_excludes_test_files(self): ep_files_all = {ep.file_path for ep in eps_all} assert "src/handler.spec.ts" in ep_files_all + def test_detect_entry_points_module_scope_caller_is_still_root(self): + """A function called only from module scope (File-sourced CALLS) is a root. + + Regression guard: the parser attributes module-scope calls to the File + node. Without filtering File-sourced callers, ``run_job`` here would + look "called" by ``script.py`` and be excluded from flow analysis, + even though in practice it IS an entry point (the script itself is + invoked externally). + """ + self._add_func("run_job", path="script.py") + # Ensure the File node exists so its qualified_name resolves cleanly + # (production code creates this automatically during parsing). + self.store.upsert_node(NodeInfo( + kind="File", name="script.py", file_path="script.py", + line_start=1, line_end=10, language="python", + )) + self.store.commit() + # Module-scope call: source is the File node's qualified_name. + self._add_call("script.py", "script.py::run_job", path="script.py") + + eps = detect_entry_points(self.store) + ep_names = {ep.name for ep in eps} + assert "run_job" in ep_names + def test_trace_simple_flow(self): """BFS traces a linear call chain: A -> B -> C.""" self._add_func("entry") diff --git a/tests/test_parser.py b/tests/test_parser.py index 1c629a5f..5587a062 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -143,6 +143,67 @@ def test_multiple_calls_to_same_function(self): lines = {e.line for e in calls} assert len(lines) == 2 # distinct line numbers + def test_module_scope_calls_attributed_to_file(self): + """Module-scope calls (script glue, top-level code) emit CALLS edges + attributed to the File node, so callees aren't flagged as dead by + find_dead_code. + + Regression test: prior to this fix, _extract_calls dropped the edge + entirely when enclosing_func was None, leaving notebooks, CLI scripts, + and top-level entry points with zero outgoing CALLS edges. + """ + with tempfile.NamedTemporaryFile(mode="w", suffix=".py", delete=False) as f: + f.write( + "def helper():\n" + " return 42\n" + "\n" + "# Module-scope call — no enclosing function\n" + "result = helper()\n" + ) + tmp = Path(f.name) + + try: + _, edges = self.parser.parse_file(tmp) + calls = [e for e in edges if e.kind == "CALLS"] + module_scope_calls = [e for e in calls if e.source == str(tmp)] + assert any( + "helper" in e.target for e in module_scope_calls + ), f"Expected module-scope CALLS edge to helper(); got: {[(e.source, e.target) for e in calls]}" + finally: + tmp.unlink() + + def test_module_scope_calls_in_notebook(self): + """Notebook code cells are entirely module-scope — every call inside + them should produce a CALLS edge attributed to the .ipynb File node.""" + import json + + notebook = { + "cells": [ + { + "cell_type": "code", + "source": [ + "from helper_module import do_work\n", + "do_work()\n", + ], + }, + ], + "metadata": {"language_info": {"name": "python"}}, + "nbformat": 4, + "nbformat_minor": 5, + } + with tempfile.NamedTemporaryFile(mode="w", suffix=".ipynb", delete=False) as f: + json.dump(notebook, f) + tmp = Path(f.name) + + try: + _, edges = self.parser.parse_file(tmp) + calls = [e for e in edges if e.kind == "CALLS"] + assert any( + "do_work" in e.target and e.source == str(tmp) for e in calls + ), f"Expected notebook CALLS edge to do_work(); got: {[(e.source, e.target) for e in calls]}" + finally: + tmp.unlink() + def test_parse_nonexistent_file(self): nodes, edges = self.parser.parse_file(Path("/nonexistent/file.py")) assert nodes == [] @@ -902,3 +963,112 @@ def test_resolve_references_targets(self): # At least some targets should be fully qualified qualified_refs = [e for e in refs if "::" in e.target] assert len(qualified_refs) > 0 + + +class TestModuleScopeCalls: + """Module-scope calls (no enclosing function) must attribute to the File node. + + Previously these edges were silently dropped, causing ``find_dead_code`` to + flag CLI entrypoints, notebook-helper functions, and top-level JSX renders + as dead. The fix emits a CALLS edge with ``source = file_path`` (the File + node's qualified name). + """ + + def setup_method(self): + self.parser = CodeParser() + + def test_python_top_level_call_attributes_to_file(self): + source = ( + b"def worker():\n" + b" return 1\n" + b"\n" + b"worker()\n" + ) + path = FIXTURES / "module_scope_py.py" + _, edges = self.parser.parse_bytes(path, source) + + calls = [e for e in edges if e.kind == "CALLS"] + top_level = [ + e for e in calls + if e.source == str(path) and e.target.endswith("worker") + ] + assert len(top_level) == 1 + # Edge originates at the call site (line 4), not the def (line 1). + assert top_level[0].line == 4 + + def test_python_if_main_block_call_attributes_to_file(self): + source = ( + b"def run_job():\n" + b" return 1\n" + b"\n" + b"if __name__ == '__main__':\n" + b" run_job()\n" + ) + path = FIXTURES / "module_scope_cli.py" + _, edges = self.parser.parse_bytes(path, source) + + calls = [e for e in edges if e.kind == "CALLS"] + top_level = [ + e for e in calls + if e.source == str(path) and e.target.endswith("run_job") + ] + assert len(top_level) == 1 + # Edge originates inside the `if __name__` block (line 5). + assert top_level[0].line == 5 + + def test_tsx_top_level_jsx_render_attributes_to_file(self): + # Bare top-level JSX expression statement exercises the + # _extract_jsx_child path specifically (not a value-reference + # fallback from the `const element = ...` assignment). + source = ( + b"import App from './App';\n" + b"\n" + b";\n" + ) + path = FIXTURES / "module_scope_entry.tsx" + _, edges = self.parser.parse_bytes(path, source) + + calls = [e for e in edges if e.kind == "CALLS"] + top_level = [ + e for e in calls + if e.source == str(path) and e.target.endswith("App") + ] + assert len(top_level) == 1 + # Edge originates at the JSX site (line 3), not the import (line 1). + assert top_level[0].line == 3 + + def test_r_top_level_call_attributes_to_file(self): + # R scripts are overwhelmingly module-scope by convention; this is + # the highest-leverage language for the fix after Python. + source = ( + b"worker <- function() {\n" + b" 1\n" + b"}\n" + b"\n" + b"worker()\n" + ) + path = FIXTURES / "module_scope_sample.R" + _, edges = self.parser.parse_bytes(path, source) + + top_level = [ + e for e in edges + if e.kind == "CALLS" + and e.source == str(path) + and e.target.endswith("worker") + ] + assert len(top_level) == 1 + + def test_elixir_top_level_dotted_call_attributes_to_file(self): + # `.exs` scripts and mix tasks commonly have module-scope `IO.puts`, + # which is what the parser comment explicitly calls out. + source = b'IO.puts("hello")\n' + path = FIXTURES / "module_scope_script.exs" + _, edges = self.parser.parse_bytes(path, source) + + top_level = [ + e for e in edges + if e.kind == "CALLS" + and e.source == str(path) + and e.target.endswith("puts") + ] + assert len(top_level) == 1 diff --git a/tests/test_refactor.py b/tests/test_refactor.py index 435a0aee..6858e532 100644 --- a/tests/test_refactor.py +++ b/tests/test_refactor.py @@ -6,7 +6,7 @@ from pathlib import Path from code_review_graph.graph import GraphStore -from code_review_graph.parser import EdgeInfo, NodeInfo +from code_review_graph.parser import CodeParser, EdgeInfo, NodeInfo from code_review_graph.refactor import ( REFACTOR_EXPIRY_SECONDS, _pending_refactors, @@ -821,3 +821,65 @@ def test_transitive_import_via_barrel_file(self): assert "safeJsonParse" not in dead_names, ( "2-hop import chain should make consumer a plausible caller" ) + + +class TestFindDeadCodeModuleScope: + """End-to-end regression: parse → store → find_dead_code. + + Pins the contract that functions invoked only from module scope are not + flagged as dead. Bypasses the hand-built graph fixtures used elsewhere in + this file so that a regression in any of the parser's 5 module-scope + CALLS paths is caught. + """ + + def setup_method(self): + self.tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) + self.store = GraphStore(self.tmp.name) + self.parser = CodeParser() + + def teardown_method(self): + self.store.close() + Path(self.tmp.name).unlink(missing_ok=True) + + def _store_parsed(self, path: Path, source: bytes) -> None: + nodes, edges = self.parser.parse_bytes(path, source) + for n in nodes: + self.store.upsert_node(n) + for e in edges: + self.store.upsert_edge(e) + self.store.commit() + + def test_module_scope_caller_prevents_dead_code_flag(self, tmp_path): + """A function called only from top-level script glue is not dead.""" + # ``run_job`` has no non-dunder name match and no framework decorator, + # so without the module-scope CALLS fix it would be flagged dead. + path = tmp_path / "script.py" + path.write_bytes( + b"def run_job():\n" + b" return 1\n" + b"\n" + b"run_job()\n" + ) + self._store_parsed(path, path.read_bytes()) + + dead = find_dead_code(self.store) + dead_names = {d["name"] for d in dead} + assert "run_job" not in dead_names, ( + "module-scope caller should prevent run_job from being flagged dead" + ) + + def test_if_main_block_caller_prevents_dead_code_flag(self, tmp_path): + """A function called only inside ``if __name__ == '__main__'`` is not dead.""" + path = tmp_path / "cli.py" + path.write_bytes( + b"def launch():\n" + b" return 1\n" + b"\n" + b"if __name__ == '__main__':\n" + b" launch()\n" + ) + self._store_parsed(path, path.read_bytes()) + + dead = find_dead_code(self.store) + dead_names = {d["name"] for d in dead} + assert "launch" not in dead_names