diff --git a/graphify/extract.py b/graphify/extract.py index a05d5c6..f6719f3 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -11,6 +11,29 @@ from typing import Callable, Any from .cache import load_cached, save_cached +# AST node types that represent a member-expression callee +# (`x.foo()`, `obj.bar()`, `Pkg::baz()`). Cross-file name-only resolution of +# these is unsafe — without receiver type info we routinely link them to the +# wrong target, producing phantom god nodes (e.g. every `Logger.log(...)` call +# in a NestJS codebase collapsing onto a one-off `function log(...)` defined +# in a smoke-test script). The cross-file resolver in `extract()` skips +# entries whose `callee_node_type` falls in this set. +_MEMBER_CALL_NODE_TYPES = frozenset({ + "member_expression", # JS / TS / Python attribute calls + "selector_expression", # Go + "field_expression", # Rust, C++, Scala + "navigation_expression", # Swift, Kotlin + "qualified_identifier", # C++ (Foo::bar()) + "scoped_identifier", # Rust (foo::bar()) + "scoped_call_expression", # PHP (Foo::bar()) + "member_call_expression", # PHP ($obj->method()) + "field_access", # Java + "method_invocation", # Java (when receiver-qualified) + "dot", # Elixir + "_zig_dotted_callee", # synthetic marker for Zig (callee text contains ".") +}) + + def _make_id(*parts: str) -> str: """Build a stable node ID from one or more name parts.""" combined = "_".join(p.strip("_.") for p in parts if p) @@ -1172,13 +1195,16 @@ def _extract_generic(path: Path, config: LanguageConfig) -> dict: "weight": 1.0, }) elif callee_name and not tgt_nid: - # Callee not in this file — save for cross-file resolution in extract() + # Callee not in this file — save for cross-file resolution in extract(). + # Track the AST node type so the cross-file resolver can refuse to + # name-match member-expression callees (`x.foo()`). raw_calls.append({ "caller_nid": caller_nid, "callee": callee_name, "is_member_call": is_member_call, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + "callee_node_type": func_node.type if func_node is not None else None, }) # Helper function calls: config('foo.bar') → uses_config edge to "foo" @@ -2131,6 +2157,7 @@ def extract_go(path: Path) -> dict: "is_member_call": is_member_call, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + "callee_node_type": func_node.type if func_node is not None else None, }) for child in node.children: walk_calls(child, caller_nid) @@ -2311,6 +2338,7 @@ def extract_rust(path: Path) -> dict: "is_member_call": is_member_call, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + "callee_node_type": func_node.type if func_node is not None else None, }) for child in node.children: walk_calls(child, caller_nid) @@ -2482,6 +2510,13 @@ def extract_zig(path: Path) -> dict: "is_member_call": is_member_call, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + # Zig: callee is `_read_text(fn, source).split(".")[-1]`, + # so a "." in the original text means it was a member call. + "callee_node_type": ( + "_zig_dotted_callee" + if "." in _read_text(fn, source) + else fn.type + ), }) for child in node.children: walk_calls(child, caller_nid) @@ -2647,6 +2682,7 @@ def extract_powershell(path: Path) -> dict: "is_member_call": False, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + "callee_node_type": "command_name", }) for child in node.children: walk_calls(child, caller_nid) @@ -3265,6 +3301,9 @@ def extract_elixir(path: Path) -> dict: "is_member_call": is_member_call, "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", + # Elixir: `child` is the node we just inspected — `dot` for + # `Mod.fn(...)`, `identifier` for a bare local call. + "callee_node_type": child.type, }) for child in node.children: walk_calls(child, caller_nid) @@ -3462,6 +3501,14 @@ def extract(paths: list[Path], cache_root: Path | None = None) -> dict: existing_pairs = {(e["source"], e["target"]) for e in all_edges} for result in per_file: for rc in result.get("raw_calls", []): + # Member-expression callees (`x.foo()`, `obj.bar()`, `Pkg::baz()`) + # cannot be safely resolved by bare property name across files — + # without receiver-type analysis we routinely link them to the + # wrong target, producing phantom god nodes (e.g. every + # `Logger.log(...)` call collapsing onto a one-off + # `function log(...)` defined in a smoke-test script). + if rc.get("callee_node_type") in _MEMBER_CALL_NODE_TYPES: + continue callee = rc.get("callee", "") if not callee: continue diff --git a/tests/test_member_call_resolution.py b/tests/test_member_call_resolution.py new file mode 100644 index 0000000..3e7ab8f --- /dev/null +++ b/tests/test_member_call_resolution.py @@ -0,0 +1,177 @@ +""" +Regression test for the cross-file member-call name-collision bug fixed by +the `_MEMBER_CALL_NODE_TYPES` resolver guard. + +Bug summary +----------- +graphify's per-language AST extractors strip member-expression callees +(`this.logger.log(...)`, `obj.find(...)`, `Pkg::baz(...)`) down to the +trailing identifier (`log`, `find`, `baz`) and queue them in `raw_calls` +for cross-file resolution. The cross-file resolver then matches that bare +identifier against a global lowercase name → id map. If any other file in +the corpus defines a top-level helper with the same name (e.g. a +`function log(id, name, pass) { ... }` in a smoke-test script), every +unrelated `Logger.log` / `this.logger.log` / `logger.log` call across the +codebase resolves to that single helper, producing a phantom god node with +hundreds of bogus INFERRED edges. + +Fix +--- +Each `raw_calls.append(...)` now records `callee_node_type` (the AST node +type of the callee). The cross-file resolver in `extract()` skips entries +whose `callee_node_type` is in `_MEMBER_CALL_NODE_TYPES`. + +Run with: + pytest tests/test_member_call_resolution.py -v +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from graphify.extract import extract + +try: + from graphify.extract import _MEMBER_CALL_NODE_TYPES +except ImportError: + _MEMBER_CALL_NODE_TYPES = None # patched constant absent on unpatched code + + +# --------------------------------------------------------------------------- +# Source fixtures — written verbatim to a tmp dir so tree-sitter sees real files +# --------------------------------------------------------------------------- + +_SMOKE_TEST_JS = """\ +const results = []; + +function log(id, name, pass, detail = '') { + const status = pass ? 'PASS' : 'FAIL'; + console.log(`${id} ${status} ${name}`); + results.push({ id, name, pass }); +} + +function testAuth() { + log('A1', 'auth-login', true); + log('A2', 'auth-refresh', true); +} + +testAuth(); +""" + +_NESTJS_SERVICE_TS = """\ +import { Logger } from '@nestjs/common'; + +export class AppointmentsService { + private readonly logger = new Logger('AppointmentsService'); + + async create(payload: unknown): Promise { + this.logger.log('Creating appointment'); + Logger.log('static-style log call'); + } + + async cancel(id: string): Promise { + this.logger.log(`Cancelled ${id}`); + } +} +""" + +_NESTJS_OTHER_SERVICE_TS = """\ +import { Logger } from '@nestjs/common'; + +const logger = new Logger('eprescribing-metrics'); + +export function logTransmissionAttempt(rxId: string): void { + logger.log({ event: 'tx-attempt', rxId }); +} + +export function logWebhookReceived(payload: unknown): void { + logger.log({ event: 'webhook', payload }); +} +""" + + +@pytest.fixture +def project(tmp_path: Path) -> Path: + (tmp_path / "validation").mkdir() + (tmp_path / "src").mkdir() + (tmp_path / "validation" / "smoke-test.mjs").write_text(_SMOKE_TEST_JS) + (tmp_path / "src" / "appointments.service.ts").write_text(_NESTJS_SERVICE_TS) + (tmp_path / "src" / "eprescribing-metrics.ts").write_text(_NESTJS_OTHER_SERVICE_TS) + return tmp_path + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +def test_member_call_node_types_constant_present() -> None: + """The constant must exist and cover every language extractor.""" + assert _MEMBER_CALL_NODE_TYPES is not None, ( + "_MEMBER_CALL_NODE_TYPES is missing from graphify.extract — patch not applied" + ) + assert isinstance(_MEMBER_CALL_NODE_TYPES, frozenset) + # Spot-check that the most important node types are tracked. + for required in ( + "member_expression", # JS/TS + "selector_expression", # Go + "field_expression", # Rust / C++ + "navigation_expression", # Swift / Kotlin + "dot", # Elixir + ): + assert required in _MEMBER_CALL_NODE_TYPES, f"missing: {required}" + + +def test_smoke_test_log_does_not_steal_nestjs_logger_calls(project: Path) -> None: + """ + Regression: with a corpus containing both `validation/smoke-test.mjs` + (which defines `function log(...)`) AND NestJS service files (which + call `this.logger.log(...)` / `Logger.log(...)` / `logger.log(...)`), + NONE of the NestJS calls should resolve to the smoke-test `log` node. + """ + files = sorted(project.rglob("*.mjs")) + sorted(project.rglob("*.ts")) + result = extract(files, cache_root=project) + + log_nodes = [n for n in result["nodes"] + if n.get("label", "").strip().rstrip("()") == "log"] + assert len(log_nodes) == 1, "expected exactly one top-level log() symbol" + + log_id = log_nodes[0]["id"] + log_source_file = log_nodes[0]["source_file"] + + # Every inbound edge to log() must come from the same file (validation/smoke-test.mjs). + inbound = [e for e in result["edges"] if e.get("target") == log_id] + cross_file = [e for e in inbound if e.get("source_file") != log_source_file] + + assert not cross_file, ( + f"phantom edges leaked to log() from other files: " + f"{[(e['source_file'], e['source_location']) for e in cross_file]}" + ) + + # And we should still capture the legitimate intra-file calls + # (testAuth → log in the smoke-test file). + assert any(e for e in inbound if e.get("source_file") == log_source_file), ( + "lost legitimate intra-file call edges to log()" + ) + + +def test_raw_calls_emit_callee_node_type_for_member_expressions(project: Path) -> None: + """ + Direct check on the raw-calls payload: any unresolved member-expression + callee should carry a `callee_node_type` that's in _MEMBER_CALL_NODE_TYPES. + + We can't easily inspect raw_calls after extract() returns (they get + consumed by the resolver), so we test indirectly: the absence of phantom + edges (above test) is the observable proof. + """ + # Sanity: just exercise extract() with something that triggers raw_calls + # so we know our test corpus is meaningful. + files = sorted(project.rglob("*.mjs")) + sorted(project.rglob("*.ts")) + result = extract(files, cache_root=project) + # We expect nodes from all three files. + source_files = {n.get("source_file") for n in result["nodes"]} + assert any("smoke-test" in (s or "") for s in source_files) + assert any("appointments.service" in (s or "") for s in source_files) + assert any("eprescribing-metrics" in (s or "") for s in source_files)