From 7aa45fdac2f7f7899c78397d42694cd9967413d2 Mon Sep 17 00:00:00 2001 From: himanshupatro-334 Date: Tue, 25 Aug 2026 16:44:24 +0530 Subject: [PATCH] fix(ruby): preserve suffixed method node IDs --- graphify/extract.py | 14 ++++ graphify/extractors/engine.py | 11 ++- graphify/extractors/models.py | 3 + graphify/ruby_resolution.py | 7 +- tests/test_languages.py | 25 ++++++ tests/test_ruby_resolution.py | 141 ++++++++++++++++++++++++++++++++++ 6 files changed, 195 insertions(+), 6 deletions(-) diff --git a/graphify/extract.py b/graphify/extract.py index 89082af878..f68757a85f 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -901,6 +901,19 @@ def _get_c_func_name(node, source: bytes) -> str | None: 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 @@ def _get_c_func_name(node, source: bytes) -> str | None: 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( diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 00b50c2577..4ca625f460 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -4085,19 +4085,24 @@ def _emit_java_parent_type(type_node, rel: str, at_line: int) -> None: 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 diff --git a/graphify/extractors/models.py b/graphify/extractors/models.py index 63c1d8a181..bd8677aec1 100644 --- a/graphify/extractors/models.py +++ b/graphify/extractors/models.py @@ -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 diff --git a/graphify/ruby_resolution.py b/graphify/ruby_resolution.py index 33a5478b43..2d0d21b5f1 100644 --- a/graphify/ruby_resolution.py +++ b/graphify/ruby_resolution.py @@ -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 _emit(caller: str, target: str, rc: dict[str, Any], # 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 _emit(caller: str, target: str, rc: dict[str, Any], 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) diff --git a/tests/test_languages.py b/tests/test_languages.py index 4fddebac56..6fd3a45c55 100644 --- a/tests/test_languages.py +++ b/tests/test_languages.py @@ -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(): diff --git a/tests/test_ruby_resolution.py b/tests/test_ruby_resolution.py index 7bde4ffdc6..4141ca2185 100644 --- a/tests/test_ruby_resolution.py +++ b/tests/test_ruby_resolution.py @@ -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"