From 22bc4c6ab488d53cb0a3452c429c96c24468a9d3 Mon Sep 17 00:00:00 2001 From: rohit-jsfreaky Date: Sun, 11 Oct 2026 20:47:55 +0100 Subject: [PATCH] fix(affected): walk callers of the supertype member an overriding method overrides (#4330) Landed from PR #4331 (squashed, authorship preserved). --- graphify/affected.py | 71 ++++++++++++- tests/test_affected_overrides.py | 173 +++++++++++++++++++++++++++++++ 2 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 tests/test_affected_overrides.py diff --git a/graphify/affected.py b/graphify/affected.py index 4289c6684..b011f2756 100644 --- a/graphify/affected.py +++ b/graphify/affected.py @@ -40,6 +40,11 @@ DEFAULT_AFFECTED_RELATIONS = ( ) +# Edges from a type to the types it derives from. A method declared again in a +# subtype overrides the supertype's same-named member. +_SUPERTYPE_RELATIONS = frozenset({"inherits", "extends", "implements"}) + + @dataclass(frozen=True) class AffectedHit: node_id: str @@ -195,6 +200,47 @@ def resolve_seed(graph: nx.Graph, query: str, root: Path | None = None) -> str | return None +def _supertypes(graph: nx.DiGraph, type_id: str, cache: dict[str, list[str]]) -> list[str]: + """Every type ``type_id`` derives from, nearest first (cycles cut).""" + if type_id in cache: + return cache[type_id] + found: list[str] = [] + seen = {type_id} + frontier = [type_id] + while frontier: + nxt = [] + for t in frontier: + for _s, base, data in graph.out_edges(t, data=True): + base = str(base) + if str(data.get("relation", "")) in _SUPERTYPE_RELATIONS and base not in seen: + seen.add(base) + found.append(base) + nxt.append(base) + frontier = nxt + cache[type_id] = found + return found + + +def _overridden_members(graph: nx.DiGraph, node: str, cache: dict[str, list[str]]) -> list[str]: + """The supertype members ``node`` overrides: same-named methods of the types + its class derives from. A call bound to one of them can run ``node`` at + runtime (`visitor.head()` typed `NodeVisitor` runs `FormattingVisitor.head`).""" + owners = [str(s) for s, _t, d in graph.in_edges(node, data=True) + if str(d.get("relation", "")) == "method"] + if not owners: + return [] + label = graph.nodes[node].get("label") + found: list[str] = [] + for owner in owners: + for base in _supertypes(graph, owner, cache): + for _s, member, data in graph.out_edges(base, data=True): + member = str(member) + if (str(data.get("relation", "")) == "method" and member != node + and graph.nodes[member].get("label") == label): + found.append(member) + return found + + def affected_nodes( graph: nx.Graph, seed: str, @@ -228,10 +274,27 @@ def affected_nodes( seen.add(member) queue.append((member, 0)) + supertype_cache: dict[str, list[str]] = {} + # Overridden members walked without being reported; one that a real edge + # also reaches (an overload calling the abstract one) is reported then. + stepping_stones: set[str] = set() while queue: current, current_depth = queue.popleft() if current_depth >= depth: continue + # A caller bound to the supertype member a method overrides depends on + # the override too, so walk that member's callers at the same depth. The + # member itself did not change: like the seed's members above, it is a + # stepping stone, not a hit. Needs edge direction, so directed graphs only + # (MultiDiGraph, what load_graph returns, is a DiGraph subclass). + if isinstance(graph, nx.DiGraph): + for overridden in _overridden_members(graph, current, supertype_cache): + if overridden not in seen: + seen.add(overridden) + stepping_stones.add(overridden) + # front of the queue: same depth as `current`, so it must be + # walked before the deeper nodes already queued + queue.appendleft((overridden, current_depth)) if hasattr(graph, "in_edges"): incoming = graph.in_edges(current, data=True) else: @@ -245,7 +308,10 @@ def affected_nodes( if relation not in relation_set: continue source = str(source) - if source in seen: + queued = source in stepping_stones + if queued: + stepping_stones.discard(source) # walked already; report it now + elif source in seen: continue seen.add(source) # Carry the matched edge's location (taken from the SAME edge dict @@ -258,7 +324,8 @@ def affected_nodes( via_location=str(data.get("source_location") or "") or None, ) hits.append(hit) - queue.append((source, current_depth + 1)) + if not queued: + queue.append((source, current_depth + 1)) return hits diff --git a/tests/test_affected_overrides.py b/tests/test_affected_overrides.py new file mode 100644 index 000000000..55252982a --- /dev/null +++ b/tests/test_affected_overrides.py @@ -0,0 +1,173 @@ +"""`affected` on a method that overrides a supertype member must reach the callers bound to that member. + +A call site names the declared type: `visitor.head()` with `visitor: NodeVisitor` binds to +`NodeVisitor.head`, and at runtime runs `FormattingVisitor.head`. Changing the override therefore +affects those callers, although no edge points at the override. The overridden member is walked as a +stepping stone (it did not change), unless a real edge reaches it too. +""" +from __future__ import annotations + +import json +from pathlib import Path + +import networkx as nx + +from graphify.affected import affected_nodes, load_graph +from graphify.extract import extract + + +def _graph(*edges: tuple[str, str, str]) -> nx.DiGraph: + g = nx.DiGraph() + for src, tgt, rel in edges: + for nid in (src, tgt): + if nid not in g: + g.add_node(nid, label=f".{nid.split('.')[-1]}()" if "." in nid else nid) + g.add_edge(src, tgt, relation=rel) + return g + + +def _hits(g, seed, depth=2): + return {h.node_id: h.depth for h in affected_nodes(g, seed, depth=depth)} + + +def test_caller_of_the_overridden_interface_method_is_affected(): + g = _graph( + ("Visitor", "Visitor.head", "method"), + ("Impl", "Impl.head", "method"), + ("Impl", "Visitor", "implements"), + ("walk", "Visitor.head", "calls"), + ) + hits = _hits(g, "Impl.head") + assert hits.get("walk") == 1 + assert "Visitor.head" not in hits, "the overridden member did not change" + + +def test_override_through_a_superclass_chain(): + g = _graph( + ("Root", "Root.close", "method"), + ("Mid", "Root", "inherits"), + ("Leaf", "Mid", "inherits"), + ("Leaf", "Leaf.close", "method"), + ("client", "Root.close", "calls"), + ("outer", "client", "calls"), + ) + hits = _hits(g, "Leaf.close") + assert hits == {"client": 1, "outer": 2} + + +def test_overridden_member_reached_by_a_real_edge_is_reported(): + # An overload in the base calls the abstract member the subclass overrides; + # both share one node, so the base member really is a caller. + g = _graph( + ("Base", "Base.read", "method"), + ("Base.read", "Base.read_all", "calls"), + ("Base", "Base.read_all", "method"), + ("Impl", "Base", "inherits"), + ("Impl", "Impl.read", "method"), + ("Base.read_all", "Base.read", "calls"), + ) + hits = _hits(g, "Impl.read") + assert hits.get("Base.read_all") == 1 + + +def test_recursive_overridden_member_is_a_caller_of_the_override(): + # `Base.m` calling `m()` on its own object dispatches to `Impl.m` for an Impl + # instance, so the self-loop is a real edge into it: Base.m is affected. + g = _graph( + ("Base", "Base.m", "method"), + ("Base.m", "Base.m", "calls"), + ("Impl", "Base", "inherits"), + ("Impl", "Impl.m", "method"), + ) + assert _hits(g, "Impl.m") == {"Base.m": 1} + + +def test_same_depth_keeps_the_depth_limit_exact(): + # depth=1 must still list the supertype member's direct callers. + g = _graph( + ("Iface", "Iface.run", "method"), + ("Impl", "Iface", "implements"), + ("Impl", "Impl.run", "method"), + ("a", "Iface.run", "calls"), + ("b", "a", "calls"), + ) + assert _hits(g, "Impl.run", depth=1) == {"a": 1} + + +def test_unrelated_same_named_method_is_not_walked(): + g = _graph( + ("Other", "Other.head", "method"), + ("Impl", "Impl.head", "method"), + ("walk", "Other.head", "calls"), + ) + assert "walk" not in _hits(g, "Impl.head") + + +def test_inheritance_cycle_terminates(): + g = _graph( + ("A", "B", "inherits"), + ("B", "A", "inherits"), + ("A", "A.m", "method"), + ("B", "B.m", "method"), + ("x", "B.m", "calls"), + ) + assert _hits(g, "A.m") == {"x": 1} + + +def test_undirected_graph_is_unchanged(): + g = _graph( + ("Visitor", "Visitor.head", "method"), + ("Impl", "Impl.head", "method"), + ("Impl", "Visitor", "implements"), + ("walk", "Visitor.head", "calls"), + ).to_undirected() + # no edge direction, no way to tell a subtype from a supertype: no expansion + assert "walk" not in _hits(g, "Impl.head") + + +def _extract_graph(tmp_path: Path, files: dict[str, str]) -> nx.Graph: + paths = [] + for name, body in files.items(): + p = tmp_path / name + p.write_text(body, encoding="utf-8") + paths.append(p) + result = extract(paths, cache_root=tmp_path / "graphify-out") + # loaded the way `graphify affected` loads graph.json (parallel edges kept) + graph_json = tmp_path / "graph.json" + graph_json.write_text(json.dumps({"nodes": result["nodes"], "links": result["edges"]}), encoding="utf-8") + return load_graph(graph_json) + + +def _node(g, label, id_suffix): + return next(n for n, a in g.nodes(data=True) if a.get("label") == label and n.endswith(id_suffix)) + + +def test_java_interface_caller_end_to_end(tmp_path: Path): + g = _extract_graph(tmp_path, { + "NodeVisitor.java": "interface NodeVisitor { void head(String n); }\n", + "Printer.java": "class Printer implements NodeVisitor { public void head(String n) {} }\n", + "Walker.java": "class Walker { void walk(NodeVisitor v) { v.head(\"x\"); } }\n", + }) + hits = _hits(g, _node(g, ".head()", "printer_head")) + assert _node(g, ".walk()", "walker_walk") in hits + + +def test_typescript_abstract_method_caller_end_to_end(tmp_path: Path): + g = _extract_graph(tmp_path, { + "cache.ts": "export abstract class Cache { abstract connect(): void; }\n", + "db.ts": ('import { Cache } from "./cache";\n' + "export class DbCache extends Cache { connect(): void {} }\n"), + "app.ts": ('import { Cache } from "./cache";\n' + "export function boot(c: Cache) { c.connect(); }\n"), + }) + hits = _hits(g, _node(g, ".connect()", "dbcache_connect")) + assert _node(g, "boot()", "app_boot") in hits + + +def test_python_base_method_caller_end_to_end(tmp_path: Path): + g = _extract_graph(tmp_path, { + "base.py": "class Base:\n def run(self):\n pass\n\n def go(self):\n self.run()\n", + "impl.py": "from base import Base\n\n\nclass Impl(Base):\n def run(self):\n pass\n", + }) + hits = _hits(g, _node(g, ".run()", "impl_impl_run")) + assert _node(g, ".go()", "base_base_go") in hits