mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-16 17:09:17 +00:00
fix(build): keep a method edge when a member constructs its own type
The default graph keeps one undirected edge per node pair. A factory
that returns new Report() also has a calls edge back to Report, and
that call was the one that survived. Report then had no method edge
to Fault, so anything that walks a type's members missed it.
A method or contains edge now wins over a calls edge in the opposite
direction. A directed graph still keeps both.
Fixes #4248
Co-Authored-By: Grok Bot <noreply@x.ai>
(cherry picked from commit 95546d6784)
This commit is contained in:
1 parent
a7040f53db
commit
2250f205b9
3 files changed
+118
-7
No files matched your search
+30
-5
@@ -73,10 +73,21 @@ def _backfill_origin(item: dict) -> None:
|
||||
# inheritance — routinely emits one of these for the same pair as well, so when the
|
||||
# simple graph collapses the pair to one edge, the generic one must never be the
|
||||
# survivor. Deliberately a small denylist rather than a full precedence order over
|
||||
# every relation: ranking `contains` against `calls` would be inventing a
|
||||
# cross-axis judgement, whereas "specific beats generic" is the only comparison
|
||||
# this collapse actually needs.
|
||||
# every relation: ranking `contains` against `calls` along the SAME direction would
|
||||
# be inventing a cross-axis judgement, whereas "specific beats generic" is the only
|
||||
# same-direction comparison this collapse needs. The one opposite-direction
|
||||
# exception is _STRUCTURAL_MEMBER_RELATIONS.
|
||||
_GENERIC_RELATIONS: frozenset[str] = frozenset({"references", "uses", "mentions"})
|
||||
# Membership of a type or file. On an undirected graph these share a node pair
|
||||
# with a `calls` edge in the opposite direction when a member calls or constructs
|
||||
# its owner (`static Report Fault() { return new Report(); }`). Last-write kept
|
||||
# `calls` whenever the member id sorted after the owner id, so the membership
|
||||
# edge never reached graph.json and the member disappeared from type walks
|
||||
# (member lookup, explain, call resolution into the type). The membership edge
|
||||
# wins; the call is what this collapse drops. Directed graphs keep both, because
|
||||
# the two directions are different arcs. Same-direction `contains`/`method` vs
|
||||
# `calls` is not ranked here.
|
||||
_STRUCTURAL_MEMBER_RELATIONS: frozenset[str] = frozenset({"method", "contains"})
|
||||
_CONFIDENCE_RANK: dict[str, int] = {"EXTRACTED": 3, "INFERRED": 2, "AMBIGUOUS": 1}
|
||||
|
||||
# Import-family relations whose target may legitimately be a module OUTSIDE the
|
||||
@@ -1557,8 +1568,22 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
# reverse-direction duplicate so the original direction is preserved (#1061).
|
||||
if not G.is_directed() and G.has_edge(src, tgt):
|
||||
existing = edge_data(G, src, tgt)
|
||||
if existing.get("relation") == attrs.get("relation") and (
|
||||
existing.get("_src") == tgt and existing.get("_tgt") == src
|
||||
existing_rel = existing.get("relation")
|
||||
incoming_rel = attrs.get("relation")
|
||||
reverse = existing.get("_src") == tgt and existing.get("_tgt") == src
|
||||
if reverse and existing_rel == incoming_rel:
|
||||
continue
|
||||
# Different relations in opposite directions are two facts that only
|
||||
# share a pair because the graph is undirected. A type's
|
||||
# `method`/`contains` edge and its member's `calls` edge back are
|
||||
# that case: last-write kept whichever source id sorted later, and
|
||||
# when the call won the member vanished from every walk of the
|
||||
# type. The membership edge wins. If the call is already stored,
|
||||
# the membership edge falls through and replaces it below.
|
||||
if (
|
||||
reverse
|
||||
and existing_rel in _STRUCTURAL_MEMBER_RELATIONS
|
||||
and incoming_rel == "calls"
|
||||
):
|
||||
continue
|
||||
# A pair that already carries a SPECIFIC relation must not be downgraded
|
||||
|
||||
@@ -18,6 +18,7 @@ import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from graphify.build import build_from_json, edge_data
|
||||
from graphify.extract import extract
|
||||
|
||||
|
||||
@@ -238,6 +239,38 @@ def test_qualified_construction_resolves_when_the_method_also_declares_the_type(
|
||||
assert not any(target in sourceless for _, target in calls)
|
||||
|
||||
|
||||
def test_factory_constructing_its_own_type_keeps_the_method_edge(tmp_path):
|
||||
# `Fault` both belongs to `Report` and constructs it. On the default
|
||||
# undirected graph those are one pair, and the `calls` edge used to be
|
||||
# the one that survived, so `Report` had no `method` edge to `Fault`.
|
||||
# The siblings that do not call the type were never affected.
|
||||
_, r = _extract(tmp_path, {"Report.cs": (
|
||||
"namespace Demo\n"
|
||||
"{\n"
|
||||
" public sealed class Report\n"
|
||||
" {\n"
|
||||
" public string Verb;\n"
|
||||
" public static Report Fault(string verb) { return new Report { Verb = verb }; }\n"
|
||||
" public static string NotImplementedJson(string verb) { return verb; }\n"
|
||||
" public string ToJson() { return Verb; }\n"
|
||||
" }\n"
|
||||
"}\n"
|
||||
)})
|
||||
report = _find(r, "Report", "report")
|
||||
fault = _find(r, ".Fault()", "fault")
|
||||
G = build_from_json(r)
|
||||
kept = edge_data(G, report, fault)
|
||||
assert kept.get("relation") == "method"
|
||||
assert (kept.get("_src"), kept.get("_tgt")) == (report, fault)
|
||||
assert edge_data(G, report, _find(r, ".NotImplementedJson()", "notimplemented")).get(
|
||||
"relation") == "method"
|
||||
assert edge_data(G, report, _find(r, ".ToJson()", "tojson")).get("relation") == "method"
|
||||
# Directed storage has room for both facts.
|
||||
D = build_from_json(r, directed=True)
|
||||
assert edge_data(D, report, fault).get("relation") == "method"
|
||||
assert edge_data(D, fault, report).get("relation") == "calls"
|
||||
|
||||
|
||||
def test_qualified_construction_of_a_foreign_type_makes_no_stub_edge(tmp_path):
|
||||
# `new System.Text.StringBuilder()` has no declaration in the corpus. No edge
|
||||
# beats an edge into a sourceless placeholder that stands for nothing.
|
||||
|
||||
@@ -18,6 +18,11 @@ sampling — and callflow's relation filter
|
||||
Alphabetical order carries no meaning. These tests pin that a generic relation
|
||||
never overwrites a specific one, in either arrival order, and that nothing else
|
||||
about the collapse changed.
|
||||
|
||||
One opposite-direction exception: a `method` or `contains` edge and a `calls`
|
||||
edge running back the other way share one undirected pair (a member that
|
||||
constructs or calls its owner). The membership edge survives. Same-direction
|
||||
`contains` vs `calls` stays unranked. A directed graph keeps both arcs.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
@@ -99,8 +104,10 @@ def test_two_generic_relations_keep_previous_behaviour():
|
||||
|
||||
|
||||
def test_two_specific_relations_keep_previous_behaviour():
|
||||
"""Deliberately NOT ranked against each other — `contains` vs `calls` is a
|
||||
cross-axis judgement this collapse does not need to make."""
|
||||
"""Same direction is deliberately NOT ranked — `contains` vs `calls` along
|
||||
one direction is a cross-axis judgement this collapse does not make.
|
||||
Opposite directions are a different pair of facts; see
|
||||
test_structural_membership_beats_an_opposite_call."""
|
||||
G = build_from_json(_extraction([_edge("calls"), _edge("contains")]))
|
||||
assert _relation(G) in {"calls", "contains"}
|
||||
|
||||
@@ -184,6 +191,52 @@ def test_unknown_relation_is_treated_as_specific():
|
||||
assert _relation(G) == "custom_rel", f"order {[e['relation'] for e in order]}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("structural", ["method", "contains"])
|
||||
@pytest.mark.parametrize("structural_src, structural_tgt", [("a", "b"), ("b", "a")])
|
||||
def test_structural_membership_beats_an_opposite_call(structural, structural_src, structural_tgt):
|
||||
"""Type -method/contains-> member and member -calls-> type share one
|
||||
undirected pair. Whichever endpoint id sorts later used to win, so a
|
||||
factory's constructor call erased the membership edge and the member
|
||||
dropped out of every type walk. The membership edge survives, with its
|
||||
own direction and location, in either id order. Still exactly one edge."""
|
||||
call_src, call_tgt = structural_tgt, structural_src
|
||||
G = build_from_json(_extraction([
|
||||
_edge(structural, src=structural_src, tgt=structural_tgt, source_location="L4"),
|
||||
_edge("calls", src=call_src, tgt=call_tgt, source_location="L5"),
|
||||
]))
|
||||
d = edge_data(G, "a", "b")
|
||||
assert d.get("relation") == structural
|
||||
assert (d.get("_src"), d.get("_tgt")) == (structural_src, structural_tgt)
|
||||
assert d.get("source_location") == "L4"
|
||||
assert G.number_of_edges() == 1
|
||||
|
||||
|
||||
def test_directed_graph_keeps_method_and_the_reverse_call():
|
||||
"""Opposite directions are different arcs once the graph is directed, so
|
||||
both facts survive. The undirected rule must not drop the call here."""
|
||||
G = build_from_json(_extraction([
|
||||
_edge("method", src="a", tgt="b", source_location="L4"),
|
||||
_edge("calls", src="b", tgt="a", source_location="L5"),
|
||||
]), directed=True)
|
||||
assert G.is_directed()
|
||||
assert G.number_of_edges() == 2
|
||||
assert edge_data(G, "a", "b").get("relation") == "method"
|
||||
assert (edge_data(G, "a", "b").get("_src"), edge_data(G, "a", "b").get("_tgt")) == ("a", "b")
|
||||
assert edge_data(G, "b", "a").get("relation") == "calls"
|
||||
assert edge_data(G, "b", "a").get("source_location") == "L5"
|
||||
|
||||
|
||||
def test_same_direction_method_and_calls_stay_unranked():
|
||||
"""The membership-vs-call rule applies only when the edges run in opposite
|
||||
directions. Along one direction the pair is still a single unranked edge."""
|
||||
G = build_from_json(_extraction([
|
||||
_edge("method", src="a", tgt="b"),
|
||||
_edge("calls", src="a", tgt="b"),
|
||||
]))
|
||||
assert G.number_of_edges() == 1
|
||||
assert _relation(G) in {"method", "calls"}
|
||||
|
||||
|
||||
def test_same_relation_collision_preserves_extracted_over_inferred():
|
||||
"""An EXTRACTED AST edge must not be downgraded by a later INFERRED edge."""
|
||||
for order in (
|
||||
|
||||
Reference in new issue
Block a user