From ab8d63bf4ced49fa88050ce5f897d6bc6c425c22 Mon Sep 17 00:00:00 2001 From: JensD-git <299119159+JensD-git@users.noreply.github.com> Date: Tue, 13 Oct 2026 12:44:31 +0200 Subject: [PATCH] fix(csharp): salt colliding ids before the partial merge, so a half keeps its members (#4411) Two files can mint the same stem (`Spec.cs` / `_Spec.cs`, `Cfg/Helpers.cs` / `Cfg_Helpers.cs`), so the type ids in them collide. The C# partial merge ran before _disambiguate_colliding_node_ids: it rewired the edges of every half onto the canonical half's still-colliding id, and the disambiguation, which remaps by (id, source_file of the edge), then missed every edge that came from another half and left it on a dead id. The members of that half had no owning type. When both colliding files declared a half of the same partial type, the second node kept the canonical id, was not removed by the merge, and was split off again, so the type stayed two nodes. And since the merge removes merged-away halves by id, an unrelated node in another file that shared such an id was removed with them. Run the disambiguation first. Every edge is still on its own half when ids are salted, and the merge joins the halves by type path, whatever their ids. Fixes #4411. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit 78575576a85771e9c0a2cf6b37b2ed896702ed07) --- ARCHITECTURE.md | 4 +- graphify/extract.py | 14 ++++- tests/test_csharp_partial_classes.py | 88 +++++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 817472ac9..5584dc147 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -122,7 +122,9 @@ C# partial types merge across files by their path inside the namespace, enclosing types first and each with its arity (`` Outer`1.Inner ``), within one assembly. Nested halves merge only under the same enclosing chain, and `Foo` and `Foo` stay separate types. AST cache schema 9 retires cached C# results that -lack this path. +lack this path. The merge runs after colliding ids are salted apart by path, so +a half whose file stem collides with another file's (`Spec.cs` and `_Spec.cs`, +`Cfg/Helpers.cs` and `Cfg_Helpers.cs`) still merges and keeps its members. R extraction preserves symbol-only bindings such as `%||%` as separate nodes. When normal ID normalization removes an entire binding name, its UTF-8 bytes diff --git a/graphify/extract.py b/graphify/extract.py index b5af1b456..76bcb9aa6 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -4230,7 +4230,9 @@ def _merge_csharp_partial_class_nodes( onto it. Member node ids are left untouched — only the class-level nodes collapse. - Must run BEFORE _disambiguate_colliding_node_ids / _rewire_unique_stub_nodes / + Must run AFTER _disambiguate_colliding_node_ids (so a half whose id collides + with another file's node is already salted apart, and every edge still sits + on its own half when it is rewired) and BEFORE _rewire_unique_stub_nodes / _resolve_csharp_type_references and the resolver registry, so every later pass sees one definition per partial type. """ @@ -9858,8 +9860,16 @@ def extract( _repoint_python_package_imports(paths, all_nodes, all_edges, root) _repoint_python_sibling_imports(paths, all_nodes, all_edges, root) _merge_swift_extensions(per_file, all_nodes, all_edges) - _merge_csharp_partial_class_nodes(per_file, all_nodes, all_edges, paths, root) + # Salt colliding ids apart BEFORE the C# partial merge. The merge rewires the + # edges of every half onto the canonical half's id; if that id collides with + # another file's node (`Spec.cs` next to `_Spec.cs`, `Cfg/Helpers.cs` next to + # `Cfg_Helpers.cs`), the disambiguation would look those rewired edges up by + # their own source_file, i.e. the file of the half they came from, not of the + # canonical half; it would miss and leave them on a dead id. Run first, it + # splits only genuinely distinct nodes; the merge then joins the halves by + # type path, whatever their (salted) ids. _disambiguate_colliding_node_ids(all_nodes, all_edges, all_raw_calls, root) + _merge_csharp_partial_class_nodes(per_file, all_nodes, all_edges, paths, root) _canonicalize_csharp_namespace_nodes(all_nodes, all_edges) # PHP namespace/use disambiguation must run BEFORE the unique-stub rewire: # the false merge (#1923) happens inside the rewire when a bare-name stub diff --git a/tests/test_csharp_partial_classes.py b/tests/test_csharp_partial_classes.py index e9369d530..bb4c7bdc7 100644 --- a/tests/test_csharp_partial_classes.py +++ b/tests/test_csharp_partial_classes.py @@ -4,9 +4,9 @@ carries the per-file stem), so the type's members split across the halves and every receiver-typed lookup on `Foo` bailed as ambiguous — cross-half calls never resolved. `_merge_csharp_partial_class_nodes` collapses the halves onto -one canonical node, keyed by (assembly, namespace, label); same-named types in -other namespaces, non-partial declarations, and nested partial types are left -alone. The assembly key is the nearest ancestor dir holding a `*.csproj`/ +one canonical node, keyed by (assembly, namespace, type path); same-named types +in other namespaces, non-partial declarations, and nested types under a +different enclosing chain are left alone (#4363). The assembly key is the nearest ancestor dir holding a `*.csproj`/ `*.fsproj`/`*.vbproj` (#2411: `partial` never fuses across assemblies), with "" — halves under no project at all, which still merge — as the sentinel. """ @@ -625,3 +625,85 @@ def test_receiver_qualified_calls_are_not_rebound_to_the_own_type(tmp_path): 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 + + +def _methods_owned_by(r, nid): + targets = {e["target"] for e in r["edges"] if e["relation"] == "method" and e["source"] == nid} + return {n["label"] for n in r["nodes"] if n["id"] in targets} + + +def test_partial_half_with_a_path_colliding_id_keeps_its_members(tmp_path): + """`Cfg/Helpers.cs` and `Cfg_Helpers.cs` mint the same file stem, so the + halves declared there share one id and get salted apart by path. The merge + used to run first: it rewired the third half's `method` edges onto that + shared id, the salting then keyed them by the third half's own file, missed, + and left them dangling, so `Gamma()` had no owner and the two colliding + halves stayed two `Cfg` nodes.""" + _, r = _extract(tmp_path, { + "Lib.csproj": "\n", + "Cfg/Helpers.cs": "namespace App { public partial class Cfg { public void Alpha() {} } }\n", + "Cfg_Helpers.cs": "namespace App { public partial class Cfg { public void Beta() {} } }\n", + "generated/Cfg.cs": ( + "namespace App { public partial class Cfg {" + " public void Gamma() { Alpha(); Beta(); } } }\n" + ), + }) + cfgs = _nodes_labeled(r, "Cfg") + assert len(cfgs) == 1, cfgs + assert _methods_owned_by(r, cfgs[0]["id"]) == {".Alpha()", ".Beta()", ".Gamma()"} + + +def test_partial_half_colliding_with_an_underscore_sibling_keeps_its_members(tmp_path): + """`Spec.cs` and `_Spec.cs` collide the same way (the leading underscore is + dropped from the stem). The merged `Spec` must own the generated half's + methods, and the unrelated `_Spec` must stay a separate type deriving from it.""" + _, r = _extract(tmp_path, { + "Spec.cs": "namespace App { public partial class Spec { public int Size; } }\n", + "_Spec.cs": "namespace App { public class _Spec : Spec { public void Extra() { Move(); } } }\n", + "generated/Spec.cs": ( + "namespace App { public partial class Spec {" + " public void Move() {} public void Copy() {} } }\n" + ), + }) + specs = _nodes_labeled(r, "Spec") + assert len(specs) == 1, specs + spec = specs[0]["id"] + assert _methods_owned_by(r, spec) == {".Move()", ".Copy()"} + sub = _nodes_labeled(r, "_Spec") + assert len(sub) == 1 and sub[0]["id"] != spec, sub + assert (sub[0]["id"], spec) in {(e["source"], e["target"]) for e in r["edges"] if e["relation"] == "inherits"} + + +def test_outer_partial_halves_in_path_colliding_files_merge(tmp_path): + """The colliding files both declare `partial class Tools`: identical ids + from two files. Salted apart first, the halves now merge by type path + instead of staying two `Tools` nodes (and losing the third half).""" + _, r = _extract(tmp_path, { + "Spec.cs": "namespace App { public static partial class Tools { public static void A() {} } }\n", + "_Spec.cs": "namespace App { public static partial class Tools { public static void B() {} } }\n", + "generated/Spec.cs": "namespace App { public static partial class Tools { public static void C() {} } }\n", + }) + tools = _nodes_labeled(r, "Tools") + assert len(tools) == 1, tools + assert _methods_owned_by(r, tools[0]["id"]) == {".A()", ".B()", ".C()"} + + +def test_merge_does_not_delete_an_unrelated_node_sharing_a_half_id(tmp_path): + """A half's id can collide with an unrelated node in another file: here the + method `Helpers.App.Cfg()` in `Cfg.cs` mints the same id as the half of + `App.Cfg` in `Cfg/Helpers.cs`. The merge removes merged-away halves by id, + so before ids were salted apart it also deleted that method, and the + method's `calls` edge was rewired onto the type.""" + calls, r = _extract(tmp_path, { + "A/Cfg.cs": "namespace App { public partial class Cfg { public void Alpha() {} } }\n", + "Cfg/Helpers.cs": "namespace App { public partial class Cfg { public void Beta() {} } }\n", + "Cfg.cs": ( + "namespace Helpers { public class App {" + " public void Cfg() { Other(); } public void Other() {} } }\n" + ), + }) + method = _nodes_labeled(r, ".Cfg()") + assert len(method) == 1 and method[0]["source_file"] == "Cfg.cs", method + assert (method[0]["id"], _find(r, ".Other()", "")) in calls + cfgs = _nodes_labeled(r, "Cfg") + assert len(cfgs) == 1 and _methods_owned_by(r, cfgs[0]["id"]) == {".Alpha()", ".Beta()"}, cfgs