diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index b1e6959e1..3073008c9 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1246,8 +1246,13 @@ def _python_pre_scan_underscore_collisions(root_node, source: bytes, stem: str) way: ``__x``/``__x__``/``x`` all collapse too) occupies the public name (#3302). Returns ``{plain_nid: {raw names that would collide on it}}`` for every - module-level function and every direct method of a module-level class — the - two cases the id-minting code actually distinguishes via ``parent_class_nid``. + module-level function, module-level class, and direct method of a + module-level class — the cases the id-minting code actually distinguishes + via ``parent_class_nid``. Module-level classes share the functions' scope, so + ``class Session`` / ``def session()`` and ``class Foo`` / ``class FOO`` + (casefold) collide there too (#2810). Methods are keyed under their class's + *resolved* id: a salted class mints its methods under the salted id, so + keying them under the plain one would miss every method collision inside it. Deliberately does not recurse into nested classes or nested functions: this keeps the scope key trivially exact (matching ``_make_id(stem, class_name)``, which only holds because Python never populates ``namespace_stack``) rather @@ -1261,7 +1266,15 @@ def _python_pre_scan_underscore_collisions(root_node, source: bytes, stem: str) def _record(plain_nid: str, name: str) -> None: groups.setdefault(plain_nid, set()).add(name) - for child in root_node.children: + def _undecorated(node): + # `@dataclass class X` / `@property def x` wrap the definition in a + # decorated_definition; the walker mints the wrapped one's id unchanged. + if node.type == "decorated_definition": + return node.child_by_field_name("definition") or node + return node + + classes: list = [] # (class_name, body) for the method pass below + for child in map(_undecorated, root_node.children): if child.type == "function_definition": name_node = child.child_by_field_name("name") if name_node is not None: @@ -1276,16 +1289,21 @@ def _python_pre_scan_underscore_collisions(root_node, source: bytes, stem: str) class_name = _read_text(class_name_node, source) if not class_name: continue - class_nid = _make_id(stem, class_name) - for member in body.children: - if member.type != "function_definition": - continue - name_node = member.child_by_field_name("name") - if name_node is None: - continue - name = _read_text(name_node, source) - if name: - _record(_make_id(class_nid, name), name) + _record(_make_id(stem, class_name), class_name) + classes.append((class_name, body)) + + module_groups = {nid: names for nid, names in groups.items() if len(names) >= 2} + for class_name, body in classes: + class_nid = _underscore_salted_nid(_make_id(stem, class_name), class_name, module_groups) + for member in map(_undecorated, body.children): + if member.type != "function_definition": + continue + name_node = member.child_by_field_name("name") + if name_node is None: + continue + name = _read_text(name_node, source) + if name: + _record(_make_id(class_nid, name), name) return {nid: names for nid, names in groups.items() if len(names) >= 2} @@ -5727,6 +5745,8 @@ def _extract_generic( nid = _make_id(stem, ".".join(namespace_stack), name) if config.ts_module == "tree_sitter_c_sharp": nid = _underscore_salted_nid(nid, name, csharp_type_groups) + elif config.ts_module == "tree_sitter_python": + nid = _underscore_salted_nid(nid, name, python_underscore_groups) if nid in seen_ids: return nid nid = _make_id(name) @@ -5880,6 +5900,9 @@ def _extract_generic( class_nid = _underscore_salted_nid(class_nid, class_name, csharp_type_groups) if config.ts_module == "tree_sitter_python" and parent_class_nid: class_nid = _make_id(parent_class_nid, class_name) + elif config.ts_module == "tree_sitter_python": + # `Session` / `session()`, `Foo` / `FOO` in one module (#2810). + class_nid = _underscore_salted_nid(class_nid, class_name, python_underscore_groups) line = node.start_point[0] + 1 metadata = None ruby_reopened = ( diff --git a/tests/test_python_underscore_resolution.py b/tests/test_python_underscore_resolution.py index 3e3cd26e7..2e4b65f17 100644 --- a/tests/test_python_underscore_resolution.py +++ b/tests/test_python_underscore_resolution.py @@ -218,3 +218,209 @@ def test_collision_in_one_class_does_not_salt_unrelated_class(tmp_path: Path) -> assert b_foo["id"] == f"{class_b_id}_foo", ( "an unrelated class's non-colliding _foo was needlessly salted" ) + + +# --- Module-level classes join the collision map (#2810) ------------------- +# +# The pre-scan above records module-level functions and class methods but not +# module-level class names, so a class colliding with a function or another +# class on one id (`Session` / `session()`, `Foo` / `FOO`, `Pair` / `_Pair`) +# still lost one declaration to first-wins `add_node`, and which one survived +# depended on declaration order. + + +def _labels_to_ids(result: dict) -> dict[str, str]: + return {n["label"]: n["id"] for n in _rationale_free_nodes(result)} + + +def test_class_and_function_case_collision_both_extracted(tmp_path: Path) -> None: + """requests/sessions.py's shape: a class and its lowercase factory.""" + path = _write_py(tmp_path, ''' + class Session: + pass + + def session(): + return Session() + ''') + ids = _labels_to_ids(extract_python(path)) + assert "Session" in ids, f"class missing (#2810): {sorted(ids)}" + assert "session()" in ids, f"function missing (#2810): {sorted(ids)}" + assert ids["Session"] != ids["session()"] + + +def test_class_and_function_case_collision_is_order_independent(tmp_path: Path) -> None: + """Function declared first must not change which declarations survive or + what ids they get.""" + class_first = _labels_to_ids(extract_python(_write_py(tmp_path, ''' + class Session: + pass + + def session(): + return Session() + '''))) + func_first = _labels_to_ids(extract_python(_write_py(tmp_path, ''' + def session(): + return Session() + + class Session: + pass + '''))) + assert class_first["Session"] == func_first["Session"] + assert class_first["session()"] == func_first["session()"] + + +def test_class_and_class_case_collision_both_extracted(tmp_path: Path) -> None: + path = _write_py(tmp_path, ''' + class Foo: + pass + + class FOO: + pass + ''') + ids = _labels_to_ids(extract_python(path)) + assert "Foo" in ids and "FOO" in ids, f"a class was dropped: {sorted(ids)}" + assert ids["Foo"] != ids["FOO"] + + +def test_class_underscore_collision_public_class_keeps_plain_id(tmp_path: Path) -> None: + """`Pair` / `_Pair`: same unique-public-member rule as functions -- the + public class keeps the id it has with no private sibling.""" + solo_id = _labels_to_ids(extract_python(_write_py(tmp_path, ''' + class Pair: + pass + ''')))["Pair"] + ids = _labels_to_ids(extract_python(_write_py(tmp_path, ''' + class _Pair: + pass + + class Pair: + pass + '''))) + assert "_Pair" in ids and "Pair" in ids + assert ids["Pair"] == solo_id + assert ids["_Pair"] != solo_id + + +def test_methods_of_a_salted_class_keep_underscore_protection(tmp_path: Path) -> None: + """Once the class id is salted its methods are minted under the salted id, + so the pre-scan must key their collision groups under it too -- otherwise + `_get`/`get` inside the salted class collapse again.""" + path = _write_py(tmp_path, ''' + class Session: + def _get(self): + return 1 + + def get(self): + return self._get() + + def session(): + return Session() + ''') + ids = _labels_to_ids(extract_python(path)) + assert "._get()" in ids and ".get()" in ids, f"a method was dropped: {sorted(ids)}" + assert ids["._get()"] != ids[".get()"] + assert ids[".get()"].startswith(ids["Session"]), "method not scoped under its class" + + +def test_constructor_call_binds_to_the_salted_class(tmp_path: Path) -> None: + """`session()`'s `Session()` call must reach the class node, not dangle or + bind back to `session()` itself.""" + path = _write_py(tmp_path, ''' + class Session: + pass + + def session(): + return Session() + ''') + result = extract_python(path) + ids = _labels_to_ids(result) + pairs = {(e["source"], e["target"]) for e in result["edges"]} + assert (ids["session()"], ids["Session"]) in pairs, ( + f"no edge from session() to the Session class: {result['edges']}" + ) + assert (ids["session()"], ids["session()"]) not in pairs + + +def test_cross_file_import_resolves_to_the_salted_class(tmp_path: Path) -> None: + from graphify.build import build + from graphify.extract import extract + + (tmp_path / "sessions.py").write_text(textwrap.dedent(''' + class Session: + pass + + def session(): + return Session() + ''')) + (tmp_path / "api.py").write_text(textwrap.dedent(''' + from sessions import Session, session + + def make(): + return Session(), session() + ''')) + G = build([extract(sorted(tmp_path.glob("*.py")), root=tmp_path, cache_root=tmp_path)]) + label = {n: d.get("label") for n, d in G.nodes(data=True)} + targets = { + label.get(d.get("_tgt", v)) + for u, v, d in G.edges(data=True) + if label.get(d.get("_src", u)) in ("api.py", "make()") + } + assert "Session" in targets, f"nothing from api.py reaches the class: {targets}" + assert "session()" in targets, f"nothing from api.py reaches the function: {targets}" + + +def test_same_file_subclass_inherits_from_the_salted_class(tmp_path: Path) -> None: + """`class Sub(Session)` must point at the real (salted) class, not mint a + second sourceless `Session` stub under the plain id.""" + path = _write_py(tmp_path, ''' + class Session: + pass + + def session(): + return Session() + + class Sub(Session): + pass + ''') + result = extract_python(path) + nodes = _rationale_free_nodes(result) + assert [n["label"] for n in nodes].count("Session") == 1, ( + f"a phantom Session stub was minted: {[(n['id'], n['label']) for n in nodes]}" + ) + ids = _labels_to_ids(result) + inherits = {(e["source"], e["target"]) for e in result["edges"] if e.get("relation") == "inherits"} + assert (ids["Sub"], ids["Session"]) in inherits + + +def test_decorated_class_and_function_case_collision_both_extracted(tmp_path: Path) -> None: + """`@dataclass class Session` is wrapped in a decorated_definition; the + pre-scan must see through it.""" + path = _write_py(tmp_path, ''' + from dataclasses import dataclass + + @dataclass + class Session: + x: int = 0 + + def session(): + return Session() + ''') + ids = _labels_to_ids(extract_python(path)) + assert "Session" in ids and "session()" in ids, f"a declaration was dropped: {sorted(ids)}" + assert ids["Session"] != ids["session()"] + + +def test_decorated_method_underscore_collision_both_extracted(tmp_path: Path) -> None: + """The `_x` / `@property x` idiom: a decorated method collides like a bare one.""" + path = _write_py(tmp_path, ''' + class C: + def _x(self): + return 1 + + @property + def x(self): + return self._x() + ''') + ids = _labels_to_ids(extract_python(path)) + assert "._x()" in ids and ".x()" in ids, f"a method was dropped: {sorted(ids)}" + assert ids["._x()"] != ids[".x()"]