diff --git a/CHANGELOG.md b/CHANGELOG.md index cb82de9a5..791142ef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 3e2ac823b..e16d1fc78 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -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 diff --git a/graphify/ruby_resolution.py b/graphify/ruby_resolution.py index c7ed67e30..33a5478b4 100644 --- a/graphify/ruby_resolution.py +++ b/graphify/ruby_resolution.py @@ -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 ` 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 ` 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") diff --git a/tests/test_ruby_resolution.py b/tests/test_ruby_resolution.py index 7d402524b..7bde4ffdc 100644 --- a/tests/test_ruby_resolution.py +++ b/tests/test_ruby_resolution.py @@ -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`."""