diff --git a/graphify/cross_repo_calls.py b/graphify/cross_repo_calls.py index 1b9601128..7e17aaddb 100644 --- a/graphify/cross_repo_calls.py +++ b/graphify/cross_repo_calls.py @@ -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( diff --git a/tests/test_cross_repo_member_calls.py b/tests/test_cross_repo_member_calls.py index dc8a3564e..af7017f25 100644 --- a/tests/test_cross_repo_member_calls.py +++ b/tests/test_cross_repo_member_calls.py @@ -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