From f4505e59ab175bd588fd9adcb96c7083e8faab30 Mon Sep 17 00:00:00 2001 From: rohit-jsfreaky Date: Tue, 25 Aug 2026 19:24:54 +0530 Subject: [PATCH] fix(ruby): match a qualified receiver by its constant path (#3078) --- graphify/extractors/engine.py | 11 +++-- graphify/ruby_resolution.py | 30 +++++++++++- tests/test_ruby_resolution.py | 89 +++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 6 deletions(-) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 00b50c2577..dbde75f3f8 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -5282,10 +5282,13 @@ def walk_calls( if recv.type in ("identifier", "constant"): member_receiver = _read_text(recv, source) elif recv.type == "scope_resolution": - # Namespaced receiver `Billing::Processor.call` — capture the - # last constant so cross-file resolution can bind it by the - # bare class name (the god-node guard bails if ambiguous). - member_receiver = _ruby_const_last_name(recv, source) or None + # Namespaced receiver `Billing::Processor.call` — keep the whole + # constant path. Truncating to the last segment discarded the + # namespace, so `ActiveRecord::Base.transaction` bound to + # whatever single class named `Base` the corpus defined: the + # god-node guard only catches an ambiguous match, not a + # unique-but-wrong one (#3078). + member_receiver = _ruby_const_full_name(recv, source) or None else: # Generic: get callee from call_function_field func_node = node.child_by_field_name(config.call_function_field) if config.call_function_field else None diff --git a/graphify/ruby_resolution.py b/graphify/ruby_resolution.py index 33a5478b43..090cbb6409 100644 --- a/graphify/ruby_resolution.py +++ b/graphify/ruby_resolution.py @@ -119,6 +119,28 @@ def _unique_class(name: str) -> str | None: nids = class_def_nids.get(_key(name), []) return nids[0] if len(nids) == 1 else None + def _class_by_const_path(raw: str) -> str | None: + """Resolve a qualified constant receiver (``Billing::Processor``) to one class. + + Matches on the constant path rather than its tail: a class qualifies when its + own label ends with the referenced segments, so ``Billing::Processor`` still + finds an ``App::Billing::Processor`` while ``ActiveRecord::Base`` no longer + binds to an unrelated ``Thing::Base`` (#3078). A leading ``::`` pins the + reference to top level, so it must match the label whole. Ambiguous, or + matching nothing in the corpus (the usual case for a framework constant) -> + no edge, never a guess. + """ + segs = tuple(_segment_path(raw)) + if not segs: + return None + if raw.strip().startswith("::"): + nids = fq_label_map.get(segs, []) + return nids[0] if len(nids) == 1 else None + hits = {nid for path, nids in fq_label_map.items() + if len(path) >= len(segs) and path[-len(segs):] == segs + for nid in nids} + return next(iter(hits)) if len(hits) == 1 else None + def _emit(caller: str, target: str, rc: dict[str, Any], relation: str = "calls", context: str = "call") -> None: if not caller or not target or caller == target: @@ -187,8 +209,12 @@ def _emit(caller: str, target: str, rc: dict[str, Any], # collide with unrelated same-named methods, so we resolve by the # receiver's class under the single-owning-class god-node guard. receiver = rc.get("receiver") - if receiver and str(receiver)[:1].isupper(): - class_nid = _unique_class(str(receiver)) + # `lstrip(":")` so a top-level-pinned `::Processor.call` is still recognised + # as a constant receiver now that the whole path is captured (#3078). + if receiver and str(receiver).lstrip(":")[:1].isupper(): + recv_raw = str(receiver) + class_nid = (_class_by_const_path(recv_raw) if "::" in recv_raw + else _unique_class(recv_raw)) if class_nid is not None: if callee == "new": _emit(caller, class_nid, rc) diff --git a/tests/test_ruby_resolution.py b/tests/test_ruby_resolution.py index 7bde4ffdc6..a08db30262 100644 --- a/tests/test_ruby_resolution.py +++ b/tests/test_ruby_resolution.py @@ -422,3 +422,92 @@ def test_rake_files_extract_and_resolve_like_rb(tmp_path): calls = {(label.get(e["source"]), label.get(e["target"])) for e in result["edges"] if e["relation"] == "calls"} assert (".run()", ".tally()") in calls + + +# ── #3078: a qualified receiver must respect its namespace ──────────────────── + + +_LOCAL_BASE_RB = """\ +class Thing + class Base + def self.call(x) = x + end +end +""" + +_BILLING_RB = """\ +module Billing + class Processor + def self.run(x) = x + end +end +""" + +_SOLO_RB = """\ +class Solo + def self.go = 1 +end +""" + + +def test_framework_qualified_receiver_does_not_bind_same_named_local_class(tmp_path: Path) -> None: + """`ActiveRecord::Base.transaction` must not bind to an unrelated local `Base`. + + The receiver used to be truncated to its last constant, so any corpus with a + single class named `Base` collected every framework call as an EXTRACTED 1.0 + edge — a false hub, not a missing edge. The namespace has to be part of the + match (#3078). `ActiveJob::Base` is here too because both namespaces used to + collapse onto the very same node. + """ + _write(tmp_path, "thing.rb", _LOCAL_BASE_RB) + caller = _write(tmp_path, "other.rb", """\ +class Other + def framework_ar + ActiveRecord::Base.transaction { save! } + end + + def framework_aj + ActiveJob::Base.default_queue_name + end +end +""") + graph = extract([caller, tmp_path / "thing.rb"], cache_root=tmp_path, parallel=False) + assert _has_call_edge(graph, "framework_ar", "Thing::Base") is None, \ + "ActiveRecord::Base must not bind to an unrelated local Thing::Base" + assert _has_call_edge(graph, "framework_aj", "Thing::Base") is None, \ + "ActiveJob::Base must not bind to an unrelated local Thing::Base" + + +def test_qualified_receiver_still_resolves_inside_its_own_namespace(tmp_path: Path) -> None: + """The namespace check must not cost a genuine `Billing::Processor.run` edge.""" + _write(tmp_path, "billing.rb", _BILLING_RB) + caller = _write(tmp_path, "other.rb", """\ +class Other + def qualified + Billing::Processor.run(1) + end +end +""") + graph = extract([caller, tmp_path / "billing.rb"], cache_root=tmp_path, parallel=False) + edge = _has_call_edge(graph, "qualified", ".run()") + assert edge is not None, "a correctly-namespaced receiver must still resolve" + assert edge["confidence"] == "EXTRACTED" + + +def test_top_level_pinned_constant_receiver_still_resolves(tmp_path: Path) -> None: + """`::Solo.go` pins the constant to top level and must keep resolving. + + Worth its own case: capturing the whole path means the receiver text now starts + with `::`, so the constant-receiver check has to look past the leading colons. + """ + _write(tmp_path, "solo.rb", _SOLO_RB) + caller = _write(tmp_path, "other.rb", """\ +class Other + def pinned + ::Solo.go + end +end +""") + graph = extract([caller, tmp_path / "solo.rb"], cache_root=tmp_path, parallel=False) + assert _has_call_edge(graph, "pinned", ".go()") is not None, \ + "a top-level-pinned constant receiver must still resolve"