Finish Objective-C member calls across a repository boundary (#3152)

graphify/cross_repo_calls.py

@@ -42,6 +42,7 @@
     "cpp": frozenset({".cpp", ".cc", ".cxx", ".hpp", ".hh", ".hxx", ".h", ".cu", ".cuh"}),
     "csharp": frozenset({".cs"}),
     "java": frozenset({".java"}),
+    "objc": frozenset({".h", ".m", ".mm"}),
     "swift": frozenset({".swift"}),
 }
 
@@ -57,7 +58,8 @@ def _key(label: object) -> str:
     Type labels are plain (``Greeter``) while method labels carry the extractor's
     decoration (``.greet()``). Case is preserved: every language that parks calls
     here is case-sensitive, and folding case would let `greeter` answer for
-    `Greeter`.
+    `Greeter`. Objective-C's ``+``/``-`` sigil is deliberately kept, so an ObjC
+    ``-greet`` and a C++ ``.greet()`` stay distinct members of a shared header.
     """
     return str(label or "").strip().removeprefix(".").removesuffix("()")
 
@@ -148,6 +150,20 @@ def _member_relations(lang: str) -> tuple[str, ...]:
     return ("method", "defines") if lang == "cpp" else ("method",)
 
 
+def _member_keys(lang: str, callee: str) -> tuple[str, ...]:
+    """Which member-index keys a parked callee may answer to.
+
+    An ObjC selector carries no class/instance distinction, so both sigils are
+    tried; a type declaring `+greet` and `-greet` both is an ambiguity, not a hit.
+    A selector cannot itself begin with a sigil, so stripping one first keeps an
+    entry that already carries it from asking for `--greet`.
+    """
+    if lang == "objc":
+        selector = callee.lstrip("+-")
+        return (f"-{selector}", f"+{selector}")
+    return (callee,)
+
+
 def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
     """Emit `calls` edges for parked member calls another repo answers.
 
@@ -190,7 +206,9 @@ def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
                 continue
             targets: list[str] = []
             for relation in _member_relations(lang):
-                targets = members_by_relation[relation].get((candidates[0], callee), [])
+                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:

graphify/extract.py

