Add extensionReceiver option for layered PInvoke composition (#1477)#1701
Add extensionReceiver option for layered PInvoke composition (#1477)#1701JeremyKuhne wants to merge 1 commit into
Conversation
7dbc716 to
0f74b3f
Compare
…ft#1477) Allow multiple assemblies running CsWin32 to contribute to a single shared static class (typically `PInvoke`) so callers can reach every generated native API through one discovery symbol regardless of which assembly emitted it. A new `extensionReceiver` setting in `NativeMethods.json` names a static class ΓÇö declared in another assembly that also runs CsWin32 ΓÇö to receive this assembly's generated members. When set, members are emitted inside a C# 14 `extension(<Receiver>) { ... }` block on the configured `className`, instead of directly on it. The receiver must live in the same namespace as `className`, be a static class, be accessible, and differ from `className`. Constants, which the C# 14 extension grammar cannot host, are emitted as fields on the host class (preserving the configured visibility) and re-exposed through a forwarder property inside the extension block so both reach paths work. Cross-assembly duplicates are skipped using a full-signature match (name + arity + per-parameter type-name comparison after normalization that strips `global::` and leading namespace segments). User-authored wrappers of the same name but different signatures are kept; only an exact-shape match suppresses re-emission. When `SuperGenerator` runs multiple generators (e.g. Windows.Win32 and Windows.Wdk side by side), each generator silently no-ops the wrap if the receiver doesn't resolve in its own namespace; the `PInvoke011` diagnostic is reported only when no generator could resolve. The analyzer is now multi-targeted: a Roslyn 4.11 floor leg with the feature disabled and a Roslyn 5.0+ leg that implements it via the new `ExtensionBlockDeclarationSyntax` / `INamedTypeSymbol.IsExtension` APIs. NuGet picks the appropriate leg based on the host's Roslyn version. Diagnostics: PInvoke011 ΓÇö extensionReceiver could not be resolved, isn't a static class, isn't accessible, or equals className. PInvoke012 ΓÇö consuming project requires C# 14. PInvoke013 ΓÇö host analyzer doesn't support C# 14 extension members. End-to-end validation includes 16 new unit tests covering shape, multi-file emission, gating, the four `TryResolveExtensionReceiver` outcomes, and the full-signature dedup matrix (name match, arity mismatch, parameter-type mismatch, generic user-wrapper vs. metadata extern of the same simple name). This change was independently vetted in dotnet/winforms: JeremyKuhne/winforms@45562f1 Feedback from that vet drove three behavioral refinements over the initial design: * Dedup upgraded from name-only to full-signature match, so user-authored wrappers no longer silently suppress the underlying metadata extern (resolves a self-recursion hazard). * Constant backing fields keep their configured visibility instead of being forced to `private`, so they remain usable in C# const contexts (enum initializers, attribute arguments, `fixed`-array sizes, `case` labels). * Receiver resolution short-circuits per generator under SuperGenerator, eliminating false `PInvoke011` reports when one of several side-by-side generators legitimately can't see the receiver. Documentation: docfx/docs/composition.md is added to the docs TOC with the conceptual model, configuration reference, dispatch rules, multi-assembly composition behavior, a step-by-step migration playbook for existing multi-layer projects, common-issue catalog, and the diagnostics matrix. features.md and getting-started.md cross-link to it. Closes microsoft#1477
0f74b3f to
396963d
Compare
| library on top, and an application on top of that — and you want callers to use a single | ||
| `PInvoke.X()` discovery surface regardless of which layer happens to declare `X`. | ||
|
|
||
| Don't use it for single-assembly projects. The default `partial class PInvoke` already gives you |
There was a problem hiding this comment.
But would it be wrong? Why provide a choice? In C# 14 shouldn't we always do this?
There was a problem hiding this comment.
@jevansaks It is a pretty big breaking change to those that already have multiple projects with CsWin32 visibility. I suppose we could make it an opt-out and default to extending PInvoke if you haven't customized the class name?
jevansaks
left a comment
There was a problem hiding this comment.
I'm about 80% done but I'll leave these comments so far. Will try to come back to it tomorrow.
Overall looking good. One thing I would love to see in this mode is that all the friendly overloads that we currently put in another namespace (and thus are totally undiscoverable and sometimes people miss them because they forget to include the Windows.Win32 namespace itself) -- can we use this extension everything approach to put them just in the main namespace or even on the main class as extension methods of that class?
|
|
||
| ### Rule 2 — When you can't use the extension path, use the host class directly | ||
|
|
||
| Some C# contexts cannot resolve through an extension property. Specifically: |
There was a problem hiding this comment.
Are these bugs that you've filed on roslyn? Please track them here
| receiver type. | ||
|
|
||
| > [!NOTE] | ||
| > Setting `public: true` on the owner publishes **every** type CsWin32 emits in that assembly, |
There was a problem hiding this comment.
I haven't looked at the winforms consumption yet but this seems problematic if we start having parts of the ecosystem shipping these things as public.
| ``` | ||
|
|
||
| Projects targeting .NET do *not* need to add these package references, although it is harmless to do so. | ||
| Projects targeting .NET do _not_ need to add these package references, although it is harmless to do so. |
There was a problem hiding this comment.
underscores are italics, asterisk is bold. this is changing the formatting unnecessarily.
| <PackageVersion Update="Microsoft.CodeAnalysis.CSharp" Version="$(CodeAnalysisVersionForAnalyzers)" /> | ||
| <PackageVersion Update="System.Collections.Immutable" Version="8.0.0" /> | ||
| <PackageVersion Update="System.Memory" Version="4.5.5" /> | ||
| <PackageVersion Update="System.Collections.Immutable" Version="9.0.0" /> |
There was a problem hiding this comment.
We've gotten in a lot of trouble changing these versions in the past see #1555
| <file src="$Roslyn4BaseOutputPath$System.Text.Json.dll" target="analyzers\dotnet\roslyn4.11\cs\System.Text.Json.dll" /> | ||
| <file src="$Roslyn4BaseOutputPath$System.Text.Encodings.Web.dll" target="analyzers\dotnet\roslyn4.11\cs\System.Text.Encodings.Web.dll" /> | ||
|
|
||
| <file src="$CsWin32GeneratorBasePath$**" target="build\tools\" exclude="**\*.exe;**\MessagePack*.dll" /> |
There was a problem hiding this comment.
Instead of multi-targeting the source generator we can also just have the new mode be part of the tool-based generator only.
| if (!this.extensionReceiverSymbolResolved) | ||
| { | ||
| this.extensionReceiverSymbolResolved = true; | ||
| this.TryResolveExtensionReceiver(out this.extensionReceiverSymbolCache, out _); |
There was a problem hiding this comment.
Are we ignoring the error because we assume we've already checked for the extension receiver and it's going to succeed?
| /// <param name="parameterTypeNames">The CsWin32-projected textual form of each parameter type, in declaration order. Pass an empty array when probing for a field/property/parameterless member (e.g. a constant). The strings are compared after a normalization that strips <c>global::</c> prefixes and leading namespace segments, so callers may pass fully-qualified names produced by <c>ToTypeSyntax(...)</c>.</param> | ||
| /// <returns><see langword="true"/> if a matching extension member is already declared on the receiver in scope; <see langword="false"/> otherwise (including when no extension receiver is configured or running on the Roslyn 4 leg).</returns> | ||
| #pragma warning disable SA1202 // Keep dedup helper next to TryResolveExtensionReceiver / receiver-related code. | ||
| internal bool IsExtensionMemberAlreadyOnReceiver(string memberName, IReadOnlyList<string> parameterTypeNames) |
There was a problem hiding this comment.
What about when there is a property/constant versus parameterless method of the same name? Should we pass in the member kind too?
Implements
extensionReceiverinNativeMethods.jsonso multiple assemblies running CsWin32 can contribute to a single shared static class (typicallyPInvoke). Callers reach every generated native API through one discovery symbol regardless of which assembly emitted it.Closes #1477.
What this adds
A new
extensionReceiversetting inNativeMethods.jsonnames a static class declared in another assembly that also runs CsWin32 to receive this assembly's generated members. When set, members are emitted inside a C# 14extension(<Receiver>) { ... }block on the configuredclassName. The receiver must live in the same namespace asclassName, be a static class, be accessible, and differ fromclassName.Constants which the C# 14 extension grammar cannot host are emitted as fields on the host class (preserving the configured visibility) and re-exposed through a forwarder property inside the extension block, so both
PInvoke.X(extension path, runtime contexts) and<HostClass>.X(const path, enum initializers / attribute args / fixed-array sizes / case labels) work.Cross-assembly duplicates are skipped using a full-signature match (name + arity + per-parameter type-name comparison after normalization that strips
global::and leading namespace segments). User-authored wrappers with the same name but a different signature do not suppress the underlying metadata extern.When
SuperGeneratorruns multiple generators (e.g.Windows.Win32andWindows.Wdkside by side), each generator silently no-ops the wrap if the receiver doesn't resolve in its own namespace;PInvoke011is reported only when no generator could resolve.The analyzer is multi-targeted into a Roslyn 4.11 floor leg (feature disabled) and a Roslyn 5.0+ leg (feature enabled, using
ExtensionBlockDeclarationSyntax/INamedTypeSymbol.IsExtension). NuGet picks the appropriate leg from the host's Roslyn version.Diagnostics
PInvoke011extensionReceivercouldn't be resolved, isn't a static class, isn't accessible, or equalsclassName.PInvoke012PInvoke013Documentation
docfx/docs/composition.mdis added to the docs TOC with the conceptual model, configuration reference, dispatch rules, multi-assembly composition behavior, a step-by-step migration playbook for existing multi-layer projects, common-issue catalog, and the diagnostics matrix.features.mdandgetting-started.mdcross-link to it.Validation
ExtensionReceiverTests.cscovering shape, multi-file emission, gating, the fourTryResolveExtensionReceiveroutcomes, and the full-signature dedup matrix (name match, arity mismatch, parameter-type mismatch, generic user-wrapper vs. metadata extern of the same simple name).integration-tests/sdk-extensionconsuming the locally-packed package via the SDK 11 preview.dotnet/winforms: JeremyKuhne/winforms@45562f1Feedback from the WinForms vet drove three refinements over the initial design:
private, so they remain usable in C# const contexts.SuperGenerator, removing falsePInvoke011reports when one of several side-by-side generators legitimately can't see the receiver.