diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 369415cfc..f32adc850 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -5321,18 +5321,39 @@ def _extract_generic( if target_nid != parent_class_nid: add_edge(parent_class_nid, target_nid, "references", line, context=ctx) - # Emit a node for each data member. Use children_by_field_name so we + # Emit a node for each declarator. Use children_by_field_name so we # only visit declarator children, not the type node (which would give # us the type name, not the field name). Handles int x, y; via # multiple declarator fields and static const int MAX = 100; via the # init_declarator → field_identifier recursion in _get_cpp_func_name. + # + # A declarator that is a function_declarator is a member-FUNCTION + # declaration — a prototype, a `virtual f();`, or a pure virtual + # `virtual f() = 0;` — not a data member. It was being emitted as a + # `defines` field (bare label, no `()`), so a pure-virtual interface + # class had no method nodes and an override/call had nothing to bind + # to. Emit it with the same shape the function_definition branch uses + # (a `.name()` label, a `method` edge, callable) so the declaration + # and its out-of-line/overriding definition share one node — parity + # with the Java/TS/Scala abstract-method contract. for decl in decls: name = _get_cpp_func_name(decl, source) - if name: - line = decl.start_point[0] + 1 - field_nid = _make_id(parent_class_nid, name) - add_node(field_nid, name, line) - add_edge(parent_class_nid, field_nid, "defines", line, context="field") + if not name: + continue + line = decl.start_point[0] + 1 + member_nid = _make_id(parent_class_nid, name) + decl_is_method = ( + decl.type == "function_declarator" + or (decl.type in ("pointer_declarator", "reference_declarator") + and any(c.type == "function_declarator" for c in decl.children)) + ) + if decl_is_method: + add_node(member_nid, f".{name}()", line) + add_edge(parent_class_nid, member_nid, "method", line) + callable_def_nids.add(member_nid) + else: + add_node(member_nid, name, line) + add_edge(parent_class_nid, member_nid, "defines", line, context="field") return # Ruby's `class << self` contains ordinary `method` nodes. Keep them diff --git a/tests/test_cpp_method_declarations.py b/tests/test_cpp_method_declarations.py new file mode 100644 index 000000000..818bb0fe9 --- /dev/null +++ b/tests/test_cpp_method_declarations.py @@ -0,0 +1,102 @@ +"""A C++ member-function DECLARATION is a method, not a data member. + +Inside a class body a `virtual double area() = 0;` (pure virtual), a +`virtual void draw();` and a plain prototype `int run();` all parse as a +`field_declaration` whose declarator is a `function_declarator`. The field +branch emitted a node for every declarator as a data member (a bare `area` +label with a `defines`/field edge), so: + + * a pure-virtual interface class had NO method nodes, only phantom fields; + * an out-of-line or overriding definition did not share the declaration's + node, because one was `.area()` (method) and the other `area` (field); + * a call to the method had nothing of the right shape to resolve to. + +The fix emits a function_declarator with the same shape as a defined method +(`.name()` label, `method` edge, callable), while a genuine data member still +gets its `defines` field edge. +""" +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +from graphify.extract import extract + + +def _extract(tmp_path, files: dict[str, str]) -> dict: + for name, body in files.items(): + p = tmp_path / name + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body) + old = os.getcwd() + try: + os.chdir(tmp_path) + return extract([Path(n) for n in files], cache_root=Path(tempfile.mkdtemp())) + finally: + os.chdir(old) + + +def _rel(r: dict, relation: str, src: str, tgt: str) -> bool: + labels = {n["id"]: str(n.get("label", "")) for n in r["nodes"]} + return any( + e.get("relation") == relation + and labels.get(e.get("source")) == src + and labels.get(e.get("target")) == tgt + for e in r["edges"] + ) + + +def _labels(r: dict) -> list[str]: + return [str(n.get("label", "")) for n in r["nodes"]] + + +def test_pure_virtual_method_is_a_method_node(tmp_path): + r = _extract(tmp_path, {"s.cpp": ( + "class Shape {\n" + "public:\n" + " virtual double area() = 0;\n" + "};\n" + )}) + assert _rel(r, "method", "Shape", ".area()") + # Not misclassified as a data member. + assert not _rel(r, "defines", "Shape", "area") + assert "area" not in _labels(r) + + +def test_declared_virtual_and_prototype_are_methods(tmp_path): + r = _extract(tmp_path, {"s.cpp": ( + "class Svc {\n" + "public:\n" + " virtual void draw();\n" + " int run();\n" + "};\n" + )}) + assert _rel(r, "method", "Svc", ".draw()") + assert _rel(r, "method", "Svc", ".run()") + + +def test_data_member_is_still_a_field(tmp_path): + r = _extract(tmp_path, {"s.cpp": ( + "class Box {\n" + "public:\n" + " int count;\n" + " double area() = 0;\n" + "};\n" + )}) + assert _rel(r, "defines", "Box", "count") + assert _rel(r, "method", "Box", ".area()") + + +def test_override_definition_shares_the_declaration_node(tmp_path): + # The abstract declaration and the concrete override are both `.area()`, + # so a call through the base resolves instead of hitting a phantom field. + r = _extract(tmp_path, {"s.cpp": ( + "class Shape {\n" + "public:\n" + " virtual double area() = 0;\n" + " double describe() { return area(); }\n" + "};\n" + )}) + assert _rel(r, "method", "Shape", ".area()") + assert _rel(r, "calls", ".describe()", ".area()") diff --git a/tests/test_cpp_objc_cross_file_calls.py b/tests/test_cpp_objc_cross_file_calls.py index fbcd192c5..7839e3c53 100644 --- a/tests/test_cpp_objc_cross_file_calls.py +++ b/tests/test_cpp_objc_cross_file_calls.py @@ -98,9 +98,9 @@ def test_cpp_instance_member_call_resolves(tmp_path: Path): result = extract(sorted(base.glob("*")), cache_root=tmp_path / "cache") calls = _call_edges(result) - assert ("main()", "calls", "bar", "INFERRED") in calls + assert ("main()", "calls", ".bar()", "INFERRED") in calls # Exactly one bar call edge from main (no fan-out, no duplicate). - bar_calls = [c for c in calls if c[0] == "main()" and c[2] == "bar"] + bar_calls = [c for c in calls if c[0] == "main()" and c[2] == ".bar()"] assert len(bar_calls) == 1 @@ -113,7 +113,7 @@ def test_cpp_pointer_member_call_resolves(tmp_path: Path): result = extract(sorted(base.glob("*")), cache_root=tmp_path / "cache") calls = _call_edges(result) - assert ("main()", "calls", "bar", "INFERRED") in calls + assert ("main()", "calls", ".bar()", "INFERRED") in calls def test_cpp_qualified_member_call_is_extracted(tmp_path: Path): @@ -125,7 +125,7 @@ def test_cpp_qualified_member_call_is_extracted(tmp_path: Path): result = extract(sorted(base.glob("*")), cache_root=tmp_path / "cache") calls = _call_edges(result) - assert ("main()", "calls", "bar", "EXTRACTED") in calls + assert ("main()", "calls", ".bar()", "EXTRACTED") in calls def test_cpp_qualified_call_resolves_to_a_qualified_only_definition(tmp_path: Path): @@ -218,7 +218,7 @@ def test_cpp_this_member_call_resolves_to_enclosing_class(tmp_path: Path): result = extract(sorted(base.glob("*")), cache_root=tmp_path / "cache") calls = _call_edges(result) - assert ("baz", "calls", "bar", "EXTRACTED") in calls + assert (".baz()", "calls", ".bar()", "EXTRACTED") in calls def test_cpp_godnode_guard_ambiguous_and_unknown_receiver(tmp_path: Path): @@ -238,7 +238,7 @@ def test_cpp_godnode_guard_ambiguous_and_unknown_receiver(tmp_path: Path): e for e in result["edges"] if e.get("relation") == "calls" and _label(result, e["source"]) == "main()" - and _label(result, e["target"]) == "run" + and _label(result, e["target"]) == ".run()" ] # Exactly one resolved run() call, and it targets A's run (not B's, not both). assert len(run_calls) == 1 diff --git a/tests/test_languages.py b/tests/test_languages.py index ffe59b919..d8b7ec1e0 100644 --- a/tests/test_languages.py +++ b/tests/test_languages.py @@ -4033,9 +4033,9 @@ def test_cpp_paired_method_decl_and_def_are_one_node(): e["target"] for e in r["edges"] if e["source"] == foo and e["relation"] in ("method", "defines", "contains") } - bar_nodes = [n for n in r["nodes"] if n["id"] in method_targets and n["label"] in ("bar", "Foo::bar()")] + bar_nodes = [n for n in r["nodes"] if n["id"] in method_targets and n["label"] in (".bar()", "Foo::bar()")] # There must be exactly one node representing bar (decl and def merged). - bar_ids = {n["id"] for n in r["nodes"] if n["label"] in ("bar", "Foo::bar()")} + bar_ids = {n["id"] for n in r["nodes"] if n["label"] in (".bar()", "Foo::bar()")} assert len(bar_ids) == 1, f"bar decl/def should be one node, got {bar_ids}" assert bar_nodes, "the merged bar node should be a member of Foo" @@ -4045,7 +4045,7 @@ def test_cpp_paired_merged_node_records_definition_site(): DECLARATION. The survivor must still carry where the symbol is implemented, or the definition site is lost with the dropped impl node.""" r = _corpus("cpp_paired/Foo.h", "cpp_paired/Foo.cpp", "cpp_paired/Main.cpp") - bars = [n for n in r["nodes"] if n["label"] in ("bar", "Foo::bar()")] + bars = [n for n in r["nodes"] if n["label"] in (".bar()", "Foo::bar()")] assert len(bars) == 1, f"bar decl/def should be one node, got {bars}" bar = bars[0] assert str(bar["source_file"]).endswith("Foo.h"), bar