From 9e291b2c50e4d495add3dae40d71d71cee9c501e Mon Sep 17 00:00:00 2001 From: rajatnagda45 Date: Thu, 1 Oct 2026 10:16:52 +0530 Subject: [PATCH] fix(cpp): treat a member-function declaration as a method, not a field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inside a class body a `virtual double area() = 0;` (pure virtual), a `virtual void draw();` and a plain prototype `int run();` parse as a `field_declaration` whose declarator is a `function_declarator`. The field branch recognised that shape only to skip emitting a type reference — it still 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; a header-declared method and its out-of-line/overriding definition did not share one node (`area` field vs `.area()` method); and a call to such a method resolved to a field-shaped node. Fix: in the field-declaration branch, emit a function_declarator with the same shape a defined method uses — a `.name()` label, a `method` edge, and callable — so the declaration and its definition collapse to one method node (parity with the Java/TypeScript/Scala abstract-method contract). A genuine data member still gets its `defines` field edge. This corrects the label of every header-declared method, so a few existing cross-file tests that asserted the old bare `bar`/`run` field label now assert `.bar()`/`.run()`; the resolved call edges (source, target, confidence) are unchanged, and the decl/def merge still produces exactly one node carrying the definition site. Added tests/test_cpp_method_declarations.py. (cherry picked from commit 0038af9394f3b37d2b1c153d2d90ba92d9701bdd) --- graphify/extractors/engine.py | 33 ++++++-- tests/test_cpp_method_declarations.py | 102 ++++++++++++++++++++++++ tests/test_cpp_objc_cross_file_calls.py | 12 +-- tests/test_languages.py | 6 +- 4 files changed, 138 insertions(+), 15 deletions(-) create mode 100644 tests/test_cpp_method_declarations.py 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