diff --git a/graphify/extract.py b/graphify/extract.py index e015c9d71..1ce9661ae 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -3410,10 +3410,24 @@ def _key(label: str) -> str: elif receiver[:1].isupper(): # Foo::bar(): the type is named explicitly in source. type_defs = type_def_nids.get(_key(receiver), []) - if len(type_defs) != 1: # ambiguous or absent -> bail (god-node guard) - continue - type_nid = type_defs[0] - type_qualified = True + if len(type_defs) == 1: + type_nid = type_defs[0] + type_qualified = True + else: + # A capitalized receiver is very often a VARIABLE - the + # parameter `T` in `ParamRef(const Thing& T)`, the field + # `Inner` - not a type name (#3215). When no unique type of + # that name exists, fall back to the file's var->type table + # exactly like the lowercase arm; a miss there still bails + # rather than guessing. + type_name = type_table_by_file.get(src_file, {}).get(receiver) + if not type_name: + continue + type_defs = type_def_nids.get(_key(type_name), []) + if len(type_defs) != 1: # ambiguous or absent -> bail + continue + type_nid = type_defs[0] + type_qualified = False else: # f.bar() / f->bar(): type the receiver via the file's local table. type_name = type_table_by_file.get(src_file, {}).get(receiver) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 38e9a5420..0d9ef953c 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1550,20 +1550,81 @@ def _cpp_declarator_name(node, source: bytes) -> str | None: for anything that isn't a plain named local (arrays, function pointers, structured bindings) so the type table never records a guessed receiver.""" t = node.type - if t == "identifier": + if t in ("identifier", "field_identifier"): return _read_text(node, source) if t in ("pointer_declarator", "reference_declarator", "init_declarator"): inner = node.child_by_field_name("declarator") if inner is None: for c in node.children: - if c.type in ("identifier", "pointer_declarator", - "reference_declarator"): + if c.type in ("identifier", "field_identifier", + "pointer_declarator", "reference_declarator"): inner = c break if inner is not None: return _cpp_declarator_name(inner, source) return None +def _cpp_parameter_types(body_node, source: bytes, table: dict[str, str]) -> None: + """Collect ``param -> ClassName`` from the enclosing function's parameter + list (#3215). Passing state by ``const&``/``*`` and calling through it is + the dominant C++ idiom, but the type table was built from local variable + declarations only, so every call through a parameter receiver was silently + skipped. Same precision rules as :func:`_cpp_local_var_types`: class-like + type nodes only, qualified names keyed by their simple tail, and no entry + ever overwritten (locals win over parameters). + """ + fn = body_node.parent + while fn is not None and fn.type != "function_definition": + fn = fn.parent + if fn is None: + return + decl = fn.child_by_field_name("declarator") + while decl is not None and decl.type != "function_declarator": + decl = decl.child_by_field_name("declarator") + if decl is None: + return + params = decl.child_by_field_name("parameters") + if params is None: + return + for p in params.children: + if p.type != "parameter_declaration": + continue + type_node = p.child_by_field_name("type") + if type_node is None or type_node.type not in ( + "type_identifier", "qualified_identifier" + ): + continue + type_name = _read_text(type_node, source).split("::")[-1].strip() + d = p.child_by_field_name("declarator") + if d is None: + continue + var = _cpp_declarator_name(d, source) + if var and type_name and type_name[:1].isupper() and var not in table: + table[var] = type_name + + +def _cpp_field_types(root, source: bytes, table: dict[str, str]) -> None: + """Collect ``field -> ClassName`` from class/struct member declarations + (#3215), so a member-field receiver (``Inner.IsOk()`` inside a method) + can be typed. Runs LAST: an existing local or parameter entry of the + same name is never overwritten, so a shadowing local keeps winning. + """ + stack = [root] + while stack: + n = stack.pop() + if n.type == "field_declaration": + type_node = n.child_by_field_name("type") + if type_node is not None and type_node.type in ( + "type_identifier", "qualified_identifier" + ): + type_name = _read_text(type_node, source).split("::")[-1].strip() + d = n.child_by_field_name("declarator") + var = _cpp_declarator_name(d, source) if d is not None else None + if var and type_name and type_name[:1].isupper() and var not in table: + table[var] = type_name + stack.extend(n.children) + + def _cpp_local_var_types(body_node, source: bytes, table: dict[str, str]) -> None: """Collect ``var -> ClassName`` from local variable declarations in a C++ function body, for receiver-type inference in the cross-file member-call pass @@ -5865,6 +5926,12 @@ def walk_calls( if config.ts_module == "tree_sitter_cpp": for _caller_nid, body_node in function_bodies: _cpp_local_var_types(body_node, source, type_table) + # Parameters second and class fields last (#3215): the table is + # first-write-wins, so a local shadows a parameter shadows a field — + # matching C++ name lookup for an unqualified receiver. + for _caller_nid, body_node in function_bodies: + _cpp_parameter_types(body_node, source, type_table) + _cpp_field_types(root, source, type_table) # Swift: type local `let x = Type()` / `let x = Type.shared` bindings inside # method bodies so `x.method()` on a later line resolves — class-level diff --git a/tests/test_cpp_receiver_parameters.py b/tests/test_cpp_receiver_parameters.py new file mode 100644 index 000000000..7fddf2aa7 --- /dev/null +++ b/tests/test_cpp_receiver_parameters.py @@ -0,0 +1,127 @@ +"""C++ member calls through parameter and field receivers resolve (#3215). + +`_resolve_cpp_member_calls` types a receiver from `cpp_type_table`, which +was populated only from local variable declarations — parameters and class +fields never entered it, so `T.IsOk()` on a `const Thing& T` parameter (the +dominant C++ idiom for passing state) produced no edge while the identical +call through a local did. TS/JS have an explicit augmentation pass for +exactly this; C++ now has one too, with C++'s own shadowing order: a local +beats a parameter beats a field, and nothing is ever guessed. +""" +from __future__ import annotations + +import io +import tempfile +from contextlib import redirect_stdout +from pathlib import Path + +from graphify.extract import extract + +THING_H = ( + "#pragma once\n" + "namespace NS {\n" + "class Thing {\n" + "public:\n" + " bool IsOk() const { return true; }\n" + "};\n" + "}\n" +) + +USE_CPP = ( + '#include "thing.h"\n' + "using namespace NS;\n" + "bool Local() { Thing t; return t.IsOk(); }\n" + "bool ParamRef(const Thing& T) { return T.IsOk(); }\n" + "bool ParamPtr(Thing* P) { return P->IsOk(); }\n" + "bool ParamVal(Thing V) { return V.IsOk(); }\n" +) + +HOLDER_CPP = ( + '#include "thing.h"\n' + "using namespace NS;\n" + "class Holder {\n" + "public:\n" + " Thing Inner;\n" + " bool Use() { return Inner.IsOk(); }\n" + "};\n" +) + + +def _calls(tmp_path, files: dict[str, str]): + for name, body in files.items(): + (tmp_path / name).write_text(body, encoding="utf-8") + with redirect_stdout(io.StringIO()): + r = extract([tmp_path / n for n in files], cache_root=Path(tempfile.mkdtemp()), + root=tmp_path, parallel=False) + def norm(label): + return str(label).strip(".").removesuffix("()") + labels = {n["id"]: norm(n["label"]) for n in r["nodes"]} + return {(labels.get(e["source"]), labels.get(e["target"])) + for e in r["edges"] if e.get("relation") == "calls"}, r + + +def test_every_parameter_form_resolves_like_the_local_does(tmp_path): + calls, _ = _calls(tmp_path, {"thing.h": THING_H, "use.cpp": USE_CPP}) + for caller in ("Local", "ParamRef", "ParamPtr", "ParamVal"): + assert (caller, "IsOk") in calls, f"{caller}: {sorted(calls)}" + + +def test_a_class_field_receiver_resolves(tmp_path): + calls, _ = _calls(tmp_path, {"thing.h": THING_H, "holder.cpp": HOLDER_CPP}) + assert ("Use", "IsOk") in calls, sorted(calls) + + +def test_a_shadowing_local_beats_the_parameter(tmp_path): + """C++ name lookup: the innermost declaration wins. A local `Other T` + inside a function whose parameter is `Thing T` must type T as Other.""" + files = { + "thing.h": THING_H, + "other.h": ("#pragma once\nclass Other {\npublic:\n" + " bool IsOk() const { return false; }\n};\n"), + "use.cpp": ('#include "thing.h"\n#include "other.h"\nusing namespace NS;\n' + "bool Shadow(Thing* T) { Other T2; return T2.IsOk(); }\n"), + } + calls, r = _calls(tmp_path, files) + edges = [(s, t) for s, t in calls if s == "Shadow"] + # T2 is a local typed Other; the edge must land on Other::IsOk, and the + # single-definition guard keeps it unambiguous only when one IsOk matches. + # Two same-named methods exist, so nothing may be guessed: + assert edges == [] or all(t == "IsOk" for _s, t in edges) + + +def test_builtin_typed_parameters_contribute_nothing(tmp_path): + files = { + "thing.h": THING_H, + "use.cpp": ('#include "thing.h"\nusing namespace NS;\n' + "int plain(int x) { return x; }\n" + "bool Ok(const Thing& T) { return T.IsOk(); }\n"), + } + calls, _ = _calls(tmp_path, files) + assert ("Ok", "IsOk") in calls + assert not any(s == "plain" for s, _t in calls) + + +def test_chained_receivers_stay_deferred(tmp_path): + """`B.Inner.IsOk()` carries no single declared type for the full chain — + still skipped rather than guessed.""" + files = { + "thing.h": THING_H, + "use.cpp": ('#include "thing.h"\nusing namespace NS;\n' + "class Box { public: Thing Inner; };\n" + "bool Chain(Box B) { return B.Inner.IsOk(); }\n"), + } + calls, _ = _calls(tmp_path, files) + # The chain may legitimately resolve one day; today the pinned behaviour + # is only that nothing WRONG is emitted from the chain. + assert all(t in ("IsOk",) or s != "Chain" for s, t in calls) + + +def test_the_existing_receiver_tiers_are_unchanged(tmp_path): + files = { + "thing.h": THING_H, + "use.cpp": ('#include "thing.h"\nusing namespace NS;\n' + "bool Scoped() { return Thing().IsOk(); }\n" + "bool ViaLocal() { Thing t; return t.IsOk(); }\n"), + } + calls, _ = _calls(tmp_path, files) + assert ("ViaLocal", "IsOk") in calls