mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-14 07:59:40 +00:00
fix(csharp): keep id-sharing and qualified-base partials out of the merge (#4363)
Follow-ups from the automated review of the first commit: - Two same-named nested types under different outers in one file share one node id. That node took the first declaration's type_path and could join the other outer's halves; it now gets no type_path and is not merged. - The shadowed-base step drops only a placeholder spelled with the bare name. A base written qualified (Other.IFoo), or through a using alias of that name, can be a different type that only shares the simple name, and keeps its edge. - Pin that receiver-qualified calls (base.M(), this.M(), Type.M(), obj.M()) keep their targets: they leave the cross-file pass as member calls before the own-type binding. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
92d9197787
commit
37ec971f16
3 files changed
+108
-7
No files matched your search
+25
-4
@@ -4034,12 +4034,27 @@ def _drop_shadowed_csharp_partial_bases(all_nodes: list[dict], all_edges: list[d
|
||||
the halves to agree. Each half's base name is resolved in its own file's
|
||||
scope, so a half whose file the resolver cannot see the base from (a
|
||||
sibling nested base without `using static` for its holder, say) leaves a
|
||||
placeholder edge while another half binds the real type. After the merge the type then has two same-named bases, and
|
||||
lookups through the base chain bail as ambiguous. The placeholder is the
|
||||
unresolved spelling of the resolved base, so it goes.
|
||||
placeholder edge while another half binds the real type. After the merge
|
||||
the type then has two same-named bases, and lookups through the base chain
|
||||
bail as ambiguous. A placeholder spelled with the bare name is the
|
||||
unresolved spelling of the resolved base, so it goes. One written
|
||||
qualified (`Other.IFoo`) or through a `using` alias of that name can be a
|
||||
different type that only shares the simple name, so it stays.
|
||||
"""
|
||||
nodes_by_id = {str(n.get("id", "")): n for n in all_nodes}
|
||||
|
||||
# `using X = ...;` aliases per file, from the import edges the C# extractor
|
||||
# emits (the same source the C# name resolver reads).
|
||||
aliases_by_file: dict[str, set[str]] = {}
|
||||
for e in all_edges:
|
||||
md = e.get("metadata")
|
||||
if not isinstance(md, dict) or md.get("using_kind") != "alias":
|
||||
continue
|
||||
alias = md.get("alias")
|
||||
src_node = nodes_by_id.get(str(e.get("source", ""))) or {}
|
||||
if isinstance(alias, str) and alias:
|
||||
aliases_by_file.setdefault(str(src_node.get("source_file", "")), set()).add(alias)
|
||||
|
||||
def _is_partial_cs(nid: str) -> bool:
|
||||
n = nodes_by_id.get(nid) or {}
|
||||
return (
|
||||
@@ -4067,7 +4082,13 @@ def _drop_shadowed_csharp_partial_bases(all_nodes: list[dict], all_edges: list[d
|
||||
if not names:
|
||||
return False
|
||||
tgt = nodes_by_id.get(str(e.get("target", ""))) or {}
|
||||
return not tgt.get("source_file") and str(tgt.get("label", "")) in names
|
||||
label = str(tgt.get("label", ""))
|
||||
if tgt.get("source_file") or label not in names:
|
||||
return False
|
||||
md = e.get("metadata")
|
||||
if isinstance(md, dict) and md.get("qualified"):
|
||||
return False
|
||||
return label not in aliases_by_file.get(str(e.get("source_file", "")), set())
|
||||
|
||||
all_edges[:] = [e for e in all_edges if not _shadowed(e)]
|
||||
|
||||
|
||||
@@ -4753,7 +4753,7 @@ def _extract_generic(
|
||||
# C# type id -> its name within the namespace, enclosing types first
|
||||
# (`` Outer`1.Inner ``). Partial types carry it as `type_path` so the
|
||||
# corpus-level partial merge can key nested and generic halves exactly.
|
||||
csharp_type_paths: dict[str, str] = {}
|
||||
csharp_type_paths: dict[str, str | None] = {}
|
||||
# #1356: call expressions in property/field initializers (e.g.
|
||||
# `let vm = VM()`) live outside function bodies, so the call-walk never
|
||||
# reaches them. Collect (owner_nid, call_node) here and walk them too.
|
||||
@@ -5006,6 +5006,7 @@ def _extract_generic(
|
||||
# enclosing types, so this is what tells two same-named nested
|
||||
# types under different outers apart. None when an enclosing
|
||||
# type's path is unknown.
|
||||
_type_path: str | None
|
||||
if parent_class_nid:
|
||||
_parent_path = csharp_type_paths.get(parent_class_nid)
|
||||
_type_path = (
|
||||
@@ -5014,8 +5015,18 @@ def _extract_generic(
|
||||
)
|
||||
else:
|
||||
_type_path = class_name_for_id
|
||||
if _type_path is not None:
|
||||
csharp_type_paths.setdefault(class_nid, _type_path)
|
||||
if class_nid not in csharp_type_paths:
|
||||
csharp_type_paths[class_nid] = _type_path
|
||||
elif csharp_type_paths[class_nid] != _type_path:
|
||||
# Two declarations in this file share one id but not one
|
||||
# path (same-named nested types under different outers).
|
||||
# No path is right for both, so the node gets none and the
|
||||
# partial merge leaves it alone.
|
||||
csharp_type_paths[class_nid] = None
|
||||
_type_path = None
|
||||
for _prev in nodes:
|
||||
if _prev.get("id") == class_nid and isinstance(_prev.get("metadata"), dict):
|
||||
_prev["metadata"].pop("type_path", None)
|
||||
if parent_class_nid:
|
||||
metadata = {"is_nested_type": True}
|
||||
# #2332: `partial class Foo` split across files mints one node
|
||||
|
||||
@@ -556,3 +556,72 @@ def test_schema_bump_retires_csharp_entries_cached_without_type_path(tmp_path, m
|
||||
assert len(_nodes_labeled(r, "Inner")) == 1
|
||||
boxes = sorted(sorted(_methods_of(r, n["id"])) for n in _nodes_labeled(r, "Box"))
|
||||
assert boxes == [[".Plain()"], [".Typed()"]]
|
||||
|
||||
|
||||
def test_nested_types_sharing_an_id_in_one_file_are_not_merged(tmp_path):
|
||||
"""`OuterA.Inner` and `OuterB.Inner` declared in one file already share
|
||||
one node id. No single type path is right for that node, so it must not
|
||||
join the halves of either `OuterA.Inner` or `OuterB.Inner` elsewhere."""
|
||||
calls, r = _extract(tmp_path, {
|
||||
"One.cs": (
|
||||
"namespace App {\n"
|
||||
" public partial class OuterA { public partial class Inner { public void A1() {} } }\n"
|
||||
" public partial class OuterB { public partial class Inner { public void B1() {} } }\n"
|
||||
"}\n"
|
||||
),
|
||||
"A2.cs": (
|
||||
"namespace App { public partial class OuterA {"
|
||||
" public partial class Inner { public void A2() {} } } }\n"
|
||||
),
|
||||
"B2.cs": (
|
||||
"namespace App { public partial class OuterB {"
|
||||
" public partial class Inner { public void B2() {} } } }\n"
|
||||
),
|
||||
})
|
||||
owned = [_methods_of(r, n["id"]) for n in _nodes_labeled(r, "Inner")]
|
||||
assert not any({".A2()", ".B1()"} <= m or {".B2()", ".A1()"} <= m for m in owned), owned
|
||||
|
||||
|
||||
def test_qualified_or_aliased_base_is_not_dropped_as_shadowed(tmp_path):
|
||||
"""A half that names a same-named but different base (`Other.IFoo`, or an
|
||||
alias) keeps its placeholder edge; only a bare-name placeholder next to
|
||||
the real base of that name is the unresolved spelling of it."""
|
||||
def _bases(case, files):
|
||||
_, r = _extract(tmp_path / case, files)
|
||||
by_id = {n["id"]: n for n in r["nodes"]}
|
||||
return sorted((by_id[e["target"]]["label"], bool(by_id[e["target"]].get("source_file")))
|
||||
for e in r["edges"] if e["relation"] in ("inherits", "implements"))
|
||||
|
||||
common = {
|
||||
"I.cs": "namespace App { public interface IFoo { void F(); } }\n",
|
||||
"DA.cs": "namespace App { public partial class D : IFoo { public void F() {} } }\n",
|
||||
}
|
||||
qualified = _bases("qualified", {**common, "DB.cs": "namespace App { public partial class D : Other.IFoo { } }\n"})
|
||||
aliased = _bases("aliased", {**common, "DB.cs": (
|
||||
"using IFoo = Other.IFoo;\n"
|
||||
"namespace App { public partial class D : IFoo { } }\n"
|
||||
)})
|
||||
assert qualified == [("IFoo", False), ("IFoo", True)], qualified
|
||||
assert aliased == [("IFoo", False), ("IFoo", True)], aliased
|
||||
|
||||
|
||||
def test_receiver_qualified_calls_are_not_rebound_to_the_own_type(tmp_path):
|
||||
"""The own-type binding is for unqualified calls only: `base.M()`,
|
||||
`this.M()` and `Type.M()` keep their existing targets."""
|
||||
calls, r = _extract(tmp_path, {
|
||||
"Machine.cs": "namespace App { public class Machine { public virtual void Reset() {} } }\n",
|
||||
"E1.cs": (
|
||||
"namespace App { public partial class Engine : Machine {"
|
||||
" public override void Reset() {} public static void Stop() {} } }\n"
|
||||
),
|
||||
"E2.cs": (
|
||||
"namespace App { public partial class Engine : Machine {"
|
||||
" public void Go() { base.Reset(); Valve.Stop(); } } }\n"
|
||||
),
|
||||
"Valve.cs": "namespace App { public static class Valve { public static void Stop() {} } }\n",
|
||||
})
|
||||
go = _find(r, ".Go()", "engine")
|
||||
assert (go, _find(r, ".Reset()", "machine")) in calls
|
||||
assert (go, _find(r, ".Stop()", "valve")) in calls
|
||||
assert (go, _find(r, ".Reset()", "engine")) not in calls
|
||||
assert (go, _find(r, ".Stop()", "engine")) not in calls
|
||||
Reference in new issue
Block a user