fix(extract): merge header/impl class fragmentation + C++/ObjC header routing (#1556, #1547)

A class declared in a header (Foo.h/@interface) and defined in its impl
(Foo.cpp/Foo.m/@implementation) fragmented into two nodes: _file_stem
drops the extension so Foo.h and Foo.cpp share a node id, which
_disambiguate_colliding_node_ids then split apart by path — and the two
"defs" tripped every resolver's single-definition god-node guard,
cascading into missing .h<->.m/.cpp linkage and cross-file/cross-language
edges.

- Routing: a `.h` using `#import` now routes to extract_objc (#1556 bridging
  headers — extract_c drops `#import` as a preproc_call), and a `.h` with
  C++-only signals (class/namespace/template/::/access-specifiers) routes
  to extract_cpp (#1547 — the C grammar has no class_specifier, so a C++
  header previously yielded a junk node and lost every method). ObjC sniff
  keeps priority; a plain C header still routes to extract_c.
- Merge: a new _merge_decl_def_classes post-pass collapses the header/impl
  id-collision onto the header (declaration) variant, modeled on
  _merge_swift_extensions, gated so it fires ONLY for a clean sibling
  header/impl pair (same dir, same base stem, exactly one header) — two
  same-named classes in different directories have different stems and
  never collide, so they are never merged (god-node guard verified). C++
  method definitions retain their `Foo::` qualifier so a `Foo::bar` def
  keys onto the header declaration (one method node, not two); free
  functions keep their bare-name ids.

Result: one canonical class node per .h/.m or .h/.cpp pair with methods
unified, which unblocks the existing member-call resolvers (verified
Swift->ObjC calls and Swift `extension` folding now resolve). Strict
improvement over v8 (which produced junk/fragmented nodes here, verified).
Still open as follow-ups: cross-file C++ #include edge resolution and a
C++/ObjC cross-file member-call resolver (a pre-existing gap, not a
regression).

Reported by @JabberYQ (#1556) and @c0dezer019 (#1547).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
safishamsi
2026-06-30 17:23:46 +01:00
co-authored by Claude Opus 4.8
parent bee3849810
commit 3bc3feed54
16 changed files with 436 additions and 6 deletions
+201 -6
View File
@@ -1846,9 +1846,15 @@ def _get_cpp_func_name(node, source: bytes) -> str | None:
if node.type in ("field_identifier", "destructor_name", "operator_name"):
return _read_text(node, source)
if node.type == "qualified_identifier":
name_node = node.child_by_field_name("name")
if name_node:
return _read_text(name_node, source)
# An out-of-class DEFINITION (`void Foo::bar() {}`) carries a
# qualified_identifier declarator. Retaining the `Foo::` qualifier makes
# _make_id(stem, "Foo::bar") normalize to the same id as the in-class
# member _make_id(class_nid, "bar"), so the decl in Foo.h and the def in
# Foo.cpp resolve to ONE method node instead of two (#1547). The full
# qualified text also handles nested scopes (`A::B::bar`). Free functions
# never have a qualified_identifier here, so their bare-name ids are
# unchanged; only qualified definitions shift onto their owning class.
return _read_text(node, source)
decl = node.child_by_field_name("declarator")
if decl:
return _get_cpp_func_name(decl, source)
@@ -9345,6 +9351,144 @@ def _resolve_cross_file_imports(
return new_edges
# Header / implementation file-extension pairing for the decl/def class merge.
_DECLDEF_HEADER_SUFFIXES = frozenset({".h", ".hpp", ".hh", ".hxx"})
_DECLDEF_IMPL_SUFFIXES = frozenset({".m", ".mm", ".cpp", ".cc", ".cxx", ".c"})
def _decldef_class_stem(source_file: str) -> tuple[str, str] | None:
"""Return ``(dir, base_stem)`` for a header/impl source file, else None.
The base stem strips an ObjC category suffix (``Foo+Cat.m`` -> ``Foo``) so a
category implementation pairs with its ``Foo.h`` declaration. Files with an
extension that is neither a header nor an impl extension return None and are
never considered for the merge.
"""
if not source_file:
return None
p = Path(source_file)
suffix = p.suffix.lower()
if suffix not in _DECLDEF_HEADER_SUFFIXES and suffix not in _DECLDEF_IMPL_SUFFIXES:
return None
stem = p.stem.split("+", 1)[0] # ObjC category: Foo+Cat -> Foo
if not stem:
return None
return (str(p.parent), stem)
def _merge_decl_def_classes(
all_nodes: list[dict],
all_edges: list[dict],
) -> None:
"""Merge a class (and its methods) declared in a header with its definition in
a sibling impl file into ONE node, for C/C++/ObjC (#1547, #1556).
A class declared in ``Foo.h`` (``class Foo`` / ``@interface Foo``) and defined
in the sibling ``Foo.cpp`` / ``Foo.m`` (``@implementation Foo``, plus after
the C++ qualified-name fix out-of-class method definitions ``Foo::bar``)
produces TWO nodes per symbol. Both are keyed off the file *stem*, and
``_file_stem`` drops the extension, so the header symbol and its impl
counterpart get the IDENTICAL id and differ only in ``source_file`` and label
(the C++ def label is ``Foo::bar()`` vs the decl's ``bar``; the ObjC impl class
label equals the interface's). Left alone, ``_disambiguate_colliding_node_ids``
SPLITS those id-collisions apart by path, fragmenting one class into two def
nodes which then trips every resolver's single-definition god-node guard
(``len(defs) != 1`` -> bail), cascading into lost .h<->.m/.cpp linkage and dead
cross-file calls.
This pass runs BEFORE disambiguation and collapses each such id-collision to
ONE node the header (declaration) variant, consistent with the #1475
header_remaps direction so disambiguation sees a single source_file per id
and leaves it alone, and the downstream resolvers see ONE definition. Because
the colliding nodes already share an id, no edge re-pointing is needed: every
edge that referenced the impl symbol already points at the surviving id. We
only drop the redundant duplicate node and prefer the header's label.
GOD-NODE GUARDS (false merges are the main risk):
* Collapse fires ONLY when every node in an id-collision group comes from a
SIBLING header/impl set same directory, same base stem (ObjC categories
``Foo+Cat.m`` compare by the stem before ``+``), header extension paired
with impl extension AND the group contains exactly ONE header file.
* Two unrelated ``class Logger`` in DIFFERENT directories never collide on id
(the id embeds the full file stem / directory path), so they are never
grouped and never merge. Two same-named classes in the SAME directory but
different base stems likewise key to different ids. Any id-collision that
is NOT a clean single-header sibling set is left untouched for
disambiguation to split (the conservative default).
The class and its method/field members fold in together: members are keyed
``_make_id(class_id, name)`` (ObjC) or, for an out-of-class C++ definition,
``_make_id(stem, "Foo::bar")`` which normalizes to the same id as the in-class
member ``_make_id(class_id, "bar")``. So every decl/def member pair is itself an
id-collision across the same sibling file set and collapses by the same rule.
"""
# Group every code node by id, recording the distinct source files involved.
by_id: dict[str, list[dict]] = {}
for n in all_nodes:
if n.get("file_type") != "code":
continue
nid = n.get("id")
sf = str(n.get("source_file", ""))
if not isinstance(nid, str) or not nid or not sf:
continue
by_id.setdefault(nid, []).append(n)
# Identify, per surviving id, which node to keep (header preferred). We can't
# mutate all_nodes mid-scan, so collect a set of node object ids to drop.
drop_objs: set[int] = set()
for nid, group in by_id.items():
if len(group) < 2:
continue
# The distinct source files of this collision must form a clean sibling
# header/impl set with exactly one header. Each file must parse as a
# header/impl file (others -> bail), share one directory + base stem.
sibling_keys: set[tuple[str, str]] = set()
headers: list[dict] = []
ok = True
for node in group:
sf = str(node.get("source_file", ""))
ds = _decldef_class_stem(sf)
if ds is None:
ok = False
break
sibling_keys.add(ds)
if Path(sf).suffix.lower() in _DECLDEF_HEADER_SUFFIXES:
headers.append(node)
if not ok:
continue
# All from one (dir, base_stem) sibling family, with a UNIQUE header.
if len(sibling_keys) != 1 or len(headers) != 1:
continue
keeper = headers[0]
for node in group:
if node is not keeper:
drop_objs.add(id(node))
if not drop_objs:
return
# Drop the redundant duplicate nodes. The surviving (header) node keeps its
# own label/source_file; edges are unchanged because the id is identical. Then
# de-dup any now-identical edges (e.g. the impl file's `contains`/`method`
# edge that duplicates the header's after the collapse).
all_nodes[:] = [n for n in all_nodes if id(n) not in drop_objs]
seen_keys: set[tuple] = set()
rewritten: list[dict] = []
for e in all_edges:
src = e.get("source")
tgt = e.get("target")
if src == tgt:
continue
k = (src, tgt, e.get("relation"), e.get("context"))
if k in seen_keys:
continue
seen_keys.add(k)
rewritten.append(e)
all_edges[:] = rewritten
def _merge_swift_extensions(
per_file: list[dict],
all_nodes: list[dict],
@@ -13486,7 +13630,14 @@ _DISPATCH: dict[str, Any] = {
# belongs to extract_objc, not extract_c). `@property` is deliberately excluded: it
# doubles as a Doxygen comment command and ObjC properties only ever live inside an
# @interface/@protocol anyway, so the stronger directives already cover them.
_OBJC_HEADER_MARKERS = (b"@interface", b"@protocol", b"@implementation", b"@import")
#
# `#import` is included because an ObjC *bridging* header is often nothing but
# `#import "X.h"` lines with no @interface (#1556). Routed to extract_c it parses
# `#import` as a `preproc_call` (not `preproc_include`), so every import edge is
# dropped and the header is isolated. `#import` is an ObjC-only directive (illegal
# in C and C++), so this won't hijack genuine C/C++ headers, and extract_objc
# resolves quoted imports via _resolve_c_include_path.
_OBJC_HEADER_MARKERS = (b"@interface", b"@protocol", b"@implementation", b"@import", b"#import")
def _is_objc_header(path: Path) -> bool:
@@ -13504,6 +13655,35 @@ def _is_objc_header(path: Path) -> bool:
return any(marker in head for marker in _OBJC_HEADER_MARKERS)
# C++-only signals. None of these are valid in a plain C header, so finding one
# in a `.h` is a high-confidence signal the header is C++ (#1547). The C grammar
# has no class_specifier, so a `class Foo { ... };` header routed to extract_c
# loses the class and its method prototypes (a junk `foo_foo` node + a sourceless
# `class` stub); routing to extract_cpp recovers the real type. Kept CONSERVATIVE:
# a plain C header with none of these stays on extract_c. ObjC sniffing keeps
# priority (an ObjC header can legitimately contain `::`/`class` inside an inline
# C++ block when compiled as Objective-C++).
_CPP_HEADER_MARKERS = (
b"class ", b"namespace ", b"template", b"::",
b"public:", b"private:", b"protected:",
)
def _is_cpp_header(path: Path) -> bool:
"""Whether a `.h` file is C++ rather than plain C (#1547).
Mirrors `_is_objc_header`: sniffs for a C++-only token. Used only to reroute
a `.h` from extract_c to extract_cpp when no ObjC marker is present (ObjC has
priority). Conservative by construction a plain C header matches nothing
here and keeps its existing extract_c routing.
"""
try:
head = path.read_bytes()[:256 * 1024]
except OSError:
return False
return any(marker in head for marker in _CPP_HEADER_MARKERS)
def _get_extractor(path: Path) -> Any | None:
"""Return the correct extractor function for a file, or None if unsupported."""
if path.name.endswith(".blade.php"):
@@ -13520,8 +13700,15 @@ def _get_extractor(path: Path) -> Any | None:
return extract_package_manifest
# `.h` is C/C++/ObjC-ambiguous; route Objective-C headers to extract_objc
# (the suffix map sends `.h` to extract_c, which can't read @interface etc.).
if path.suffix == ".h" and _is_objc_header(path):
return extract_objc
# ObjC sniffing has priority over the C++ sniff: an Objective-C++ header can
# contain both `@interface` and inline C++ (`::`), and it must parse as ObjC.
if path.suffix == ".h":
if _is_objc_header(path):
return extract_objc
# A C++ class header routed to extract_c loses the class entirely (the C
# grammar has no class_specifier). Reroute to extract_cpp (#1547).
if _is_cpp_header(path):
return extract_cpp
return _DISPATCH.get(path.suffix)
@@ -13794,6 +13981,14 @@ def extract(
_augment_symbol_resolution_edges(paths, all_nodes, all_edges, root)
# Merge a header-declared class (and its methods) with its sibling-impl
# definition into ONE node (C/C++/ObjC #1547/#1556). Runs BEFORE the id-remap
# below: a header symbol and its impl counterpart share an id only while both
# still carry the raw file-stem prefix; the per-file prefix remap then diverges
# them (foo_h vs foo_cpp), so the collapse must happen first. Collapsing here
# also means disambiguation sees one source_file per id and won't split them.
_merge_decl_def_classes(all_nodes, all_edges)
# Remap file node IDs from absolute-path-derived to the canonical
# {parent_dir}_{stem} spec form so (a) graph.json edge endpoints are stable
# across machines (#502) and (b) AST file nodes match the IDs semantic
+3
View File
@@ -0,0 +1,3 @@
#include "Logger.h"
void Logger::log() {}
+4
View File
@@ -0,0 +1,4 @@
class Logger {
public:
void log();
};
+3
View File
@@ -0,0 +1,3 @@
#include "Logger.h"
void Logger::log() {}
+4
View File
@@ -0,0 +1,4 @@
class Logger {
public:
void log();
};
+5
View File
@@ -0,0 +1,5 @@
#include "Foo.h"
void Foo::bar() {
value = 1;
}
+10
View File
@@ -0,0 +1,10 @@
#ifndef FOO_H
#define FOO_H
class Foo {
public:
void bar();
int value;
};
#endif
+7
View File
@@ -0,0 +1,7 @@
#include "Foo.h"
int main() {
Foo f;
f.bar();
return 0;
}
+4
View File
@@ -0,0 +1,4 @@
class Dup {
public:
void a();
};
+4
View File
@@ -0,0 +1,4 @@
class Dup {
public:
void b();
};
+7
View File
@@ -0,0 +1,7 @@
#ifndef PLAIN_H
#define PLAIN_H
int add(int a, int b);
struct Point { int x; int y; };
#endif
+1
View File
@@ -0,0 +1 @@
#import "Widget.h"
+4
View File
@@ -0,0 +1,4 @@
@interface Widget
- (void)render;
- (void)refresh;
@end
+9
View File
@@ -0,0 +1,9 @@
#import "Widget.h"
@implementation Widget
- (void)render {
[self refresh];
}
- (void)refresh {
}
@end
+5
View File
@@ -0,0 +1,5 @@
extension Widget {
func describe() -> String {
return "widget"
}
}
+165
View File
@@ -2582,3 +2582,168 @@ def test_systemverilog_no_dangling_edges():
for e in r["edges"]:
assert e["source"] in node_ids, f"dangling source: {e}"
assert e["target"] in node_ids, f"dangling target: {e}"
# ── Header/impl class merge + .h routing (#1547 C++, #1556 ObjC/Swift) ─────────
from graphify.extract import (
extract as _extract_corpus,
_get_extractor,
_is_cpp_header,
_is_objc_header,
)
def _corpus(*relpaths):
"""Run the full extract() pipeline on fixture files (absolute, resolved
paths so the per-file id-remap behaves like real usage), no shared cache."""
import tempfile
paths = [(FIXTURES / rp).resolve() for rp in relpaths]
with tempfile.TemporaryDirectory() as td:
return _extract_corpus(paths, cache_root=Path(td))
def _nodes_with_label(r, label):
return [n for n in r["nodes"] if n["label"] == label]
def _assert_no_dangling(r):
ids = {n["id"] for n in r["nodes"]}
for e in r["edges"]:
assert e["source"] in ids, f"dangling source: {e}"
assert e["target"] in ids, f"dangling target: {e}"
# --- #1547: C++ paired header/impl --------------------------------------------
def test_cpp_header_routes_to_cpp_extractor():
"""A `.h` with a C++ class must route to extract_cpp, not extract_c (which has
no class_specifier and would drop the class entirely)."""
p = (FIXTURES / "cpp_paired" / "Foo.h").resolve()
assert _get_extractor(p).__name__ == "extract_cpp"
assert _is_cpp_header(p)
def test_plain_c_header_stays_on_c_extractor():
"""A plain C header (no C++ signal) must keep its extract_c routing."""
p = (FIXTURES / "cpp_samedir" / "plain.h").resolve()
assert not _is_cpp_header(p)
assert _get_extractor(p).__name__ == "extract_c"
def test_cpp_paired_single_class_node():
"""Foo.h (class) + Foo.cpp (Foo::bar def) + Main.cpp must yield exactly ONE
Foo class node — not a foo_h + foo_cpp pair, and no junk `class` stub."""
r = _corpus("cpp_paired/Foo.h", "cpp_paired/Foo.cpp", "cpp_paired/Main.cpp")
foos = _nodes_with_label(r, "Foo")
assert len(foos) == 1, f"expected one Foo, got {[n['id'] for n in foos]}"
assert not _nodes_with_label(r, "class"), "no sourceless `class` stub should exist"
assert not _nodes_with_label(r, "foo_foo")
def test_cpp_paired_method_decl_and_def_are_one_node():
"""`void bar();` in Foo.h and `void Foo::bar() {}` in Foo.cpp must collapse to
ONE method node owned by the single Foo class."""
r = _corpus("cpp_paired/Foo.h", "cpp_paired/Foo.cpp", "cpp_paired/Main.cpp")
foo = _nodes_with_label(r, "Foo")[0]["id"]
method_targets = {
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()")]
# 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()")}
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"
def test_cpp_paired_includes_resolve_to_real_header():
"""Foo.cpp and Main.cpp `#include "Foo.h"` must resolve to the real Foo.h file
node (no dangling import)."""
r = _corpus("cpp_paired/Foo.h", "cpp_paired/Foo.cpp", "cpp_paired/Main.cpp")
ids = {n["id"] for n in r["nodes"]}
foo_h = _nodes_with_label(r, "Foo.h")[0]["id"]
imports = [e for e in r["edges"] if e["relation"] == "imports"]
assert len(imports) >= 2
for e in imports:
assert e["target"] in ids, f"dangling import target: {e}"
assert any(e["target"] == foo_h for e in imports), "includes should target Foo.h"
def test_cpp_paired_no_dangling_edges():
r = _corpus("cpp_paired/Foo.h", "cpp_paired/Foo.cpp", "cpp_paired/Main.cpp")
_assert_no_dangling(r)
# --- #1556: ObjC paired header/impl + bridging header -------------------------
def test_objc_header_with_import_routes_to_objc():
"""A bridging header that is only `#import "X.h"` (no @interface) must route to
extract_objc; extract_c parses `#import` as preproc_call and drops the edge."""
p = (FIXTURES / "objc_mixed" / "Bridging-Header.h").resolve()
assert _is_objc_header(p)
assert _get_extractor(p).__name__ == "extract_objc"
def test_objc_paired_single_class_methods_not_duplicated():
"""Widget.h (@interface) + Widget.m (@implementation) -> ONE Widget class node
with its methods present once each."""
r = _corpus("objc_mixed/Widget.h", "objc_mixed/Widget.m")
widgets = _nodes_with_label(r, "Widget")
assert len(widgets) == 1, f"expected one Widget, got {[n['id'] for n in widgets]}"
render = _nodes_with_label(r, "-render")
refresh = _nodes_with_label(r, "-refresh")
assert len(render) == 1, f"-render duplicated: {render}"
assert len(refresh) == 1, f"-refresh duplicated: {refresh}"
def test_objc_bridging_header_not_isolated():
"""A bridging header of only `#import "Widget.h"` must produce an imports edge
to the real Widget.h node (not be an isolated node)."""
r = _corpus("objc_mixed/Widget.h", "objc_mixed/Widget.m", "objc_mixed/Bridging-Header.h")
bridge = _nodes_with_label(r, "Bridging-Header.h")[0]["id"]
widget_h = _nodes_with_label(r, "Widget.h")[0]["id"]
out = [e for e in r["edges"] if e["source"] == bridge and e["relation"] == "imports"]
assert out, "bridging header should emit an imports edge"
assert any(e["target"] == widget_h for e in out), "bridging import should target Widget.h"
def test_objc_paired_no_dangling_edges():
r = _corpus("objc_mixed/Widget.h", "objc_mixed/Widget.m", "objc_mixed/Bridging-Header.h")
_assert_no_dangling(r)
# --- #1556: Swift extension folds onto canonical ObjC class -------------------
def test_swift_extension_folds_onto_objc_class():
"""`extension Widget` in Swift over an ObjC `Widget` must fold onto the single
canonical Widget node, with its members anchored there."""
r = _corpus("objc_mixed/Widget.h", "objc_mixed/Widget.m", "objc_mixed/WidgetExtras.swift")
widgets = _nodes_with_label(r, "Widget")
assert len(widgets) == 1, f"expected one Widget, got {[n['id'] for n in widgets]}"
wid = widgets[0]["id"]
method_targets = {e["target"] for e in r["edges"] if e["relation"] == "method" and e["source"] == wid}
labels = {n["label"] for n in r["nodes"] if n["id"] in method_targets}
assert any("describe" in l for l in labels), f"Swift extension method should anchor on Widget, got {labels}"
_assert_no_dangling(r)
# --- god-node guard negatives -------------------------------------------------
def test_decldef_merge_does_not_merge_across_directories():
"""Two unrelated `class Logger` in DIFFERENT directories (each its own .h/.cpp)
must NOT merge — assert TWO distinct Logger nodes."""
r = _corpus(
"cpp_logger/a/Logger.h", "cpp_logger/a/Logger.cpp",
"cpp_logger/b/Logger.h", "cpp_logger/b/Logger.cpp",
)
loggers = _nodes_with_label(r, "Logger")
assert len(loggers) == 2, f"cross-dir Loggers must stay distinct, got {[n['id'] for n in loggers]}"
assert len({n["id"] for n in loggers}) == 2
def test_decldef_merge_does_not_merge_same_name_same_dir_distinct_files():
"""Two same-named `class Dup` in the SAME dir but different base stems
(Alpha.h, Beta.h) must stay distinct (no unique header/impl sibling pair)."""
r = _corpus("cpp_samedir/Alpha.h", "cpp_samedir/Beta.h")
dups = _nodes_with_label(r, "Dup")
assert len(dups) == 2, f"same-dir distinct Dups must stay distinct, got {[n['id'] for n in dups]}"