diff --git a/graphify/cross_repo_calls.py b/graphify/cross_repo_calls.py index 63a319e47..9ca92d9cf 100644 --- a/graphify/cross_repo_calls.py +++ b/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: diff --git a/graphify/extract.py b/graphify/extract.py index 7cc9e62c9..fb45633ec 100644 --- a/graphify/extract.py +++ b/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 diff --git a/graphify/extractors/objc.py b/graphify/extractors/objc.py index 8b2820f46..5c9568f16 100644 --- a/graphify/extractors/objc.py +++ b/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 `. 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)) diff --git a/tests/test_cross_repo_member_calls.py b/tests/test_cross_repo_member_calls.py index 3c1672464..108e1bbdf 100644 --- a/tests/test_cross_repo_member_calls.py +++ b/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 ``, + # which no parked receiver type ever spells. + G = _graph( + caller=_caller("a", PARKED_OBJC, source_file="src/App.m"), + declarations=[(_declaration("b", "", "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" diff --git a/tests/test_objc_callable_markers.py b/tests/test_objc_callable_markers.py new file mode 100644 index 000000000..eff87efbc --- /dev/null +++ b/tests/test_objc_callable_markers.py @@ -0,0 +1,106 @@ +"""ObjC declarations carry the `_callable` / `_callable_class` markers. + +Every other extractor stamps its definitions with `_callable` — "a real callable, +not a same-named data symbol" (#2438) — and narrows a type to `_callable_class`, +callable only through a constructor (#2137). The generic engine does it for all of +them in one place (`extractors/engine.py`, `callable_def_nids` / +`callable_class_nids`). + +The ObjC extractor builds its nodes by hand and set neither, so an ObjC class was +invisible to every pass that indexes declarations by those markers, and an ObjC +method could never be told apart from a data symbol of the same name. This pins the +markers onto the four node kinds ObjC produces, including the two that must stay +unmarked. +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract + +GREETER_H = ( + "@interface Greeter : NSObject\n" + "- (void)greet;\n" + "+ (instancetype)shared;\n" + "@end\n" +) +GREETER_M = ( + "#import \"Greeter.h\"\n" + "@implementation Greeter\n" + "- (void)greet {}\n" + "@end\n" +) + + +def _extract(tmp_path: Path, files: dict[str, str]) -> dict: + paths = [] + for name, body in files.items(): + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + paths.append(path) + return extract(paths, cache_root=tmp_path / "graphify-out") + + +def _node(result: dict, label: str) -> dict: + matches = [n for n in result["nodes"] if n.get("label") == label] + assert len(matches) == 1, [n.get("label") for n in result["nodes"]] + return matches[0] + + +def test_a_class_is_marked_as_a_type(tmp_path: Path): + result = _extract(tmp_path, {"Greeter.h": GREETER_H}) + greeter = _node(result, "Greeter") + assert greeter.get("_callable") is True + assert greeter.get("_callable_class") is True + + +def test_an_implementation_only_class_is_marked_too(tmp_path: Path): + # A class whose `@interface` is not in this corpus is still a declaration. + result = _extract(tmp_path, {"Greeter.m": "@implementation Greeter\n- (void)greet {}\n@end\n"}) + greeter = _node(result, "Greeter") + assert greeter.get("_callable") is True + assert greeter.get("_callable_class") is True + + +def test_a_header_and_impl_pair_keeps_the_markers_after_folding(tmp_path: Path): + # `_merge_decl_def_classes` folds the `.h`/`.m` pair into one node; the markers + # have to be on whichever node survives. + result = _extract(tmp_path, {"Greeter.h": GREETER_H, "Greeter.m": GREETER_M}) + greeter = _node(result, "Greeter") + assert greeter.get("_callable") is True + assert greeter.get("_callable_class") is True + + +def test_a_method_is_callable_but_is_not_a_type(tmp_path: Path): + result = _extract(tmp_path, {"Greeter.h": GREETER_H}) + for label in ("-greet", "+shared"): + method = _node(result, label) + assert method.get("_callable") is True, label + assert "_callable_class" not in method, label + + +def test_a_protocol_is_marked_like_any_other_interface(tmp_path: Path): + # A Java or C# interface gets `_callable_class` from the generic engine, and a + # protocol is the same kind of declaration. + result = _extract(tmp_path, {"Greeting.h": "@protocol Greeting\n- (void)greet;\n@end\n"}) + protocol = _node(result, "") + assert protocol.get("_callable") is True + assert protocol.get("_callable_class") is True + + +def test_a_dangling_reference_is_not_marked(tmp_path: Path): + # `NSObject` is a stub minted for a name this corpus never declares, so it has + # no source file and no declaration behind it. + result = _extract(tmp_path, {"Greeter.h": GREETER_H}) + stub = _node(result, "NSObject") + assert not stub.get("source_file") + assert "_callable" not in stub + assert "_callable_class" not in stub + + +def test_the_file_node_is_not_marked(tmp_path: Path): + result = _extract(tmp_path, {"Greeter.h": GREETER_H}) + file_node = _node(result, "Greeter.h") + assert "_callable" not in file_node + assert "_callable_class" not in file_node