fix(ruby): resolve compact/nested mixin targets, prevent phantom concern hub (#2302)
module Foo::Bar and nested module Foo; module Bar are canonicalized to one fully-qualified label, and include/extend/prepend keep the full constant path, so include Foo::Bar resolves. Mixin resolution is scoped and lexical: a qualified external name (extend ActiveSupport::Concern) no longer binds to a local module named Concern, while a genuine in-corpus include Foo::Concern still resolves. Nested-declared classes keep a last-segment index so typed-receiver calls (Processor.new) keep resolving. Adapts the approach from #1394 (thanks @FolatheDuckofDuckingburg) to the post-refactor extractors/engine.py, with an added last-segment guard for nested-class receiver resolution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
7db00fcc1a
commit
00efd6e796
@@ -13,6 +13,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
- Fix: a custom `GRAPHIFY_OUT` name no longer prunes every same-named directory in the tree; only the configured output path is excluded (#2273, thanks @oleksii-tumanov).
|
||||
- Fix: C# member calls resolve for receivers declared inline via `out var`, `is`, `case`, and switch-arm patterns (#2346, thanks @JensD-git), and members of a `partial class` split across files now attach to one merged class node so cross-half calls resolve (#2332).
|
||||
- Fix: members of a Kotlin anonymous object (`object : Foo { ... }`) are now extracted, with their `implements` and `calls` edges (#2347).
|
||||
- Fix: Ruby mixins declared with compact/nested syntax now resolve, and a qualified external mixin can no longer fabricate a phantom hub (#2302, thanks @FolatheDuckofDuckingburg). `module Foo::Bar` and `module Foo; module Bar` are canonicalized to the same fully-qualified label, and `include`/`extend`/`prepend` keep the full constant path, so `include Foo::Bar` resolves. Mixin resolution is now scoped and lexical: a qualified external name like `extend ActiveSupport::Concern` no longer binds to any local module named `Concern`, while a genuine in-corpus `include Foo::Concern` still resolves. Nested-declared classes keep their last-segment index so typed-receiver calls (`Processor.new`) continue to resolve.
|
||||
- Perf: dedup drops an O(nodes x components) scan in remap construction (#2328, thanks @stupidprogrammer4), with identical results.
|
||||
|
||||
## 0.9.31 (2026-07-30)
|
||||
|
||||
@@ -2188,12 +2188,19 @@ def _ruby_const_last_name(node, source: bytes) -> str:
|
||||
return _read_text(consts[-1], source)
|
||||
return ""
|
||||
|
||||
def _ruby_const_full_name(node, source: bytes) -> str:
|
||||
"""Full constant path of a ``constant`` or ``scope_resolution`` (``A::B::C`` kept whole)."""
|
||||
if node is None or node.type not in ("constant", "scope_resolution"):
|
||||
return ""
|
||||
return _read_text(node, source).strip()
|
||||
|
||||
_RUBY_CLASS_FACTORIES = frozenset({("Struct", "new"), ("Class", "new"), ("Data", "define")})
|
||||
|
||||
def _ruby_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: str,
|
||||
nodes: list, edges: list, seen_ids: set, function_bodies: list,
|
||||
parent_class_nid: str | None, add_node, add_edge, walk,
|
||||
callable_def_nids: set, callable_class_nids: set) -> bool:
|
||||
callable_def_nids: set, callable_class_nids: set,
|
||||
ruby_namespace: list) -> bool:
|
||||
"""Ruby: a constant assignment whose RHS is ``Struct.new(...)``,
|
||||
``Class.new(Super)`` or ``Data.define(...)`` defines a class named after the
|
||||
constant (#1640). Synthesize the class node, attach block-defined methods via
|
||||
@@ -2216,6 +2223,11 @@ def _ruby_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: st
|
||||
const_name = _read_text(left, source)
|
||||
if not const_name:
|
||||
return False
|
||||
# Qualify the factory-defined const against the enclosing scope, mirroring
|
||||
# the generic class branch (#2302): `module Billing; Invoice = Struct.new`
|
||||
# labels `Billing::Invoice`.
|
||||
const_segments = const_name.split("::")
|
||||
const_name = "::".join(ruby_namespace + const_segments)
|
||||
line = node.start_point[0] + 1
|
||||
class_nid = _make_id(stem, const_name)
|
||||
add_node(class_nid, const_name, line)
|
||||
@@ -2259,8 +2271,12 @@ def _ruby_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: st
|
||||
block = next((c for c in right.children if c.type in ("do_block", "block")), None)
|
||||
if block is not None:
|
||||
body = next((c for c in block.children if c.type == "body_statement"), block)
|
||||
for child in body.children:
|
||||
walk(child, parent_class_nid=class_nid)
|
||||
ruby_namespace.extend(const_segments)
|
||||
try:
|
||||
for child in body.children:
|
||||
walk(child, parent_class_nid=class_nid)
|
||||
finally:
|
||||
del ruby_namespace[-len(const_segments):]
|
||||
return True
|
||||
|
||||
def _extract_generic(
|
||||
@@ -2310,6 +2326,11 @@ def _extract_generic(
|
||||
edges: list[dict] = []
|
||||
seen_ids: set[str] = set()
|
||||
namespace_stack: list[str] = []
|
||||
# Ruby only: enclosing module/class segments, so `module Foo::Bar` (compact)
|
||||
# and `module Foo; module Bar` (nested) label the same node `Foo::Bar` and
|
||||
# `include Foo::Bar` resolves for both spellings (#2302). Kept separate from
|
||||
# namespace_stack so Ruby method ids/labels are unchanged.
|
||||
ruby_namespace: list[str] = []
|
||||
scope_stack: list[str] = []
|
||||
function_bodies: list[tuple[str, object]] = []
|
||||
# nids of function / method / class definitions in this file. The indirect-
|
||||
@@ -2484,6 +2505,13 @@ def _extract_generic(
|
||||
if not name_node:
|
||||
return
|
||||
class_name = _read_text(name_node, source)
|
||||
# Ruby: fully qualify the module/class label with its enclosing
|
||||
# scope, splitting compact `Foo::Bar` names into segments so both
|
||||
# declaration styles converge on one `Foo::Bar` label (#2302).
|
||||
ruby_segments: list[str] = []
|
||||
if config.ts_module == "tree_sitter_ruby":
|
||||
ruby_segments = class_name.split("::")
|
||||
class_name = "::".join(ruby_namespace + ruby_segments)
|
||||
class_nid = _make_id(stem, ".".join(namespace_stack), class_name)
|
||||
line = node.start_point[0] + 1
|
||||
metadata = None
|
||||
@@ -2737,7 +2765,11 @@ def _extract_generic(
|
||||
for _arg in _args.children:
|
||||
if _arg.type not in ("constant", "scope_resolution"):
|
||||
continue
|
||||
_mod = _ruby_const_last_name(_arg, source)
|
||||
# Full path, not last segment: `include Foo::Bar`
|
||||
# must reference `Foo::Bar`, and truncating
|
||||
# `ActiveSupport::Concern` to `Concern` fabricated
|
||||
# edges to any local `Concern` module (#2302).
|
||||
_mod = _ruby_const_full_name(_arg, source)
|
||||
if _mod:
|
||||
_ruby_mixin_calls.append({
|
||||
"caller_nid": class_nid,
|
||||
@@ -2996,11 +3028,18 @@ def _extract_generic(
|
||||
add_edge(class_nid, target_nid, "references",
|
||||
line, context="generic_arg")
|
||||
|
||||
# Find body and recurse
|
||||
# Find body and recurse. Ruby pushes its scope segments so nested
|
||||
# declarations qualify against the enclosing module/class (#2302);
|
||||
# ruby_segments is empty for every other language.
|
||||
body = _find_body(node, config)
|
||||
if body:
|
||||
for child in body.children:
|
||||
walk(child, parent_class_nid=class_nid)
|
||||
ruby_namespace.extend(ruby_segments)
|
||||
try:
|
||||
for child in body.children:
|
||||
walk(child, parent_class_nid=class_nid)
|
||||
finally:
|
||||
if ruby_segments:
|
||||
del ruby_namespace[-len(ruby_segments):]
|
||||
return
|
||||
|
||||
# Event listener property arrays: $listen = [Event::class => [Listener::class]]
|
||||
@@ -3853,7 +3892,8 @@ def _extract_generic(
|
||||
if _ruby_extra_walk(node, source, file_nid, stem, str_path,
|
||||
nodes, edges, seen_ids, function_bodies,
|
||||
parent_class_nid, add_node, add_edge, walk,
|
||||
callable_def_nids, callable_class_nids):
|
||||
callable_def_nids, callable_class_nids,
|
||||
ruby_namespace):
|
||||
return
|
||||
|
||||
# Python's `@property` / `@staticmethod` / `@classmethod` wrap the
|
||||
|
||||
+59
-13
@@ -28,11 +28,12 @@ def _key(label: str) -> str:
|
||||
return re.sub(r"[^a-zA-Z0-9]+", "", str(label)).lower()
|
||||
|
||||
|
||||
# A Ruby class/module container node is labelled with a bare constant
|
||||
# (``Processor``, ``TaxCalculator``); methods end in ``()`` and files in ``.rb``.
|
||||
# Lets us register method-less containers (a ``Class.new(StandardError)`` error
|
||||
# class, an empty module) that have no `method` edge to be found by.
|
||||
_BARE_CONST_RE = re.compile(r"^[A-Z][A-Za-z0-9_]*$")
|
||||
# A Ruby class/module container node is labelled with a constant path, bare or
|
||||
# ``::``-qualified (``Processor``, ``Billing::Rounding``); methods end in ``()``
|
||||
# and files in ``.rb``. Lets us register method-less containers (a
|
||||
# ``Class.new(StandardError)`` error class, an empty module) that have no
|
||||
# `method` edge to be found by.
|
||||
_BARE_CONST_RE = re.compile(r"^[A-Z][A-Za-z0-9_]*(?:::[A-Z][A-Za-z0-9_]*)*$")
|
||||
|
||||
|
||||
def _ruby_raw_calls(per_file: list[dict]) -> list[dict]:
|
||||
@@ -72,7 +73,14 @@ def resolve_ruby_member_calls(
|
||||
src, tgt = e.get("source"), e.get("target")
|
||||
cnode = node_by_id.get(src)
|
||||
if cnode is not None:
|
||||
class_def_nids.setdefault(_key(cnode.get("label", "")), []).append(str(src))
|
||||
clabel = str(cnode.get("label", ""))
|
||||
class_def_nids.setdefault(_key(clabel), []).append(str(src))
|
||||
# A nested/compact declaration labels the node fully qualified
|
||||
# (`Billing::Processor`), but its receivers reference the bare last
|
||||
# segment (`Processor.new`), so index that too — the unique-match
|
||||
# guard below still bails on genuine collisions (#2302).
|
||||
if "::" in clabel:
|
||||
class_def_nids.setdefault(_key(clabel.split("::")[-1]), []).append(str(src))
|
||||
tnode = node_by_id.get(tgt)
|
||||
if tnode is not None:
|
||||
method_index[(str(src), _key(tnode.get("label", "")))] = str(tgt)
|
||||
@@ -83,11 +91,28 @@ def resolve_ruby_member_calls(
|
||||
for n in all_nodes:
|
||||
nid = n.get("id")
|
||||
sf = str(n.get("source_file", ""))
|
||||
if nid and sf.endswith((".rb", ".rake")) and _BARE_CONST_RE.match(str(n.get("label", ""))):
|
||||
class_def_nids.setdefault(_key(n.get("label", "")), []).append(str(nid))
|
||||
label = str(n.get("label", ""))
|
||||
if nid and sf.endswith((".rb", ".rake")) and _BARE_CONST_RE.match(label):
|
||||
class_def_nids.setdefault(_key(label), []).append(str(nid))
|
||||
if "::" in label:
|
||||
class_def_nids.setdefault(_key(label.split("::")[-1]), []).append(str(nid))
|
||||
for k in list(class_def_nids):
|
||||
class_def_nids[k] = sorted(set(class_def_nids[k]))
|
||||
|
||||
def _segment_path(label: str) -> list[str]:
|
||||
return [s.strip().lower() for s in str(label).split("::") if s.strip()]
|
||||
|
||||
# Fully-qualified and last-segment views of the same definitions, for the
|
||||
# scoped mixin lookup: `include Foo::Bar` must match a `Foo::Bar` label as a
|
||||
# whole path, never just its tail.
|
||||
fq_label_map: dict[tuple[str, ...], list[str]] = {}
|
||||
last_segment_map: dict[str, list[str]] = {}
|
||||
for nid in sorted({nid for nids in class_def_nids.values() for nid in nids}):
|
||||
segs = _segment_path(str(node_by_id.get(nid, {}).get("label", "")))
|
||||
if segs:
|
||||
fq_label_map.setdefault(tuple(segs), []).append(nid)
|
||||
last_segment_map.setdefault(segs[-1], []).append(nid)
|
||||
|
||||
existing_pairs = {(e.get("source"), e.get("target")) for e in all_edges}
|
||||
|
||||
def _unique_class(name: str) -> str | None:
|
||||
@@ -113,10 +138,14 @@ def resolve_ruby_member_calls(
|
||||
"weight": 1.0,
|
||||
})
|
||||
|
||||
# `include`/`extend`/`prepend <Const>` mixins (#1668): resolve the module by
|
||||
# its constant name to the single owning module/class node and emit a
|
||||
# `mixes_in` edge, under the same single-definition god-node guard. An
|
||||
# ambiguous or unresolved constant produces no edge.
|
||||
# `include`/`extend`/`prepend <Const>` mixins (#1668): resolve the module
|
||||
# reference lexically, the way Ruby constant lookup works (#2302) — try the
|
||||
# reference under each enclosing scope of the including class, innermost
|
||||
# first, then top level (a leading `::` pins it to top level). A qualified
|
||||
# external like `ActiveSupport::Concern` matches no in-corpus path and
|
||||
# produces no edge; an unqualified reference with no lexical match falls
|
||||
# back to a globally unique last segment, under the same single-definition
|
||||
# god-node guard. Ambiguous at any step -> bail, no wrong edge.
|
||||
for rc in _ruby_raw_calls(per_file):
|
||||
if not rc.get("is_mixin"):
|
||||
continue
|
||||
@@ -124,7 +153,24 @@ def resolve_ruby_member_calls(
|
||||
module_name = rc.get("callee")
|
||||
if not caller or not module_name:
|
||||
continue
|
||||
target = _unique_class(str(module_name))
|
||||
raw_ref = str(module_name)
|
||||
absolute = raw_ref.startswith("::")
|
||||
ref_segs = _segment_path(raw_ref)
|
||||
caller_node = node_by_id.get(caller)
|
||||
caller_segs = [] if absolute else _segment_path(
|
||||
str(caller_node.get("label", "")) if caller_node else "")
|
||||
target: str | None = None
|
||||
for i in range(len(caller_segs), -1, -1):
|
||||
nids = fq_label_map.get(tuple(caller_segs[:i] + ref_segs), [])
|
||||
if len(nids) == 1:
|
||||
target = nids[0]
|
||||
break
|
||||
if len(nids) > 1:
|
||||
break # reopened/ambiguous definition: bail
|
||||
if target is None and len(ref_segs) == 1 and not absolute:
|
||||
nids = last_segment_map.get(ref_segs[0], [])
|
||||
if len(nids) == 1:
|
||||
target = nids[0]
|
||||
if target is not None:
|
||||
_emit(caller, target, rc, relation="mixes_in", context="mixin")
|
||||
|
||||
|
||||
@@ -220,12 +220,13 @@ def test_plain_module_gets_a_node_with_methods(tmp_path: Path) -> None:
|
||||
|
||||
|
||||
def test_nested_modules_each_get_a_node(tmp_path: Path) -> None:
|
||||
"""#1640 shape 1, nested."""
|
||||
"""#1640 shape 1, nested — the inner module is labelled fully qualified
|
||||
(#2302), so nested and compact declarations converge on one label."""
|
||||
r = extract_ruby(_write(tmp_path, "n.rb",
|
||||
"module Billing\n module Rounding\n def round(x)\n x.round(2)\n end\n end\nend\n"))
|
||||
labels = _node_labels(r)
|
||||
assert "Billing" in labels and "Rounding" in labels
|
||||
assert ("Rounding", ".round()") in _method_edges(r)
|
||||
assert "Billing" in labels and "Billing::Rounding" in labels
|
||||
assert ("Billing::Rounding", ".round()") in _method_edges(r)
|
||||
|
||||
|
||||
def test_struct_new_constant_creates_class_with_methods(tmp_path: Path) -> None:
|
||||
@@ -350,6 +351,61 @@ def test_mixin_is_not_emitted_as_calls_edge(tmp_path: Path) -> None:
|
||||
assert ("K", "C") in _mixes_in(g)
|
||||
|
||||
|
||||
# ── #2302 compact-syntax mixins + qualified constant lookup ──────────────────
|
||||
|
||||
|
||||
def test_compact_and_nested_module_includes_resolve(tmp_path: Path) -> None:
|
||||
"""#2302: `module Billing::TotalsConcern` (compact) and a top-level module
|
||||
both resolve as mixin targets, lexically from the including class."""
|
||||
_write(tmp_path, "totals_concern.rb",
|
||||
"module Billing::TotalsConcern\n def total; end\nend\n")
|
||||
_write(tmp_path, "archivable_concern.rb",
|
||||
"module ArchivableConcern\n extend ActiveSupport::Concern\n def archive; end\nend\n")
|
||||
_write(tmp_path, "models.rb",
|
||||
"module Billing\n class Invoice\n include TotalsConcern\n end\nend\n"
|
||||
"\nclass Account\n extend ArchivableConcern\nend\n")
|
||||
g = extract(sorted(tmp_path.glob("*.rb")), cache_root=tmp_path, parallel=False)
|
||||
mix = _mixes_in(g)
|
||||
assert ("Billing::Invoice", "Billing::TotalsConcern") in mix
|
||||
assert ("Account", "ArchivableConcern") in mix
|
||||
# `extend ActiveSupport::Concern` must not fabricate an edge to any local
|
||||
# module — no phantom `Concern` target of any spelling.
|
||||
assert not any(t.split("::")[-1] == "Concern" for _s, t in mix)
|
||||
|
||||
|
||||
def test_qualified_external_mixin_does_not_bind_to_local(tmp_path: Path) -> None:
|
||||
"""#2302: `extend ActiveSupport::Concern` must NOT resolve to an unrelated
|
||||
local `module Concern` just because the last segment matches."""
|
||||
_write(tmp_path, "concern.rb", "module Concern\n def local_thing; end\nend\n")
|
||||
_write(tmp_path, "post.rb", "class Post\n extend ActiveSupport::Concern\nend\n")
|
||||
mix = _mixes_in(extract(sorted(tmp_path.glob("*.rb")), cache_root=tmp_path, parallel=False))
|
||||
assert ("Post", "Concern") not in mix
|
||||
assert not mix
|
||||
|
||||
|
||||
def test_in_corpus_qualified_mixin_resolves(tmp_path: Path) -> None:
|
||||
"""#2302 over-suppression guard: a qualified reference whose full path IS
|
||||
defined in the corpus still resolves."""
|
||||
_write(tmp_path, "foo.rb", "module Foo\n module Concern\n def helper; end\n end\nend\n")
|
||||
_write(tmp_path, "k.rb", "class K\n include Foo::Concern\nend\n")
|
||||
mix = _mixes_in(extract(sorted(tmp_path.glob("*.rb")), cache_root=tmp_path, parallel=False))
|
||||
assert ("K", "Foo::Concern") in mix
|
||||
|
||||
|
||||
def test_nested_declared_class_still_resolves_as_receiver(tmp_path: Path) -> None:
|
||||
"""#2302 regression guard: qualifying labels must not break bare constant
|
||||
receivers — `Processor.new` / typed `p.run` still find `Billing::Processor`."""
|
||||
_write(tmp_path, "billing.rb",
|
||||
"module Billing\n class Processor\n def run\n 42\n end\n end\nend\n")
|
||||
_write(tmp_path, "caller.rb",
|
||||
"def process_all\n p = Processor.new\n p.run\nend\n")
|
||||
g = extract(sorted(tmp_path.glob("*.rb")), cache_root=tmp_path, parallel=False)
|
||||
assert _has_call_edge(g, "process_all", "Processor") is not None, \
|
||||
"Processor.new should still resolve to the nested-declared class"
|
||||
assert _has_call_edge(g, "process_all", "run") is not None, \
|
||||
"typed p.run should still resolve to Billing::Processor#run"
|
||||
|
||||
|
||||
def test_rake_files_extract_and_resolve_like_rb(tmp_path):
|
||||
"""#1784: `.rake` files are plain Ruby and must route to the Ruby extractor
|
||||
and participate in Ruby cross-file resolution exactly like `.rb`."""
|
||||
|
||||
Reference in New Issue
Block a user