feat(csharp): unify partial-class parts for member and base resolution#732
feat(csharp): unify partial-class parts for member and base resolution#732vitali87 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements support for C# partial class member unification, ensuring that when a class is split across multiple files using the partial modifier, its parts are grouped together. This allows typed receiver resolution to correctly span all parts of the partial class (and their respective base classes) when resolving members and bases. A high-severity issue was identified in the slicing logic used to generate the grouping key when a class is defined in the global namespace (i.e., when module_qn is empty), which could lead to incorrect grouping of unrelated classes. A suggestion was provided to handle empty module_qn values correctly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if cs.TS_CSHARP_MODIFIER_PARTIAL in modifiers: | ||
| key = class_qn[len(module_qn) + 1 :] | ||
| group = self._csharp_partial_index.setdefault(key, []) | ||
| group.append(class_qn) | ||
| self.csharp_partial_groups[class_qn] = group |
There was a problem hiding this comment.
If a C# class is defined in the global namespace (i.e., outside of any namespace declaration), module_qn will be empty (""). In this case, len(module_qn) + 1 evaluates to 1, causing class_qn[1:] to slice off the first character of the class name. This can lead to incorrect grouping of unrelated classes (for example, Widget and Fidget would both resolve to the key "idget" and be grouped together as parts of the same partial class). Checking if module_qn is non-empty before slicing prevents this issue.
| if cs.TS_CSHARP_MODIFIER_PARTIAL in modifiers: | |
| key = class_qn[len(module_qn) + 1 :] | |
| group = self._csharp_partial_index.setdefault(key, []) | |
| group.append(class_qn) | |
| self.csharp_partial_groups[class_qn] = group | |
| if cs.TS_CSHARP_MODIFIER_PARTIAL in modifiers: | |
| key = class_qn[len(module_qn) + 1 :] if module_qn else class_qn | |
| group = self._csharp_partial_index.setdefault(key, []) | |
| group.append(class_qn) | |
| self.csharp_partial_groups[class_qn] = group |
There was a problem hiding this comment.
Fixed in the latest push. The partial-group key is now the declaring DIRECTORY (module_qn minus the file stem) plus the namespace-qualified name, so two independent projects that both declare N.Widget live in different directories and are not merged across the assembly boundary. Parts in different directories of one project fall back to generic resolution (safe under-merge) rather than risk a cross-project wrong edge. Added test_same_name_partial_in_different_projects_not_merged (two project dirs + a decoy so the generic fallback can't mask it; verified RED before, GREEN after).
|
@greptile review |
…to avoid cross-project merges
|
@greptile review |
What
Third and final C# follow-up gap. A
partialtype split across files becomes N path-distinct Class nodes (proj.A.N.Widget,proj.B.N.Widget); they are one logical type. This PR unifies the parts for call resolution so a typed receiver binds to members and bases declared on any part, not just the one it happened to resolve to.Why it's needed
Unique-named cross-part calls already resolve via the generic name-only fallback, but that fallback is ambiguous (and silently mis-binds) when a method name is shared. Probing
var w = new Widget(); w.Beta();whereBetaexists on both a Widget part and an unrelatedOtherclass showed the call binding toOther.Beta— a real mis-resolution. Typed partial-group resolution fixes it.How
partialmodifier is grouped by its namespace-qualified name into a shared list (DefinitionProcessor.csharp_partial_groups: each part qn → the shared list of all part qns), threaded by reference into the type-inference engine (same pattern asclass_field_types)._type_name_to_qn: when a type name resolves to several candidates that are all parts of one partial group, return one part (a single logical type) instead of bailing on ambiguity._find_method: for a partial class, the arity-first-then-name search spans every part and each part's bases, so a member/base declared on another part binds.Non-partial classes are unaffected (
.get(qn) or [qn]). Single-node collapse (deduping the Class node, mergingINHERITS/IMPLEMENTSonto one node) is deliberately left as Roslyn-follow-up per the plan — exact symbol-identity merge needs the semantic model; this PR delivers the resolution-level member merge.Tests (red → green)
test_csharp_partial_classes.py: a member on another part resolved through a typed receiver (with anOther.Betadecoy so only cross-part typing can bind it — verified RED: it bound toOther.Betabefore the fix), and an inherited member reached through a base declared on the other part.Full suite: 5022 passed, 10 skipped. ruff + ty clean.
Independent of #728 and #731.