From 8cd79577eeff7019962369fd0be17b4626353ee1 Mon Sep 17 00:00:00 2001 From: Jaeung Jang Date: Sat, 22 Aug 2026 21:44:56 +0900 Subject: [PATCH] fix(js): scope factory object literals to the method, not the class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `const api = {}; api.foo = fn` inside a class method minted the owner node as `_make_id(function_owner_nid, name)`, and `function_owner_nid` is the enclosing *class* when one exists. The object is a local of the method, so two sibling methods that pick the same local name collapsed onto one node: class Registry { buildRead() { const api = {}; api.get = function () {}; return api; } buildWrite() { const api = {}; api.put = function () {}; return api; } } 0.9.48 emits a single `api` node carrying both `.get()` and `.put()`, and the identical `Registry -contains-> api` edge twice. The second is the duplication `contained_owners` exists to prevent — that set is scoped to one function, so it cannot see the sibling method. Key the object on `func_nid` instead. `this.X = fn` keeps `function_owner_nid`: `this` really is the instance, so those members belong to the class. --- graphify/extractors/engine.py | 14 ++++- tests/test_extract.py | 106 ++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index ab6ed0c90..1bfde6b87 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -4443,12 +4443,22 @@ def _emit_java_parent_type(type_node, rel: str, at_line: int) -> None: owner_nid = function_owner_nid elif tgt[0] == "object" and tgt[1] in object_bindings: object_name = tgt[1] - owner_nid = _make_id(function_owner_nid, object_name) + # Namespace the object by the function it is declared in, + # NOT by `function_owner_nid`. The two differ for a method: + # `this.X = fn` belongs to the class, but `const api = {}` + # is a local of the method. Keying the object on the class + # merged the factory objects of two sibling methods that + # picked the same local name, so the second method's + # members hung off the first method's node and the + # `contains` edge was emitted once per method — the very + # duplication `contained_owners` exists to prevent (it is + # scoped to one function, so it cannot see the sibling). + owner_nid = _make_id(func_nid, object_name) owner_line = object_bindings[object_name].start_point[0] + 1 add_node(owner_nid, object_name, owner_line) if owner_nid not in contained_owners: contained_owners.add(owner_nid) - add_edge(function_owner_nid, owner_nid, "contains", owner_line) + add_edge(func_nid, owner_nid, "contains", owner_line) else: continue m_name = tgt[2] diff --git a/tests/test_extract.py b/tests/test_extract.py index c9790e4ab..38112f5a1 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -934,6 +934,112 @@ def test_extract_js_factory_object_contains_edge_not_duplicated(tmp_path): assert len(methods) == 4 +def test_extract_js_factory_objects_in_sibling_methods_stay_separate(tmp_path): + """Two methods of one class that build same-named local factory objects must + get one node each. + + The object is a local of the method, so it has to be namespaced by the + method. Namespacing it by the enclosing class instead collapsed both + `const api = {}` locals onto a single node: `.get()` and `.put()` hung off + the same object and the second method's object never appeared at all. + """ + from graphify.extract import extract_js + f = tmp_path / "registry.js" + f.write_text( + "class Registry {\n" + " buildRead() {\n" + " const api = {};\n" + " api.get = function () { return 1; };\n" + " return api;\n" + " }\n" + " buildWrite() {\n" + " const api = {};\n" + " api.put = function () { return 2; };\n" + " return api;\n" + " }\n" + "}\n" + ) + result = extract_js(f) + by_label = {} + for n in result["nodes"]: + by_label.setdefault(n["label"], []).append(n) + + api_nodes = by_label.get("api", []) + assert len(api_nodes) == 2, ( + "each method's local object needs its own node, got " + f"{[n['id'] for n in api_nodes]}" + ) + + edges = {(e["source"], e["relation"], e["target"]) for e in result["edges"]} + read_nid = by_label[".buildRead()"][0]["id"] + write_nid = by_label[".buildWrite()"][0]["id"] + read_api = next(n["id"] for n in api_nodes + if (read_nid, "contains", n["id"]) in edges) + write_api = next(n["id"] for n in api_nodes + if (write_nid, "contains", n["id"]) in edges) + assert read_api != write_api + + # Each object owns only its own method. + assert (read_api, "method", by_label[".get()"][0]["id"]) in edges + assert (write_api, "method", by_label[".put()"][0]["id"]) in edges + assert (read_api, "method", by_label[".put()"][0]["id"]) not in edges + assert (write_api, "method", by_label[".get()"][0]["id"]) not in edges + + +def test_extract_js_factory_contains_edge_not_duplicated_across_methods(tmp_path): + """`contained_owners` is scoped to one function, so it cannot dedup across + sibling methods. With the object keyed on the class, both methods minted the + same owner id and emitted the identical `contains` edge twice.""" + from graphify.extract import extract_js + f = tmp_path / "dup.js" + f.write_text( + "class Builder {\n" + " one() {\n" + " const api = {};\n" + " api.a = () => 1;\n" + " return api;\n" + " }\n" + " two() {\n" + " const api = {};\n" + " api.b = () => 2;\n" + " return api;\n" + " }\n" + "}\n" + ) + result = extract_js(f) + contains = [(e["source"], e["target"]) for e in result["edges"] + if e["relation"] == "contains"] + assert len(contains) == len(set(contains)), ( + f"duplicate contains edges: {contains}" + ) + + +def test_extract_js_this_assignment_still_belongs_to_the_class(tmp_path): + """Control: `this.X = fn` keeps its class-level owner. + + Only the object-literal arm moves to the function; `this` in a constructor + really does refer to the instance, so both methods must land on the class. + """ + from graphify.extract import extract_js + f = tmp_path / "self.js" + f.write_text( + "class Handler {\n" + " constructor() {\n" + " this.onOpen = function () { return 1; };\n" + " }\n" + " wire() {\n" + " this.onClose = function () { return 2; };\n" + " }\n" + "}\n" + ) + result = extract_js(f) + by_label = {n["label"]: n["id"] for n in result["nodes"]} + cls = by_label["Handler"] + edges = {(e["source"], e["relation"], e["target"]) for e in result["edges"]} + assert (cls, "method", by_label[".onOpen()"]) in edges + assert (cls, "method", by_label[".onClose()"]) in edges + + def test_extract_js_factory_object_arrow_assigned_methods(tmp_path): """Arrow functions assigned to a factory object are captured just like function expressions (the dominant modern factory shape)."""