mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-25 16:22:30 +00:00
fix(cross-repo): bind ObjC split decl/impl across repos to its definition
An ObjC class whose @interface(.h) and @implementation(.m) live in different repositories surfaces as two same-named candidate type nodes that _merge_decl_def_classes folds within a repo but cannot across the boundary (its id embeds the directory, #1556). The cross-repo member-call pass then tripped the single-definition guard (len(candidates) != 1) and dropped the call edge even though exactly one candidate DEFINES the selector. Scoped to lang == "objc": when more than one same-named candidate survives, union their members and bind the selector to the sole candidate that DEFINES it (a method node with definition_file set, or a .m/.mm source). Two candidates that both define it are two unrelated classes and stay ambiguous, so the guard still catches the collision it was meant to. Other languages, the C# resolver path, and the single-candidate path are unchanged. Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
1 parent
c588b72d93
commit
a82a375c2d
2 files changed
+105
-24
No files matched your search
@@ -14,7 +14,10 @@ The resolvers now park those calls on the caller node as
|
||||
``_park_unresolved_member_call``). This pass reads them back after the graphs are
|
||||
composed and emits the ``calls`` edge when the receiver's type resolves to
|
||||
exactly one declaration in another repo, keeping the single-definition guard the
|
||||
single-repo resolvers use: an ambiguous name still fabricates nothing.
|
||||
single-repo resolvers use: an ambiguous name still fabricates nothing. An ObjC
|
||||
class split ``@interface``(.h)/``@implementation``(.m) across repos is the one
|
||||
exception — several same-named candidates bind to the sole one that DEFINES the
|
||||
selector (#1556), two definitions still being an ambiguity.
|
||||
|
||||
Edges only, no node merging or renaming, so the pass composes with the prefixing
|
||||
and pruning already in place. Every edge it adds is tagged, and it clears its own
|
||||
@@ -59,6 +62,11 @@ _LANG_SUFFIXES: dict[str, frozenset[str]] = {
|
||||
# carries `defines` instead. `method` wins when both name the same member.
|
||||
_MEMBER_RELATIONS = ("defines", "method")
|
||||
|
||||
# An ObjC @implementation lives in a `.m`/`.mm`; a `.h` carries only the @interface
|
||||
# declaration. This is what tells the one DEFINITION apart from the declarations when
|
||||
# a class is split across repositories.
|
||||
_OBJC_IMPL_SUFFIXES = frozenset({".m", ".mm"})
|
||||
|
||||
|
||||
def _key(label: object) -> str:
|
||||
"""Normalize a node label or a parked name to its bare identifier.
|
||||
@@ -172,6 +180,34 @@ def _member_keys(lang: str, callee: str) -> tuple[str, ...]:
|
||||
return (callee,)
|
||||
|
||||
|
||||
def _objc_union_definition(
|
||||
merged: "nx.Graph",
|
||||
members: dict[tuple[str, str], list[str]],
|
||||
candidates: list[str],
|
||||
callee: str,
|
||||
) -> str | None:
|
||||
"""The sole DEFINITION of an ObjC selector across same-named candidates.
|
||||
|
||||
An ObjC class whose `@interface` (`.h`) and `@implementation` (`.m`) live in
|
||||
different repositories surfaces as several same-named candidates that
|
||||
``_merge_decl_def_classes`` folds within a repo but cannot across the boundary
|
||||
(its id embeds the directory, #1556). The candidate that DEFINES the selector —
|
||||
an impl in `.m`/`.mm`, or a folded node naming its impl in ``definition_file`` —
|
||||
is the target; a bare header declaration is not. Two candidates that both define
|
||||
it are two unrelated classes and stay ambiguous, so the single-definition guard
|
||||
still holds for the collision it was meant to catch.
|
||||
"""
|
||||
defs: list[str] = []
|
||||
for cand in candidates:
|
||||
for member_key in _member_keys("objc", callee):
|
||||
for member in members.get((cand, member_key), []):
|
||||
data = merged.nodes[member]
|
||||
if data.get("definition_file") or _suffix(data.get("source_file")) in _OBJC_IMPL_SUFFIXES:
|
||||
defs.append(member)
|
||||
defs = list(dict.fromkeys(defs))
|
||||
return defs[0] if len(defs) == 1 else None
|
||||
|
||||
|
||||
def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
|
||||
"""Emit `calls` edges for parked member calls another repo answers.
|
||||
|
||||
@@ -228,31 +264,42 @@ def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
|
||||
if merged.nodes[node].get("repo") != caller_repo
|
||||
and _suffix(merged.nodes[node].get("source_file")) in suffixes
|
||||
]
|
||||
if len(candidates) != 1:
|
||||
# The same guard the single-repo resolvers apply: two repos
|
||||
# declaring the same name is an ambiguity, not a hit.
|
||||
if not candidates:
|
||||
continue
|
||||
if lang == "csharp":
|
||||
# Reuse the extractor's authoritative C# visibility rules. A
|
||||
# candidate is valid only when the caller's namespace, using,
|
||||
# or alias resolves this exact receiver name to that node.
|
||||
resolved, _decisive = csharp_resolver.resolve_type_name(
|
||||
receiver_type,
|
||||
dict(caller_data, id=caller),
|
||||
str(caller_data.get("source_file") or ""),
|
||||
)
|
||||
if resolved != candidates[0]:
|
||||
if lang == "objc" and len(candidates) > 1:
|
||||
# A class split @interface(.h)/@implementation(.m) across repos
|
||||
# leaves several same-named candidates; bind the selector to its
|
||||
# sole definition rather than bailing on the collision guard.
|
||||
target = _objc_union_definition(
|
||||
merged, members_by_relation["method"], candidates, callee)
|
||||
if target is None:
|
||||
continue
|
||||
targets: list[str] = []
|
||||
for relation in _member_relations(lang):
|
||||
members = members_by_relation[relation]
|
||||
for member_key in _member_keys(lang, callee):
|
||||
targets.extend(members.get((candidates[0], member_key), []))
|
||||
if targets:
|
||||
break
|
||||
if len(targets) != 1:
|
||||
continue
|
||||
target = targets[0]
|
||||
else:
|
||||
if len(candidates) != 1:
|
||||
# The same guard the single-repo resolvers apply: two repos
|
||||
# declaring the same name is an ambiguity, not a hit.
|
||||
continue
|
||||
if lang == "csharp":
|
||||
# Reuse the extractor's authoritative C# visibility rules. A
|
||||
# candidate is valid only when the caller's namespace, using,
|
||||
# or alias resolves this exact receiver name to that node.
|
||||
resolved, _decisive = csharp_resolver.resolve_type_name(
|
||||
receiver_type,
|
||||
dict(caller_data, id=caller),
|
||||
str(caller_data.get("source_file") or ""),
|
||||
)
|
||||
if resolved != candidates[0]:
|
||||
continue
|
||||
targets: list[str] = []
|
||||
for relation in _member_relations(lang):
|
||||
members = members_by_relation[relation]
|
||||
for member_key in _member_keys(lang, callee):
|
||||
targets.extend(members.get((candidates[0], member_key), []))
|
||||
if targets:
|
||||
break
|
||||
if len(targets) != 1:
|
||||
continue
|
||||
target = targets[0]
|
||||
if target == caller or merged.has_edge(caller, target):
|
||||
continue
|
||||
merged.add_edge(
|
||||
|
||||
@@ -333,6 +333,40 @@ def test_an_objc_protocol_does_not_answer_a_parked_call():
|
||||
assert link_cross_repo_member_calls(G) == 0
|
||||
|
||||
|
||||
def test_an_objc_class_split_decl_impl_across_repos_binds_to_the_definition():
|
||||
# `@interface` in one repo's header, `@implementation` in another repo's `.m`:
|
||||
# `_merge_decl_def_classes` folds this within a repo but not across the boundary,
|
||||
# so both surface as `Greeter` and the collision guard would drop the call. The
|
||||
# `.m` definition is the target; the header declaration is not.
|
||||
G = _graph(
|
||||
caller=_caller("a", PARKED_OBJC, source_file="src/App.m"),
|
||||
declarations=[
|
||||
(_declaration("b", "Greeter", "include/Greeter.h"),
|
||||
_method("b", "-greet", source_file="include/Greeter.h")),
|
||||
(_declaration("c", "Greeter", "src/Greeter.m"),
|
||||
_method("c", "-greet", source_file="src/Greeter.m")),
|
||||
],
|
||||
)
|
||||
assert link_cross_repo_member_calls(G) == 1
|
||||
assert _added_calls(G) == {("a::app_run", "c::greeter_greet")}
|
||||
|
||||
|
||||
def test_two_objc_definitions_of_the_same_class_still_bind_nothing():
|
||||
# Two unrelated `Greeter` classes, each with its own `.m` definition, are a real
|
||||
# collision: the split-class union must not paper over the guard it loosened.
|
||||
G = _graph(
|
||||
caller=_caller("a", PARKED_OBJC, source_file="src/App.m"),
|
||||
declarations=[
|
||||
(_declaration("b", "Greeter", "b/Greeter.m"),
|
||||
_method("b", "-greet", source_file="b/Greeter.m")),
|
||||
(_declaration("c", "Greeter", "c/Greeter.m"),
|
||||
_method("c", "-greet", source_file="c/Greeter.m")),
|
||||
],
|
||||
)
|
||||
assert link_cross_repo_member_calls(G) == 0
|
||||
assert _added_calls(G) == set()
|
||||
|
||||
|
||||
def test_the_definition_answers_before_a_same_named_declaration():
|
||||
# A C++ class declares `void greet();` in its header (`defines`) and defines
|
||||
# it out of line in the `.cpp` (`method`). Both hang off the one folded class
|
||||
|
||||
Reference in new issue
Block a user