From 03beac9d09e9c4ab67e55c4481a36d85b2546fdc Mon Sep 17 00:00:00 2001 From: Mpasha17 Date: Thu, 15 Oct 2026 20:52:46 +0530 Subject: [PATCH] fix(js): keep class members that differ only by _, $ or # as separate nodes make_id drops _, $ and #, so updateHash/_updateHash, close/#close and set/$set in one JS/TS class body minted one id and the second member disappeared, with its calls attributed to the survivor. Group each class body's member names by id and reuse the Python/C# collision rule (_underscore_salted_nid, now with configurable private prefixes): a unique public name keeps its id, colliding private ones get a salted id. Non-colliding members keep their ids. The TS cross-file member resolver now matches the exact member name before falling back to the normalized key, so c.set() still binds to set and not $set. Fixes #4475 Co-Authored-By: Grok Bot --- graphify/extract.py | 12 ++- graphify/extractors/engine.py | 67 +++++++++++++-- tests/test_js_member_collisions.py | 129 +++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+), 7 deletions(-) create mode 100644 tests/test_js_member_collisions.py diff --git a/graphify/extract.py b/graphify/extract.py index 327ee2ec9..2c715ac36 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -5044,6 +5044,10 @@ def _resolve_typescript_member_calls( type_def_nids.setdefault(_key(n.get("label", "")), []).append(n["id"]) method_index: dict[tuple[str, str], str] = {} + # Exact member name -> node. `_key` drops `_`/`$`/`#`, and since #4475 + # `updateHash` / `_updateHash` are two nodes of one class, so the exact + # name is tried first and `_key` stays the fallback. + method_exact: dict[tuple[str, str], str] = {} class_of_method: dict[str, str] = {} for e in all_edges: if e.get("relation") != "method": @@ -5052,6 +5056,7 @@ def _resolve_typescript_member_calls( tnode = node_by_id.get(tgt) if tnode is not None: method_index[(src, _key(tnode.get("label", "")))] = tgt + method_exact[(src, str(tnode.get("label", "")).removesuffix("()").removeprefix("."))] = tgt if isinstance(src, str) and isinstance(tgt, str): class_of_method[tgt] = src @@ -5181,7 +5186,10 @@ def _resolve_typescript_member_calls( if len(visible) != 1: continue type_nid = visible[0] - method_nid = method_index.get((type_nid, _key(callee))) + method_nid = method_exact.get((type_nid, str(callee))) + exact = method_nid is not None + if not exact: + method_nid = method_index.get((type_nid, _key(callee))) if not method_nid: # Receiver typed, but the type has no such method. The old fallback # (a `references` edge to the type node) was another fabrication @@ -5192,7 +5200,7 @@ def _resolve_typescript_member_calls( # declares a public `foo` (the node is marked); otherwise its `foo` comes # from elsewhere and the private method is not the target. method_node = node_by_id.get(method_nid, {}) - if (str(method_node.get("label", "")).lstrip(".").startswith("#") + if (not exact and str(method_node.get("label", "")).lstrip(".").startswith("#") and not str(callee).startswith("#") and not method_node.get("_ts_public_twin")): continue diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index b1e6959e1..3357fa74a 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1290,7 +1290,8 @@ def _python_pre_scan_underscore_collisions(root_node, source: bytes, stem: str) return {nid: names for nid, names in groups.items() if len(names) >= 2} -def _underscore_salted_nid(plain_nid: str, name: str, groups: dict[str, set[str]]) -> str: +def _underscore_salted_nid(plain_nid: str, name: str, groups: dict[str, set[str]], + private_prefixes: tuple[str, ...] = ("_",)) -> str: """Resolve a declaration's real node id against the collision map. Shared by the Python and C# pre-scans (#3302, #4415). A name with no @@ -1303,17 +1304,63 @@ def _underscore_salted_nid(plain_nid: str, name: str, groups: dict[str, set[str] `_x`/`__x` colliding with no public member at all) is salted, so the outcome never depends on declaration order — mirrors the exported/unexported rule the Go extractor uses for its own case-only collision (#2779). + ``private_prefixes`` widens what counts as non-public: JS/TS also marks + members private with ``$`` and ``#`` (#4475). """ names = groups.get(plain_nid) if not names or len(names) < 2: return plain_nid - public = [n for n in names if not n.startswith("_")] + public = [n for n in names if not n.startswith(private_prefixes)] if len(public) == 1 and name == public[0]: return plain_nid salt = hashlib.sha1(name.encode("utf-8"), usedforsecurity=False).hexdigest()[:6] return _make_id(plain_nid, salt) +_JS_PRIVATE_MEMBER_PREFIXES = ("_", "$", "#") +_JS_MEMBER_METHOD_TYPES = ("method_definition", "method_signature", "abstract_method_signature") + + +def _js_member_nid(class_nid: str, member_node, name: str, source: bytes, + groups_cache: dict) -> str: + """Node id for a JS/TS class member, salted when a sibling collides (#4475). + + ``make_id`` drops ``_``, ``$`` and ``#``, so ``updateHash`` / ``_updateHash``, + ``close`` / ``#close`` and ``set`` / ``$set`` in one class body mint one id + and the second declaration vanished. The member names of the enclosing + class body are grouped by the id they would get (methods, signatures and + function-valued fields, the members the walk turns into ``.name()`` nodes), + then ``_underscore_salted_nid`` applies the Python/C# rule: a unique public + name keeps the plain id, every other colliding name gets a salted one. + Members that collide with nothing keep their old id. Getter/setter pairs + and overloads share one name, so they are not a collision. + """ + body = member_node.parent + if body is None: + return _make_id(class_nid, name) + key = (class_nid, body.start_byte, body.end_byte) + groups = groups_cache.get(key) + if groups is None: + names: dict[str, set[str]] = {} + for c in body.children: + if c.type in _JS_MEMBER_METHOD_TYPES: + name_node = c.child_by_field_name("name") + elif c.type in ("field_definition", "public_field_definition"): + value = c.child_by_field_name("value") + if value is None or value.type not in _JS_FUNCTION_VALUE_TYPES: + continue + name_node = c.child_by_field_name("property") or c.child_by_field_name("name") + else: + continue + text = _read_text(name_node, source) if name_node is not None else "" + if text: + names.setdefault(_make_id(class_nid, text), set()).add(text) + groups = {nid: ns for nid, ns in names.items() if len(ns) >= 2} + groups_cache[key] = groups + return _underscore_salted_nid(_make_id(class_nid, name), name, groups, + _JS_PRIVATE_MEMBER_PREFIXES) + + def _swift_pre_scan(root_node, source: bytes) -> tuple[set[str], set[str]]: """Pre-scan a Swift compilation unit and return (protocol_names, class_like_names).""" protocols: set[str] = set() @@ -3927,7 +3974,7 @@ def _js_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: str, callable_def_nids: set | None = None, local_bound_names: dict | None = None, closure_locals_by_body: dict | None = None, - config=None) -> bool: + config=None, js_member_groups: dict | None = None) -> bool: """Handle lexical_declaration (arrow functions, CJS requires, module-level const literals) for JS/TS. Returns True if handled.""" # CommonJS / prototype member assignments whose value is a function: # exports.X = () => {} → file-contained function X() @@ -4065,7 +4112,8 @@ def _js_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: str, field_name = _read_text(prop, source) if field_name: line = node.start_point[0] + 1 - nid = _make_id(parent_class_nid, field_name) + nid = _js_member_nid(parent_class_nid, node, field_name, source, + js_member_groups if js_member_groups is not None else {}) add_node_fn(nid, f".{field_name}()", line) add_edge_fn(parent_class_nid, nid, "method", line) if callable_def_nids is not None: @@ -5677,6 +5725,9 @@ def _extract_generic( if config.ts_module == "tree_sitter_swift": swift_protocol_names, swift_class_names = _swift_pre_scan(root, source) + # JS/TS class body -> member collision groups, filled lazily (#4475). + js_member_groups: dict = {} + python_underscore_groups: dict[str, set[str]] = {} if config.ts_module == "tree_sitter_python": python_underscore_groups = _python_pre_scan_underscore_collisions(root, source, stem) @@ -7333,6 +7384,11 @@ def _extract_generic( func_nid = _underscore_salted_nid( func_nid, sanitized_name, csharp_member_groups ) + elif (config.ts_module in ("tree_sitter_javascript", "tree_sitter_typescript") + and t in _JS_MEMBER_METHOD_TYPES): + func_nid = _js_member_nid( + parent_class_nid, node, sanitized_name, source, js_member_groups + ) ruby_method_metadata = None if ruby_method_kind is not None: kinds = ruby_method_kinds.setdefault(func_nid, set()) @@ -7923,7 +7979,8 @@ def _extract_generic( nodes, edges, seen_ids, function_bodies, parent_class_nid, add_node, add_edge, callable_def_nids, local_bound_names, - closure_locals_by_body, config=config): + closure_locals_by_body, config=config, + js_member_groups=js_member_groups): return # TS enum members, and namespace / module containers diff --git a/tests/test_js_member_collisions.py b/tests/test_js_member_collisions.py new file mode 100644 index 000000000..c302b2604 --- /dev/null +++ b/tests/test_js_member_collisions.py @@ -0,0 +1,129 @@ +"""#4475: JS/TS class members that differ only by `_`, `$` or `#` stay separate nodes.""" +from __future__ import annotations + +import warnings +from pathlib import Path + +import pytest + +from graphify.extract import extract + + +def _extract(tmp_path: Path, files: dict[str, str]) -> dict: + for name, src in files.items(): + (tmp_path / name).write_text(src, encoding="utf-8") + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + return extract([tmp_path / n for n in files], cache_root=tmp_path, parallel=False) + + +def _members(r: dict) -> dict[str, str]: + return {n["label"]: n["id"] for n in r["nodes"] if n["label"].startswith(".")} + + +def _calls(r: dict) -> set[tuple[str, str]]: + label = {n["id"]: n["label"] for n in r["nodes"]} + return {(label.get(e["source"]), label.get(e["target"])) for e in r["edges"] if e["relation"] == "calls"} + + +@pytest.mark.parametrize("fname,src,expected", [ + ("a.js", "export class R { _onBegin(a) { return a; } onBegin(b) { return b; } }\n", {"._onBegin()", ".onBegin()"}), + ("a.js", "export class W { #close(a) { return a; } close(b) { return this.#close(b); } }\n", {".#close()", ".close()"}), + ("a.ts", "export declare class Q { $set(a: number): void; set(b: number): void; }\n", {".$set()", ".set()"}), + ("a.js", "export class A {\n updateHash() { return 1; }\n _updateHash() { return 2; }\n}\n", {".updateHash()", "._updateHash()"}), + ("a.ts", "export abstract class B { abstract _run(): void; abstract run(): void; }\n", {"._run()", ".run()"}), + ("a.js", "export class F { _go = () => 1; go = () => 2; }\n", {"._go()", ".go()"}), +]) +def test_issue_cases_get_two_nodes(tmp_path, fname, src, expected): + assert expected <= set(_members(_extract(tmp_path, {fname: src}))) + + +def test_public_member_keeps_the_plain_id_and_private_one_is_salted(tmp_path): + m = _members(_extract(tmp_path, {"a.js": "export class W { #close(a) { return a; } close(b) { return b; } }\n"})) + assert m[".close()"] == "a_w_close" + assert m[".#close()"].startswith("a_w_close_") and m[".#close()"] != "a_w_close" + + +def test_order_does_not_matter(tmp_path): + (tmp_path / "1").mkdir() + (tmp_path / "2").mkdir() + a = _members(_extract(tmp_path / "1", {"a.js": "export class A { _x() {} x() {} }\n"})) + b = _members(_extract(tmp_path / "2", {"a.js": "export class A { x() {} _x() {} }\n"})) + assert a == b + + +def test_no_public_member_salts_both(tmp_path): + m = _members(_extract(tmp_path, {"a.js": "export class P { _x() { return 1; } $x() { return 2; } }\n"})) + assert set(m) == {"._x()", ".$x()"} + assert "a_p_x" not in m.values() and len(set(m.values())) == 2 + + +def test_non_colliding_member_ids_unchanged(tmp_path): + m = _members(_extract(tmp_path, {"a.ts": ( + "export class C {\n run() {}\n _helper() {}\n #secret() {}\n $id() {}\n" + " get size() { return 1; }\n set size(v) {}\n static make() {}\n" + " over(a: number): void;\n over(a: string): void;\n over(a: any) {}\n}\n")})) + assert m == {".run()": "a_c_run", "._helper()": "a_c_helper", ".#secret()": "a_c_secret", + ".$id()": "a_c_id", ".size()": "a_c_size", ".make()": "a_c_make", ".over()": "a_c_over"} + + +def test_call_edges_go_to_the_right_member(tmp_path): + r = _extract(tmp_path, {"w.js": ( + "export class W {\n #close(a) { return helper(a); }\n close(b) { return this.#close(b); }\n" + " shut() { return this.close(1); }\n}\nfunction helper(x) { return x; }\n")}) + calls = _calls(r) + assert (".close()", ".#close()") in calls + assert (".shut()", ".close()") in calls + assert (".#close()", "helper()") in calls # the private body's own call is credited to it + assert (".close()", "helper()") not in calls + + +@pytest.mark.parametrize("call,target", [("a.updateHash()", ".updateHash()"), ("a._updateHash()", "._updateHash()")]) +@pytest.mark.parametrize("cross_file", [False, True]) +def test_callers_reach_the_right_member(tmp_path, call, target, cross_file): + cls = "export class A {\n updateHash() { return 1; }\n _updateHash() { return 2; }\n}\n" + fn = f"export function f() {{ const a = new A(); return {call}; }}\n" + files = ({"a.js": cls, "u.js": "import { A } from './a.js';\n" + fn} if cross_file else {"a.js": cls + fn}) + members = {t for s, t in _calls(_extract(tmp_path, files)) if s == "f()" and t and t.startswith(".")} + assert members == {target} + + +def test_cross_file_ts_typed_receiver_hits_each_member(tmp_path): + r = _extract(tmp_path, { + "ctx.ts": "export class Ctx {\n set(v: number): void {}\n $set(v: number): void {}\n}\n", + "use.ts": ('import { Ctx } from "./ctx";\n' + "export function a(c: Ctx) { c.set(1); }\nexport function b(c: Ctx) { c.$set(1); }\n"), + }) + calls = _calls(r) + assert ("a()", ".set()") in calls and ("a()", ".$set()") not in calls + assert ("b()", ".$set()") in calls and ("b()", ".set()") not in calls + + +def test_dedup_build_and_merge_keep_both(tmp_path): + from graphify.build import build, build_merge + from graphify.cluster import cluster + from graphify.export import to_json + src = {"a.js": "export class A {\n close() { return this.#close(); }\n #close() { return 1; }\n}\n"} + r = _extract(tmp_path, src) + G = build([r], dedup=True, root=str(tmp_path)) + labels = {G.nodes[n]["label"] for n in G.nodes} + assert {".close()", ".#close()"} <= labels + to_json(G, cluster(G), str(tmp_path / "graph.json")) + G2 = build_merge([_extract(tmp_path, src)], graph_path=str(tmp_path / "graph.json"), root=str(tmp_path)) + assert set(G2.nodes) == set(G.nodes) + + + +def test_pre_fix_ast_cache_entry_is_never_served_for_js(tmp_path): + """JS/TS bypass the AST cache, so a collapsed entry left by an older run can't resurface (#4475).""" + from graphify.cache import save_cached + from graphify.extract import cache_salt, extract_js + src = tmp_path / "a.js" + src.write_text("export class A {\n updateHash() { return 1; }\n _updateHash() { return 2; }\n}\n", encoding="utf-8") + stale = extract_js(src) + stale["nodes"] = [n for n in stale["nodes"] if n.get("label") != "._updateHash()"] + save_cached(src, stale, root=tmp_path, cache_root=tmp_path, kind="ast", salt=cache_salt(src)) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + r = extract([src], root=tmp_path, cache_root=tmp_path, parallel=False) + assert {".updateHash()", "._updateHash()"} <= {n["label"] for n in r["nodes"]}