diff --git a/docs/design/0210-module-level-call-extraction.md b/docs/design/0210-module-level-call-extraction.md new file mode 100644 index 00000000..e6ca2bb0 --- /dev/null +++ b/docs/design/0210-module-level-call-extraction.md @@ -0,0 +1,174 @@ +# Design Doc: Module-top-level Call Extraction (Issue #210) + +> **Status:** Proposed +> **Date:** 2026-07-12 +> **Author:** Wolfvin +> **Related issues:** #210 +> **Related PRs:** (this PR) + +--- + +## Problem + +CodeLens `reference_count` (`rc` field in `search --mode symbol` output) was +severely undercounting cross-file calls for any function only called via +the two patterns below — both extremely common in modern Express/Router +modular backends: + +1. **Middleware-factory argument pattern.** A function call passed as an + argument to another call at module top-level, e.g. + `router.post(path, requirePermission('admin'), handler)`. The + `requirePermission('admin')` call is inside `router.post(...)`'s + argument list — at module top-level, not inside any function body. + +2. **Inline arrow callback body pattern.** A function call inside an + inline arrow function passed as a callback, e.g. the route handler + `(req, res) => { if (hasPermission(...)) {...} }`. The arrow function + is NOT assigned to a variable, so it's not registered as a function + declaration, and its body is never walked for call extraction. + +Concrete cases reported in issue #210 (verified against the +Coretax-Auto-Downloader / KDS backend): + +- `requirePermission` (middleware factory in `src/middleware/permission-gate.ts`) + was reported with `rc: 0`. Ground truth: 13+ call sites across route files + via `router.post(path, requirePermission('admin'), handler)` at module top-level. + Verified via SQLite: `SELECT * FROM graph_edges WHERE target_id = ...` + returned 0 rows. + +- `hasPermission` (utility in `src/lib/permissions.ts`) was reported with + `rc: 1`. Ground truth: 7 call sites in 3 files (3 in `assignments.ts`, + 3 in `task-templates.ts`, 1 in `permission-gate.ts`). Only the call + inside `requirePermission`'s direct body was extracted. + +Both bugs have the same root cause: the TS/JS backend parsers' per-function +pass only walks bodies of registered function/class declarations. Module- +top-level calls and inline arrow callback bodies are invisible. + +## Goal + +`search --mode symbol` reports `ref_count` that matches the actual number +of call sites for functions called via the two patterns above, without +regressing any existing callgraph consumer (dead-code, impact, trace). + +## Changes + +### Architecture / Data Model + +Introduce a **module-level pass** in the TS/JS backend parsers, run after +the per-function pass. The pass walks the AST root (`program` node) and +extracts every `call_expression` / `new_expression` that hasn't already +been covered by the per-function pass, by skipping the subtrees of +registered declarations: + +- `function_declaration` / `generator_function_declaration` +- `class_declaration` (its `class_body` is walked by the per-function pass) +- `variable_declarator` whose value is `arrow_function` / + `function_expression` / a `defineStore(...)` call (Pinia store pattern) + +Everything else — including inline arrow functions, call arguments, and +non-function `variable_declarator` values like `const router = Router()` — +is walked normally, so calls inside them are extracted. + +Edges from module-level calls use a **synthetic source_id** of the form +`:0:`, with NO corresponding `graph_nodes` entry. This +mirrors the convention already used by +`hybrid_type_resolver._file_node_id` for IMPORTS edges. Benefits: + +- `ref_count` (computed from the target side) is correct — each call + site produces one edge, incrementing the target's count. +- `cmd_list` and `search --mode symbol` output remains clean — no fake + `` function entries appear in user-facing output. +- `dead-code` correctly marks previously-falsely-dead functions as + active (their `ref_count > 0` now). +- `trace` / `impact` continue to work without crash — their JOINs on + `source_id = node_id` simply produce no rows for `` sources, + same as the existing `:0:file` sources for IMPORTS edges. + This is NOT a regression: before the fix, these commands also saw 0 + callers for `requirePermission` because no CALLS edges existed at all. + +### Modified Files + +- `scripts/parsers/ts_backend_parser.py` — add `_find_module_level_calls` + method; call it from `extract_references` after the per-function pass; + update docstring. +- `scripts/parsers/js_backend_parser.py` — add `_find_module_level_calls` + method (using the parser's iterative DFS + `keep_alive` pattern from + issue #116/#163); call it from `extract_references` after the main + walk loop; update docstring. +- `scripts/parsers/fallback_js_backend.py` — add module-level scan for + lines not covered by any function's approximate scope; emit edges with + synthetic `` source. + +### Tests + +- `tests/test_js_backend_parser.py::TestIssue210ModuleLevelCalls` — 6 new + tests covering middleware-factory argument, inline arrow callback body, + multi-site accumulation, no-double-count for function bodies, sanity + extraction of `router.post` itself. +- `tests/test_ts_backend_parser.py` — new file with 10 tests covering + baseline TS parsing plus the same issue #210 patterns (TS is the + primary fix target since the KDS backend is TypeScript). + +## Trade-offs + +### Alternative A: Create a synthetic `` node per file + +- **Pros:** Trace/impact commands would see real caller counts via + SQLite JOINs (source_id would match a real graph_nodes row). +- **Cons:** Adds fake function entries to the backend registry. Would + require modifying `commands/list.py` and `search_engine.py` to filter + them out — violating the issue constraint "Jangan ubah command files + (commands/*.py) kecuali memang diperlukan". The `test_list_backend_only` + test would also need updating. +- **Why rejected:** Violates issue constraint. The fix should stay scoped + to parsers. + +### Alternative B: Modify the per-function pass to also walk module top-level + +- **Pros:** Single-pass design; no separate module-level walk needed. +- **Cons:** Would require invasive changes to the existing walk logic in + both parsers. The TS parser's two-pass design (declarations then calls) + and the JS parser's single-pass design (with issue #116 SIGSEGV + mitigation) both have non-trivial structure that doesn't naturally + accommodate "also extract calls outside function bodies". +- **Why rejected:** Higher regression risk; larger blast radius. + +### Alternative C: Register inline arrow callbacks as anonymous function declarations + +- **Pros:** Per-function pass would naturally walk their bodies. +- **Cons:** Anonymous functions have no name — can't be registered as + function nodes with a meaningful `fn` field. Would require generating + synthetic names, polluting the registry. +- **Why rejected:** Pollutes the function registry with anonymous entries. + +### Chosen approach: Separate module-level pass with synthetic source_id + +- **Why:** Minimal blast radius (parsers only). Follows existing precedent + for synthetic source_ids (hybrid_type_resolver._file_node_id). No + changes to command files or the registry shape. Satisfies the issue's + DoD points 1 and 2 (correct `rc` in `search --mode symbol`). + +## Open Questions + +- [ ] Should `trace` and `impact` eventually be updated to follow + `source_id = ':0:'` edges and report module-level callers? + Currently they skip these dangling sources (same as IMPORTS edges). + This is a follow-up enhancement, not a regression — filed as a future + improvement once issue #210 lands. + +## Migration / Rollout + +No migration impact — additive change. Existing callgraphs gain new edges +for previously-missed call sites; `ref_count` values increase for affected +functions. Functions previously marked "dead" by `audit --check dead-code` +may now be correctly marked "active" if their only callers were module- +top-level. This is a correctness improvement, not a breaking change. + +## References + +- Issue: #210 +- Prior art: `scripts/hybrid_type_resolver.py:_file_node_id` — synthetic + source_id convention for IMPORTS edges. +- Related design docs: [0004-graph-model](0004-graph-model.md) (graph_edges + schema, source_id semantics) diff --git a/scripts/parsers/fallback_js_backend.py b/scripts/parsers/fallback_js_backend.py index 18839fea..722ca27a 100644 --- a/scripts/parsers/fallback_js_backend.py +++ b/scripts/parsers/fallback_js_backend.py @@ -170,6 +170,7 @@ def parse_js_backend_fallback(content, file_path): # Detect function calls (simplified — within function bodies) # For each function found, scan its approximate scope for calls lines = content.split('\n') + covered_lines = [False] * len(lines) for node in nodes: start_line = node["line"] - 1 # 0-indexed # Approximate: scan from function start to next function or 50 lines @@ -177,9 +178,29 @@ def parse_js_backend_fallback(content, file_path): for i in range(start_line, end_line): if i >= len(lines): break + covered_lines[i] = True for m in re.finditer(r'\b([a-zA-Z_]\w*)\s*\(', lines[i]): call_name = m.group(1) if call_name not in skip_names and call_name != node["fn"]: edges.append({"from": node["id"], "to_fn": call_name}) + # Issue #210: module-top-level calls. The per-function loop above only + # scans lines inside an approximate function scope. Calls at file root + # (e.g., `router.post(path, requirePermission('admin'), handler)` in an + # Express modular route file) and calls inside inline arrow-function + # callbacks that aren't assigned to a variable are missed entirely, + # undercounting reference_count for any function only called via those + # patterns. Scan remaining uncovered lines and emit edges with a + # synthetic `` caller (no node entry — mirrors the IMPORTS-edge + # convention in hybrid_type_resolver._file_node_id). + module_node_id = f"{file_path}:0:" + for i, line in enumerate(lines): + if covered_lines[i]: + continue + for m in re.finditer(r'\b([a-zA-Z_]\w*)\s*\(', line): + call_name = m.group(1) + if call_name in skip_names: + continue + edges.append({"from": module_node_id, "to_fn": call_name}) + return {"nodes": nodes, "edges": edges} diff --git a/scripts/parsers/js_backend_parser.py b/scripts/parsers/js_backend_parser.py index 2eb6820d..819c9b36 100755 --- a/scripts/parsers/js_backend_parser.py +++ b/scripts/parsers/js_backend_parser.py @@ -9,7 +9,10 @@ - async variants of all above - Method calls: obj.method() → tracked as "method" - Member expression calls: HttpClient.new() -- Anonymous/inline callbacks → IGNORED +- Module-top-level calls (issue #210): router.post(...) at file root +- Calls inside inline arrow/function-expression callbacks (issue #210): + e.g., the (req, res) => {...} handler passed to router.post(...) +- Anonymous/inline callbacks -> body calls now extracted (issue #210) - Built-in keywords filtered out """ @@ -202,6 +205,39 @@ def extract_references(self, content: str, file_path: str) -> Dict[str, List[Dic keep_alive.append(child) stack.append((child, depth + 1)) + # Issue #210: module-top-level pass. + # The main walk above only extracts calls via _parse_and_collect_calls, + # which is invoked on declaration nodes (function_declaration, + # variable_declarator, class_declaration, export_statement children). + # Calls that live outside any registered declaration body are missed: + # - Module-top-level calls (e.g. `router.post(path, fn, handler)` at + # file root) — including calls passed as arguments like + # `requirePermission('admin')` inside `router.post(...)`. + # - Calls inside inline arrow/function-expression callbacks (e.g. + # the `(req, res) => { hasPermission(...) }` handler passed to + # `router.post(...)`). + # Walk the root again, skipping subtrees of registered declarations + # to avoid double-count, and extract remaining call_expression / + # new_expression nodes with a synthetic `` caller. + module_calls = self._find_module_level_calls(root, source, file_path) + if module_calls: + # Synthetic source_id (no corresponding node entry) — mirrors + # the convention used by hybrid_type_resolver._file_node_id for + # IMPORTS edges. CALLS-traversal code naturally skips dangling + # source_ids, and ref_count is computed from the target side, + # so no node is needed. This keeps `list`/`search` output free + # of fake `` function entries. + module_node_id = f"{file_path}:0:" + for call_info in module_calls: + edge = { + "from": module_node_id, + "to_fn": call_info["fn_name"], + "via_self": call_info.get("via_self", False) + } + if call_info.get("is_ipc_call"): + edge["is_ipc_call"] = True + edges.append(edge) + return {"nodes": nodes, "edges": edges, "skipped": []} finally: if _gc_was_enabled: @@ -505,6 +541,105 @@ def _find_calls_in_scope(self, body_node: Optional[Node], source: bytes, return calls + def _find_module_level_calls(self, root: Node, source: bytes, + file_path: str) -> List[Dict]: + """Find call sites that live outside any registered function declaration. + + Issue #210: the main walk in :meth:`extract_references` only extracts + calls via :meth:`_parse_and_collect_calls`, which is invoked on + declaration nodes. Two patterns slip through and cause + reference_count undercounting: + + 1. Module-top-level calls. Express/Router modular route files typically + do `router.post(path, requirePermission('admin'), handler)` at the + file root — not inside any function. The whole call expression + (including the `requirePermission('admin')` argument) is invisible + to the per-function pass. + 2. Calls inside inline arrow/function-expression callbacks. Route + handlers like `(req, res) => { hasPermission(...) }` are passed as + arguments to `router.post(...)` and are NOT registered as function + declarations (they're not assigned to a name), so the per-function + pass never visits their bodies. + + This pass walks the AST root and extracts every call_expression / + new_expression that hasn't already been covered, by skipping the + subtrees of registered declarations: + + - function_declaration / generator_function_declaration + - class_declaration (its method_definition bodies are walked by the + per-function pass via the class_body) + - variable_declarator whose value is arrow_function / + function_expression / defineStore(...) call (registered as a + function node or Pinia store) + + Everything else — including inline arrow functions and the argument + lists of call_expressions — is walked normally, so calls inside them + are extracted with a synthetic `` caller. + + Uses the same iterative DFS + ``keep_alive`` pattern as + :meth:`_find_calls_in_scope` (issue #116/#163) so tree-sitter Node + references don't dangle mid-walk. + """ + calls: List[Dict] = [] + MAX_DEPTH = 200 + + def _is_registered_value(child: Node) -> bool: + """Return True if a variable_declarator child is a registered value. + + Registered values are arrow_function, function_expression, or a + defineStore(...) call_expression (Pinia store pattern). + """ + if child.type in ('arrow_function', 'function_expression'): + return True + if child.type == 'call_expression': + func_node = child.child_by_field_name('function') + if func_node: + fn_text = source[func_node.start_byte:func_node.end_byte].decode( + 'utf-8', errors='replace') + if fn_text == 'defineStore': + return True + return False + + keep_alive: List[Node] = [root] + stack: List[Tuple[Node, int]] = [(root, 0)] + while stack: + node, depth = stack.pop() + if depth > MAX_DEPTH: + continue + + # Skip subtrees already covered by the per-function pass to + # avoid double-counting call sites. + skip_subtree = False + if node.type in ('function_declaration', + 'generator_function_declaration', + 'class_declaration'): + skip_subtree = True + elif node.type == 'variable_declarator': + for child in node.children: + if _is_registered_value(child): + skip_subtree = True + break + + if skip_subtree: + continue + + if node.type == 'call_expression': + call_info = self._parse_call(node, source) + if call_info: + calls.append(call_info) + elif node.type == 'new_expression': + call_info = self._parse_new_expression(node, source) + if call_info: + calls.append(call_info) + + if depth + 1 <= MAX_DEPTH: + children = node.children + for child in reversed(children): + keep_alive.append(child) + stack.append((child, depth + 1)) + + return calls + def _parse_call(self, node: Node, source: bytes) -> Optional[Dict]: """Parse a call_expression node to extract the called function name. diff --git a/scripts/parsers/ts_backend_parser.py b/scripts/parsers/ts_backend_parser.py index 34bbbd9e..2de4fb0e 100644 --- a/scripts/parsers/ts_backend_parser.py +++ b/scripts/parsers/ts_backend_parser.py @@ -14,7 +14,9 @@ - Abstract method signatures - Method calls: obj.method() -> tracked as "method" - Member expression calls: HttpClient.new() -- Anonymous/inline callbacks -> IGNORED +- Module-top-level calls (issue #210): router.post(...) at file root +- Calls inside inline arrow/function-expression callbacks (issue #210): + e.g., the (req, res) => {...} handler passed to router.post(...) - Built-in keywords filtered out """ @@ -80,6 +82,35 @@ def extract_references(self, content: str, file_path: str) -> Dict[str, List[Dic edge["is_ipc_call"] = True edges.append(edge) + # Third pass (issue #210): module-top-level calls. + # The per-function pass above only walks bodies of registered function + # declarations. Calls at module top-level (e.g. `router.post(path, + # requirePermission('admin'), handler)` at file root) and calls inside + # inline arrow/function-expression callbacks (e.g. the `(req, res) => { + # hasPermission(...) }` handler passed as an argument) are missed + # entirely, which undercounts reference_count for any function only + # called via those patterns. Walk the root and extract remaining + # call_expression / new_expression nodes, skipping subtrees that are + # already covered by the per-function pass to avoid double-counting. + module_calls = self._find_module_level_calls(tree, source, file_path) + if module_calls: + # Synthetic source_id (no corresponding node entry) — mirrors the + # convention used by hybrid_type_resolver._file_node_id for IMPORTS + # edges. CALLS-traversal code naturally skips dangling source_ids, + # and ref_count is computed from the target side, so no node is + # needed. This keeps `list`/`search` output free of fake `` + # function entries. + module_node_id = f"{file_path}:0:" + for call_info in module_calls: + edge = { + "from": module_node_id, + "to_fn": call_info["fn_name"], + "via_self": call_info.get("via_self", False) + } + if call_info.get("is_ipc_call"): + edge["is_ipc_call"] = True + edges.append(edge) + return {"nodes": nodes, "edges": edges} def _find_function_declarations(self, root: Node, source: bytes, @@ -344,6 +375,84 @@ def visit(node: Node, _, depth): self.walk_tree(body_node, source, visit) return calls + def _find_module_level_calls(self, root: Node, source: bytes, + file_path: str) -> List[Dict]: + """Find call sites that live outside any registered function declaration. + + Issue #210: the per-function pass only walks bodies of registered + function/class declarations. Two patterns slip through and cause + reference_count undercounting: + + 1. Module-top-level calls. Express/Router modular route files typically + do `router.post(path, requirePermission('admin'), handler)` at the + file root — not inside any function. The whole call expression + (including the `requirePermission('admin')` argument) is invisible + to the per-function pass. + 2. Calls inside inline arrow/function-expression callbacks. Route + handlers like `(req, res) => { hasPermission(...) }` are passed as + arguments to `router.post(...)` and are NOT registered as function + declarations (they're not assigned to a name), so the per-function + pass never visits their bodies. + + This pass walks the AST root and extracts every call_expression / + new_expression that hasn't already been covered, by skipping the + subtrees of registered declarations: + + - function_declaration / generator_function_declaration + - class_declaration (its method_definition bodies are walked by the + per-function pass via the class_body) + - variable_declarator whose value is arrow_function / + function_expression / defineStore(...) call (registered as a + function node or Pinia store) + + Everything else — including inline arrow functions and the argument + lists of call_expressions — is walked normally, so calls inside them + are extracted with a synthetic `` caller. + """ + calls: List[Dict] = [] + + def _is_registered_value(child: Node) -> bool: + """Return True if a variable_declarator child is a registered value. + + Registered values are arrow_function, function_expression, or a + defineStore(...) call_expression (Pinia store pattern). + """ + if child.type in ('arrow_function', 'function_expression'): + return True + if child.type == 'call_expression': + func_node = child.child_by_field_name('function') + if func_node: + fn_text = source[func_node.start_byte:func_node.end_byte].decode( + 'utf-8', errors='replace') + if fn_text == 'defineStore': + return True + return False + + def visit(node: Node, _, depth): + # Skip subtrees already covered by the per-function pass to + # avoid double-counting call sites. + if node.type in ('function_declaration', + 'generator_function_declaration', + 'class_declaration'): + return False + if node.type == 'variable_declarator': + for child in node.children: + if _is_registered_value(child): + return False + + if node.type == 'call_expression': + call_info = self._parse_call(node, source) + if call_info: + calls.append(call_info) + elif node.type == 'new_expression': + call_info = self._parse_new_expression(node, source) + if call_info: + calls.append(call_info) + return True + + self.walk_tree(root, source, visit) + return calls + def _parse_call(self, node: Node, source: bytes) -> Optional[Dict]: """Parse a call_expression node to extract the called function name. diff --git a/tests/test_js_backend_parser.py b/tests/test_js_backend_parser.py index dcdbbb44..6ddb2697 100644 --- a/tests/test_js_backend_parser.py +++ b/tests/test_js_backend_parser.py @@ -165,3 +165,159 @@ def test_fallback_returns_nodes_and_edges_keys(self): assert "edges" in result assert isinstance(result["nodes"], list) assert isinstance(result["edges"], list) + + +# ─── Issue #210: module-top-level call extraction ────────────────────────── +# These tests reproduce two real-world patterns from the KDS backend audit +# (see GitHub issue #210) where reference_count was severely undercounted +# because the per-function pass missed them: +# 1. Middleware-factory argument: `router.post(path, requirePermission('admin'), handler)` +# — `requirePermission('admin')` is a call_expression passed as an argument +# to another call_expression at module top-level. The per-function pass +# only walks registered function bodies; module-top-level calls are +# invisible. Issue #210 fix adds a module-level pass that walks the AST +# root, skips registered declaration subtrees, and extracts remaining +# call_expression nodes (including those nested inside other calls' +# argument lists). +# 2. Inline arrow callback body: `router.post(path, fn, (req, res) => { +# hasPermission(...) })` — the arrow function is passed as an argument +# and is NOT registered as a function declaration (not assigned to a +# name), so the per-function pass never visits its body. The module- +# level pass walks into inline arrow functions and extracts their calls. +class TestIssue210ModuleLevelCalls: + """Issue #210 regression tests: module-top-level call extraction. + + Before the fix, both patterns below produced 0 call edges because the + per-function pass only walks bodies of registered function/class + declarations. Module-top-level calls and calls inside inline arrow + function callbacks were missed entirely, causing reference_count + undercounting for any function only called via those patterns. + """ + + def test_middleware_factory_argument_extracted(self): + """Call passed as argument to another call (router.post pattern). + + Before fix: `requirePermission('admin')` inside `router.post(...)` at + module top-level was not extracted — 0 edges to `requirePermission`. + After fix: 1 edge with synthetic `` source_id. + """ + js = """ + const router = Router(); + router.post('/foo', requirePermission('admin'), (req, res) => { + res.json({ ok: true }); + }); + """ + result = _parse(js, "routes.js") + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + # requirePermission must be extracted as a call target + assert "requirePermission" in edge_to_fns, ( + f"requirePermission edge missing — module-top-level call passed " + f"as argument not extracted. edges={edge_to_fns}" + ) + # The edge source must be the synthetic id (file:0:) + rp_edges = [e for e in result["edges"] if e.get("to_fn") == "requirePermission"] + assert all(e["from"].endswith(":0:") for e in rp_edges), ( + f"requirePermission edge source should be synthetic id, " + f"got: {[e['from'] for e in rp_edges]}" + ) + + def test_inline_arrow_callback_body_calls_extracted(self): + """Calls inside inline arrow function callback body (route handler). + + Before fix: `hasPermission(...)` inside `(req, res) => { ... }` was + not extracted because the arrow function isn't a registered function + declaration. After fix: edge extracted via module-level pass walking + into inline arrow functions. + """ + js = """ + const router = Router(); + router.post('/foo', authMiddleware, (req, res) => { + if (hasPermission(req.user, 'admin')) { + return res.json({ ok: true }); + } + res.status(403).send('forbidden'); + }); + """ + result = _parse(js, "routes.js") + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + assert "hasPermission" in edge_to_fns, ( + f"hasPermission edge missing — inline arrow callback body call " + f"not extracted. edges={edge_to_fns}" + ) + + def test_multi_site_same_file_accumulates_edges(self): + """Multiple call sites of the same function in one file each emit + their own edge — no dedup collapse that would undercount rc. + + Mirrors the `hasPermission` case from KDS backend (3 call sites per + route file × 2 route files → rc=6 from module-level alone). + """ + js = """ + const router = Router(); + router.post('/a', fn, (req, res) => { + if (hasPermission(req.user, 'a')) { return res.json({ ok: true }); } + res.status(403).send('forbidden'); + }); + router.post('/b', fn, (req, res) => { + if (hasPermission(req.user, 'b')) { return res.json({ ok: true }); } + res.status(403).send('forbidden'); + }); + router.post('/c', fn, (req, res) => { + if (hasPermission(req.user, 'c')) { return res.json({ ok: true }); } + res.status(403).send('forbidden'); + }); + """ + result = _parse(js, "routes.js") + hp_edges = [e for e in result["edges"] if e.get("to_fn") == "hasPermission"] + assert len(hp_edges) == 3, ( + f"Expected 3 hasPermission edges (one per call site), got " + f"{len(hp_edges)}. edges={hp_edges}" + ) + + def test_no_double_count_for_calls_inside_function_body(self): + """Calls inside a registered function body must NOT be re-extracted + by the module-level pass — otherwise ref_count would be inflated. + + The module-level pass skips subtrees of registered declarations + (function_declaration, class_declaration, variable_declarator with + arrow/function value) to avoid double-counting. + """ + js = """ + function handler() { + return helper(); + } + const router = Router(); + router.post('/foo', handler); + """ + result = _parse(js, "test.js") + helper_edges = [e for e in result["edges"] if e.get("to_fn") == "helper"] + # `helper()` is called once inside `handler()` body. The per-function + # pass extracts it with from=handler's node id. The module-level pass + # must NOT re-extract it (handler's subtree is skipped). + assert len(helper_edges) == 1, ( + f"Expected exactly 1 helper edge (no double-count), got " + f"{len(helper_edges)}. edges={helper_edges}" + ) + # The single edge should come from handler's node id, not + assert not helper_edges[0]["from"].endswith(":0:"), ( + f"helper edge should come from handler's node id, not . " + f"edge={helper_edges[0]}" + ) + + def test_module_level_call_extracts_router_post(self): + """Sanity: module-top-level `router.post(...)` itself is extracted + (its target is `post` — a method call, resolved via short-name match + by edge_resolver). Before fix, this was also missed. + """ + js = """ + const router = Router(); + router.post('/foo', handler); + """ + result = _parse(js, "routes.js") + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + # `post` is the method name from `router.post(...)` — extracted as + # to_fn="post" by _parse_call (member_expression → method name). + assert "post" in edge_to_fns, ( + f"router.post call not extracted at module top-level. " + f"edges={edge_to_fns}" + ) diff --git a/tests/test_ts_backend_parser.py b/tests/test_ts_backend_parser.py new file mode 100644 index 00000000..26ca507d --- /dev/null +++ b/tests/test_ts_backend_parser.py @@ -0,0 +1,255 @@ +""" +Tests for the TS Backend Parser — function call graph extraction. + +Issue #210 regression coverage: module-top-level call extraction for +TypeScript backend files. Mirrors the test coverage in +``test_js_backend_parser.py::TestIssue210ModuleLevelCalls`` but for the +TypeScript parser (which uses a different code path — two-pass design +vs. JS parser's single-pass design). + +The KDS backend (Coretax-Auto-Downloader) reported in issue #210 is a +TypeScript Express app, so the TS parser is the primary fix target. +""" + +import os +import sys +import pytest + +SCRIPT_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts") +sys.path.insert(0, SCRIPT_DIR) + +# Try to import tree-sitter based parser +_ts_be_parser = None +_ts_be_parser_available = False +try: + from parsers.ts_backend_parser import TSBackendParser + _ts_be_parser = TSBackendParser() + _ts_be_parser_available = True +except Exception: + pass + + +def _parse(content, path="server.ts"): + """Parse TS backend using tree-sitter parser. Skips if unavailable.""" + if not _ts_be_parser_available: + pytest.skip("Tree-sitter TypeScript grammar not installed") + return _ts_be_parser.extract_references(content, path) + + +@pytest.mark.skipif(not _ts_be_parser_available, reason="Tree-sitter TypeScript grammar not installed") +class TestTSBackendParser: + """Baseline tests for TSBackendParser function declaration extraction.""" + + def test_function_declaration(self): + ts = "function processData(input: string): string { return input; }" + result = _parse(ts) + fn_names = [n["fn"] for n in result["nodes"]] + assert "processData" in fn_names + + def test_exported_function_declaration(self): + ts = "export function verifyToken(token: string): boolean { return true; }" + result = _parse(ts) + for node in result["nodes"]: + if node["fn"] == "verifyToken": + assert node.get("exported") is True + + def test_arrow_function_with_types(self): + ts = "const fetchData = async (url: string): Promise => { return; };" + result = _parse(ts) + fn_names = [n["fn"] for n in result["nodes"]] + assert "fetchData" in fn_names + + def test_function_call_edge(self): + ts = """ + function hashPassword(pw: string): string { return crypto.hash(pw); } + function verifyPassword(input: string): boolean { return hashPassword(input); } + """ + result = _parse(ts) + assert len(result["edges"]) > 0 + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + assert "hashPassword" in edge_to_fns + + +@pytest.mark.skipif(not _ts_be_parser_available, reason="Tree-sitter TypeScript grammar not installed") +class TestTSBackendParserIssue210ModuleLevel: + """Issue #210 regression tests: module-top-level call extraction for TS. + + Before the fix, the per-function pass only walked bodies of registered + function/class declarations. Module-top-level calls (e.g. + `router.post(path, requirePermission('admin'), handler)` at file root) + and calls inside inline arrow/function-expression callbacks (e.g. route + handlers) were missed entirely, causing reference_count undercounting. + + The KDS backend reported in issue #210 is a TypeScript Express app, so + these TS tests reproduce the exact patterns from the bug report. + """ + + def test_middleware_factory_argument_extracted(self): + """Call passed as argument to another call (router.post pattern). + + This is the exact `requirePermission('admin')` pattern from + permission-gate.ts in the KDS backend. Before fix: 0 edges to + requirePermission. After fix: 1 edge with synthetic source. + """ + ts = """ + import { Router } from 'express'; + import { requirePermission } from './middleware/permission-gate'; + + const router = Router(); + + router.post('/accounting/journal', requirePermission('admin'), (req, res) => { + res.json({ ok: true }); + }); + """ + result = _parse(ts, "src/routes/accounting.ts") + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + assert "requirePermission" in edge_to_fns, ( + f"requirePermission edge missing — module-top-level call passed " + f"as argument not extracted. edges={edge_to_fns}" + ) + rp_edges = [e for e in result["edges"] if e.get("to_fn") == "requirePermission"] + assert all(e["from"].endswith(":0:") for e in rp_edges), ( + f"requirePermission edge source should be synthetic id, " + f"got: {[e['from'] for e in rp_edges]}" + ) + + def test_inline_arrow_callback_body_calls_extracted(self): + """Calls inside inline arrow function callback body (route handler). + + This is the exact `hasPermission(...)` pattern from the KDS backend + route handlers. Before fix: 0 edges to hasPermission from route + files. After fix: edge extracted via module-level pass. + """ + ts = """ + import { Router } from 'express'; + import { hasPermission } from '../lib/permissions'; + + const router = Router(); + + router.post('/foo', authMiddleware, (req, res) => { + if (hasPermission(req.user, 'admin')) { + return res.json({ ok: true }); + } + res.status(403).send('forbidden'); + }); + """ + result = _parse(ts, "src/routes/foo.ts") + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + assert "hasPermission" in edge_to_fns, ( + f"hasPermission edge missing — inline arrow callback body call " + f"not extracted. edges={edge_to_fns}" + ) + + def test_multi_site_same_file_accumulates_edges(self): + """Multiple call sites of the same function in one file each emit + their own edge — no dedup collapse that would undercount rc. + + Mirrors the KDS backend case: 3 `requirePermission('admin')` call + sites per route file × 4 route files → rc=12 from module-level alone. + """ + ts = """ + import { Router } from 'express'; + import { requirePermission } from './middleware/permission-gate'; + + const router = Router(); + + router.post('/a', requirePermission('admin'), (req, res) => { res.json({ ok: true }); }); + router.post('/b', requirePermission('admin'), (req, res) => { res.json({ ok: true }); }); + router.post('/c', requirePermission('admin'), (req, res) => { res.json({ ok: true }); }); + """ + result = _parse(ts, "src/routes/foo.ts") + rp_edges = [e for e in result["edges"] if e.get("to_fn") == "requirePermission"] + assert len(rp_edges) == 3, ( + f"Expected 3 requirePermission edges (one per call site), got " + f"{len(rp_edges)}. edges={rp_edges}" + ) + + def test_no_double_count_for_calls_inside_function_body(self): + """Calls inside a registered function body must NOT be re-extracted + by the module-level pass — otherwise ref_count would be inflated. + + The module-level pass skips subtrees of registered declarations + (function_declaration, class_declaration, variable_declarator with + arrow/function value) to avoid double-counting. + """ + ts = """ + function handler(): void { + helper(); + } + const router = Router(); + router.post('/foo', handler); + """ + result = _parse(ts, "test.ts") + helper_edges = [e for e in result["edges"] if e.get("to_fn") == "helper"] + assert len(helper_edges) == 1, ( + f"Expected exactly 1 helper edge (no double-count), got " + f"{len(helper_edges)}. edges={helper_edges}" + ) + assert not helper_edges[0]["from"].endswith(":0:"), ( + f"helper edge should come from handler's node id, not . " + f"edge={helper_edges[0]}" + ) + + def test_module_level_call_extracts_router_post(self): + """Sanity: module-top-level `router.post(...)` itself is extracted. + """ + ts = """ + const router = Router(); + router.post('/foo', handler); + """ + result = _parse(ts, "routes.ts") + edge_to_fns = [e.get("to_fn", "") for e in result["edges"]] + assert "post" in edge_to_fns, ( + f"router.post call not extracted at module top-level. " + f"edges={edge_to_fns}" + ) + + def test_class_method_body_not_double_counted(self): + """Calls inside an exported class method body are extracted by the + per-function pass (which walks class_body). The module-level pass + must skip the class_declaration subtree (inside export_statement) + to avoid double-counting. + + Note: the TS parser only registers classes via the export_statement + branch of _find_function_declarations, so we use `export class` + here. Non-exported class registration is a separate pre-existing + limitation outside issue #210's scope. + """ + ts = """ + export class Service { + process(): void { + this.helper(); + } + } + const router = Router(); + router.post('/foo', handler); + """ + result = _parse(ts, "test.ts") + # `helper` is called once inside Service.process. The per-function + # pass walks class_body → finds helper call. The module-level pass + # skips class_declaration subtree (inside export_statement) → no + # double-count. + helper_edges = [e for e in result["edges"] if e.get("to_fn") == "helper"] + assert len(helper_edges) == 1, ( + f"Expected exactly 1 helper edge (no double-count from class " + f"subtree), got {len(helper_edges)}. edges={helper_edges}" + ) + + def test_exported_arrow_function_not_double_counted(self): + """`export const foo = () => {...}` — the arrow function is registered + as a function declaration. Module-level pass must skip the + variable_declarator subtree to avoid double-counting its body calls. + """ + ts = """ + export const handler = () => { + return helper(); + }; + const router = Router(); + router.post('/foo', handler); + """ + result = _parse(ts, "test.ts") + helper_edges = [e for e in result["edges"] if e.get("to_fn") == "helper"] + assert len(helper_edges) == 1, ( + f"Expected exactly 1 helper edge (no double-count from exported " + f"arrow function), got {len(helper_edges)}. edges={helper_edges}" + )