@@ -4106,7 +4106,9 @@ def _resolve_objc_member_calls(
         captured; a dotted receiver like ``Foo.shared`` is never passed through,
         because ``_key`` would strip the dot and collide with a real ``FooShared``.
     An uninferable receiver is SKIPPED (no guess), so an ambiguous selector across
-    classes never fans out. ``_merge_decl_def_classes`` folds each @interface/@impl
+    classes never fans out. A receiver typed to a class this corpus declares nowhere
+    is parked on the caller for a merged graph to finish (#3152).
+    ``_merge_decl_def_classes`` folds each @interface/@impl
     pair into one node, so a paired class clears the single-definition guard.
     ``@protocol`` declarations are excluded from the receiver-type index: a protocol
     is a contract, not a message receiver, and ObjC keeps protocol and class names in
@@ -4205,6 +4207,16 @@ def _field_type_up_chain(cls, receiver):
             queue.extend(_objc_bases.get(c, []))
         return None
 
+    def _park_absent(caller: str, callee: str, type_name: str, rc: dict) -> None:
+        """Park a typed receiver whose class is declared nowhere here (#3152).
+
+        A builtin type (NSString, DispatchQueue, ...) is not what another repo
+        declares, so parking one would only let a same-named user class answer it.
+        """
+        if type_name in _LANGUAGE_BUILTIN_GLOBALS:
+            return
+        _park_unresolved_member_call(node_by_id.get(caller), callee, type_name, "objc", rc)
+
     for rc in all_raw_calls:
         if not rc.get("is_member_call"):
             continue
@@ -4225,7 +4237,10 @@ def _field_type_up_chain(cls, receiver):
             if not type_name:
                 continue
             type_defs = type_def_nids.get(_key(type_name), [])
-            if len(type_defs) != 1:  # ambiguous or absent -> bail (god-node guard)
+            if not type_defs:
+                _park_absent(caller, callee, type_name, rc)
+                continue
+            if len(type_defs) != 1:  # ambiguous -> bail (god-node guard)
                 continue
             type_nid = type_defs[0]
             type_qualified = False
@@ -4236,7 +4251,10 @@ def _field_type_up_chain(cls, receiver):
             type_qualified = True
         elif receiver[:1].isupper():
             type_defs = type_def_nids.get(_key(receiver), [])
-            if len(type_defs) != 1:  # ambiguous or absent -> bail (god-node guard)
+            if not type_defs:
+                _park_absent(caller, callee, receiver, rc)
+                continue
+            if len(type_defs) != 1:  # ambiguous -> bail (god-node guard)
                 continue
             type_nid = type_defs[0]
             type_qualified = True
@@ -4250,7 +4268,10 @@ def _field_type_up_chain(cls, receiver):
             if not type_name:
                 continue
             type_defs = type_def_nids.get(_key(type_name), [])
-            if len(type_defs) != 1:  # ambiguous or absent -> bail (god-node guard)
+            if not type_defs:
+                _park_absent(caller, callee, type_name, rc)
+                continue
+            if len(type_defs) != 1:  # ambiguous -> bail (god-node guard)
                 continue
             type_nid = type_defs[0]
             type_qualified = False

graphify/extractors/objc.py

@@ -12,6 +12,14 @@
 # `C++Bridge.h` and `Foo+.h` are left intact.
 _OBJC_STEM_PART = re.compile(r"[A-Za-z_][A-Za-z0-9_]*")
 
+# Declaration markers, matching what the generic engine puts on every other
+# language's definitions (#2438): `_callable` says "a real callable, not a
+# same-named data symbol", and `_callable_class` narrows that to a type, which is
+# callable only through a constructor. Passes that index declarations gate on
+# these, so a node without them is invisible to them.
+_CALLABLE = ("_callable",)
+_CALLABLE_CLASS = ("_callable", "_callable_class")
+
 
 def _objc_category_base_stem(stem: str) -> str:
     """Strip an ObjC category/extension suffix from a file stem (``Foo+Cat`` -> ``Foo``).
@@ -115,11 +123,14 @@ def extract_objc(path: Path) -> dict:
     # same (class, field) tombstones the entry (None) — drop, don't guess.
     objc_field_types: dict[str, dict[str, str | None]] = {}
 
-    def add_node(nid: str, label: str, line: int) -> None:
+    def add_node(nid: str, label: str, line: int, markers: tuple[str, ...] = ()) -> None:
         if nid not in seen_ids:
             seen_ids.add(nid)
-            nodes.append({"id": nid, "label": label, "file_type": "code",
-                          "source_file": str_path, "source_location": f"L{line}"})
+            node = {"id": nid, "label": label, "file_type": "code",
+                    "source_file": str_path, "source_location": f"L{line}"}
+            for marker in markers:
+                node[marker] = True
+            nodes.append(node)
 
     def add_edge(src: str, tgt: str, relation: str, line: int,
                  confidence: str = "EXTRACTED", weight: float = 1.0,
@@ -284,7 +295,7 @@ def walk(node, parent_nid: str | None = None) -> None:
             # produced fine when the members lived in `Foo.h` (#1556).
             cls_stem = _objc_category_base_stem(stem) if _objc_is_category(node) else stem
             cls_nid = _make_id(cls_stem, name)
-            add_node(cls_nid, name, line)
+            add_node(cls_nid, name, line, _CALLABLE_CLASS)
             add_edge(file_nid, cls_nid, "contains", line)
             # superclass is second identifier after ':'
             colon_seen = False
@@ -349,7 +360,7 @@ def walk(node, parent_nid: str | None = None) -> None:
             impl_stem = _objc_category_base_stem(stem) if _objc_is_category(node) else stem
             impl_nid = _make_id(impl_stem, name)
             if impl_nid not in seen_ids:
-                add_node(impl_nid, name, line)
+                add_node(impl_nid, name, line, _CALLABLE_CLASS)
                 add_edge(file_nid, impl_nid, "contains", line)
             for child in node.children:
                 if child.type == "instance_variables":
@@ -367,7 +378,9 @@ def walk(node, parent_nid: str | None = None) -> None:
                     break
             if name:
                 proto_nid = _make_id(stem, name)
-                add_node(proto_nid, f"<{name}>", line)
+                # A protocol is a type declaration like any other interface, and
+                # the engine marks a Java/C# interface the same way.
+                add_node(proto_nid, f"<{name}>", line, _CALLABLE_CLASS)
                 add_edge(file_nid, proto_nid, "contains", line)
                 # Adopted protocols: `@protocol Derived <Base, Other>`. These
                 # nest under a protocol_reference_list node (distinct from the
@@ -402,7 +415,7 @@ def walk(node, parent_nid: str | None = None) -> None:
             method_name = "".join(parts) if parts else None
             if method_name:
                 method_nid = _make_id(container, method_name)
-                add_node(method_nid, f"{prefix}{method_name}", line)
+                add_node(method_nid, f"{prefix}{method_name}", line, _CALLABLE)
                 add_edge(container, method_nid, "method", line)
                 if t == "method_definition":
                     method_bodies.append((method_nid, node, container))

tests/test_cross_repo_member_calls.py

@@ -6,11 +6,11 @@
 `merge-graphs` and `global add` read. The two-repo graph was missing precisely the
 edges that make it a call graph.
 
-The Java, C++, C# and Swift resolvers now park those calls on the caller node and
-this pass finishes them after the merge. The cases below pin what it must NOT do
-as much as what it must: the single-definition guard, the cross-repo-only scope,
-and the language guard are what keep it from fabricating an edge from a name
-collision.
+The Java, C++, C#, Objective-C and Swift resolvers now park those calls on the
+caller node and this pass finishes them after the merge. The cases below pin what
+it must NOT do as much as what it must: the single-definition guard, the
+cross-repo-only scope, and the language guard are what keep it from fabricating an
+edge from a name collision.
 """
 from __future__ import annotations
 
@@ -44,6 +44,7 @@ def _needs(module: str):
 needs_java = _needs("tree_sitter_java")
 needs_cpp = _needs("tree_sitter_cpp")
 needs_csharp = _needs("tree_sitter_c_sharp")
+needs_objc = _needs("tree_sitter_objc")
 needs_swift = _needs("tree_sitter_swift")
 
 
@@ -177,6 +178,79 @@ def test_a_defines_member_does_not_answer_a_java_call():
     assert link_cross_repo_member_calls(G) == 0
 
 
+PARKED_OBJC = [{"callee": "greet", "receiver_type": "Greeter", "lang": "objc", "line": "L5"}]
+
+
+def test_an_objc_selector_answers_through_either_sigil():
+    # An ObjC method label keeps its +/- sigil while a parked selector carries no
+    # class/instance distinction, so both spellings have to be tried.
+    for label in ("-greet", "+greet"):
+        G = _graph(
+            caller=_caller("a", PARKED_OBJC, source_file="src/App.m"),
+            declarations=[(_declaration("b", "Greeter", "Greeter.m"),
+                           _method("b", label, source_file="Greeter.m"))],
+        )
+        assert link_cross_repo_member_calls(G) == 1, label
+        assert _added_calls(G) == {("a::app_run", "b::greeter_greet")}
+
+
+def test_an_objc_callee_that_already_carries_a_sigil_still_binds():
+    # A parked selector is written bare, but the payload comes from `graph.json`
+    # and a sigiled one must not ask the index for `--greet`.
+    G = _graph(
+        caller=_caller("a", [dict(PARKED_OBJC[0], callee="-greet")],
+                       source_file="src/App.m"),
+        declarations=[(_declaration("b", "Greeter", "Greeter.m"),
+                       _method("b", "-greet", source_file="Greeter.m"))],
+    )
+    assert link_cross_repo_member_calls(G) == 1
+    assert _added_calls(G) == {("a::app_run", "b::greeter_greet")}
+
+
+def test_a_type_declaring_both_objc_sigils_binds_nothing():
+    # `+greet` and `-greet` on one class are two different methods and the parked
+    # selector cannot say which was meant.
+    decl = _declaration("b", "Greeter", "Greeter.m")
+    G = _graph(
+        caller=_caller("a", PARKED_OBJC, source_file="src/App.m"),
+        declarations=[(decl, _method("b", "-greet", "greeter_inst", "Greeter.m"))],
+    )
+    class_method = _method("b", "+greet", "greeter_cls", "Greeter.m")
+    G.add_node(class_method[0], **class_method[1])
+    G.add_edge(decl[0], class_method[0], relation="method")
+
+    assert link_cross_repo_member_calls(G) == 0
+
+
+def test_an_objc_call_does_not_bind_to_a_cpp_member_of_a_shared_header():
+    # `.h` belongs to both languages, so the sigil is what keeps an ObjC `-greet`
+    # and a C++ `.greet()` from answering for each other inside one header.
+    G = _graph(
+        caller=_caller("a", PARKED_OBJC, source_file="src/App.m"),
+        declarations=[(_declaration("b", "Greeter", "greeter.h"),
+                       _method("b", ".greet()", source_file="greeter.h"))],
+    )
+    assert link_cross_repo_member_calls(G) == 0
+
+    G = _graph(
+        caller=_caller("a", PARKED_CPP, source_file="src/app.cpp"),
+        declarations=[(_declaration("b", "Greeter", "greeter.h"),
+                       _method("b", "-greet", source_file="greeter.h"))],
+    )
+    assert link_cross_repo_member_calls(G) == 0
+
+
+def test_an_objc_protocol_does_not_answer_a_parked_call():
+    # A protocol carries the same markers as a class but is labelled `<Greeter>`,
+    # which no parked receiver type ever spells.
+    G = _graph(
+        caller=_caller("a", PARKED_OBJC, source_file="src/App.m"),
+        declarations=[(_declaration("b", "<Greeter>", "Greeter.h"),
+                       _method("b", "-greet", source_file="Greeter.h"))],
+    )
+    assert link_cross_repo_member_calls(G) == 0
+
+
 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
@@ -364,6 +438,22 @@ def test_a_java_build_parks_the_call_and_the_merge_finishes_it(tmp_path: Path):
         ("src/Greeter.cs", "class Greeter { public void Greet() {} }\n"),
         marks=needs_csharp, id="csharp-field-receiver",
     ),
+    pytest.param(
+        "objc", "greet",
+        ("src/App.m", "@interface App : NSObject\n"
+                      "@property (nonatomic, strong) Greeter *greeter;\n"
+                      "@end\n"
+                      "@implementation App\n"
+                      "- (void)run { [self.greeter greet]; }\n"
+                      "@end\n"),
+        ("src/Greeter.m", "@interface Greeter : NSObject\n"
+                          "- (void)greet;\n"
+                          "@end\n"
+                          "@implementation Greeter\n"
+                          "- (void)greet {}\n"
+                          "@end\n"),
+        marks=needs_objc, id="objc-property-receiver",
+    ),
     pytest.param(
         "swift", "greet",
         ("src/App.swift", "class App {\n"