fix(ruby): extract module/Struct/Class.new containers and resolve constant-receiver calls (#1640, #1634)
#1640 (node extraction): the extractor only created nodes for `class Foo`, so plain `module Foo`, `Foo = Struct.new(...) do ... end`, `Foo = Class.new(Super)` and `Result = Data.define(...)` produced no container node — their methods hung off the file via `contains` with dot-less labels and no edge could target them. `module` is now a container type (methods attach via `method`, nested modules included), and a constant assignment whose RHS is Struct.new/Class.new/Data.define synthesizes a class node named after the constant, attaches block-defined methods to it, and emits an `inherits` edge for `Class.new(Super)`. Plain constant assignments (MAX = 100, X = Foo.new) are untouched. #1634 (resolution): constant-receiver singleton calls (`Service.call`, `Model.where`, `SomeJob.perform_async`) emitted no edge, so a Zeitwerk-autoloaded Rails app (no requires) had near-zero cross-file edges. resolve_ruby_member_calls now handles a capitalized receiver with any callee: bind to the class's owned singleton/instance method (`def self.call`) when present, else to the class node itself so inherited/dynamic class methods (ActiveRecord where/find_by) still give blast-radius. Namespaced receivers resolve by bare class name. The single-owning-class god-node guard is kept — ambiguous receivers resolve to nothing, never a wrong edge. The two compound: PaymentProcessor#process -> TaxCalculator.rate_for needs the module node (#1640) AND the resolver (#1634); both now land. Full suite: 2893 passed, 3 skipped. Adversarial smoke confirms no false class nodes from plain/multiple assignments and no self-loops on self-class calls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
safishamsi
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
5737388f17
commit
13e2bddf4d
@@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Fix: Ruby plain modules and `Struct.new` / `Class.new` / `Data.define` constant assignments now get container nodes (#1640, thanks @krishnateja7). The extractor only created nodes for `class Foo`, so `module Foo` (utility/`module_function` modules), `Foo = Struct.new(...) do ... end`, `Foo = Class.new(StandardError)`, and `Result = Data.define(...)` produced no node at all — their methods hung off the file via `contains` with dot-less labels, and no edge could ever target them. `module` is now a container type (methods attach via `method` like a class, nested modules included), and a constant assignment whose RHS is one of those factories synthesizes a class node named after the constant, attaches block-defined methods to it, and emits an `inherits` edge for `Class.new(Super)`. Plain constant assignments (`MAX = 100`, `X = Foo.new`) are untouched.
|
||||
- Fix: Ruby constant-receiver singleton calls now resolve cross-file (#1634, thanks @krishnateja7). `Service.call`, `Model.where`, `SomeJob.perform_async` — the dominant Rails idiom — emitted no `calls` edge, so with Zeitwerk autoloading (no `require`s) a Rails app had essentially no cross-file edges and `affected`/`path` came up empty. `resolve_ruby_member_calls` now handles a capitalized (constant) receiver with any callee: it binds to the class's singleton/instance method when one is owned (`def self.call`, which the extractor indexes), else to the class node itself so inherited/dynamic class methods (ActiveRecord `where`/`find_by`) still give correct blast-radius. Namespaced receivers (`Billing::Processor.call`) resolve by the bare class name. The single-owning-class god-node guard is kept throughout — an ambiguous receiver resolves to nothing rather than a wrong edge.
|
||||
- Fix: Apex `interface X extends A, B` now emits an `extends` edge per parent (#1645, thanks @Synvoya). The interface regex captured the parent list in group 2, but the handler only read the interface name (group 1), so multiple-inheritance parents were dropped and only the `contains` edge survived. The interface branch now iterates the parent list and resolves each the same way the class branch already does.
|
||||
- Fix: Kotlin interface delegation (`class Foo : Bar by baz`) now emits the `implements` edge (#1644, thanks @Synvoya). The `by` form wraps the delegated interface in an `explicit_delegation` node, so neither the `constructor_invocation` nor the bare `user_type` branch fired and the edge was silently dropped. The delegation-specifier loop now unwraps `explicit_delegation` to its `user_type` (generic-argument recovery still runs), so idiomatic Kotlin delegation shows up in the graph.
|
||||
- Fix: a malformed semantic chunk no longer crashes `extract` and discards every successful chunk (#1631, thanks @ssazy). When an LLM returned a well-formed object whose `edges` (or `nodes`/`hyperedges`) array carried a stray non-dict entry — a nested list where an edge object belongs — the AST+semantic merge and the semantic-cache write both called `.get()` per entry and raised `AttributeError: 'list' object has no attribute 'get'`. On a 34-chunk run where 33 succeeded, that meant no `graph.json` was written and the cache write failed too, so a re-run re-extracted everything. `_parse_llm_json` now sanitizes each fragment at the single parse chokepoint (keeping only dict entries and coercing a non-list value to `[]`), so the cache writer, the adaptive-retry merge, and the CLI merge are all protected in one place.
|
||||
|
||||
+104
-1
@@ -2993,7 +2993,12 @@ _CPP_CONFIG = LanguageConfig(
|
||||
|
||||
_RUBY_CONFIG = LanguageConfig(
|
||||
ts_module="tree_sitter_ruby",
|
||||
class_types=frozenset({"class"}),
|
||||
# `module Foo` is a container node just like `class Foo` in tree-sitter's
|
||||
# Ruby grammar (name in a `constant` child, body in `body_statement`), so it
|
||||
# gets a node and its methods attach via `method` (#1640). Without it, plain
|
||||
# utility/`module_function` modules produced no node and their methods hung
|
||||
# off the file via `contains` with dot-less labels.
|
||||
class_types=frozenset({"class", "module"}),
|
||||
function_types=frozenset({"method", "singleton_method"}),
|
||||
import_types=frozenset(),
|
||||
call_types=frozenset({"call"}),
|
||||
@@ -3283,6 +3288,92 @@ def _ruby_local_class_bindings(body_node, source: bytes) -> dict[str, str | None
|
||||
return bindings
|
||||
|
||||
|
||||
def _ruby_const_last_name(node, source: bytes) -> str:
|
||||
"""Last constant of a ``constant`` or ``scope_resolution`` (``A::B::C`` -> ``C``)."""
|
||||
if node is None:
|
||||
return ""
|
||||
if node.type == "constant":
|
||||
return _read_text(node, source)
|
||||
if node.type == "scope_resolution":
|
||||
consts = [c for c in node.children if c.type == "constant"]
|
||||
if consts:
|
||||
return _read_text(consts[-1], source)
|
||||
return ""
|
||||
|
||||
|
||||
# `Const = <factory>(...)` shapes that define a lightweight class named after the
|
||||
# constant. tree-sitter parses each as an `assignment`, not a `class`, so the
|
||||
# generic class branch never saw them (#1640).
|
||||
_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) -> 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
|
||||
``method`` (by recursing the block with the new node as parent), and emit an
|
||||
``inherits`` edge for ``Class.new(Super)``. Returns True if handled.
|
||||
"""
|
||||
if node.type != "assignment":
|
||||
return False
|
||||
left = node.child_by_field_name("left")
|
||||
right = node.child_by_field_name("right")
|
||||
if left is None or right is None or left.type != "constant" or right.type != "call":
|
||||
return False
|
||||
recv = right.child_by_field_name("receiver")
|
||||
meth = right.child_by_field_name("method")
|
||||
if recv is None or meth is None or recv.type != "constant":
|
||||
return False
|
||||
if (_read_text(recv, source), _read_text(meth, source)) not in _RUBY_CLASS_FACTORIES:
|
||||
return False
|
||||
|
||||
const_name = _read_text(left, source)
|
||||
if not const_name:
|
||||
return False
|
||||
line = node.start_point[0] + 1
|
||||
class_nid = _make_id(stem, const_name)
|
||||
add_node(class_nid, const_name, line)
|
||||
callable_def_nids.add(class_nid) # a class is callable (its constructor)
|
||||
# Mirror the generic class branch: containment always hangs off the file node.
|
||||
add_edge(file_nid, class_nid, "contains", line)
|
||||
|
||||
# `Class.new(Super)` — the first positional constant argument is the superclass.
|
||||
if _read_text(recv, source) == "Class":
|
||||
args = next((c for c in right.children if c.type == "argument_list"), None)
|
||||
if args is not None:
|
||||
for arg in args.children:
|
||||
if arg.type in ("constant", "scope_resolution"):
|
||||
base = _ruby_const_last_name(arg, source)
|
||||
if base:
|
||||
base_nid = _make_id(stem, base)
|
||||
if base_nid not in seen_ids:
|
||||
base_nid = _make_id(base)
|
||||
if base_nid not in seen_ids:
|
||||
nodes.append({
|
||||
"id": base_nid, "label": base,
|
||||
"file_type": "code", "source_file": "",
|
||||
"source_location": "",
|
||||
})
|
||||
seen_ids.add(base_nid)
|
||||
add_edge(class_nid, base_nid, "inherits", line)
|
||||
break
|
||||
|
||||
# Recurse the do/brace block so block-defined methods attach to the class.
|
||||
# The block wraps its statements in a `body_statement` (like a class body);
|
||||
# descend into it so the method handler sees parent_class_nid — otherwise the
|
||||
# default recurse resets the parent to None and the method hangs off the file
|
||||
# with a dot-less label.
|
||||
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)
|
||||
return True
|
||||
|
||||
|
||||
# ── Generic extractor ─────────────────────────────────────────────────────────
|
||||
|
||||
def _extract_generic(
|
||||
@@ -4658,6 +4749,13 @@ def _extract_generic(
|
||||
ensure_named_node):
|
||||
return
|
||||
|
||||
if config.ts_module == "tree_sitter_ruby":
|
||||
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):
|
||||
return
|
||||
|
||||
# Python's `@property` / `@staticmethod` / `@classmethod` wrap the
|
||||
# inner function_definition in a `decorated_definition` node. The
|
||||
# default recurse below clears parent_class_nid, which would cause the
|
||||
@@ -5042,6 +5140,11 @@ def _extract_generic(
|
||||
is_member_call = True
|
||||
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
|
||||
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
|
||||
|
||||
@@ -28,6 +28,13 @@ 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_]*$")
|
||||
|
||||
|
||||
def _ruby_raw_calls(per_file: list[dict]) -> list[dict]:
|
||||
calls: list[dict] = []
|
||||
for result in per_file:
|
||||
@@ -69,6 +76,15 @@ def resolve_ruby_member_calls(
|
||||
tnode = node_by_id.get(tgt)
|
||||
if tnode is not None:
|
||||
method_index[(str(src), _key(tnode.get("label", "")))] = str(tgt)
|
||||
# Also register class/module container nodes that own no `method` edge — a
|
||||
# method-less `Class.new(StandardError)` or an empty module — so a constant
|
||||
# receiver still resolves to a real node (#1640/#1634). External base stubs
|
||||
# carry an empty source_file, so the `.rb` filter keeps them out.
|
||||
for n in all_nodes:
|
||||
nid = n.get("id")
|
||||
sf = str(n.get("source_file", ""))
|
||||
if nid and sf.endswith(".rb") and _BARE_CONST_RE.match(str(n.get("label", ""))):
|
||||
class_def_nids.setdefault(_key(n.get("label", "")), []).append(str(nid))
|
||||
for k in list(class_def_nids):
|
||||
class_def_nids[k] = sorted(set(class_def_nids[k]))
|
||||
|
||||
@@ -104,12 +120,24 @@ def resolve_ruby_member_calls(
|
||||
if not caller or not callee:
|
||||
continue
|
||||
|
||||
# `Processor.new` -> instantiation edge to the class.
|
||||
# Constant receiver: `Processor.new` (instantiation) or `Service.call` /
|
||||
# `Model.where` (singleton / class method). The bare method name would
|
||||
# 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 callee == "new" and receiver and str(receiver)[:1].isupper():
|
||||
if receiver and str(receiver)[:1].isupper():
|
||||
class_nid = _unique_class(str(receiver))
|
||||
if class_nid is not None:
|
||||
_emit(caller, class_nid, rc)
|
||||
if callee == "new":
|
||||
_emit(caller, class_nid, rc)
|
||||
else:
|
||||
# Emit to the singleton/instance method the class owns
|
||||
# (`def self.call`, which the extractor indexes); otherwise
|
||||
# to the class node itself, so inherited/dynamic class methods
|
||||
# like ActiveRecord `where`/`find_by` still give correct
|
||||
# blast-radius. An ambiguous receiver bails to nothing.
|
||||
method_nid = method_index.get((class_nid, _key(str(callee))))
|
||||
_emit(caller, method_nid or class_nid, rc)
|
||||
continue
|
||||
|
||||
# `p.run` where p's type is known -> edge to that class's method.
|
||||
|
||||
@@ -193,3 +193,100 @@ def test_class_new_creates_instantiation_edge(tmp_path: Path) -> None:
|
||||
edge = _has_call_edge(graph, "process_all", "Processor")
|
||||
assert edge is not None, "Processor.new should resolve a call to the Processor class"
|
||||
assert edge["confidence"] == "EXTRACTED"
|
||||
|
||||
|
||||
# ── #1640 node extraction + #1634 constant-receiver resolution ───────────────
|
||||
|
||||
|
||||
def _node_labels(result: dict) -> set[str]:
|
||||
return {str(n.get("label", "")) for n in result["nodes"]}
|
||||
|
||||
|
||||
def _method_edges(result: dict) -> set[tuple[str, str]]:
|
||||
labels = _labels(result["nodes"])
|
||||
return {
|
||||
(labels.get(e["source"], ""), labels.get(e["target"], ""))
|
||||
for e in result["edges"] if e.get("relation") == "method"
|
||||
}
|
||||
|
||||
|
||||
def test_plain_module_gets_a_node_with_methods(tmp_path: Path) -> None:
|
||||
"""#1640 shape 1: `module Foo` must get a node and own its methods."""
|
||||
r = extract_ruby(_write(tmp_path, "tax.rb",
|
||||
"module TaxCalculator\n module_function\n def rate_for(order)\n 0.2\n end\nend\n"))
|
||||
assert "TaxCalculator" in _node_labels(r)
|
||||
# method attaches to the module (dot label), not the file (dot-less).
|
||||
assert ("TaxCalculator", ".rate_for()") in _method_edges(r)
|
||||
|
||||
|
||||
def test_nested_modules_each_get_a_node(tmp_path: Path) -> None:
|
||||
"""#1640 shape 1, nested."""
|
||||
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)
|
||||
|
||||
|
||||
def test_struct_new_constant_creates_class_with_methods(tmp_path: Path) -> None:
|
||||
"""#1640 shape 2: `Foo = Struct.new(...) do ... end`."""
|
||||
r = extract_ruby(_write(tmp_path, "invoice.rb",
|
||||
"Invoice = Struct.new(:total, :tax) do\n def grand_total\n total + tax\n end\nend\n"))
|
||||
assert "Invoice" in _node_labels(r)
|
||||
assert ("Invoice", ".grand_total()") in _method_edges(r)
|
||||
|
||||
|
||||
def test_class_new_constant_creates_class_and_inherits(tmp_path: Path) -> None:
|
||||
"""#1640 shape 3: `Foo = Class.new(Super)` — node + inherits edge."""
|
||||
r = extract_ruby(_write(tmp_path, "err.rb", "ApiError = Class.new(StandardError)\n"))
|
||||
assert "ApiError" in _node_labels(r)
|
||||
labels = _labels(r["nodes"])
|
||||
inh = {(labels.get(e["source"], ""), labels.get(e["target"], ""))
|
||||
for e in r["edges"] if e.get("relation") == "inherits"}
|
||||
assert ("ApiError", "StandardError") in inh
|
||||
|
||||
|
||||
def test_data_define_constant_creates_class(tmp_path: Path) -> None:
|
||||
r = extract_ruby(_write(tmp_path, "res.rb", "Result = Data.define(:ok, :value)\n"))
|
||||
assert "Result" in _node_labels(r)
|
||||
|
||||
|
||||
def test_constant_receiver_singleton_call_resolves(tmp_path: Path) -> None:
|
||||
"""#1634: `Processor.call` (def self.call) resolves to the singleton method."""
|
||||
_write(tmp_path, "processor.rb", "class Processor\n def self.call; end\nend\n")
|
||||
runner = _write(tmp_path, "runner.rb",
|
||||
"class Runner\n def run\n Processor.call\n end\nend\n")
|
||||
graph = extract([runner, tmp_path / "processor.rb"], cache_root=tmp_path, parallel=False)
|
||||
assert _has_call_edge(graph, "run", "call") is not None
|
||||
|
||||
|
||||
def test_constant_receiver_module_function_call_resolves(tmp_path: Path) -> None:
|
||||
"""#1634 + #1640: `TaxCalculator.rate_for` resolves across files to a
|
||||
module_function — needs both the module node (#1640) and the resolver (#1634)."""
|
||||
_write(tmp_path, "tax.rb",
|
||||
"module TaxCalculator\n module_function\n def rate_for(o)\n 0.2\n end\nend\n")
|
||||
pp = _write(tmp_path, "pp.rb",
|
||||
"class PaymentProcessor\n def process(order)\n TaxCalculator.rate_for(order)\n end\nend\n")
|
||||
graph = extract([pp, tmp_path / "tax.rb"], cache_root=tmp_path, parallel=False)
|
||||
assert _has_call_edge(graph, "process", "rate_for") is not None
|
||||
|
||||
|
||||
def test_constant_receiver_unknown_class_method_falls_back_to_class(tmp_path: Path) -> None:
|
||||
"""#1634: `Model.where` (no `where` def, e.g. ActiveRecord) still links to the
|
||||
class node for blast-radius, rather than dropping the edge."""
|
||||
_write(tmp_path, "model.rb", "class Model\n def self.create; end\nend\n")
|
||||
caller = _write(tmp_path, "svc.rb",
|
||||
"class Svc\n def run\n Model.where(id: 1)\n end\nend\n")
|
||||
graph = extract([caller, tmp_path / "model.rb"], cache_root=tmp_path, parallel=False)
|
||||
# No `where` method node exists, so the edge lands on the class node itself.
|
||||
assert _has_call_edge(graph, "run", "Model") is not None
|
||||
|
||||
|
||||
def test_ambiguous_constant_receiver_emits_no_edge(tmp_path: Path) -> None:
|
||||
"""Two classes named `Processor` => ambiguous receiver => bail (no wrong edge)."""
|
||||
_write(tmp_path, "a.rb", "module A\n class Processor\n def self.call; end\n end\nend\n")
|
||||
_write(tmp_path, "b.rb", "module B\n class Processor\n def self.call; end\n end\nend\n")
|
||||
caller = _write(tmp_path, "c.rb",
|
||||
"class Runner\n def run\n Processor.call\n end\nend\n")
|
||||
graph = extract([caller, tmp_path / "a.rb", tmp_path / "b.rb"], cache_root=tmp_path, parallel=False)
|
||||
assert _has_call_edge(graph, "run", "call") is None
|
||||
|
||||
Reference in New Issue
Block a user