mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-26 16:26:42 +00:00
fix(ruby): match a qualified receiver by its constant path (#3078)
A call on a qualified constant receiver (ActiveRecord::Base.transaction) truncated the receiver to its last segment (Base) and bound to any lone corpus class named Base, a false EXTRACTED edge. Keep the full constant path and suffix-match it against declared classes, emitting an edge only on a single unambiguous hit; a leading :: pins a whole-path match. Bare receivers are unchanged.
This commit is contained in:
committed by
safishamsi
parent
b48005276e
commit
984ff116d3
@@ -5380,10 +5380,13 @@ def _extract_generic(
|
||||
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
|
||||
|
||||
@@ -120,6 +120,28 @@ def resolve_ruby_member_calls(
|
||||
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:
|
||||
@@ -188,8 +210,12 @@ def resolve_ruby_member_calls(
|
||||
# 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)
|
||||
|
||||
@@ -563,3 +563,90 @@ end
|
||||
|
||||
assert node2_bang["id"] == id1, "foo!'s ID must remain unchanged when foo is added"
|
||||
assert node2_plain["id"] != node2_bang["id"], "foo and foo! must have distinct IDs"
|
||||
# ── #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"
|
||||
|
||||
Reference in New Issue
Block a user