mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-25 08:11:54 +00:00
fix(cross-repo): point a cross-repo call edge at its call site, not the caller's header
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
847a93f3ca
commit
3281d42492
7 files changed
+112
-6
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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"]
|
||||
@@ -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
|
||||
@@ -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", [])]
|
||||
|
||||
|
||||
Reference in new issue
Block a user