diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index b6bfac96f..369415cfc 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -3335,6 +3335,105 @@ def _ruby_local_class_bindings(body_node, source: bytes) -> dict[str, str | None visit(body_node) return bindings + +def _ruby_local_names(body_node, source: bytes) -> frozenset[str]: + """Collect every name bound as a local variable or parameter in one Ruby + method body (the method's own parameters plus in-body assignments, block + parameters, ``for`` variables and ``rescue`` captures). + + A paren-less ``identifier`` in Ruby is a method call on ``self`` (``build``) + *unless* a local of that name is in scope — the exact rule Ruby's own parser + uses to tell ``build`` (a send) from ``x`` (a variable read). The call-walk + uses this set to make that distinction; see the bare-self-send branch there. + + Deliberately over-inclusive and does not descend into nested ``def`` bodies + (which open their own scope): a missed binding only suppresses a candidate + call (fail-closed), it never invents a wrong edge. + """ + names: set[str] = set() + boundary = {"method", "singleton_method"} + param_lists = { + "method_parameters", + "block_parameters", + "lambda_parameters", + "bare_parameters", + } + param_wrappers = { + "optional_parameter", + "splat_parameter", + "hash_splat_parameter", + "keyword_parameter", + "block_parameter", + "destructured_parameter", + "forward_parameter", + } + + def _first_identifier(n): + if n.type == "identifier": + return n + for c in n.children: + found = _first_identifier(c) + if found is not None: + return found + return None + + def _add_params(param_list) -> None: + for child in param_list.children: + if child.type == "identifier": + names.add(_read_text(child, source)) + elif child.type in param_wrappers: + ident = _first_identifier(child) + if ident is not None: + names.add(_read_text(ident, source)) + + def _add_targets(n) -> None: + # Identifiers reachable as assignment targets (`a, b = …`, destructuring) + # without descending into a value expression on the right. + if n.type == "identifier": + names.add(_read_text(n, source)) + return + for c in n.children: + _add_targets(c) + + # Parameters live on the enclosing method/block node, not inside the body. + parent = body_node.parent + if parent is not None: + for child in parent.children: + if child.type in param_lists: + _add_params(child) + + def visit(n) -> None: + for child in n.children: + if child.type in boundary: + continue # nested method opens its own local scope + if child.type in ("assignment", "operator_assignment"): + left = child.child_by_field_name("left") + if left is not None: + if left.type == "identifier": + names.add(_read_text(left, source)) + else: + # `a, b = …` / `(a, b) = …`; `obj.attr = …` contributes no + # local and _add_targets simply finds no bare target there. + if left.type not in ("call", "element_reference"): + _add_targets(left) + elif child.type in param_lists: + _add_params(child) + elif child.type == "exception_variable": + ident = _first_identifier(child) + if ident is not None: + names.add(_read_text(ident, source)) + elif child.type == "for": + for sub in child.children: + if sub.type == "identifier": + names.add(_read_text(sub, source)) + elif sub.type == "left_assignment_list": + _add_targets(sub) + visit(child) + + visit(body_node) + return frozenset(names) + + 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: @@ -6040,6 +6139,9 @@ def _extract_generic( # populated before walk_calls runs. Lets member-call raw_calls carry a # receiver_type so the cross-file pass resolves `var.method` by type (#ruby). ruby_var_types: dict[str, dict[str, str | None]] = {} + # Ruby: per-method set of bound local/parameter names, so the call-walk can + # tell a paren-less self-send (`build`) from a plain variable read (`x`). + ruby_local_names: dict[str, frozenset[str]] = {} # Fields declared on a SUPERCLASS type receivers in a subclass too (#3151): # fold each class's table with its ancestors', nearest declaration winning. # Local `inherits` edges only - the cross-file half lives in the corpus @@ -6289,7 +6391,35 @@ def _extract_generic( and node.type in ("lexical_declaration", "variable_declaration")): _require_imports_js(node, source, caller_nid, stem, edges, str_path) - if node.type in config.call_types: + # Ruby paren-less self-send: `build` (no args, no parens) parses as a bare + # `identifier`, indistinguishable in the grammar from a local read `x`. + # It is a method call on `self` unless a local of that name is in scope — + # Ruby's own disambiguation rule (see `_ruby_local_names`). Route it + # through the normal callee path as a non-member call so it resolves + # in-file (EXTRACTED) or becomes a raw_call the Ruby resolver can prove. + _ruby_bare_self_send = False + if config.ts_module == "tree_sitter_ruby" and node.type == "identifier": + _parent = node.parent + # The `method`/`receiver` of a `call` node are handled by the call + # branch; don't also read them as bare sends here. + _call_field = ( + _parent is not None + and _parent.type == "call" + and node in ( + _parent.child_by_field_name("method"), + _parent.child_by_field_name("receiver"), + ) + ) + if not _call_field: + _bare_name = _read_text(node, source) + if ( + _bare_name + and _bare_name not in ruby_local_names.get(caller_nid, frozenset()) + and _bare_name not in extra_locals + ): + _ruby_bare_self_send = True + + if node.type in config.call_types or _ruby_bare_self_send: # JS/TS dynamic imports: await import('./foo.js') if config.ts_module in ("tree_sitter_javascript", "tree_sitter_typescript"): if _dynamic_import_js(node, source, caller_nid, str_path, @@ -6310,7 +6440,11 @@ def _extract_generic( lua_self_qualified: bool = False # Lua self:m() rewritten to Table:m # Special handling per language - if config.ts_module == "tree_sitter_swift": + if _ruby_bare_self_send: + # A bare `identifier` has no receiver and no method/argument + # fields: the callee is the identifier itself, implicit `self`. + callee_name = _read_text(node, source) + elif config.ts_module == "tree_sitter_swift": # Swift: first child may be simple_identifier or navigation_expression first = node.children[0] if node.children else None if first: @@ -7165,6 +7299,7 @@ def _extract_generic( if config.ts_module == "tree_sitter_ruby": for caller_nid, body_node in function_bodies: ruby_var_types[caller_nid] = _ruby_local_class_bindings(body_node, source) + ruby_local_names[caller_nid] = _ruby_local_names(body_node, source) # C++: build the per-file `var -> ClassName` table from local declarations in # every function body so the cross-file member-call pass can type a receiver diff --git a/tests/test_ruby_bare_self_send_calls.py b/tests/test_ruby_bare_self_send_calls.py new file mode 100644 index 000000000..94a2b91c9 --- /dev/null +++ b/tests/test_ruby_bare_self_send_calls.py @@ -0,0 +1,178 @@ +"""A paren-less Ruby method call is a self-send and must keep its `calls` edge. + +In Ruby the most common call form has no receiver and no parentheses:: + + def run + build # a method call on self, not a variable read + end + +Tree-sitter parses that `build` as a bare `identifier`, structurally identical +to a local-variable read `x`. The call-walk only looked at `call` nodes, so the +edge was silently dropped — `run` → `build` never appeared, even though +`run(1)`, `self.run` and `run arg` all resolved. The Ruby resolver already had +a dedicated bare-call promotion path, but it had nothing to promote because the +extraction step never produced the call. + +The fix mirrors Ruby's own rule: a bare identifier is a send on `self` unless a +local or parameter of that name is in scope. These specs pin both halves — the +send is captured, and a genuine local read is NOT turned into a call. +""" +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +from graphify.extract import extract + + +def _extract(tmp_path, files: dict[str, str]) -> dict: + for name, body in files.items(): + p = tmp_path / name + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body) + old = os.getcwd() + try: + os.chdir(tmp_path) + return extract([Path(n) for n in files], cache_root=Path(tempfile.mkdtemp())) + finally: + os.chdir(old) + + +def _labels(r: dict) -> dict[str, str]: + return {n["id"]: str(n.get("label", "")) for n in r["nodes"]} + + +def _calls(r: dict, src: str, tgt: str) -> list[dict]: + labels = _labels(r) + return [ + e + for e in r["edges"] + if e.get("relation") == "calls" + and labels.get(e.get("source")) == src + and labels.get(e.get("target")) == tgt + ] + + +def test_bare_self_send_in_a_class_is_a_call(tmp_path): + r = _extract(tmp_path, {"w.rb": ( + "class Widget\n" + " def run\n" + " build\n" + " end\n" + " def build\n" + " 1\n" + " end\n" + "end\n" + )}) + assert _calls(r, ".run()", ".build()") + + +def test_bare_call_between_module_methods_is_a_call(tmp_path): + # The whole module layer used to be a set of leaves with no internal edges. + r = _extract(tmp_path, {"g.rb": ( + "module Greetable\n" + " def greet\n" + " build_message\n" + " end\n" + " def build_message\n" + " \"hi\"\n" + " end\n" + "end\n" + )}) + assert _calls(r, ".greet()", ".build_message()") + + +def test_bare_call_to_a_mixed_in_method_resolves(tmp_path): + r = _extract(tmp_path, {"m.rb": ( + "module Greetable\n" + " def greet\n" + " \"hi\"\n" + " end\n" + "end\n" + "class Person\n" + " include Greetable\n" + " def initialize\n" + " greet\n" + " end\n" + "end\n" + )}) + assert _calls(r, ".initialize()", ".greet()") + + +def test_bare_call_as_an_argument_is_a_call(tmp_path): + # `log hello` — `hello` is itself a paren-less send passed as the argument. + r = _extract(tmp_path, {"a.rb": ( + "class W\n" + " def run\n" + " log hello\n" + " end\n" + " def hello\n" + " end\n" + " def log(x)\n" + " end\n" + "end\n" + )}) + assert _calls(r, ".run()", ".hello()") + + +def test_a_local_variable_read_is_not_a_call(tmp_path): + # `build` is assigned first, so the later bare `build` is a variable read, + # not a send — even though a method named `build` exists. No edge. + r = _extract(tmp_path, {"v.rb": ( + "class W\n" + " def run\n" + " build = 1\n" + " build\n" + " end\n" + " def build\n" + " 2\n" + " end\n" + "end\n" + )}) + assert not _calls(r, ".run()", ".build()") + + +def test_a_parameter_read_is_not_a_call(tmp_path): + # A parameter named like a method shadows it; the bare read is the parameter. + r = _extract(tmp_path, {"p.rb": ( + "class W\n" + " def run(build)\n" + " build\n" + " end\n" + " def build\n" + " 1\n" + " end\n" + "end\n" + )}) + assert not _calls(r, ".run()", ".build()") + + +def test_a_block_parameter_read_is_not_a_call(tmp_path): + # `item` is a block parameter inside the method, not a send. + r = _extract(tmp_path, {"b.rb": ( + "class W\n" + " def run\n" + " [1].each do |item|\n" + " item\n" + " end\n" + " end\n" + " def item\n" + " end\n" + "end\n" + )}) + assert not _calls(r, ".run()", ".item()") + + +def test_self_send_and_bare_send_do_not_duplicate_an_edge(tmp_path): + r = _extract(tmp_path, {"s.rb": ( + "class W\n" + " def run\n" + " build\n" + " self.build\n" + " end\n" + " def build\n" + " end\n" + "end\n" + )}) + assert len(_calls(r, ".run()", ".build()")) == 1