From 3281d42492ba3fec4a1dc0c980383ca6e36ebb12 Mon Sep 17 00:00:00 2001 From: xiongjianxu <6457197+xiongjianxu@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:47:48 +0800 Subject: [PATCH] fix(cross-repo): point a cross-repo call edge at its call site, not the caller's header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cross-repo `calls` edge took its `source_file` from the caller node (an ObjC method node is minted at its `@interface` in the `.h`) while its `source_location` is the call-site line, recorded in the `.m`. Pairing the two pointed `source_file:source_location` into the wrong file — often past the end of a short header. The park payload now carries the call-site file alongside the line, and `link_cross_repo_member_calls` prefers it, falling back to the caller node's file for pre-#3152 graphs. The parked path is relativized in the same extract-time pass that relativizes node source_files, so it never leaks the absolute checkout path into graph.json. Co-Authored-By: Claude Code --- graphify/cross_repo_calls.py | 7 +++- graphify/extract.py | 15 ++++++++ tests/test_cross_repo_member_calls.py | 29 ++++++++++++++ tests/test_go_receiver_member_calls.py | 5 ++- tests/test_parked_call_site_file.py | 51 +++++++++++++++++++++++++ tests/test_php_receiver_member_calls.py | 5 ++- tests/test_ts_receiver_member_calls.py | 6 ++- 7 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 tests/test_parked_call_site_file.py diff --git a/graphify/cross_repo_calls.py b/graphify/cross_repo_calls.py index a82818861..1b9601128 100644 --- a/graphify/cross_repo_calls.py +++ b/graphify/cross_repo_calls.py @@ -262,7 +262,12 @@ def link_cross_repo_member_calls(merged: "nx.Graph") -> int: context="cross_repo", confidence="INFERRED", confidence_score=0.85, - source_file=str(caller_data.get("source_file") or ""), + # The parked call-site file (the .m), not the caller node's own + # source_file (often the .h where the method is declared): the + # location below is the call-site line, so pairing it with the + # .h would point past the end of an unrelated file. Pre-#3152 + # graphs have no `file`, so fall back to the caller's file. + source_file=str(entry.get("file") or caller_data.get("source_file") or ""), source_location=entry.get("line"), weight=1.0, _src=caller, diff --git a/graphify/extract.py b/graphify/extract.py index 0a6037d7d..2c689bf6f 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -4534,6 +4534,9 @@ def _park_unresolved_member_call( location = raw_call.get("source_location") if location: entry["line"] = str(location) + call_file = raw_call.get("source_file") + if call_file: + entry["file"] = str(call_file) parked.append(entry) @@ -10908,6 +10911,18 @@ def extract( if df_path.is_absolute(): new_df, _, _ = _sf_entry(str(df), df_path) item["definition_file"] = new_df + # A parked member call records the call-SITE file (the .m), which differs + # from the caller node's own source_file (often the .h) and so is not + # relativized by the source_file arm above. Left absolute it leaks the + # checkout path into graph.json and into the cross-repo edge built from it + # (#3152 parking carries names only for the same reason). Relativize it + # here, where the canonical source_file form is already computed. + md = item.get("metadata") + if isinstance(md, dict): + for _pc in md.get(UNRESOLVED_CALLS_KEY) or (): + _pf = isinstance(_pc, dict) and _pc.get("file") + if _pf and Path(_pf).is_absolute(): + _pc["file"], _, _ = _sf_entry(str(_pf), Path(_pf)) if ext_id_remap: # Bash entrypoint ids are the file-level id + "__entry" diff --git a/tests/test_cross_repo_member_calls.py b/tests/test_cross_repo_member_calls.py index 0d5d3e9e6..dc8a3564e 100644 --- a/tests/test_cross_repo_member_calls.py +++ b/tests/test_cross_repo_member_calls.py @@ -107,6 +107,35 @@ def test_a_parked_call_binds_to_the_one_declaration_in_another_repo(): assert data["source_location"] == "L10" +def test_edge_source_file_is_the_parked_call_site_not_the_caller_node_file(): + # The caller node's source_file is the declaration file (an ObjC caller lives + # in its `.h`); the call happens in the `.m`. Pairing the call-site line with + # the `.h` points past the end of an unrelated file (#3152 issue 3). The parked + # `file` carries the call-site, relativized at extract time. + parked = [{"callee": "greet", "receiver_type": "Greeter", "lang": "java", + "line": "L10", "file": "src/App.m"}] + G = _graph( + caller=_caller("a", parked, source_file="src/App.h"), + declarations=[(_declaration("b", "Greeter"), _method("b"))], + ) + assert link_cross_repo_member_calls(G) == 1 + data = G.edges["a::app_run", "b::greeter_greet"] + assert data["source_file"] == "src/App.m" + assert data["source_location"] == "L10" + + +def test_a_pre_3152_parked_entry_without_file_falls_back_to_the_caller_file(): + # Graphs built before the call-site `file` was parked still merge: the edge + # falls back to the caller node's own source_file, the pre-fix behavior. + G = _graph( + caller=_caller("a", PARKED_GREET, source_file="src/App.h"), + declarations=[(_declaration("b", "Greeter"), _method("b"))], + ) + assert link_cross_repo_member_calls(G) == 1 + data = G.edges["a::app_run", "b::greeter_greet"] + assert data["source_file"] == "src/App.h" + + def test_two_repos_declaring_the_same_name_bind_nothing(): # The single-definition guard the single-repo resolvers apply. Two `Greeter` # declarations mean the call is ambiguous, and guessing one is worse than diff --git a/tests/test_go_receiver_member_calls.py b/tests/test_go_receiver_member_calls.py index a3a74caa6..03aee940a 100644 --- a/tests/test_go_receiver_member_calls.py +++ b/tests/test_go_receiver_member_calls.py @@ -150,8 +150,9 @@ def test_a_type_from_another_language_never_answers_a_go_receiver(tmp_path): "java/Greeter.java": "public class Greeter { public void Greet() {} }\n", }) assert _greet_edge(calls) is None, calls - parked = [(n.get("metadata") or {}).get("unresolved_calls") - for n in result["nodes"] if n["label"] == "Run()"] + parked = [[{k: v for k, v in e.items() if k != "file"} for e in uc] + for n in result["nodes"] if n["label"] == "Run()" + for uc in [(n.get("metadata") or {}).get("unresolved_calls")] if uc] assert parked == [[{"callee": "Greet", "receiver_type": "Greeter", "lang": "go", "line": "L3"}]], parked diff --git a/tests/test_parked_call_site_file.py b/tests/test_parked_call_site_file.py new file mode 100644 index 000000000..9eb7477f5 --- /dev/null +++ b/tests/test_parked_call_site_file.py @@ -0,0 +1,51 @@ +"""A parked member call records its call-site file, relativized (#3152 issue 3). + +`_park_unresolved_member_call` keeps an unresolved cross-repo call on the caller +node so a later merge can finish the edge. The call-site file (the `.m`) differs +from the caller node's own source_file (the `.h` where the method is declared), +and the raw_call carries it as an ABSOLUTE path. Left absolute it leaks the +checkout directory into graph.json — the same non-reproducibility class as the +category-id leak — and into the cross-repo edge built from it. The extract-time +source_file relativization pass now relativizes the parked `file` too. +""" +from __future__ import annotations + +import importlib.util + +import pytest + +from graphify.extract import extract + +pytestmark = pytest.mark.skipif( + importlib.util.find_spec("tree_sitter_objc") is None, + reason="tree_sitter_objc not installed", +) + + +def _parked(result: dict) -> list[dict]: + out: list[dict] = [] + for n in result["nodes"]: + out.extend((n.get("metadata") or {}).get("unresolved_calls") or []) + return out + + +def test_parked_call_site_file_is_repo_relative(tmp_path): + src = tmp_path / "appB" + src.mkdir() + (src / "Host.h").write_text( + "@interface Host : NSObject\n- (void)start;\n@end\n", encoding="utf-8") + (src / "Host.m").write_text( + '#import "Host.h"\n' + "@implementation Host\n" + "- (void)start {\n" + " Player *p = [[Player alloc] init];\n" + " [p play];\n" + "}\n@end\n", + encoding="utf-8") + result = extract(sorted(src.glob("*")), root=src, cache_root=tmp_path / "c") + + play = [e for e in _parked(result) if e.get("callee") == "play"] + assert play, "the call into the undeclared Player type should be parked" + entry = play[0] + assert entry["file"] == "Host.m" # the call-site .m, not the caller's Host.h + assert "private" not in entry["file"] and tmp_path.name not in entry["file"] diff --git a/tests/test_php_receiver_member_calls.py b/tests/test_php_receiver_member_calls.py index bf1d0a39e..2a779d1ea 100644 --- a/tests/test_php_receiver_member_calls.py +++ b/tests/test_php_receiver_member_calls.py @@ -178,7 +178,8 @@ def test_a_class_from_another_language_never_answers_a_php_receiver(tmp_path): "Greeter.java": "public class Greeter { public void greet() {} }\n", }) assert _greet_edge(calls) is None, calls - parked = [(n.get("metadata") or {}).get("unresolved_calls") for n in result["nodes"] - if "run" in str(n["label"]) and n.get("metadata")] + parked = [[{k: v for k, v in e.items() if k != "file"} for e in uc] + for n in result["nodes"] if "run" in str(n["label"]) and n.get("metadata") + for uc in [(n.get("metadata") or {}).get("unresolved_calls")] if uc] assert parked == [[{"callee": "greet", "receiver_type": "Greeter", "lang": "php", "line": "L4"}]], parked diff --git a/tests/test_ts_receiver_member_calls.py b/tests/test_ts_receiver_member_calls.py index b7a6822fb..2e8f0ee0e 100644 --- a/tests/test_ts_receiver_member_calls.py +++ b/tests/test_ts_receiver_member_calls.py @@ -310,7 +310,11 @@ def test_a_pure_esm_corpus_still_activates_the_resolver(tmp_path): def _parked(r) -> list[dict]: - return [entry for n in r["nodes"] + # `file` (the parked call-site path, #3152 issue 3) is dropped: its exact + # value depends on the root these cases extract without, and they assert the + # receiver/callee/lang/line shape, not the location. + return [{k: v for k, v in entry.items() if k != "file"} + for n in r["nodes"] if isinstance(n.get("metadata"), dict) for entry in n["metadata"].get("unresolved_calls", [])]