mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-14 07:59:40 +00:00
fix(ruby): capture paren-less self-send calls
A receiver-less Ruby method call written without parentheses — the most
common call form in idiomatic Ruby — was dropped from the call graph:
def run
build # a method call on self
end
Tree-sitter parses that `build` as a bare `identifier`, structurally
identical to a local-variable read (`x`). The call-walk only inspected
`call` nodes, so `build(1)`, `self.build` and `build arg` all produced a
`calls` edge while the bare `build` produced nothing. A whole module of
mutually-calling helpers could end up with no internal edges at all.
The Ruby resolver already had a dedicated bare-call promotion path, but it
had nothing to promote because extraction never produced the call.
Fix: mirror Ruby's own disambiguation rule — a bare identifier is a send on
`self` unless a local or parameter of that name is in scope. `_ruby_local_names`
collects the bound names of a method body (parameters, assignments, block
params, `for` variables, `rescue` captures); a bare identifier not among them
is routed through the normal callee path as a non-member call, resolving
in-file (EXTRACTED) or becoming a raw_call the resolver can prove.
The collector is deliberately over-inclusive, so a missed binding only
suppresses a candidate call (fail-closed) rather than inventing a wrong edge.
A receiver-position send (`logger.info` where `logger` is itself a send) is
left for a follow-up and stays unresolved rather than guessed.
Added tests/test_ruby_bare_self_send_calls.py covering the self-send, the
cross-module and mixed-in cases, and the negatives (a local, a parameter and
a block parameter that shadow a method name must NOT become calls).
(cherry picked from commit c95b11c8e6)
This commit is contained in:
1 parent
18b1498009
commit
7340b82812
2 files changed
+315
-2
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user