mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-26 16:26:42 +00:00
fix(ruby): preserve suffixed method node IDs (#3077)
normalize_id strips !/?/= from Ruby method names, so save and save! (or foo and foo=) minted the same node id and the second was dropped as a seen_ids collision. Add a Ruby symbol sanitizer (!->_bang, ?->_pred, =->_eq) applied to the id only (the label keeps the raw spelling), and rekey method_index on the raw name so member-call resolution still matches. Ids stay stable across incremental re-extraction.
This commit is contained in:
@@ -901,6 +901,19 @@ _CPP_CONFIG = LanguageConfig(
|
||||
resolve_function_name_fn=_get_cpp_func_name,
|
||||
)
|
||||
|
||||
def _ruby_sanitize_method_name(name: str) -> str:
|
||||
"""Encode trailing Ruby method suffixes (!, ?, =) into safe node ID components (#3077)."""
|
||||
if not name:
|
||||
return name
|
||||
if name.endswith("!"):
|
||||
return f"{name[:-1]}_bang"
|
||||
if name.endswith("?"):
|
||||
return f"{name[:-1]}_pred"
|
||||
if name.endswith("="):
|
||||
return f"{name[:-1]}_eq"
|
||||
return name
|
||||
|
||||
|
||||
_RUBY_CONFIG = LanguageConfig(
|
||||
ts_module="tree_sitter_ruby",
|
||||
# `module Foo` is a container node just like `class Foo` in tree-sitter's
|
||||
@@ -917,6 +930,7 @@ _RUBY_CONFIG = LanguageConfig(
|
||||
name_fallback_child_types=("constant", "scope_resolution", "identifier"),
|
||||
body_fallback_child_types=("body_statement",),
|
||||
function_boundary_types=frozenset({"method", "singleton_method"}),
|
||||
sanitize_symbol_name_fn=_ruby_sanitize_method_name,
|
||||
)
|
||||
|
||||
_CSHARP_CONFIG = LanguageConfig(
|
||||
|
||||
@@ -4178,19 +4178,24 @@ def _extract_generic(
|
||||
|
||||
if not func_name:
|
||||
return
|
||||
sanitized_name = (
|
||||
config.sanitize_symbol_name_fn(func_name)
|
||||
if config.sanitize_symbol_name_fn is not None
|
||||
else func_name
|
||||
)
|
||||
# A name that normalizes to nothing collapses `_make_id(prefix, name)`
|
||||
# onto the (absolute-path-derived) prefix, leaking the scan path and
|
||||
# colliding with the file/class node (#1899). No graph signal; skip.
|
||||
if not normalize_id(func_name):
|
||||
if not normalize_id(sanitized_name):
|
||||
return
|
||||
|
||||
line = node.start_point[0] + 1
|
||||
if parent_class_nid:
|
||||
func_nid = _make_id(parent_class_nid, func_name)
|
||||
func_nid = _make_id(parent_class_nid, sanitized_name)
|
||||
add_node(func_nid, f".{func_name}()", line)
|
||||
add_edge(parent_class_nid, func_nid, "method", line)
|
||||
else:
|
||||
func_nid = _make_id(stem, func_name)
|
||||
func_nid = _make_id(stem, sanitized_name)
|
||||
add_node(func_nid, f"{func_name}()", line)
|
||||
add_edge(file_nid, func_nid, "contains", line)
|
||||
callable_def_nids.add(func_nid) # function / method def is callable
|
||||
|
||||
@@ -47,6 +47,9 @@ class LanguageConfig:
|
||||
# Optional custom name resolver for functions (C, C++ declarator unwrapping)
|
||||
resolve_function_name_fn: Callable | None = None
|
||||
|
||||
# Optional symbol name sanitizer for node ID generation (e.g. Ruby suffixed methods)
|
||||
sanitize_symbol_name_fn: Callable[[str], str] | None = None
|
||||
|
||||
# Extra label formatting for functions: if True, functions get "name()" label
|
||||
function_label_parens: bool = True
|
||||
|
||||
|
||||
@@ -83,7 +83,8 @@ def resolve_ruby_member_calls(
|
||||
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)
|
||||
method_name = str(tnode.get("label", "")).strip("()").lstrip(".")
|
||||
method_index[(str(src), method_name)] = 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
|
||||
@@ -198,7 +199,7 @@ def resolve_ruby_member_calls(
|
||||
# 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))))
|
||||
method_nid = method_index.get((class_nid, str(callee)))
|
||||
_emit(caller, method_nid or class_nid, rc)
|
||||
continue
|
||||
|
||||
@@ -209,7 +210,7 @@ def resolve_ruby_member_calls(
|
||||
class_nid = _unique_class(str(receiver_type))
|
||||
if class_nid is None:
|
||||
continue
|
||||
method_nid = method_index.get((class_nid, _key(str(callee))))
|
||||
method_nid = method_index.get((class_nid, str(callee)))
|
||||
if method_nid is None:
|
||||
continue
|
||||
_emit(caller, method_nid, rc)
|
||||
|
||||
@@ -390,6 +390,31 @@ def test_ruby_inherits_edge():
|
||||
assert found, "TimeoutApiClient should have inherits edge to ApiClient"
|
||||
|
||||
|
||||
def test_ruby_suffixed_methods_survive_extraction(tmp_path: Path):
|
||||
"""#3077: foo, foo!, foo?, and foo= must all survive extraction with distinct IDs."""
|
||||
f = tmp_path / "service.rb"
|
||||
f.write_text("""\
|
||||
class Service
|
||||
def foo; end
|
||||
def foo!; end
|
||||
def foo?; end
|
||||
def foo=(val); end
|
||||
end
|
||||
""")
|
||||
r = extract_ruby(f)
|
||||
assert "error" not in r
|
||||
methods = [n for n in r["nodes"] if n["label"].startswith(".")]
|
||||
assert len(methods) == 4
|
||||
labels = {n["label"] for n in methods}
|
||||
assert labels == {".foo()", ".foo!()", ".foo?()", ".foo=()"}
|
||||
ids = {n["id"] for n in methods}
|
||||
assert len(ids) == 4
|
||||
assert any(i.endswith("_foo") for i in ids)
|
||||
assert any(i.endswith("_foo_bang") for i in ids)
|
||||
assert any(i.endswith("_foo_pred") for i in ids)
|
||||
assert any(i.endswith("_foo_eq") for i in ids)
|
||||
|
||||
|
||||
# ── C# ───────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_csharp_no_error():
|
||||
|
||||
@@ -422,3 +422,144 @@ def test_rake_files_extract_and_resolve_like_rb(tmp_path):
|
||||
calls = {(label.get(e["source"]), label.get(e["target"]))
|
||||
for e in result["edges"] if e["relation"] == "calls"}
|
||||
assert (".run()", ".tally()") in calls
|
||||
|
||||
|
||||
def test_ruby_suffixed_methods_extraction_and_labels(tmp_path: Path) -> None:
|
||||
"""#3077: def foo, def foo!, def foo?, and def foo=(val) in the same class
|
||||
must all survive extraction with distinct IDs and raw labels."""
|
||||
f = _write(tmp_path, "thing.rb", """\
|
||||
class Thing
|
||||
def foo; end
|
||||
def foo!; end
|
||||
def foo?; end
|
||||
def foo=(val); end
|
||||
end
|
||||
""")
|
||||
r = extract_ruby(f)
|
||||
assert "error" not in r
|
||||
method_nodes = [n for n in r["nodes"] if n["id"] != r["nodes"][0]["id"] and n.get("label") != "Thing"]
|
||||
assert len(method_nodes) == 4, f"Expected 4 distinct method nodes, got {method_nodes}"
|
||||
|
||||
node_by_label = {n["label"]: n["id"] for n in r["nodes"]}
|
||||
assert ".foo()" in node_by_label
|
||||
assert ".foo!()" in node_by_label
|
||||
assert ".foo?()" in node_by_label
|
||||
assert ".foo=()" in node_by_label
|
||||
|
||||
assert node_by_label[".foo()"].endswith("_foo")
|
||||
assert node_by_label[".foo!()"].endswith("_foo_bang")
|
||||
assert node_by_label[".foo?()"].endswith("_foo_pred")
|
||||
assert node_by_label[".foo=()"].endswith("_foo_eq")
|
||||
|
||||
# Verify all 4 method edges exist
|
||||
method_edges = [e for e in r["edges"] if e.get("relation") == "method"]
|
||||
assert len(method_edges) == 4
|
||||
targets = {e["target"] for e in method_edges}
|
||||
assert len(targets) == 4
|
||||
|
||||
|
||||
def test_ruby_suffixed_singleton_methods_extraction(tmp_path: Path) -> None:
|
||||
"""#3077: Singleton methods (def self.foo!) must use the same sanitizer."""
|
||||
f = _write(tmp_path, "service.rb", """\
|
||||
class Service
|
||||
def self.run!; end
|
||||
def self.valid?; end
|
||||
end
|
||||
""")
|
||||
r = extract_ruby(f)
|
||||
node_by_label = {n["label"]: n["id"] for n in r["nodes"]}
|
||||
assert ".run!()" in node_by_label
|
||||
assert ".valid?()" in node_by_label
|
||||
assert node_by_label[".run!()"].endswith("_run_bang")
|
||||
assert node_by_label[".valid?()"].endswith("_valid_pred")
|
||||
|
||||
|
||||
def test_ruby_suffixed_toplevel_functions_extraction(tmp_path: Path) -> None:
|
||||
"""#3077: Top-level functions (def parse!) must use the same sanitizer."""
|
||||
f = _write(tmp_path, "utils.rb", """\
|
||||
def parse!; end
|
||||
def valid?; end
|
||||
""")
|
||||
r = extract_ruby(f)
|
||||
node_by_label = {n["label"]: n["id"] for n in r["nodes"]}
|
||||
assert "parse!()" in node_by_label
|
||||
assert "valid?()" in node_by_label
|
||||
assert node_by_label["parse!()"].endswith("_parse_bang")
|
||||
assert node_by_label["valid?()"].endswith("_valid_pred")
|
||||
|
||||
|
||||
def test_ruby_suffixed_methods_call_resolution(tmp_path: Path) -> None:
|
||||
"""#3077: Calls to p.save and p.save! must resolve to different target nodes."""
|
||||
acc_path = _write(tmp_path, "account.rb", """\
|
||||
class Account
|
||||
def save
|
||||
1
|
||||
end
|
||||
def save!
|
||||
2
|
||||
end
|
||||
def valid?
|
||||
true
|
||||
end
|
||||
end
|
||||
""")
|
||||
client_path = _write(tmp_path, "client.rb", """\
|
||||
def perform_save
|
||||
a = Account.new
|
||||
a.save
|
||||
end
|
||||
|
||||
def perform_save_bang
|
||||
a = Account.new
|
||||
a.save!
|
||||
end
|
||||
|
||||
def perform_valid_query
|
||||
a = Account.new
|
||||
a.valid?
|
||||
end
|
||||
""")
|
||||
g = extract([acc_path, client_path], cache_root=tmp_path / ".cache", parallel=False)
|
||||
node_by_id = {n["id"]: n for n in g["nodes"]}
|
||||
|
||||
calls_by_caller = {}
|
||||
for e in g["edges"]:
|
||||
if e.get("relation") == "calls":
|
||||
caller_node = node_by_id.get(e["source"])
|
||||
target_node = node_by_id.get(e["target"])
|
||||
if caller_node and target_node:
|
||||
calls_by_caller.setdefault(caller_node["label"], []).append(target_node["label"])
|
||||
|
||||
assert ".save()" in calls_by_caller.get("perform_save()", []), \
|
||||
f"perform_save should call .save(), got {calls_by_caller.get('perform_save()')}"
|
||||
assert ".save!()" in calls_by_caller.get("perform_save_bang()", []), \
|
||||
f"perform_save_bang should call .save!(), got {calls_by_caller.get('perform_save_bang()')}"
|
||||
assert ".valid?()" in calls_by_caller.get("perform_valid_query()", []), \
|
||||
f"perform_valid_query should call .valid?(), got {calls_by_caller.get('perform_valid_query()')}"
|
||||
|
||||
|
||||
def test_ruby_suffixed_methods_id_stability(tmp_path: Path) -> None:
|
||||
"""#3077: ID of foo! must remain stable when foo is added later."""
|
||||
f1 = _write(tmp_path, "model.rb", """\
|
||||
class Model
|
||||
def foo!; end
|
||||
end
|
||||
""")
|
||||
r1 = extract_ruby(f1)
|
||||
node1 = next(n for n in r1["nodes"] if n.get("label") == ".foo!()")
|
||||
id1 = node1["id"]
|
||||
assert id1.endswith("_foo_bang")
|
||||
|
||||
# Add def foo
|
||||
f2 = _write(tmp_path, "model.rb", """\
|
||||
class Model
|
||||
def foo; end
|
||||
def foo!; end
|
||||
end
|
||||
""")
|
||||
r2 = extract_ruby(f2)
|
||||
node2_bang = next(n for n in r2["nodes"] if n.get("label") == ".foo!()")
|
||||
node2_plain = next(n for n in r2["nodes"] if n.get("label") == ".foo()")
|
||||
|
||||
assert node2_bang["id"] == id1, "foo!'s ID must remain unchanged when foo is added"
|
||||
assert node2_plain["id"] != node2_bang["id"], "foo and foo! must have distinct IDs"
|
||||
|
||||
Reference in New Issue
Block a user