From 96d9c14a4469f142322d8bcfeb0f3285cad6fa62 Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Fri, 3 Jul 2026 09:09:11 +0200 Subject: [PATCH 1/5] Add design plan for flattening image generation Working reference for the v1.5.0 effort to name generated image types after their registration (like Custom APIs) and delete the aliased-usings subsystem. Co-Authored-By: Claude via Conducktor --- docs/plans/flatten-image-generation.md | 199 +++++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 docs/plans/flatten-image-generation.md diff --git a/docs/plans/flatten-image-generation.md b/docs/plans/flatten-image-generation.md new file mode 100644 index 0000000..5a5e511 --- /dev/null +++ b/docs/plans/flatten-image-generation.md @@ -0,0 +1,199 @@ +# Plan: Flatten plugin image generation to eliminate the namespace/aliasing subsystem + +> Status: **Approved — implementing.** Ships as a new minor **v1.5.0** that folds in the pending +> (unreleased) v1.4.2 XPC3007 work; nothing is released before this change. + +## Goal + +Generate `PreImage`/`PostImage`/`ActionWrapper` the same way Custom APIs generate +`Request`/`Response`/`ActionWrapper`: **uniquely-named types in the plugin's own namespace**, +referenced by **fully-qualified name** in generated handler signatures. This removes the reason the +per-registration namespace exists, which lets us delete the entire aliased-usings subsystem. + +A new **XPC2002** error makes the uniqueness precondition explicit at compile time (mirroring the +existing runtime block), so generation can never collide. + +--- + +## 1. New naming scheme + +Today (`PluginStepMetadata.cs:52`, `RegisterStepHelper.cs:46`, runtime `Plugin.cs:335-339`): + +``` +namespace: {PluginNamespace}.PluginRegistrations.{PluginClass}.{Entity}{Operation}{Stage} +types: PreImage, PostImage, ActionWrapper +``` + +Proposed (mirrors Custom API at `Plugin.cs:474`): + +``` +namespace: {PluginNamespace} +prefix: Sanitize("{PluginClass}{Entity}{Operation}{Stage}") +types: {prefix}PreImage, {prefix}PostImage, {prefix}ActionWrapper +``` + +- **Why the plugin-class part is required (and Custom API doesn't need one):** a Custom API name is + globally unique by construction; an image's identity is the derived tuple `Entity+Operation+Stage`, + which repeats across plugin classes in the same namespace. Plugin class names are unique within a + namespace, and — per the registration rule, enforced by XPC2002 below — the tuple is unique within a + plugin class. So `{PluginClass}{Entity}{Operation}{Stage}` is unique within the namespace. No + index/disambiguator needed. +- **Sanitization:** route the whole prefix through the existing `IdentifierSanitizer.Sanitize` + (already used for Custom API names at `Plugin.cs:474`). This also hardens the + custom-message-operation case (`EventOperation` can be an arbitrary string like + `"contextand_DoThing"`), which the current raw-concatenation scheme handles only by luck. The + generator and the runtime must use the identical sanitizer so the reflected type name matches. +- **Verbosity is the accepted trade-off**, identical in spirit to Custom API's `SomeApiRequest`. We do + **not** steer handlers toward the shared interfaces to avoid the long name — declaring + `IPluginImage` throws away the generated typed property wrappers, which are the entire point + of the feature. The interfaces stay a documented escape hatch for when you genuinely only need the + raw `Entity`; the docs mention they exist but do not recommend them as the default parameter type. + +--- + +## 2. New diagnostic: XPC2002 — duplicate step registration + +- **Descriptor** (`DiagnosticDescriptors.cs`, category XPC2xxx "plugin class structure", **Error**): + "Plugin '{0}' registers {Entity}/{Operation}/{Stage} more than once. Each entity/operation/stage + combination may be registered at most once per plugin." +- **Analyzer** `Analyzers/DuplicateStepRegistrationAnalyzer.cs`: register a symbol/compilation-level + action over each `Plugin`-derived class, collect all `RegisterStep`/`RegisterPluginStep` + invocations in its constructor, key each by `(sanitized entity, sanitized operation, stage)`, and + report on the 2nd+ occurrence. Reuse `RegisterStepHelper` for extraction. +- **Rationale:** codifies the existing runtime block at compile time and *guarantees* the naming + scheme in section 1 is collision-free, so generation never breaks. Error severity because it's + already illegal. +- Docs: `rules/XPC2002.md`; entry in `AnalyzerReleases.Unshipped.md`. + +--- + +## 3. Source-generator changes + +- `Models/PluginStepMetadata.cs:52` — replace `RegistrationNamespace` with + `TypePrefix => Sanitize($"{PluginClassName}{EntityTypeName}{EventOperation}{ExecutionStage}")`; the + emit namespace becomes `Namespace` (plugin namespace). +- `Helpers/RegisterStepHelper.cs:46` — update/remove the duplicated namespace formula to match. +- `CodeGeneration/WrapperClassGenerator.cs:29` — emit into `metadata.Namespace`; prefix the three + class names with `metadata.TypePrefix`. Update per-file hint names to use the prefix so they stay + unique. +- Confirm the `IActionWrapper` implementation and interface list on the image classes are unchanged + (only names/namespace move). + +## 4. Runtime changes + +- `Plugin.cs:335-339` — build the image wrapper type name as + `$"{ChildClassNamespace}.{IdentifierSanitizer.Sanitize(ChildClassShortName + typeof(TEntity).Name + eventOperation + executionStage)}ActionWrapper"`, + exactly paralleling the Custom API path at `Plugin.cs:474`. This is the only runtime discovery + point; it must use the same sanitizer/format as the generator. + +## 5. Deletions & simplifications (the payoff) + +- **Delete** `CodeFixes/AliasedImageUsingsFixAllProvider.cs`. +- **Remove from `Helpers/SyntaxFactoryHelper.cs`:** `ApplyAliasedImageUsings` (both overloads), + `ConvertToAliasedUsingsAndQualifyRefs` (all overloads), `ImageAmbiguityRewriter`, + `DetectImageAmbiguity`, `GetAliasForImageNamespace`, `IsImageRegistrationNamespace`, and the + `qualifier` overload of `CreateImageParameterList`. Keep `CreateNameofExpression`, + `BuildSignatureDescription`, and a simplified `CreateImageParameterList` that takes a + **fully-qualified namespace** (see section 7). +- `AddUsingDirectiveIfMissing` — likely droppable for images (we go fully-qualified instead of adding + usings); keep only if still used elsewhere. + +## 6. Analyzer changes + +- `HandlerSignatureMismatchAnalyzer` (XPC4002/4003): update the *expected* image type resolution to + the new namespace + prefixed names. Comparison stays semantic (by symbol), so a user handler that + imports the type with a short name still matches. **Also make an unresolved/error-typed image + parameter count as a mismatch** so post-migration handlers (whose old `PreImage` type no longer + resolves) trigger the existing code fix rather than only a raw CS0246 — this is the migration hook + (see section 8). +- `ImageWithoutMethodReferenceAnalyzer` (XPC3003) and `FullEntityImageAnalyzer` (XPC3005): + naming-independent, no change expected — add a regression test to confirm. + +## 7. Code-fix changes (converge on the Custom API pattern) + +- `CreateHandlerMethodCodeFixProvider` and `FixHandlerSignatureCodeFixProvider`: + - Emit **fully-qualified** generated type names (`{PluginNamespace}.{prefix}PreImage`) in the + handler signature — exactly what the Custom API fixes already do (their tests assert + `TestNamespace.SomeApiResponse Handle(TestNamespace.SomeApiRequest request)`). No `using`, no + alias. + - Swap `GetFixAllProvider()` from `AliasedImageUsingsFixAllProvider` to + `WellKnownFixAllProviders.BatchFixer` — identical to the Custom API code fixes now. +- `PreferNameofCodeFixProvider`: unaffected by naming; verify. + +## 8. Migration strategy (breaking change) + +Existing handlers reference short `PreImage`/`PostImage` + `using {Ns}.PluginRegistrations.…;`. After +a rebuild the old namespace is gone, so those break (CS0234 on the using, CS0246 on the params). +Mitigation (pending decision — see "Open questions"): + +- **Auto-fix via enhanced XPC4002/4003** (section 6): once the old param type fails to resolve, the + signature analyzer flags a mismatch and its code fix rewrites the signature to the new + fully-qualified type; the stale `using` becomes an unused-using (CS8019/IDE0005), cleaned by + standard tooling. **OR** +- **Documented manual edit only**: rely on the compiler errors + a `docs` migration note. + +Note: we deliberately do **not** offer "switch to `IPluginImage`" as the migration —that +discards the generated typed properties (see section 1). Call the change out prominently as breaking +in the CHANGELOG with a short migration note either way. + +## 9. Tests + +- `GenerationTests/WrapperClassGenerationTests.cs` — update expected namespace + prefixed names; add a + case with **two registrations in one plugin** and **two plugins in one namespace** asserting unique, + collision-free names with no aliasing. +- `DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs`, + `CreateHandlerMethodCodeFixProviderTests.cs` — assert fully-qualified names; **delete** + aliasing/multi-registration-alias tests. +- New `DiagnosticTests/DuplicateStepRegistrationAnalyzerTests.cs` for XPC2002 (fires on dup; not on + distinct tuples; not across different plugins). +- New migration regression test: a handler with an unresolved old-style image param triggers XPC4002 + and the fix produces the new fully-qualified signature. +- Full `dotnet test` on both test projects; the whole suite (currently 119 in the SG tests) must stay + green minus the intentionally-removed aliasing tests. + +## 10. Docs & release tracking + +- **CLAUDE.md** "Type-Safe Images" section: rewrite the generated-code example to the flat scheme, + drop the "namespace isolation" bullet, and update the "How It Works" namespace line. Add XPC2002 to + the diagnostics coverage. Keep the "Image Interfaces" subsection as a documented escape hatch (only + when you need the raw `Entity`) — mention the interfaces exist but do not present them as the + recommended handler parameter type. +- `rules/XPC2002.md` new; `AnalyzerReleases.Unshipped.md` add XPC2002. +- **CHANGELOG** (`XrmPluginCore/CHANGELOG.md`): add a `Breaking:` line (the project already uses + `Breaking:` lines, e.g. v1.3.0). + +## 11. Versioning + +This changes generated type names + namespaces = breaking for consumers. Ship as **v1.5.0**, and +**rename the pending `### v1.4.2 - Unreleased` heading to `### v1.5.0 - Unreleased`**, folding the +XPC3007 bullets in alongside the new image bullets (1.4.2 is never released on its own). The repo +expresses breaking changes as `Breaking:` lines within minor bumps (precedent: v1.3.0), so add a +prominent `Breaking:` line + migration link under the 1.5.0 heading. + +Analyzer release tracking: XPC3007 currently sits in `AnalyzerReleases.Unshipped.md` alongside the +new XPC2002 — both move to `AnalyzerReleases.Shipped.md` under `## Release 1.5.0` when 1.5.0 ships. + +## 12. Suggested PR sequence + +1. **XPC2002** analyzer + rule doc + tests (independently valuable; establishes the invariant). +2. **Generator + runtime rename** behind the same discovery contract (sections 3-4) + generation + tests. +3. **Code-fix convergence + subsystem deletion** (sections 5, 7) + analyzer update (section 6) + + migration hook + test updates. +4. **Docs + CHANGELOG + release tracking** (sections 10-11). + +Splitting this way keeps each PR reviewable and green; step 2 is the only one that must land +atomically (generator and runtime must agree on the name format in one commit). + +## 13. Open questions / risks + +- **Concatenation ambiguity** (`"AB"+"CD…"` vs `"ABC"+"D…"`): theoretically possible with + separator-less concatenation, but it's the *same* risk the current `{Entity}{Operation}{Stage}` + scheme already accepts, and now bounded further by XPC2002 + unique class names. If we want it fully + eliminated, insert an unambiguous separator that's still a valid identifier char (e.g. `_`) between + segments — cheap, at the cost of an underscore in the type name. **Recommend adding the separator** + since we're touching this anyway. +- **Custom-message operations**: sanitization must be identical on generator and runtime; covered by + reusing `IdentifierSanitizer`, but worth an explicit shared test. +- **Non-image registrations**: plugins that register steps *without* images generate no wrapper types + today; confirm the rename doesn't accidentally start emitting types for them. From 3a0064fcd43cc50799477d252e3fe9c75048056e Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Fri, 3 Jul 2026 10:46:18 +0200 Subject: [PATCH 2/5] Flatten plugin image generation into the plugin namespace Generate PreImage/PostImage/ActionWrapper into the plugin's own namespace with registration-derived names ({PluginClass}{Entity}{Operation}{Stage}{Kind}), mirroring the Custom API request/response scheme, instead of a per-registration {...}.PluginRegistrations.{...} namespace with bare type names. The runtime wrapper discovery in Plugin.RegisterStep is updated to match (shared IdentifierSanitizer). Because generated type names are now unique within the plugin namespace, the entire aliased-usings subsystem is removed: the signature/create-method code fixes emit fully-qualified image parameter types (no using directives) and use WellKnownFixAllProviders.BatchFixer. Deletes AliasedImageUsingsFixAllProvider and the alias/requalification helpers in SyntaxFactoryHelper. The signature-mismatch analyzer now matches the concrete wrapper by fully-qualified name and reports the expected type names to the code fixes. Uniqueness within the namespace is guaranteed by XPC2002 (one registration per entity/operation/stage per plugin) plus distinct plugin class names. Tests and end-to-end TypeSafe plugins updated to the new names; alias-specific code-fix tests removed. CHANGELOG marks the breaking change under v1.5.0. Co-Authored-By: Claude via Conducktor --- ...CreateHandlerMethodCodeFixProviderTests.cs | 335 ++------- .../DiagnosticReportingTests.cs | 10 +- ...FixHandlerSignatureCodeFixProviderTests.cs | 653 +----------------- .../WrapperClassGenerationTests.cs | 48 +- .../Helpers/TestFixtures.cs | 40 +- .../IntegrationTests/CompilationTests.cs | 24 +- .../HandlerMethodNotFoundAnalyzer.cs | 17 +- .../HandlerSignatureMismatchAnalyzer.cs | 90 +-- .../AliasedImageUsingsFixAllProvider.cs | 348 ---------- .../CreateHandlerMethodCodeFixProvider.cs | 74 +- .../FixHandlerSignatureCodeFixProvider.cs | 93 +-- .../CodeGeneration/WrapperClassGenerator.cs | 23 +- XrmPluginCore.SourceGenerator/Constants.cs | 5 +- .../Helpers/RegisterStepHelper.cs | 20 +- .../Helpers/SyntaxFactoryHelper.cs | 381 +--------- .../Models/PluginStepMetadata.cs | 23 +- .../TypeSafe/TypeSafeAccountPlugin.cs | 14 +- .../TypeSafe/TypeSafeAccountService.cs | 8 +- .../TypeSafe/TypeSafeContactPlugin.cs | 11 +- .../TypeSafe/TypeSafeContactService.cs | 8 +- XrmPluginCore/CHANGELOG.md | 1 + XrmPluginCore/Plugin.cs | 9 +- 22 files changed, 326 insertions(+), 1909 deletions(-) delete mode 100644 XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs index b11b814..9738efc 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs @@ -7,179 +7,67 @@ namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; /// -/// Tests for CreateHandlerMethodCodeFixProvider that creates missing handler methods -/// and adds using directives for generated image namespaces. +/// Tests for CreateHandlerMethodCodeFixProvider that creates missing handler methods. Image parameters +/// are emitted with their fully-qualified generated wrapper type names (which live in the plugin's own +/// namespace), so no using directive is added. /// public class CreateHandlerMethodCodeFixProviderTests : CodeFixTestBase { + private const string Pre = "TestNamespace.TestPluginAccountUpdatePostOperationPreImage"; + private const string Post = "TestNamespace.TestPluginAccountUpdatePostOperationPostImage"; + [Fact] - public async Task Should_Create_Method_And_Add_Using_For_PreImage() + public async Task Should_Create_Method_For_PreImage() { - // Arrange - Method doesn't exist on interface, WithPreImage registered - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - "HandleUpdate") + var source = WrapPlugin(""" .AddFilteredAttributes(x => x.Name) .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - } - - public class TestService : ITestService - { - } - } - """; + """); - // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Method created with alias-qualified PreImage parameter - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - - // Assert - Aliased using directive added - fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + fixedSource.Should().Contain($"void HandleUpdate({Pre} preImage)"); + fixedSource.Should().NotContain("using TestNamespace.PluginRegistrations"); } [Fact] - public async Task Should_Create_Method_And_Add_Using_For_Both_Images() + public async Task Should_Create_Method_For_Both_Images() { - // Arrange - Method doesn't exist on interface, both images registered - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - "HandleUpdate") + var source = WrapPlugin(""" .AddFilteredAttributes(x => x.Name) .WithPreImage(x => x.Name, x => x.Revenue) .WithPostImage(x => x.Name, x => x.AccountNumber); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - } - - public class TestService : ITestService - { - } - } - """; + """); - // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Method created with both alias-qualified image parameters - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage, AccountUpdatePostOperation.PostImage postImage)"); - - // Assert - Aliased using directive added - fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + fixedSource.Should().Contain($"void HandleUpdate({Pre} preImage, {Post} postImage)"); } [Fact] - public async Task Should_Create_Method_Without_Using_When_No_Images() + public async Task Should_Create_Method_Without_Images() { - // Arrange - Method doesn't exist on interface, no images registered - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - "HandleUpdate") + var source = WrapPlugin(""" .AddFilteredAttributes(x => x.Name); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } + """); - public interface ITestService - { - } - - public class TestService : ITestService - { - } - } - """; - - // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Method created without image parameters fixedSource.Should().Contain("void HandleUpdate()"); - - // Assert - No image namespace using directive added fixedSource.Should().NotContain("using TestNamespace.PluginRegistrations"); } [Fact] - public async Task Should_Use_Aliased_Usings_For_Multiple_Registrations() + public async Task FixAll_Should_Create_Methods_Across_Services() { - // Arrange - A plain using already exists for the Update registration; the Delete handler is - // missing from the interface. Creating Delete must requalify the existing Update reference to - // its alias (resolved via the semantic model) and add the Delete using in aliased form. + // Two registrations, each on its own service, both missing their handler method. const string source = """ using System; - using System.ComponentModel; using Microsoft.Xrm.Sdk; using XrmPluginCore; using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using XrmPluginCore.Tests.Context.BusinessDomain; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -187,78 +75,54 @@ public class TestPlugin : Plugin { public TestPlugin() { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + "HandleUpdate") + .WithPreImage(x => x.Name); - RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, - "HandleDelete") - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); + RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, + "HandleDelete") + .WithPreImage(x => x.Name); } protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(PreImage preImage); - } - - public class TestService : ITestService - { - public void HandleUpdate(PreImage preImage) { } + => services.AddScoped().AddScoped(); } - } - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } + public interface IUpdateService { } + public interface IDeleteService { } - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } + public class UpdateService : IUpdateService { } + public class DeleteService : IDeleteService { } } """; - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - New method created with its own alias-qualified type - fixedSource.Should().Contain("void HandleDelete(AccountDeletePostOperation.PreImage preImage)"); - - // Assert - Existing bare PreImage references requalified with the Update alias - CountOccurrences(fixedSource, "AccountUpdatePostOperation.PreImage preImage").Should().BeGreaterOrEqualTo(1); - - // Assert - Each namespace aliased exactly once - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the existing plain using should be converted to aliased form"); - CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;") - .Should().Be(1, "the new using should be added in aliased form"); + var fixedSource = await ApplyFixAllAsync(source); - AssertNoAmbiguousReferences(fixedSource); + fixedSource.Should().Contain("void HandleUpdate(TestNamespace.TestPluginAccountUpdatePostOperationPreImage preImage)"); + fixedSource.Should().Contain("void HandleDelete(TestNamespace.TestPluginAccountDeletePostOperationPreImage preImage)"); } [Fact] - public async Task FixAll_Should_Create_Multiple_Methods_On_Same_Interface() + public async Task Should_Add_Method_To_Correct_Interface_When_Names_Collide() { - // Arrange - Two same-service registrations, both missing their handler methods. + // A decoy interface with the SAME simple name lives in another namespace and is declared FIRST. + // The registered service is TestNamespace.ITestService; the fix must target it, not the decoy. const string source = """ using System; - using System.ComponentModel; using Microsoft.Xrm.Sdk; using XrmPluginCore; using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using XrmPluginCore.Tests.Context.BusinessDomain; + namespace DecoyNamespace + { + public interface ITestService + { + void SomethingElse(); + } + } + namespace TestNamespace { public class TestPlugin : Plugin @@ -267,19 +131,11 @@ public TestPlugin() { RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, "HandleUpdate") - .AddFilteredAttributes(x => x.Name) .WithPreImage(x => x.Name, x => x.Revenue); - - RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, - "HandleDelete") - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); } protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } + => services.AddScoped(); } public interface ITestService @@ -290,57 +146,29 @@ public class TestService : ITestService { } } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } """; - // Act - Apply the consolidating FixAll across all diagnostics in one pass - var fixedSource = await ApplyFixAllAsync(source); - - // Assert - Both methods created with their own alias-qualified parameter - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - fixedSource.Should().Contain("void HandleDelete(AccountDeletePostOperation.PreImage preImage)"); + var fixedSource = await ApplyCodeFixAsync(source); - // Assert - One aliased using per distinct namespace (no duplicates) - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;").Should().Be(1); - CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;").Should().Be(1); + var compilation = CompilationHelper.CreateCompilation(fixedSource); + var registered = compilation.GetTypeByMetadataName("TestNamespace.ITestService"); + var decoy = compilation.GetTypeByMetadataName("DecoyNamespace.ITestService"); - AssertNoAmbiguousReferences(fixedSource); + registered.Should().NotBeNull(); + decoy.Should().NotBeNull(); + registered!.GetMembers("HandleUpdate").Should().HaveCount(1, "the method must be added to the registered interface"); + decoy!.GetMembers("HandleUpdate").Should().BeEmpty("the method must not be added to the same-named decoy interface"); } - [Fact] - public async Task Should_Add_Method_To_Correct_Interface_When_Names_Collide() - { - // Arrange - A decoy interface with the SAME simple name lives in another namespace and is - // declared FIRST in the document. The registered service is TestNamespace.ITestService. The - // fix must add the method to the registered interface, not the first one matching by name. - const string source = """ + private static string WrapPlugin(string registrationTail) => + $$""" using System; - using System.ComponentModel; using Microsoft.Xrm.Sdk; using XrmPluginCore; using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using XrmPluginCore.Tests.Context.BusinessDomain; - namespace DecoyNamespace - { - public interface ITestService - { - void SomethingElse(); - } - } - namespace TestNamespace { public class TestPlugin : Plugin @@ -349,14 +177,11 @@ public TestPlugin() { RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, "HandleUpdate") - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); + {{registrationTail}} } protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } + => services.AddScoped(); } public interface ITestService @@ -367,52 +192,8 @@ public class TestService : ITestService { } } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } """; - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - The method landed on the registered interface, not the decoy - var compilation = CompilationHelper.CreateCompilation(fixedSource); - var registered = compilation.GetTypeByMetadataName("TestNamespace.ITestService"); - var decoy = compilation.GetTypeByMetadataName("DecoyNamespace.ITestService"); - - registered.Should().NotBeNull(); - decoy.Should().NotBeNull(); - registered!.GetMembers("HandleUpdate").Should().HaveCount(1, "the method must be added to the registered interface"); - decoy!.GetMembers("HandleUpdate").Should().BeEmpty("the method must not be added to the same-named decoy interface"); - - AssertNoAmbiguousReferences(fixedSource); - } - - private static void AssertNoAmbiguousReferences(string source) - { - var compilation = CompilationHelper.CreateCompilation(source); - var ambiguous = compilation.GetDiagnostics() - .Where(d => d.Id == "CS0104") - .Select(d => d.GetMessage()) - .ToList(); - ambiguous.Should().BeEmpty("the fixed source should not contain ambiguous references"); - } - - private static int CountOccurrences(string source, string search) - { - var count = 0; - var index = 0; - while ((index = source.IndexOf(search, index, StringComparison.Ordinal)) != -1) - { - count++; - index += search.Length; - } - return count; - } - private static Task ApplyCodeFixAsync(string source) => ApplyCodeFixAsync(source, new HandlerMethodNotFoundAnalyzer(), new CreateHandlerMethodCodeFixProvider(), DiagnosticDescriptors.HandlerMethodNotFound.Id); diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs index fec4594..5810c52 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs @@ -1004,7 +1004,7 @@ public void Process() { } // Verify PreImage class is generated var generatedSource = result.GeneratedTrees.First().ToString(); - generatedSource.Should().Contain("public sealed class PreImage", + generatedSource.Should().Contain("public sealed class TestPluginAccountUpdatePostOperationPreImage", "PreImage class should be generated to allow developers to create the handler method with correct signature"); } @@ -1064,10 +1064,10 @@ public void MethodWithoutImage() { } // Verify PreImage class is generated var generatedSource = result.GeneratedTrees.First().ToString(); - generatedSource.Should().Contain("public sealed class PreImage", + generatedSource.Should().Contain("public sealed class TestPluginAccountUpdatePostOperationPreImage", "PreImage class should be generated to allow developers to create the handler method with correct signature"); - generatedSource.Should().Contain("namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation", + generatedSource.Should().Contain("namespace TestNamespace", "Generated types should be in the correct namespace"); } @@ -1160,14 +1160,14 @@ public void HandleUpdate() { } // Verify Namespace1 source: correct namespace AND correct property (Name) var namespace1Source = sourcesByHintName[namespace1HintName]; - namespace1Source.Should().Contain("namespace Namespace1.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation", + namespace1Source.Should().Contain("namespace Namespace1", "Namespace1 hint name should map to Namespace1 generated namespace"); namespace1Source.Should().Contain("public string? Name =>", "Namespace1 plugin registered Name attribute"); // Verify Namespace2 source: correct namespace AND correct property (AccountNumber) var namespace2Source = sourcesByHintName[namespace2HintName]; - namespace2Source.Should().Contain("namespace Namespace2.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation", + namespace2Source.Should().Contain("namespace Namespace2", "Namespace2 hint name should map to Namespace2 generated namespace"); namespace2Source.Should().Contain("public string? AccountNumber =>", "Namespace2 plugin registered AccountNumber attribute"); diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs index 881e6d8..bc27492 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs @@ -7,389 +7,70 @@ namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; /// -/// Tests for FixHandlerSignatureCodeFixProvider that fixes handler method signatures -/// and adds using directives for generated image namespaces. +/// Tests for FixHandlerSignatureCodeFixProvider that fixes handler method signatures to match +/// registered images. Image parameters are emitted with their fully-qualified generated wrapper type +/// names (which live in the plugin's own namespace), so no using directive is added. /// public class FixHandlerSignatureCodeFixProviderTests : CodeFixTestBase { + private const string Pre = "TestNamespace.TestPluginAccountUpdatePostOperationPreImage"; + private const string Post = "TestNamespace.TestPluginAccountUpdatePostOperationPostImage"; + [Fact] - public async Task Should_Fix_Signature_And_Add_Using_For_PreImage() + public async Task Should_Fix_Signature_For_PreImage() { - // Arrange - Handler has no params, but WithPreImage is registered - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + var source = WrapPlugin(""" nameof(ITestService.HandleUpdate)) .AddFilteredAttributes(x => x.Name) .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } + """, "void HandleUpdate();", "public void HandleUpdate() { }"); - public interface ITestService - { - void HandleUpdate(); - } - - public class TestService : ITestService - { - public void HandleUpdate() { } - } - } - """; - - // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed (alias-qualified) - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - - // Assert - Aliased using directive added - fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + fixedSource.Should().Contain($"void HandleUpdate({Pre} preImage)"); + fixedSource.Should().NotContain("using TestNamespace.PluginRegistrations"); } [Fact] - public async Task Should_Fix_Signature_And_Add_Using_For_PostImage() + public async Task Should_Fix_Signature_For_PostImage() { - // Arrange - Handler has no params, but WithPostImage is registered - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + var source = WrapPlugin(""" nameof(ITestService.HandleUpdate)) .AddFilteredAttributes(x => x.Name) .WithPostImage(x => x.Name, x => x.AccountNumber); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } + """, "void HandleUpdate();", "public void HandleUpdate() { }"); - public interface ITestService - { - void HandleUpdate(); - } - - public class TestService : ITestService - { - public void HandleUpdate() { } - } - } - """; - - // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed (alias-qualified) - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PostImage postImage)"); - - // Assert - Aliased using directive added - fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + fixedSource.Should().Contain($"void HandleUpdate({Post} postImage)"); } [Fact] - public async Task Should_Fix_Signature_And_Add_Using_For_Both_Images() + public async Task Should_Fix_Signature_For_Both_Images() { - // Arrange - Handler has no params, but both images are registered - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + var source = WrapPlugin(""" nameof(ITestService.HandleUpdate)) .AddFilteredAttributes(x => x.Name) .WithPreImage(x => x.Name, x => x.Revenue) .WithPostImage(x => x.Name, x => x.AccountNumber); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(); - } - - public class TestService : ITestService - { - public void HandleUpdate() { } - } - } - """; - - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - Signature fixed with both image parameters (alias-qualified) - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage, AccountUpdatePostOperation.PostImage postImage)"); - - // Assert - Aliased using directive added - fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); - } - - [Fact] - public async Task Should_Not_Duplicate_Using_When_Already_Present() - { - // Arrange - Using directive already exists, handler has wrong signature - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(); - } - - public class TestService : ITestService - { - public void HandleUpdate() { } - } - } - """; - - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - Signature fixed (alias-qualified) - fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - - // Assert - The pre-existing plain using is converted to the aliased form (not duplicated) - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the using directive should be converted to aliased form exactly once"); - CountOccurrences(fixedSource, "using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(0, "the plain using should no longer be present"); - } - - [Fact] - public async Task Should_Use_Aliased_Usings_For_Multiple_Registrations() - { - // Arrange - A plain using already exists for the Update registration; the Delete handler is - // missing its image parameter. Fixing Delete must requalify the existing Update reference to - // its alias (resolved via the semantic model) and add the Delete using in aliased form. - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - - RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, - nameof(ITestService.HandleDelete)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(PreImage preImage); - void HandleDelete(); - } - - public class TestService : ITestService - { - public void HandleUpdate(PreImage preImage) { } - public void HandleDelete() { } - } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - """; + """, "void HandleUpdate();", "public void HandleUpdate() { }"); - // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Both signatures alias-qualified (interface + implementation) - CountOccurrences(fixedSource, "void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)").Should().Be(2, "both the interface and implementation are updated"); - CountOccurrences(fixedSource, "void HandleDelete(AccountDeletePostOperation.PreImage preImage)").Should().Be(2, "both the interface and implementation are updated"); - - // Assert - Each namespace aliased exactly once - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the existing plain using should be converted to aliased form exactly once"); - CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;") - .Should().Be(1, "the new using should be added in aliased form exactly once"); - - // Assert - Result compiles without ambiguous-reference (CS0104) errors - AssertNoAmbiguousReferences(fixedSource); - } - - [Fact] - public async Task Should_Not_Duplicate_Already_Aliased_Using() - { - // Arrange - The aliased using is already present, but the handler still needs its parameter. - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(); - } - - public class TestService : ITestService - { - public void HandleUpdate() { } - } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - """; - - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - Signature fixed with the alias-qualified parameter - CountOccurrences(fixedSource, "void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)").Should().Be(2); - - // Assert - The already-present aliased using is not duplicated - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the existing aliased using should not be duplicated"); - - AssertNoAmbiguousReferences(fixedSource); + fixedSource.Should().Contain($"void HandleUpdate({Pre} preImage, {Post} postImage)"); } [Fact] - public async Task Should_Realias_CleanedUp_Plain_Using() + public async Task FixAll_Should_Fix_Multiple_Registrations() { - // Arrange - The Update registration is fully aliased and correct (no diagnostic). A developer - // has "cleaned up" the Delete import back to a plain using, and the Delete handler is missing - // its parameter. After the fix, every image using must be aliased exactly once. + // Two registrations on the same service, both with mismatched (empty) handler signatures. const string source = """ using System; - using System.ComponentModel; using Microsoft.Xrm.Sdk; using XrmPluginCore; using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using XrmPluginCore.Tests.Context.BusinessDomain; - using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; - using TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation; namespace TestNamespace { @@ -399,255 +80,46 @@ public TestPlugin() { RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); + .WithPreImage(x => x.Name); - RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, - nameof(ITestService.HandleDelete)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); + RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, + nameof(ITestService.HandleDelete)) + .WithPreImage(x => x.Name); } protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(AccountUpdatePostOperation.PreImage preImage); - void HandleDelete(); - } - - public class TestService : ITestService - { - public void HandleUpdate(AccountUpdatePostOperation.PreImage preImage) { } - public void HandleDelete() { } - } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - """; - - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - The stale plain Delete using is converted to aliased form - CountOccurrences(fixedSource, "using TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;") - .Should().Be(0, "the plain using should be re-aliased"); - - // Assert - Every image namespace is aliased exactly once - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1); - CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;") - .Should().Be(1); - - // Assert - Delete signature fixed, Update signature preserved - CountOccurrences(fixedSource, "void HandleDelete(AccountDeletePostOperation.PreImage preImage)").Should().Be(2); - CountOccurrences(fixedSource, "void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)").Should().Be(2); - - AssertNoAmbiguousReferences(fixedSource); - } - - [Fact] - public async Task Should_Requalify_Each_Reference_Under_Multiple_Plain_Usings() - { - // Arrange - Two plain image usings are present at once. The Update handler references a bare - // PreImage and the Delete handler a bare PostImage (each generated namespace only exposes the - // image type its registration declared, so each bare reference still binds uniquely). The - // Create handler is missing its parameter and triggers the fix. The fix must requalify each - // pre-existing reference to its OWN alias via the semantic model - the old break-on-first - // logic would have mis-qualified one of them. - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; - using TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - - RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, - nameof(ITestService.HandleDelete)) - .AddFilteredAttributes(x => x.Name) - .WithPostImage(x => x.Name, x => x.AccountNumber); - - RegisterStep(EventOperation.Create, ExecutionStage.PostOperation, - nameof(ITestService.HandleCreate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } - } - - public interface ITestService - { - void HandleUpdate(PreImage preImage); - void HandleDelete(PostImage postImage); - void HandleCreate(); - } - - public class TestService : ITestService - { - public void HandleUpdate(PreImage preImage) { } - public void HandleDelete(PostImage postImage) { } - public void HandleCreate() { } - } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation - { - public sealed class PostImage { } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountCreatePostOperation - { - public sealed class PreImage { } - } - """; - - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - Each pre-existing reference requalified to its OWN alias (never cross-qualified) - CountOccurrences(fixedSource, "void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)").Should().Be(2); - CountOccurrences(fixedSource, "void HandleDelete(AccountDeletePostOperation.PostImage postImage)").Should().Be(2); - - // Assert - The triggering handler created with its own alias - CountOccurrences(fixedSource, "void HandleCreate(AccountCreatePostOperation.PreImage preImage)").Should().Be(2); - - // Assert - Each namespace aliased exactly once - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;").Should().Be(1); - CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;").Should().Be(1); - CountOccurrences(fixedSource, "using AccountCreatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountCreatePostOperation;").Should().Be(1); - - AssertNoAmbiguousReferences(fixedSource); - } - - [Fact] - public async Task FixAll_Should_Consolidate_Multiple_Registrations() - { - // Arrange - Two same-service registrations both trigger the diagnostic. - const string source = """ - using System; - using System.ComponentModel; - using Microsoft.Xrm.Sdk; - using XrmPluginCore; - using XrmPluginCore.Enums; - using Microsoft.Extensions.DependencyInjection; - using XrmPluginCore.Tests.Context.BusinessDomain; - - namespace TestNamespace - { - public class TestPlugin : Plugin - { - public TestPlugin() - { - RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - - RegisterStep(EventOperation.Delete, ExecutionStage.PostOperation, - nameof(ITestService.HandleDelete)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); - } - - protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } + => services.AddScoped(); } public interface ITestService { void HandleUpdate(); - void HandleDelete(); + void HandleDelete(); } public class TestService : ITestService { public void HandleUpdate() { } - public void HandleDelete() { } + public void HandleDelete() { } } } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } """; - // Act - Apply the consolidating FixAll across all diagnostics in one pass var fixedSource = await ApplyFixAllAsync(source); - // Assert - Every signature alias-qualified (interface + implementation) - CountOccurrences(fixedSource, "void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)").Should().Be(2); - CountOccurrences(fixedSource, "void HandleDelete(AccountDeletePostOperation.PreImage preImage)").Should().Be(2); - - // Assert - One aliased using per distinct namespace (no duplicates) - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;").Should().Be(1); - CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;").Should().Be(1); - - AssertNoAmbiguousReferences(fixedSource); + // Both handlers fixed on interface + implementation, each with its own registration-derived type. + CountOccurrences(fixedSource, "void HandleUpdate(TestNamespace.TestPluginAccountUpdatePostOperationPreImage preImage)").Should().Be(2); + CountOccurrences(fixedSource, "void HandleDelete(TestNamespace.TestPluginAccountDeletePostOperationPreImage preImage)").Should().Be(2); } - [Fact] - public async Task Should_Add_Standard_Alias_When_Namespace_Imported_Under_Different_Alias() - { - // Arrange - The image namespace is already imported, but under a NON-standard alias ("Foo"). - // The emitted parameter type uses the standard alias (the last namespace segment), so the fix - // must still add the standard aliased using - otherwise the qualified type fails to resolve. - const string source = """ + private static string WrapPlugin(string registrationTail, string interfaceMethod, string implMethod) => + $$""" using System; - using System.ComponentModel; using Microsoft.Xrm.Sdk; using XrmPluginCore; using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using XrmPluginCore.Tests.Context.BusinessDomain; - using Foo = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -656,76 +128,25 @@ public class TestPlugin : Plugin public TestPlugin() { RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, - nameof(ITestService.HandleUpdate)) - .AddFilteredAttributes(x => x.Name) - .WithPreImage(x => x.Name, x => x.Revenue); + {{registrationTail}} } protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) - { - return services.AddScoped(); - } + => services.AddScoped(); } public interface ITestService { - void HandleUpdate(); + {{interfaceMethod}} } public class TestService : ITestService { - public void HandleUpdate() { } + {{implMethod}} } } - - namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation - { - public sealed class PreImage { } - public sealed class PostImage { } - } """; - // Act - var fixedSource = await ApplyCodeFixAsync(source); - - // Assert - Signature uses the standard alias - CountOccurrences(fixedSource, "void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)").Should().Be(2); - - // Assert - The standard aliased using is added even though "Foo" already imports the namespace - CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the standard alias must be present so the qualified parameter type resolves"); - - // Assert - The pre-existing non-standard alias is left untouched - CountOccurrences(fixedSource, "using Foo = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the existing alias should not be removed"); - - // Assert - Compiles cleanly: no ambiguity AND no unresolved standard alias - AssertCompilesWithoutReferenceErrors(fixedSource); - } - - private static void AssertNoAmbiguousReferences(string source) - { - var compilation = CompilationHelper.CreateCompilation(source); - var ambiguous = compilation.GetDiagnostics() - .Where(d => d.Id == "CS0104") - .Select(d => d.GetMessage()) - .ToList(); - ambiguous.Should().BeEmpty("the fixed source should not contain ambiguous references"); - } - - private static void AssertCompilesWithoutReferenceErrors(string source) - { - var compilation = CompilationHelper.CreateCompilation(source); - - // CS0104 = ambiguous reference; CS0246/CS0234 = unresolved type / namespace member, which is - // how a missing alias (qualified type whose alias was never added) surfaces. - var errors = compilation.GetDiagnostics() - .Where(d => d.Id is "CS0104" or "CS0246" or "CS0234") - .Select(d => d.GetMessage()) - .ToList(); - errors.Should().BeEmpty("the fixed source should resolve all image references"); - } - private static int CountOccurrences(string source, string search) { var count = 0; diff --git a/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs b/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs index 38c8b40..42808cb 100644 --- a/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs @@ -27,13 +27,13 @@ public void Should_Generate_PreImage_Class_With_Properties() var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify class structure - generatedSource.Should().Contain($"public sealed class PreImage : IPluginPreImage<{ContextNamespace}.Account>"); + generatedSource.Should().Contain($"public sealed class TestPluginAccountUpdatePostOperationPreImage : IPluginPreImage<{ContextNamespace}.Account>"); generatedSource.Should().Contain($"public {ContextNamespace}.Account Entity {{ get; }}"); - generatedSource.Should().Contain("public PreImage(Entity entity)"); + generatedSource.Should().Contain("public TestPluginAccountUpdatePostOperationPreImage(Entity entity)"); generatedSource.Should().Contain($"Entity = entity.ToEntity<{ContextNamespace}.Account>();"); // Verify NO PostImage class is generated - generatedSource.Should().NotContain("public sealed class PostImage"); + generatedSource.Should().NotContain("class TestPluginAccountUpdatePostOperationPostImage"); // Verify NO PostEntityImages retrieval generatedSource.Should().NotContain("PostEntityImages"); @@ -55,9 +55,9 @@ public void Should_Generate_PostImage_Class_With_Properties() var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify class structure - generatedSource.Should().Contain($"public sealed class PostImage : IPluginPostImage<{ContextNamespace}.Account>"); + generatedSource.Should().Contain($"public sealed class TestPluginAccountUpdatePostOperationPostImage : IPluginPostImage<{ContextNamespace}.Account>"); generatedSource.Should().Contain($"public {ContextNamespace}.Account Entity {{ get; }}"); - generatedSource.Should().Contain("public PostImage(Entity entity)"); + generatedSource.Should().Contain("public TestPluginAccountUpdatePostOperationPostImage(Entity entity)"); generatedSource.Should().Contain($"Entity = entity.ToEntity<{ContextNamespace}.Account>();"); // Verify properties forward to Entity @@ -66,7 +66,7 @@ public void Should_Generate_PostImage_Class_With_Properties() HasAccountNumberSummary().Matches(generatedSource).Count.Should().Be(1, $"AccountNumber property should have correct XML summary. Generated source:\n{generatedSource}"); // Verify NO PreImage class is generated - generatedSource.Should().NotContain("public sealed class PreImage"); + generatedSource.Should().NotContain("class TestPluginAccountUpdatePostOperationPreImage"); // Verify NO PreEntityImages retrieval generatedSource.Should().NotContain("PreEntityImages"); @@ -93,9 +93,9 @@ public void Should_Generate_Both_Image_Classes_In_Same_Namespace() namespaceCount.Should().Be(1, "all classes should be in the same namespace"); // All classes should exist - generatedSource.Should().Contain($"public sealed class PreImage : IPluginPreImage<{ContextNamespace}.Account>"); - generatedSource.Should().Contain($"public sealed class PostImage : IPluginPostImage<{ContextNamespace}.Account>"); - generatedSource.Should().Contain("internal sealed class ActionWrapper : IActionWrapper"); + generatedSource.Should().Contain($"public sealed class TestPluginAccountUpdatePostOperationPreImage : IPluginPreImage<{ContextNamespace}.Account>"); + generatedSource.Should().Contain($"public sealed class TestPluginAccountUpdatePostOperationPostImage : IPluginPostImage<{ContextNamespace}.Account>"); + generatedSource.Should().Contain("internal sealed class TestPluginAccountUpdatePostOperationActionWrapper : IActionWrapper"); } [Theory] @@ -166,7 +166,7 @@ public void Should_Generate_ActionWrapper_Class() var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify ActionWrapper class structure (now implements IActionWrapper interface) - generatedSource.Should().Contain("internal sealed class ActionWrapper : IActionWrapper"); + generatedSource.Should().Contain("internal sealed class TestPluginAccountUpdatePostOperationActionWrapper : IActionWrapper"); generatedSource.Should().Contain("public Action CreateAction()"); generatedSource.Should().Contain("var service = serviceProvider.GetRequiredService();"); @@ -194,7 +194,7 @@ public void Should_Generate_ActionWrapper_With_PreImage_Call() // Verify PreImage handling (now inline instead of using PluginImageHelper) generatedSource.Should().Contain("var preImageEntity = context?.PreEntityImages?.Values?.FirstOrDefault();"); - generatedSource.Should().Contain("var preImage = preImageEntity != null ? new PreImage(preImageEntity) : null;"); + generatedSource.Should().Contain("var preImage = preImageEntity != null ? new TestPluginAccountUpdatePostOperationPreImage(preImageEntity) : null;"); generatedSource.Should().Contain("service.HandleAccountUpdate(preImage)"); // Should have using statement @@ -221,7 +221,7 @@ public void Should_Generate_ActionWrapper_With_PostImage_Call() // Verify PostImage handling (now inline instead of using PluginImageHelper) generatedSource.Should().Contain("var postImageEntity = context?.PostEntityImages?.Values?.FirstOrDefault();"); - generatedSource.Should().Contain("var postImage = postImageEntity != null ? new PostImage(postImageEntity) : null;"); + generatedSource.Should().Contain("var postImage = postImageEntity != null ? new TestPluginAccountUpdatePostOperationPostImage(postImageEntity) : null;"); generatedSource.Should().Contain("service.HandleAccountUpdate(postImage)"); // Should have using statement @@ -248,9 +248,9 @@ public void Should_Generate_ActionWrapper_With_Both_Images() // Verify both images are handled (now inline instead of using PluginImageHelper) generatedSource.Should().Contain("var preImageEntity = context?.PreEntityImages?.Values?.FirstOrDefault();"); - generatedSource.Should().Contain("var preImage = preImageEntity != null ? new PreImage(preImageEntity) : null;"); + generatedSource.Should().Contain("var preImage = preImageEntity != null ? new TestPluginAccountUpdatePostOperationPreImage(preImageEntity) : null;"); generatedSource.Should().Contain("var postImageEntity = context?.PostEntityImages?.Values?.FirstOrDefault();"); - generatedSource.Should().Contain("var postImage = postImageEntity != null ? new PostImage(postImageEntity) : null;"); + generatedSource.Should().Contain("var postImage = postImageEntity != null ? new TestPluginAccountUpdatePostOperationPostImage(postImageEntity) : null;"); generatedSource.Should().Contain("service.HandleAccountUpdate(preImage, postImage)"); // Should have using statement @@ -277,15 +277,15 @@ public void Should_Generate_ActionWrapper_For_Handler_Without_Images() var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify ActionWrapper class structure is generated - generatedSource.Should().Contain("internal sealed class ActionWrapper : IActionWrapper"); + generatedSource.Should().Contain("internal sealed class TestPluginAccountUpdatePostOperationActionWrapper : IActionWrapper"); generatedSource.Should().Contain("public Action CreateAction()"); // Verify service method is called WITHOUT any image parameters generatedSource.Should().Contain("service.HandleUpdate()"); // Verify NO PreImage or PostImage classes are generated - generatedSource.Should().NotContain("public class PreImage"); - generatedSource.Should().NotContain("public class PostImage"); + generatedSource.Should().NotContain("class TestPluginAccountUpdatePostOperationPreImage"); + generatedSource.Should().NotContain("class TestPluginAccountUpdatePostOperationPostImage"); // Verify NO image entity retrieval is generated generatedSource.Should().NotContain("PreEntityImages"); @@ -349,7 +349,7 @@ public void Should_Generate_PreImage_With_All_Entity_Properties_When_No_Attribut var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify PreImage class is generated - generatedSource.Should().Contain($"public sealed class PreImage : IPluginPreImage<{ContextNamespace}.Account>"); + generatedSource.Should().Contain($"public sealed class TestPluginAccountUpdatePostOperationPreImage : IPluginPreImage<{ContextNamespace}.Account>"); // Verify that multiple entity properties are present (full entity = all properties) generatedSource.Should().Contain("public string? Name => Entity.Name;"); @@ -357,7 +357,7 @@ public void Should_Generate_PreImage_With_All_Entity_Properties_When_No_Attribut generatedSource.Should().Contain("public string? AccountNumber => Entity.AccountNumber;"); // Verify NO PostImage class is generated - generatedSource.Should().NotContain("public sealed class PostImage"); + generatedSource.Should().NotContain("class TestPluginAccountUpdatePostOperationPostImage"); } [Fact] @@ -376,7 +376,7 @@ public void Should_Generate_PostImage_With_All_Entity_Properties_When_No_Attribu var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify PostImage class is generated - generatedSource.Should().Contain($"public sealed class PostImage : IPluginPostImage<{ContextNamespace}.Account>"); + generatedSource.Should().Contain($"public sealed class TestPluginAccountUpdatePostOperationPostImage : IPluginPostImage<{ContextNamespace}.Account>"); // Verify that multiple entity properties are present (full entity = all properties) generatedSource.Should().Contain("public string? Name => Entity.Name;"); @@ -384,7 +384,7 @@ public void Should_Generate_PostImage_With_All_Entity_Properties_When_No_Attribu generatedSource.Should().Contain("public string? AccountNumber => Entity.AccountNumber;"); // Verify NO PreImage class is generated - generatedSource.Should().NotContain("public sealed class PreImage"); + generatedSource.Should().NotContain("class TestPluginAccountUpdatePostOperationPreImage"); } [Fact] @@ -402,9 +402,9 @@ public void Should_Generate_ActionWrapper_With_Full_Entity_PreImage_Call() var generatedSource = result.GeneratedTrees[0].GetText().ToString(); // Verify ActionWrapper is generated and handles the PreImage - generatedSource.Should().Contain("internal sealed class ActionWrapper : IActionWrapper"); + generatedSource.Should().Contain("internal sealed class TestPluginAccountUpdatePostOperationActionWrapper : IActionWrapper"); generatedSource.Should().Contain("var preImageEntity = context?.PreEntityImages?.Values?.FirstOrDefault();"); - generatedSource.Should().Contain("var preImage = preImageEntity != null ? new PreImage(preImageEntity) : null;"); + generatedSource.Should().Contain("var preImage = preImageEntity != null ? new TestPluginAccountUpdatePostOperationPreImage(preImageEntity) : null;"); generatedSource.Should().Contain("service.HandleAccountUpdate(preImage)"); // Should NOT have PostImage retrieval @@ -435,7 +435,7 @@ public void Should_Mirror_Obsolete_Attribute_On_Deprecated_Image_Properties() generatedSource.Should().NotContain("[System.Obsolete]\n\t\tpublic string? Name"); } - [System.Text.RegularExpressions.GeneratedRegex(@"namespace\s+TestNamespace\.PluginRegistrations\.TestPlugin\.AccountUpdatePostOperation")] + [System.Text.RegularExpressions.GeneratedRegex(@"namespace\s+TestNamespace\b")] private static partial System.Text.RegularExpressions.Regex IsAccountUpdatePostOperationNamespace(); [System.Text.RegularExpressions.GeneratedRegex(@"\[CompilerGenerated\]")] diff --git a/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs b/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs index 077e5b3..001d37e 100644 --- a/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs +++ b/XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs @@ -39,7 +39,6 @@ public static string GetPluginWithPreImage(string entityClass = "Account") using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; -using TestNamespace.PluginRegistrations.TestPlugin.{entityClass}UpdatePostOperation; namespace TestNamespace {{ @@ -61,12 +60,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService {{ - void HandleAccountUpdate(PreImage preImage); + void HandleAccountUpdate(TestPlugin{entityClass}UpdatePostOperationPreImage preImage); }} public class TestService : ITestService {{ - public void HandleAccountUpdate(PreImage preImage) {{ }} + public void HandleAccountUpdate(TestPlugin{entityClass}UpdatePostOperationPreImage preImage) {{ }} }} }}"; } @@ -81,7 +80,6 @@ public void HandleAccountUpdate(PreImage preImage) {{ }} using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -103,12 +101,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService { - void HandleAccountUpdate(PostImage postImage); + void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPostImage postImage); } public class TestService : ITestService { - public void HandleAccountUpdate(PostImage postImage) { } + public void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPostImage postImage) { } } } """; @@ -123,7 +121,6 @@ public void HandleAccountUpdate(PostImage postImage) { } using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -146,12 +143,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService { - void HandleAccountUpdate(PreImage preImage, PostImage postImage); + void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage, TestPluginAccountUpdatePostOperationPostImage postImage); } public class TestService : ITestService { - public void HandleAccountUpdate(PreImage preImage, PostImage postImage) { } + public void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage, TestPluginAccountUpdatePostOperationPostImage postImage) { } } } """; @@ -167,7 +164,6 @@ public static string GetPluginWithoutImages(string action = "nameof(ITestService using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -208,7 +204,6 @@ public void HandleUpdate() { } using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -230,12 +225,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService { - void HandleAccountUpdate(PreImage preImage); + void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage); } public class TestService : ITestService { - public void HandleAccountUpdate(PreImage preImage) { } + public void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage) { } } } """; @@ -250,7 +245,6 @@ public void HandleAccountUpdate(PreImage preImage) { } using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -272,12 +266,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService { - void HandleAccountUpdate(PostImage postImage); + void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPostImage postImage); } public class TestService : ITestService { - public void HandleAccountUpdate(PostImage postImage) { } + public void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPostImage postImage) { } } } """; @@ -294,7 +288,6 @@ public void HandleAccountUpdate(PostImage postImage) { } using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -316,12 +309,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService { - void HandleAccountUpdate(PreImage preImage); + void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage); } public class TestService : ITestService { - public void HandleAccountUpdate(PreImage preImage) + public void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage) { // Only touches a non-deprecated member. var name = preImage.Name; @@ -341,7 +334,6 @@ public void HandleAccountUpdate(PreImage preImage) using XrmPluginCore.Enums; using Microsoft.Extensions.DependencyInjection; using TestNamespace; - using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation; namespace TestNamespace { @@ -363,12 +355,12 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle public interface ITestService { - void HandleAccountUpdate(PreImage preImage); + void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage); } public class TestService : ITestService { - public void HandleAccountUpdate(PreImage preImage) + public void HandleAccountUpdate(TestPluginAccountUpdatePostOperationPreImage preImage) { // Accessing the deprecated member here SHOULD raise CS0612 in this calling code. var legacy = preImage.ctx_DeprecatedField; @@ -479,9 +471,6 @@ public class CallbackService /// public static string GetCompleteSource(string pluginSource) { - var entityName = pluginSource.Contains("RegisterStep SignatureMatches(method, hasPreImage, hasPostImage, entityType, expectedNamespace)); + var hasMatchingOverload = methods.Any(method => SignatureMatches(method, hasPreImage, hasPostImage, entityType, preImageTypeName, postImageTypeName)); if (hasMatchingOverload) { return; @@ -104,11 +116,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext context) var expectedSignature = SyntaxFactoryHelper.BuildSignatureDescription(hasPreImage, hasPostImage); // Determine if generated types exist to choose appropriate diagnostic severity - var generatedTypesExist = DoGeneratedTypesExist( - context, - expectedNamespace, - hasPreImage, - hasPostImage); + var generatedTypesExist = DoGeneratedTypesExist(context, preImageTypeName, postImageTypeName); // Choose diagnostic: XPC4003 (Error) if types exist, XPC4002 (Warning) if they don't var descriptor = generatedTypesExist @@ -121,9 +129,13 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext context) properties.Add(Constants.PropertyMethodName, methodName); properties.Add(Constants.PropertyHasPreImage, hasPreImage.ToString()); properties.Add(Constants.PropertyHasPostImage, hasPostImage.ToString()); - if (expectedNamespace != null) + if (preImageTypeName != null) { - properties.Add(Constants.PropertyImageNamespace, expectedNamespace); + properties.Add(Constants.PropertyPreImageType, preImageTypeName); + } + if (postImageTypeName != null) + { + properties.Add(Constants.PropertyPostImageType, postImageTypeName); } var diagnostic = Diagnostic.Create( @@ -138,39 +150,25 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext context) private static bool DoGeneratedTypesExist( SyntaxNodeAnalysisContext context, - string expectedNamespace, - bool hasPreImage, - bool hasPostImage) + string preImageTypeName, + string postImageTypeName) { - if (expectedNamespace == null) - { - return false; - } - var compilation = context.SemanticModel.Compilation; - if (hasPreImage) + if (preImageTypeName != null && compilation.GetTypeByMetadataName(preImageTypeName) == null) { - var preImageType = compilation.GetTypeByMetadataName($"{expectedNamespace}.PreImage"); - if (preImageType == null) - { - return false; - } + return false; } - if (hasPostImage) + if (postImageTypeName != null && compilation.GetTypeByMetadataName(postImageTypeName) == null) { - var postImageType = compilation.GetTypeByMetadataName($"{expectedNamespace}.PostImage"); - if (postImageType == null) - { - return false; - } + return false; } return true; } - private static bool SignatureMatches(IMethodSymbol method, bool hasPreImage, bool hasPostImage, ITypeSymbol entityType, string expectedNamespace) + private static bool SignatureMatches(IMethodSymbol method, bool hasPreImage, bool hasPostImage, ITypeSymbol entityType, string preImageTypeName, string postImageTypeName) { var parameters = method.Parameters; var expectedParamCount = (hasPreImage ? 1 : 0) + (hasPostImage ? 1 : 0); @@ -189,7 +187,7 @@ private static bool SignatureMatches(IMethodSymbol method, bool hasPreImage, boo return false; } - if (!IsImageParameter(parameters[paramIndex], Constants.PreImageTypeName, Constants.PreImageInterfaceName, entityType, expectedNamespace)) + if (!IsImageParameter(parameters[paramIndex], preImageTypeName, Constants.PreImageInterfaceName, entityType)) { return false; } @@ -204,7 +202,7 @@ private static bool SignatureMatches(IMethodSymbol method, bool hasPreImage, boo return false; } - if (!IsImageParameter(parameters[paramIndex], Constants.PostImageTypeName, Constants.PostImageInterfaceName, entityType, expectedNamespace)) + if (!IsImageParameter(parameters[paramIndex], postImageTypeName, Constants.PostImageInterfaceName, entityType)) { return false; } @@ -223,17 +221,15 @@ private static bool SignatureMatches(IMethodSymbol method, bool hasPreImage, boo /// For the generic interfaces, the type argument must match the registered entity type, otherwise the /// generated ActionWrapper would fail to compile when passing the concrete image. /// - private static bool IsImageParameter(IParameterSymbol parameter, string expectedWrapperType, string expectedInterfaceName, ITypeSymbol entityType, string expectedNamespace) + private static bool IsImageParameter(IParameterSymbol parameter, string expectedFullTypeName, string expectedInterfaceName, ITypeSymbol entityType) { var type = parameter.Type; - // Concrete generated wrapper (PreImage / PostImage). Match by namespace + name so a same-named - // type in a different namespace is not mistaken for the generated wrapper. Fall back to name-only - // when the expected namespace can't be resolved. - if (type.Name == expectedWrapperType) + // Concrete generated wrapper: match by fully-qualified name so a same-named type in a different + // namespace is not mistaken for the generated wrapper. + if (FullTypeName(type) == expectedFullTypeName) { - return expectedNamespace == null - || type.ContainingNamespace?.ToDisplayString() == expectedNamespace; + return true; } // Shared image interfaces - must be declared in XrmPluginCore. @@ -259,4 +255,16 @@ private static bool IsImageParameter(IParameterSymbol parameter, string expected return true; } + + /// + /// The namespace-qualified name of a type (no global:: prefix), matching the format produced by + /// . + /// + private static string FullTypeName(ITypeSymbol type) + { + var containingNamespace = type.ContainingNamespace; + return containingNamespace == null || containingNamespace.IsGlobalNamespace + ? type.Name + : $"{containingNamespace.ToDisplayString()}.{type.Name}"; + } } diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs deleted file mode 100644 index 383e8e3..0000000 --- a/XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs +++ /dev/null @@ -1,348 +0,0 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using XrmPluginCore.SourceGenerator.Helpers; - -namespace XrmPluginCore.SourceGenerator.CodeFixes; - -/// -/// Shared for the image handler code fixes. Unlike -/// — which computes each diagnostic's fix -/// independently against the original document and merges text changes — this provider consolidates -/// every fixable diagnostic in a document into a single pass: it applies each target signature/method -/// edit with its own alias-qualified parameter list, then runs one always-aliased using -/// rewrite per affected document (adding every distinct aliased image using and requalifying every -/// reference once). The result is order-independent and convergent: fixing N registrations via FixAll -/// produces the same unambiguous output as fixing them one-by-one. -/// -internal sealed class AliasedImageUsingsFixAllProvider : FixAllProvider -{ - public static readonly AliasedImageUsingsFixAllProvider Instance = new(); - - // User-facing label shown in the FixAll UI. The equivalence key is a stable identity token and is - // not suitable as a title. - private const string Title = "Fix all image handler signatures"; - - private AliasedImageUsingsFixAllProvider() - { - } - - public override IEnumerable GetSupportedFixAllScopes() => new[] - { - FixAllScope.Document, - FixAllScope.Project, - FixAllScope.Solution, - }; - - public override async Task GetFixAsync(FixAllContext fixAllContext) - { - var diagnostics = await GetDiagnosticsAsync(fixAllContext).ConfigureAwait(false); - if (diagnostics.IsDefaultOrEmpty) - { - return null; - } - - var solution = fixAllContext.Solution; - - return CodeAction.Create( - Title, - c => FixAllAsync(solution, diagnostics, c), - equivalenceKey: fixAllContext.CodeActionEquivalenceKey); - } - - private static async Task> GetDiagnosticsAsync(FixAllContext context) - { - switch (context.Scope) - { - case FixAllScope.Document when context.Document != null: - return await context.GetDocumentDiagnosticsAsync(context.Document).ConfigureAwait(false); - - case FixAllScope.Project: - return await context.GetAllDiagnosticsAsync(context.Project).ConfigureAwait(false); - - case FixAllScope.Solution: - var all = ImmutableArray.CreateBuilder(); - foreach (var project in context.Solution.Projects) - { - all.AddRange(await context.GetAllDiagnosticsAsync(project).ConfigureAwait(false)); - } - - return all.ToImmutable(); - - default: - return ImmutableArray.Empty; - } - } - - private static async Task FixAllAsync( - Solution solution, - ImmutableArray diagnostics, - CancellationToken cancellationToken) - { - // Accumulate every edit by the document it lands in, so each affected document gets a single - // consolidated rewrite regardless of how many diagnostics target it. - var batches = new Dictionary(); - - foreach (var diagnostic in diagnostics) - { - await AccumulateDiagnosticAsync(solution, diagnostic, batches, cancellationToken).ConfigureAwait(false); - } - - foreach (var kvp in batches) - { - solution = await ApplyBatchAsync(solution, kvp.Key, kvp.Value, cancellationToken).ConfigureAwait(false); - } - - return solution; - } - - private static async Task AccumulateDiagnosticAsync( - Solution solution, - Diagnostic diagnostic, - Dictionary batches, - CancellationToken cancellationToken) - { - var diagnosticTree = diagnostic.Location.SourceTree; - if (diagnosticTree == null) - { - return; - } - - var diagnosticDocument = solution.GetDocument(diagnosticTree); - if (diagnosticDocument == null) - { - return; - } - - if (!diagnostic.Properties.TryGetValue(Constants.PropertyMethodName, out var methodName) || methodName == null) - { - return; - } - - diagnostic.Properties.TryGetValue(Constants.PropertyHasPreImage, out var hasPreImageStr); - diagnostic.Properties.TryGetValue(Constants.PropertyHasPostImage, out var hasPostImageStr); - diagnostic.Properties.TryGetValue(Constants.PropertyImageNamespace, out var imageNamespace); - - var hasPreImage = bool.TryParse(hasPreImageStr, out var pre) && pre; - var hasPostImage = bool.TryParse(hasPostImageStr, out var post) && post; - - var semanticModel = await diagnosticDocument.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var diagnosticRoot = await diagnosticTree.GetRootAsync(cancellationToken).ConfigureAwait(false); - if (semanticModel == null || diagnosticRoot == null) - { - return; - } - - var serviceType = RegisterStepHelper.ResolveServiceType(diagnosticRoot, semanticModel, diagnostic.Location.SourceSpan, cancellationToken); - if (serviceType == null) - { - return; - } - - if (diagnostic.Id == DiagnosticDescriptors.HandlerMethodNotFound.Id) - { - await AccumulateCreateMethodAsync(solution, serviceType, methodName, hasPreImage, hasPostImage, imageNamespace, batches, cancellationToken).ConfigureAwait(false); - } - else - { - await AccumulateSignatureFixAsync(diagnosticDocument, serviceType, methodName, hasPreImage, hasPostImage, imageNamespace, batches, cancellationToken).ConfigureAwait(false); - } - } - - private static async Task AccumulateCreateMethodAsync( - Solution solution, - INamedTypeSymbol serviceType, - string methodName, - bool hasPreImage, - bool hasPostImage, - string imageNamespace, - Dictionary batches, - CancellationToken cancellationToken) - { - var interfaceDeclaration = await CreateHandlerMethodCodeFixProvider - .FindInterfaceDeclarationAsync(solution, serviceType, cancellationToken) - .ConfigureAwait(false); - if (interfaceDeclaration == null) - { - return; - } - - var interfaceDocument = solution.GetDocument(interfaceDeclaration.SyntaxTree); - if (interfaceDocument == null) - { - return; - } - - var batch = GetBatch(batches, interfaceDocument.Id); - batch.Creations.Add(new MethodEdit(interfaceDeclaration.Identifier.Text, methodName, hasPreImage, hasPostImage, imageNamespace)); - batch.AddNamespace(imageNamespace); - } - - private static async Task AccumulateSignatureFixAsync( - Document diagnosticDocument, - INamedTypeSymbol serviceType, - string methodName, - bool hasPreImage, - bool hasPostImage, - string imageNamespace, - Dictionary batches, - CancellationToken cancellationToken) - { - var solution = diagnosticDocument.Project.Solution; - - var methods = new List(TypeHelper.GetAllMethodsIncludingInherited(serviceType, methodName)); - if (serviceType.TypeKind == TypeKind.Interface) - { - var compilation = await diagnosticDocument.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); - if (compilation != null) - { - methods.AddRange(TypeHelper.FindImplementingMethods(compilation, serviceType, methodName)); - } - } - - foreach (var method in methods) - { - foreach (var location in method.Locations) - { - if (!location.IsInSource || location.SourceTree == null) - { - continue; - } - - var document = solution.GetDocument(location.SourceTree); - if (document == null) - { - continue; - } - - var batch = GetBatch(batches, document.Id); - batch.SignatureEdits.Add(new MethodEdit(method.ContainingType.Name, methodName, hasPreImage, hasPostImage, imageNamespace)); - batch.AddNamespace(imageNamespace); - } - } - } - - private static async Task ApplyBatchAsync( - Solution solution, - DocumentId documentId, - DocumentBatch batch, - CancellationToken cancellationToken) - { - var document = solution.GetDocument(documentId); - if (document == null) - { - return solution; - } - - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - if (root == null) - { - return solution; - } - - // One consolidated always-aliased using rewrite FIRST, on the original tree, so the rewriter - // resolves bare references against a semantic model whose nodes still match. All structural - // edits below produce already alias-qualified parameters, so requalification skips them. - var newRoot = SyntaxFactoryHelper.ApplyAliasedImageUsings(root, batch.Namespaces, semanticModel); - - // Fix existing handler signatures (interface + implementations) to their alias-qualified form. - foreach (var edit in DistinctEdits(batch.SignatureEdits)) - { - var newParameters = SyntaxFactoryHelper.CreateImageParameterList(edit.HasPreImage, edit.HasPostImage, GetAlias(edit.ImageNamespace)); - var current = newRoot.DescendantNodes().OfType() - .FirstOrDefault(m => m.Identifier.Text == edit.MethodName && - m.Ancestors().OfType().FirstOrDefault()?.Identifier.Text == edit.TypeName); - if (current != null) - { - newRoot = newRoot.ReplaceNode(current, current.WithParameterList(newParameters)); - } - } - - // Create missing handler methods on their interfaces with the alias-qualified parameter list. - foreach (var edit in DistinctEdits(batch.Creations)) - { - var methodDeclaration = CreateHandlerMethodCodeFixProvider.CreateMethodDeclaration( - edit.MethodName, edit.HasPreImage, edit.HasPostImage, GetAlias(edit.ImageNamespace)); - var interfaceDeclaration = newRoot.DescendantNodes().OfType() - .FirstOrDefault(i => i.Identifier.Text == edit.TypeName); - if (interfaceDeclaration != null) - { - newRoot = newRoot.ReplaceNode(interfaceDeclaration, interfaceDeclaration.AddMembers(methodDeclaration)); - } - } - - return solution.WithDocumentSyntaxRoot(documentId, newRoot); - } - - private static string GetAlias(string imageNamespace) => - string.IsNullOrEmpty(imageNamespace) ? null : SyntaxFactoryHelper.GetAliasForImageNamespace(imageNamespace); - - private static IEnumerable DistinctEdits(IEnumerable edits) - { - var seen = new HashSet<(string, string)>(); - foreach (var edit in edits) - { - if (seen.Add((edit.TypeName, edit.MethodName))) - { - yield return edit; - } - } - } - - private static DocumentBatch GetBatch(Dictionary batches, DocumentId documentId) - { - if (!batches.TryGetValue(documentId, out var batch)) - { - batch = new DocumentBatch(); - batches[documentId] = batch; - } - - return batch; - } - - private sealed class DocumentBatch - { - public List SignatureEdits { get; } = new(); - - public List Creations { get; } = new(); - - public HashSet Namespaces { get; } = new(); - - public void AddNamespace(string imageNamespace) - { - if (!string.IsNullOrEmpty(imageNamespace)) - { - Namespaces.Add(imageNamespace); - } - } - } - - private readonly struct MethodEdit - { - public MethodEdit(string typeName, string methodName, bool hasPreImage, bool hasPostImage, string imageNamespace) - { - TypeName = typeName; - MethodName = methodName; - HasPreImage = hasPreImage; - HasPostImage = hasPostImage; - ImageNamespace = imageNamespace; - } - - public string TypeName { get; } - - public string MethodName { get; } - - public bool HasPreImage { get; } - - public bool HasPostImage { get; } - - public string ImageNamespace { get; } - } -} diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs index a46538c..446ae50 100644 --- a/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs +++ b/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs @@ -13,7 +13,8 @@ namespace XrmPluginCore.SourceGenerator.CodeFixes; /// -/// Code fix provider that creates a missing handler method on a service interface. +/// Code fix provider that creates a missing handler method on a service interface. Image parameters are +/// emitted with their fully-qualified generated wrapper type names, so no using directive is needed. /// [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(CreateHandlerMethodCodeFixProvider)), Shared] public class CreateHandlerMethodCodeFixProvider : CodeFixProvider @@ -22,7 +23,7 @@ public class CreateHandlerMethodCodeFixProvider : CodeFixProvider ImmutableArray.Create(DiagnosticDescriptors.HandlerMethodNotFound.Id); public sealed override FixAllProvider GetFixAllProvider() => - AliasedImageUsingsFixAllProvider.Instance; + WellKnownFixAllProviders.BatchFixer; public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -41,12 +42,11 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - diagnostic.Properties.TryGetValue(Constants.PropertyHasPreImage, out var hasPreImageStr); - diagnostic.Properties.TryGetValue(Constants.PropertyHasPostImage, out var hasPostImageStr); - diagnostic.Properties.TryGetValue(Constants.PropertyImageNamespace, out var imageNamespace); + diagnostic.Properties.TryGetValue(Constants.PropertyPreImageType, out var preImageType); + diagnostic.Properties.TryGetValue(Constants.PropertyPostImageType, out var postImageType); - var hasPreImage = bool.TryParse(hasPreImageStr, out var pre) && pre; - var hasPostImage = bool.TryParse(hasPostImageStr, out var post) && post; + var hasPreImage = !string.IsNullOrEmpty(preImageType); + var hasPostImage = !string.IsNullOrEmpty(postImageType); // Build the title showing expected signature var signatureDescription = SyntaxFactoryHelper.BuildSignatureDescription(hasPreImage, hasPostImage); @@ -55,7 +55,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( title: title, - createChangedSolution: c => CreateMethodAsync(context.Document, diagnostic, serviceType!, methodName!, hasPreImage, hasPostImage, imageNamespace, c), + createChangedSolution: c => CreateMethodAsync(context.Document, diagnostic, methodName!, preImageType, postImageType, c), equivalenceKey: nameof(CreateHandlerMethodCodeFixProvider)), diagnostic); } @@ -63,23 +63,19 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task CreateMethodAsync( Document document, Diagnostic diagnostic, - string serviceTypeName, string methodName, - bool hasPreImage, - bool hasPostImage, - string imageNamespace, + string preImageType, + string postImageType, CancellationToken cancellationToken) { var solution = document.Project.Solution; - // Get semantic model to find the service type declaration var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); if (semanticModel == null) { return solution; } - // Find the RegisterStep call to get the service type var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); if (root == null) { @@ -96,7 +92,6 @@ private static async Task CreateMethodAsync( return solution; } - // Get service type from generic arguments var genericName = RegisterStepHelper.GetGenericName(registerStepInvocation); if (genericName == null || genericName.TypeArgumentList.Arguments.Count < 2) { @@ -105,21 +100,17 @@ private static async Task CreateMethodAsync( var serviceTypeSyntax = genericName.TypeArgumentList.Arguments[1]; var typeInfo = semanticModel.GetTypeInfo(serviceTypeSyntax, cancellationToken); - var serviceTypeSymbol = typeInfo.Type as INamedTypeSymbol; - - if (serviceTypeSymbol == null) + if (typeInfo.Type is not INamedTypeSymbol serviceTypeSymbol) { return solution; } - // Find the interface declaration in the solution var interfaceDeclaration = await FindInterfaceDeclarationAsync(solution, serviceTypeSymbol, cancellationToken); if (interfaceDeclaration == null) { return solution; } - // Add the method to the interface var interfaceDocument = solution.GetDocument(interfaceDeclaration.SyntaxTree); if (interfaceDocument == null) { @@ -132,50 +123,21 @@ private static async Task CreateMethodAsync( return solution; } - // Always qualify the image parameters with the namespace alias. - var alias = string.IsNullOrEmpty(imageNamespace) - ? null - : SyntaxFactoryHelper.GetAliasForImageNamespace(imageNamespace); - - var methodDeclaration = CreateMethodDeclaration(methodName, hasPreImage, hasPostImage, alias); - - // Annotate the exact interface node so we can locate it precisely after the using rewrite, - // rather than re-finding by identifier text (which is ambiguous when several interfaces share - // a name across namespaces or as nested types). - var interfaceAnnotation = new SyntaxAnnotation(); - var annotatedRoot = interfaceRoot.ReplaceNode( - interfaceDeclaration, - interfaceDeclaration.WithAdditionalAnnotations(interfaceAnnotation)); - - // Emit the aliased using and requalify existing image references FIRST, so the rewriter - // resolves bare references against a semantic model whose nodes still match. Annotating the - // node above produced a new tree, so build the semantic model from a compilation with that - // tree swapped in — the annotation doesn't affect binding, so references resolve identically. - // The created method's parameters are already alias-qualified, so requalification skips them. - // The interface may live in a different tree than the trigger. - var annotatedTree = annotatedRoot.SyntaxTree; - var annotatedCompilation = semanticModel.Compilation.ReplaceSyntaxTree(interfaceRoot.SyntaxTree, annotatedTree); - var interfaceSemanticModel = annotatedCompilation.GetSemanticModel(annotatedTree); - var newRoot = SyntaxFactoryHelper.ApplyAliasedImageUsings(annotatedRoot, imageNamespace, interfaceSemanticModel); - - // Then add the method to the annotated interface declaration. - var targetInterface = newRoot.GetAnnotatedNodes(interfaceAnnotation) - .OfType() - .FirstOrDefault(); - if (targetInterface != null) - { - newRoot = newRoot.ReplaceNode(targetInterface, targetInterface.AddMembers(methodDeclaration)); - } + var methodDeclaration = CreateMethodDeclaration(methodName, preImageType, postImageType); + + // The generated wrapper types are referenced by fully-qualified name, so the method can be added + // directly with no using-directive juggling. + var newRoot = interfaceRoot.ReplaceNode(interfaceDeclaration, interfaceDeclaration.AddMembers(methodDeclaration)); return solution.WithDocumentSyntaxRoot(interfaceDocument.Id, newRoot); } - internal static MethodDeclarationSyntax CreateMethodDeclaration(string methodName, bool hasPreImage, bool hasPostImage, string qualifier = null) + internal static MethodDeclarationSyntax CreateMethodDeclaration(string methodName, string preImageType, string postImageType) { return SyntaxFactory.MethodDeclaration( SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.VoidKeyword)), SyntaxFactory.Identifier(methodName)) - .WithParameterList(SyntaxFactoryHelper.CreateImageParameterList(hasPreImage, hasPostImage, qualifier)) + .WithParameterList(SyntaxFactoryHelper.CreateImageParameterList(preImageType, postImageType)) .WithSemicolonToken(SyntaxFactory.Token(SyntaxKind.SemicolonToken)) .WithLeadingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed, SyntaxFactory.ElasticTab) .WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed); diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs index c84966e..a7056de 100644 --- a/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs +++ b/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs @@ -14,7 +14,8 @@ namespace XrmPluginCore.SourceGenerator.CodeFixes; /// -/// Code fix provider that fixes handler method signatures to match registered images. +/// Code fix provider that fixes handler method signatures to match registered images. Image parameters +/// are emitted with their fully-qualified generated wrapper type names, so no using directive is needed. /// [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(FixHandlerSignatureCodeFixProvider)), Shared] public class FixHandlerSignatureCodeFixProvider : CodeFixProvider @@ -25,7 +26,7 @@ public class FixHandlerSignatureCodeFixProvider : CodeFixProvider DiagnosticDescriptors.HandlerSignatureMismatchError.Id); public sealed override FixAllProvider GetFixAllProvider() => - AliasedImageUsingsFixAllProvider.Instance; + WellKnownFixAllProviders.BatchFixer; public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -44,12 +45,11 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - diagnostic.Properties.TryGetValue(Constants.PropertyHasPreImage, out var hasPreImageStr); - diagnostic.Properties.TryGetValue(Constants.PropertyHasPostImage, out var hasPostImageStr); - diagnostic.Properties.TryGetValue(Constants.PropertyImageNamespace, out var imageNamespace); + diagnostic.Properties.TryGetValue(Constants.PropertyPreImageType, out var preImageType); + diagnostic.Properties.TryGetValue(Constants.PropertyPostImageType, out var postImageType); - var hasPreImage = bool.TryParse(hasPreImageStr, out var pre) && pre; - var hasPostImage = bool.TryParse(hasPostImageStr, out var post) && post; + var hasPreImage = !string.IsNullOrEmpty(preImageType); + var hasPostImage = !string.IsNullOrEmpty(postImageType); // Build the title showing expected signature var signatureDescription = SyntaxFactoryHelper.BuildSignatureDescription(hasPreImage, hasPostImage, includeParameterNames: true); @@ -58,7 +58,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( title: title, - createChangedSolution: c => FixSignatureAsync(context.Document, diagnostic, serviceType!, methodName!, hasPreImage, hasPostImage, imageNamespace, c), + createChangedSolution: c => FixSignatureAsync(context.Document, diagnostic, methodName!, preImageType, postImageType, c), equivalenceKey: nameof(FixHandlerSignatureCodeFixProvider)), diagnostic); } @@ -66,23 +66,19 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task FixSignatureAsync( Document document, Diagnostic diagnostic, - string serviceTypeName, string methodName, - bool hasPreImage, - bool hasPostImage, - string imageNamespace, + string preImageType, + string postImageType, CancellationToken cancellationToken) { var solution = document.Project.Solution; - // Get semantic model to find the service type var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); if (semanticModel == null) { return solution; } - // Find the RegisterStep call to get the service type symbol var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); if (root == null) { @@ -99,7 +95,6 @@ private static async Task FixSignatureAsync( return solution; } - // Get service type from generic arguments var genericName = RegisterStepHelper.GetGenericName(registerStepInvocation); if (genericName == null || genericName.TypeArgumentList.Arguments.Count < 2) { @@ -108,17 +103,12 @@ private static async Task FixSignatureAsync( var serviceTypeSyntax = genericName.TypeArgumentList.Arguments[1]; var typeInfo = semanticModel.GetTypeInfo(serviceTypeSyntax, cancellationToken); - var serviceTypeSymbol = typeInfo.Type as INamedTypeSymbol; - - if (serviceTypeSymbol == null) + if (typeInfo.Type is not INamedTypeSymbol serviceTypeSymbol) { return solution; } - // Find the method declarations to fix (in interface and implementations) - solution = await FixMethodDeclarationsAsync(solution, semanticModel.Compilation, serviceTypeSymbol, methodName, hasPreImage, hasPostImage, imageNamespace, cancellationToken); - - return solution; + return await FixMethodDeclarationsAsync(solution, semanticModel.Compilation, serviceTypeSymbol, methodName, preImageType, postImageType, cancellationToken); } private static async Task FixMethodDeclarationsAsync( @@ -126,9 +116,8 @@ private static async Task FixMethodDeclarationsAsync( Compilation compilation, INamedTypeSymbol serviceType, string methodName, - bool hasPreImage, - bool hasPostImage, - string imageNamespace, + string preImageType, + string postImageType, CancellationToken cancellationToken) { // Collect ALL methods to fix (interface + implementations) @@ -136,8 +125,7 @@ private static async Task FixMethodDeclarationsAsync( if (serviceType.TypeKind == TypeKind.Interface) { - var implMethods = TypeHelper.FindImplementingMethods(compilation, serviceType, methodName); - allMethods.AddRange(implMethods); + allMethods.AddRange(TypeHelper.FindImplementingMethods(compilation, serviceType, methodName)); } // Group method locations by document (syntax tree) @@ -161,11 +149,11 @@ private static async Task FixMethodDeclarationsAsync( } } - // Process each document + var newParameters = SyntaxFactoryHelper.CreateImageParameterList(preImageType, postImageType); + foreach (var kvp in locationsByTree) { var tree = kvp.Key; - var locations = kvp.Value; var methodDocument = solution.GetDocument(tree); if (methodDocument == null) @@ -175,12 +163,12 @@ private static async Task FixMethodDeclarationsAsync( var methodRoot = await tree.GetRootAsync(cancellationToken).ConfigureAwait(false); - // Find all method declarations in this document + // Find all target method declarations in this document var methodDeclarations = new List(); - foreach (var location in locations) + foreach (var location in kvp.Value) { - var node = methodRoot.FindNode(location.SourceSpan); - var methodDecl = node.AncestorsAndSelf() + var methodDecl = methodRoot.FindNode(location.SourceSpan) + .AncestorsAndSelf() .OfType() .FirstOrDefault(); @@ -195,42 +183,9 @@ private static async Task FixMethodDeclarationsAsync( continue; } - // Always qualify the image parameters with the namespace alias. - var alias = string.IsNullOrEmpty(imageNamespace) - ? null - : SyntaxFactoryHelper.GetAliasForImageNamespace(imageNamespace); - var newParameters = SyntaxFactoryHelper.CreateImageParameterList(hasPreImage, hasPostImage, alias); - - // Build a set of (containingTypeName, methodName) pairs to replace - var targets = new HashSet<(string typeName, string method)>(); - foreach (var methodDecl in methodDeclarations) - { - var containingType = methodDecl.Ancestors().OfType().FirstOrDefault(); - var typeName = containingType?.Identifier.Text; - if (typeName != null) - { - targets.Add((typeName, methodDecl.Identifier.Text)); - } - } - - // Emit the aliased using and requalify existing image references FIRST, on the original - // tree, so the rewriter resolves bare references against a semantic model whose nodes still - // match. The replacement parameters below are already alias-qualified, so the requalifier - // leaves them untouched. - var treeSemanticModel = compilation.GetSemanticModel(tree); - var newRoot = SyntaxFactoryHelper.ApplyAliasedImageUsings(methodRoot, imageNamespace, treeSemanticModel); - - // Replace all matching method declarations one at a time, re-finding after each - foreach (var target in targets) - { - var current = newRoot.DescendantNodes().OfType() - .FirstOrDefault(m => m.Identifier.Text == target.method && - m.Ancestors().OfType().FirstOrDefault()?.Identifier.Text == target.typeName); - if (current != null) - { - newRoot = newRoot.ReplaceNode(current, current.WithParameterList(newParameters)); - } - } + var newRoot = methodRoot.ReplaceNodes( + methodDeclarations, + (original, _) => original.WithParameterList(newParameters)); solution = solution.WithDocumentSyntaxRoot(methodDocument.Id, newRoot); } diff --git a/XrmPluginCore.SourceGenerator/CodeGeneration/WrapperClassGenerator.cs b/XrmPluginCore.SourceGenerator/CodeGeneration/WrapperClassGenerator.cs index 2935837..b2f8e0c 100644 --- a/XrmPluginCore.SourceGenerator/CodeGeneration/WrapperClassGenerator.cs +++ b/XrmPluginCore.SourceGenerator/CodeGeneration/WrapperClassGenerator.cs @@ -26,7 +26,9 @@ public static string GenerateWrapperClasses(PluginStepMetadata metadata) // File header and using directives sb.Append(GetFileHeader(metadata.NullableAnnotationsEnabled)); - var namespaceToUse = metadata.RegistrationNamespace; + // Generated types live in the plugin's own namespace with registration-derived names, so no + // per-registration namespace (and no aliased-usings juggling) is needed. + var namespaceToUse = metadata.Namespace; // Namespace declaration sb.AppendLine($"namespace {namespaceToUse}"); @@ -51,7 +53,7 @@ public static string GenerateWrapperClasses(PluginStepMetadata metadata) /// private static void GenerateImageWrapperClass(StringBuilder sb, PluginStepMetadata metadata, ImageMetadata image) { - var className = image.WrapperClassName; + var className = metadata.ImageClassName(image.ImageType); // Class header with documentation, attribute, field, and constructor sb.Append(GetImageClassHeader( @@ -90,6 +92,7 @@ private static void GenerateActionWrapperClass(StringBuilder sb, PluginStepMetad // ActionWrapper header with documentation, class declaration, and service retrieval sb.Append(GetActionWrapperHeader( + metadata.ActionWrapperClassName, metadata.ServiceTypeName, metadata.HandlerMethodName, metadata.ServiceTypeFullName)); @@ -105,13 +108,13 @@ private static void GenerateActionWrapperClass(StringBuilder sb, PluginStepMetad if (hasPreImage) { sb.AppendLine(); - sb.Append(GetPreImageRetrieval()); + sb.Append(GetPreImageRetrieval(metadata.ImageClassName(Constants.PreImageTypeName))); args.Add("preImage"); } if (hasPostImage) { sb.AppendLine(); - sb.Append(GetPostImageRetrieval()); + sb.Append(GetPostImageRetrieval(metadata.ImageClassName(Constants.PostImageTypeName))); args.Add("postImage"); } @@ -221,12 +224,12 @@ private static string GetObsoleteAttribute(Models.AttributeMetadata attr) : $"{L2}[System.Obsolete({messageLiteral})]\n"; } - private static string GetActionWrapperHeader(string serviceTypeName, string methodName, string serviceFullName) => + private static string GetActionWrapperHeader(string actionWrapperClassName, string serviceTypeName, string methodName, string serviceFullName) => $"{L1}/// \n" + $"{L1}/// Generated action wrapper for {serviceTypeName}.{methodName}\n" + $"{L1}/// \n" + $"{L1}[CompilerGenerated]\n" + - $"{L1}internal sealed class ActionWrapper : IActionWrapper\n" + + $"{L1}internal sealed class {actionWrapperClassName} : IActionWrapper\n" + $"{L1}{{\n" + $"{L2}/// \n" + $"{L2}/// Creates the action delegate that invokes the service method with appropriate images.\n" + @@ -237,13 +240,13 @@ private static string GetActionWrapperHeader(string serviceTypeName, string meth $"{L3}{{\n" + $"{L4}var service = serviceProvider.GetRequiredService<{serviceFullName}>();"; - private static string GetPreImageRetrieval() => + private static string GetPreImageRetrieval(string preImageClassName) => $"{L4}var preImageEntity = context?.PreEntityImages?.Values?.FirstOrDefault();\n" + - $"{L4}var preImage = preImageEntity != null ? new PreImage(preImageEntity) : null;"; + $"{L4}var preImage = preImageEntity != null ? new {preImageClassName}(preImageEntity) : null;"; - private static string GetPostImageRetrieval() => + private static string GetPostImageRetrieval(string postImageClassName) => $"{L4}var postImageEntity = context?.PostEntityImages?.Values?.FirstOrDefault();\n" + - $"{L4}var postImage = postImageEntity != null ? new PostImage(postImageEntity) : null;"; + $"{L4}var postImage = postImageEntity != null ? new {postImageClassName}(postImageEntity) : null;"; private static string GetActionWrapperFooter(string methodName, string argsString) => $"{L4}service.{methodName}({argsString});\n" + diff --git a/XrmPluginCore.SourceGenerator/Constants.cs b/XrmPluginCore.SourceGenerator/Constants.cs index 1e1fd28..8d81f70 100644 --- a/XrmPluginCore.SourceGenerator/Constants.cs +++ b/XrmPluginCore.SourceGenerator/Constants.cs @@ -52,7 +52,10 @@ internal static class Constants public const string PropertyMethodName = "MethodName"; public const string PropertyHasPreImage = "HasPreImage"; public const string PropertyHasPostImage = "HasPostImage"; - public const string PropertyImageNamespace = "ImageNamespace"; + // Fully-qualified names of the generated image wrapper types, passed to the code fixes so handler + // signatures reference them directly (no using/alias needed — they live in the plugin's namespace). + public const string PropertyPreImageType = "PreImageType"; + public const string PropertyPostImageType = "PostImageType"; public const string PropertyHasArguments = "HasArguments"; // Diagnostic property keys for Custom API handler fixes diff --git a/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs b/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs index e4ecdf0..fea39f9 100644 --- a/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs +++ b/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs @@ -13,13 +13,17 @@ namespace XrmPluginCore.SourceGenerator.Helpers; internal static class RegisterStepHelper { /// - /// Computes the expected generated wrapper namespace for a RegisterStep invocation. - /// Returns: {PluginNamespace}.PluginRegistrations.{PluginClassName}.{EntityTypeName}{Operation}{Stage} + /// Computes the fully-qualified name of a generated image wrapper type for a RegisterStep invocation, + /// e.g. {PluginNamespace}.{Sanitize(PluginClass+Entity+Operation+Stage)}{imageTypeSuffix} where + /// is "PreImage" or "PostImage". Must stay in sync with the + /// generator () and the runtime discovery in + /// Plugin.RegisterStep. Returns null when the registration tuple can't be resolved. /// - public static string GetExpectedImageNamespace( + public static string GetExpectedImageTypeName( InvocationExpressionSyntax invocation, GenericNameSyntax genericName, - SemanticModel semanticModel) + SemanticModel semanticModel, + string imageTypeSuffix) { var pluginClass = invocation.Ancestors().OfType().FirstOrDefault(); if (pluginClass == null) @@ -27,11 +31,9 @@ public static string GetExpectedImageNamespace( return null; } - var pluginClassName = pluginClass.Identifier.Text; var pluginNamespace = GetNamespace(pluginClass); - var entityTypeSyntax = genericName.TypeArgumentList.Arguments[0]; - var entityTypeInfo = semanticModel.GetTypeInfo(entityTypeSyntax); + var entityTypeInfo = semanticModel.GetTypeInfo(genericName.TypeArgumentList.Arguments[0]); var entityTypeName = entityTypeInfo.Type?.Name ?? "Unknown"; var (operationExpr, stageExpr) = GetOperationAndStageArguments(invocation, semanticModel); @@ -43,7 +45,9 @@ public static string GetExpectedImageNamespace( var operation = ExtractEnumValue(operationExpr); var stage = ExtractEnumValue(stageExpr); - return $"{pluginNamespace}.PluginRegistrations.{pluginClassName}.{entityTypeName}{operation}{stage}"; + var prefix = IdentifierHelper.Sanitize($"{pluginClass.Identifier.Text}{entityTypeName}{operation}{stage}"); + var ns = string.IsNullOrEmpty(pluginNamespace) ? string.Empty : pluginNamespace + "."; + return $"{ns}{prefix}{imageTypeSuffix}"; } /// diff --git a/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs b/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs index 1eaa5ea..4c0ae48 100644 --- a/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs +++ b/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs @@ -2,7 +2,6 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using System.Collections.Generic; -using System.Linq; namespace XrmPluginCore.SourceGenerator.Helpers; @@ -30,30 +29,25 @@ public static InvocationExpressionSyntax CreateNameofExpression(string serviceTy } /// - /// Creates a parameter list for PreImage/PostImage parameters. + /// Creates a parameter list for the generated image wrappers. Each parameter is typed with the + /// fully-qualified generated wrapper type (e.g. MyNs.AccountPluginAccountUpdatePostOperationPreImage), + /// so no using directive is required at the handler's declaration site. A null type name omits that + /// parameter. /// - public static ParameterListSyntax CreateImageParameterList(bool hasPreImage, bool hasPostImage) - { - return CreateImageParameterList(hasPreImage, hasPostImage, qualifier: null); - } - - /// - /// Creates a parameter list for PreImage/PostImage parameters with optional qualifier. - /// - public static ParameterListSyntax CreateImageParameterList(bool hasPreImage, bool hasPostImage, string qualifier) + public static ParameterListSyntax CreateImageParameterList(string preImageTypeName, string postImageTypeName) { var parameters = new List(); - if (hasPreImage) + if (preImageTypeName != null) { parameters.Add(SyntaxFactory.Parameter(SyntaxFactory.Identifier("preImage")) - .WithType(CreateTypeName(Constants.PreImageTypeName, qualifier))); + .WithType(SyntaxFactory.ParseTypeName(preImageTypeName))); } - if (hasPostImage) + if (postImageTypeName != null) { parameters.Add(SyntaxFactory.Parameter(SyntaxFactory.Identifier("postImage")) - .WithType(CreateTypeName(Constants.PostImageTypeName, qualifier))); + .WithType(SyntaxFactory.ParseTypeName(postImageTypeName))); } return SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(parameters)); @@ -85,361 +79,4 @@ public static string BuildSignatureDescription(bool hasPreImage, bool hasPostIma return string.Join(", ", parts); } - - /// - /// Adds a using directive to the compilation unit if it doesn't already exist. - /// - public static SyntaxNode AddUsingDirectiveIfMissing(SyntaxNode root, string namespaceName) - { - if (string.IsNullOrEmpty(namespaceName)) - { - return root; - } - - if (root is not CompilationUnitSyntax compilationUnit) - { - return root; - } - - var alreadyExists = compilationUnit.Usings.Any(u => u.Name?.ToString() == namespaceName); - if (alreadyExists) - { - return root; - } - - var usingDirective = SyntaxFactory.UsingDirective(SyntaxFactory.ParseName(namespaceName)) - .WithTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed); - - return compilationUnit.AddUsings(usingDirective); - } - - /// - /// Detects whether adding the given image namespace would create ambiguity with existing usings. - /// - /// (needsAlias, alias) where alias is the last segment of the namespace. - public static (bool needsAlias, string alias) DetectImageAmbiguity(SyntaxNode root, string imageNamespace) - { - if (string.IsNullOrEmpty(imageNamespace) || root is not CompilationUnitSyntax compilationUnit) - { - return (false, null); - } - - foreach (var usingDirective in compilationUnit.Usings) - { - // Skip aliased usings - if (usingDirective.Alias != null) - { - continue; - } - - var existingNs = usingDirective.Name?.ToString(); - if (existingNs == null) - { - continue; - } - - if (existingNs != imageNamespace && IsImageRegistrationNamespace(existingNs)) - { - return (true, GetLastNamespaceSegment(imageNamespace)); - } - } - - return (false, null); - } - - /// - /// Returns the alias used for an image-registration namespace: its last dot-separated segment. - /// - public static string GetAliasForImageNamespace(string ns) => GetLastNamespaceSegment(ns); - - /// - /// Backward-compatible overload without a semantic model. Bare PreImage/PostImage references are - /// left unqualified (no semantic resolution). Prefer the overload that takes a . - /// - public static SyntaxNode ConvertToAliasedUsingsAndQualifyRefs(SyntaxNode root, string newImageNamespace) - => ConvertToAliasedUsingsAndQualifyRefs(root, newImageNamespace, semanticModel: null); - - /// - /// Always emits the aliased using for and re-qualifies/aliases - /// every existing image-registration using in the tree. This is the single shared entry point used - /// by the code-fix providers; alias-qualified parameter lists are produced separately via - /// . - /// - /// The compilation unit (document root) to rewrite. - /// The image-registration namespace to alias and add. - /// The semantic model for 's tree (pre-rewrite). - public static SyntaxNode ApplyAliasedImageUsings(SyntaxNode root, string imageNamespace, SemanticModel semanticModel) - { - if (string.IsNullOrEmpty(imageNamespace)) - { - // The analyzer only sets the image namespace when an expected one exists; nothing to do. - return root; - } - - return ConvertToAliasedUsingsAndQualifyRefs(root, imageNamespace, semanticModel); - } - - /// - /// Multi-namespace variant of . - /// Adds every distinct aliased image using needed and requalifies every reference in a single pass. - /// Used by the FixAll path so consolidating N registrations funnels through the same engine as a - /// single fix. - /// - /// The compilation unit (document root) to rewrite. - /// The image-registration namespaces to alias and add. - /// The semantic model for 's tree (pre-rewrite). - public static SyntaxNode ApplyAliasedImageUsings(SyntaxNode root, IEnumerable imageNamespaces, SemanticModel semanticModel) - { - if (imageNamespaces == null) - { - return root; - } - - var namespaces = imageNamespaces.Where(ns => !string.IsNullOrEmpty(ns)).Distinct().ToList(); - if (namespaces.Count == 0) - { - return root; - } - - return ConvertToAliasedUsingsAndQualifyRefs(root, namespaces, semanticModel); - } - - /// - /// Converts existing plain image usings to aliased form, qualifies all bare PreImage/PostImage - /// type references, and adds the new aliased using. - /// - /// The compilation unit (document root) to rewrite. - /// The image-registration namespace to alias and add. - /// - /// The semantic model for 's original (pre-rewrite) tree, used to resolve - /// which alias a bare PreImage/PostImage reference belongs to. May be null, in which case bare - /// references are left unqualified. - /// - public static SyntaxNode ConvertToAliasedUsingsAndQualifyRefs(SyntaxNode root, string newImageNamespace, SemanticModel semanticModel) - => ConvertToAliasedUsingsAndQualifyRefs(root, new[] { newImageNamespace }, semanticModel); - - /// - /// Converts existing plain image usings to aliased form, qualifies all bare PreImage/PostImage - /// type references, and adds an aliased using for each of . - /// This is the single engine shared by the single-fix and FixAll code paths. - /// - /// The compilation unit (document root) to rewrite. - /// The image-registration namespaces to alias and add. - /// - /// The semantic model for 's original (pre-rewrite) tree, used to resolve - /// which alias a bare PreImage/PostImage reference belongs to. May be null, in which case bare - /// references are left unqualified. - /// - private static SyntaxNode ConvertToAliasedUsingsAndQualifyRefs(SyntaxNode root, IReadOnlyCollection newImageNamespaces, SemanticModel semanticModel) - { - if (root is not CompilationUnitSyntax compilationUnit) - { - return root; - } - - // Build map of existing plain image namespace usings → alias - var plainNamespaceToAlias = new Dictionary(); - foreach (var usingDirective in compilationUnit.Usings) - { - if (usingDirective.Alias != null) - { - continue; - } - - var ns = usingDirective.Name?.ToString(); - if (ns != null && IsImageRegistrationNamespace(ns)) - { - plainNamespaceToAlias[ns] = GetLastNamespaceSegment(ns); - } - } - - // Also include each new namespace - foreach (var newImageNamespace in newImageNamespaces) - { - if (!string.IsNullOrEmpty(newImageNamespace)) - { - plainNamespaceToAlias[newImageNamespace] = GetLastNamespaceSegment(newImageNamespace); - } - } - - // Rewrite the tree - var rewriter = new ImageAmbiguityRewriter(plainNamespaceToAlias, semanticModel); - var newRoot = rewriter.Visit(root); - - // Add each new aliased using if not already present - if (newRoot is CompilationUnitSyntax newCompUnit) - { - var usingsToAdd = new List(); - foreach (var newImageNamespace in newImageNamespaces) - { - if (string.IsNullOrEmpty(newImageNamespace)) - { - continue; - } - - var newAlias = GetLastNamespaceSegment(newImageNamespace); - - // The standard alias is "already present" only if some using binds exactly that alias - // name, or a plain (non-aliased) using imports the namespace (which the rewriter above - // would already have converted to the standard alias). A using that imports the same - // namespace under a DIFFERENT alias does NOT count: the emitted parameter types are - // qualified with the standard alias, so we must still add it or the code won't compile. - var alreadyExists = newCompUnit.Usings.Any(u => - u.Alias?.Name.ToString() == newAlias || - (u.Alias == null && u.Name?.ToString() == newImageNamespace)) || - usingsToAdd.Any(u => u.Alias?.Name.ToString() == newAlias); - - if (!alreadyExists) - { - usingsToAdd.Add(SyntaxFactory.UsingDirective( - SyntaxFactory.NameEquals(SyntaxFactory.IdentifierName(newAlias)), - SyntaxFactory.ParseName(newImageNamespace)) - .WithTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed)); - } - } - - if (usingsToAdd.Count > 0) - { - newRoot = newCompUnit.AddUsings(usingsToAdd.ToArray()); - } - } - - return newRoot; - } - - private static bool IsImageRegistrationNamespace(string ns) - { - return ns.Contains(".PluginRegistrations."); - } - - private static string GetLastNamespaceSegment(string ns) - { - var lastDot = ns.LastIndexOf('.'); - return lastDot >= 0 ? ns.Substring(lastDot + 1) : ns; - } - - private static TypeSyntax CreateTypeName(string typeName, string qualifier) - { - if (qualifier == null) - { - return SyntaxFactory.IdentifierName(typeName); - } - - return SyntaxFactory.QualifiedName( - SyntaxFactory.IdentifierName(qualifier), - SyntaxFactory.IdentifierName(typeName)); - } - - /// - /// Rewrites a syntax tree to convert plain image usings to aliased form - /// and qualify bare PreImage/PostImage references. - /// - private sealed class ImageAmbiguityRewriter : CSharpSyntaxRewriter - { - private readonly Dictionary _plainNamespaceToAlias; - - // Semantic model for the original (pre-rewrite) tree, used to resolve which image - // namespace a bare PreImage/PostImage reference actually binds to. May be null. - private readonly SemanticModel _semanticModel; - - public ImageAmbiguityRewriter(Dictionary plainNamespaceToAlias, SemanticModel semanticModel) - { - _plainNamespaceToAlias = plainNamespaceToAlias; - _semanticModel = semanticModel; - } - - public override SyntaxNode VisitUsingDirective(UsingDirectiveSyntax node) - { - if (node.Alias != null) - { - return base.VisitUsingDirective(node); - } - - var ns = node.Name?.ToString(); - if (ns != null && _plainNamespaceToAlias.TryGetValue(ns, out var alias)) - { - // Convert to aliased using - return SyntaxFactory.UsingDirective( - SyntaxFactory.NameEquals(SyntaxFactory.IdentifierName(alias)), - node.Name!) - .WithLeadingTrivia(node.GetLeadingTrivia()) - .WithTrailingTrivia(node.GetTrailingTrivia()); - } - - return base.VisitUsingDirective(node); - } - - public override SyntaxNode VisitIdentifierName(IdentifierNameSyntax node) - { - var name = node.Identifier.Text; - - // Only qualify PreImage and PostImage - if (name != Constants.PreImageTypeName && name != Constants.PostImageTypeName) - { - return base.VisitIdentifierName(node); - } - - // Exclusion: already qualified (parent is QualifiedNameSyntax and we are the right side) - if (node.Parent is QualifiedNameSyntax qualifiedParent && qualifiedParent.Right == node) - { - return base.VisitIdentifierName(node); - } - - // Exclusion: member access name (e.g., obj.PreImage) — not a type reference - if (node.Parent is MemberAccessExpressionSyntax memberAccess && memberAccess.Name == node) - { - return base.VisitIdentifierName(node); - } - - // Exclusion: alias name in using directive - if (node.Parent is NameEqualsSyntax) - { - return base.VisitIdentifierName(node); - } - - // Resolve which image namespace this bare reference binds to via the semantic model - // (built from the original, pre-rewrite tree). Only qualify when it resolves uniquely - // to a single image-registration namespace whose alias we know. If the reference is - // ambiguous or unresolved, leave it unqualified so it surfaces as a normal compile error - // rather than guessing — this keeps requalification correct with any number of plain - // image usings in the file. - var matchingAlias = ResolveImageAlias(node); - if (matchingAlias == null) - { - return base.VisitIdentifierName(node); - } - - // Replace bare identifier with qualified name - var qualifiedName = SyntaxFactory.QualifiedName( - SyntaxFactory.IdentifierName(matchingAlias), - SyntaxFactory.IdentifierName(name)); - - return qualifiedName - .WithLeadingTrivia(node.GetLeadingTrivia()) - .WithTrailingTrivia(node.GetTrailingTrivia()); - } - - private string ResolveImageAlias(IdentifierNameSyntax node) - { - if (_semanticModel == null) - { - return null; - } - - var symbol = _semanticModel.GetSymbolInfo(node).Symbol; - if (symbol == null) - { - // Ambiguous (e.g. CS0104) or otherwise unresolved — do not guess. - return null; - } - - var containingNamespace = symbol.ContainingNamespace?.ToDisplayString(); - if (containingNamespace != null && _plainNamespaceToAlias.TryGetValue(containingNamespace, out var alias)) - { - return alias; - } - - return null; - } - } } diff --git a/XrmPluginCore.SourceGenerator/Models/PluginStepMetadata.cs b/XrmPluginCore.SourceGenerator/Models/PluginStepMetadata.cs index 4928e45..83882f1 100644 --- a/XrmPluginCore.SourceGenerator/Models/PluginStepMetadata.cs +++ b/XrmPluginCore.SourceGenerator/Models/PluginStepMetadata.cs @@ -1,6 +1,7 @@ using Microsoft.CodeAnalysis; using System.Collections.Generic; using System.Linq; +using XrmPluginCore.SourceGenerator.Helpers; namespace XrmPluginCore.SourceGenerator.Models; @@ -46,11 +47,19 @@ internal sealed class PluginStepMetadata public List Diagnostics { get; set; } = []; /// - /// Gets the namespace for generated wrapper classes. - /// Format: {OriginalNamespace}.PluginRegistrations.{PluginClassName}.{Entity}{Op}{Stage} + /// The identifier prefix shared by every generated type for this registration, derived from the + /// registration tuple and sanitized into a valid identifier. Uniqueness within the plugin namespace + /// is guaranteed by XPC2002 (one registration per entity/operation/stage per plugin) and by distinct + /// plugin class names. Kept in sync with the runtime discovery in Plugin.RegisterStep. /// - public string RegistrationNamespace => - $"{Namespace}.PluginRegistrations.{PluginClassName}.{EntityTypeName}{EventOperation}{ExecutionStage}"; + public string TypePrefix => + IdentifierHelper.Sanitize($"{PluginClassName}{EntityTypeName}{EventOperation}{ExecutionStage}"); + + /// The generated wrapper class name for the given image type (e.g. "...PreImage"). + public string ImageClassName(string imageType) => $"{TypePrefix}{imageType}"; + + /// The generated ActionWrapper class name for this registration. + public string ActionWrapperClassName => $"{TypePrefix}{Constants.ActionWrapperClassSuffix}"; /// /// Gets a unique identifier for this registration. @@ -177,12 +186,6 @@ internal sealed class ImageMetadata public string ImageName { get; set; } public List Attributes { get; set; } = []; - /// - /// Gets the generated wrapper class name for this image. - /// Simply "PreImage" or "PostImage" - namespace provides isolation. - /// - public string WrapperClassName => ImageType; - public override bool Equals(object obj) { if (obj is ImageMetadata other) diff --git a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountPlugin.cs b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountPlugin.cs index 21db0b1..5cdd890 100644 --- a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountPlugin.cs +++ b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountPlugin.cs @@ -2,22 +2,18 @@ using XrmPluginCore.Enums; using XrmPluginCore.Tests.Context.BusinessDomain; - -// Import the generated PreImage/PostImage from the namespace -using XrmPluginCore.Tests.TestPlugins.TypeSafe.PluginRegistrations.TypeSafeAccountPlugin.AccountUpdatePreOperation; - namespace XrmPluginCore.Tests.TestPlugins.TypeSafe; /// /// Test plugin using the type-safe image API. -/// PreImage and PostImage wrappers are generated by the source generator -/// and passed directly to the action callback. +/// PreImage and PostImage wrappers are generated by the source generator (into this namespace, named +/// after the registration) and passed directly to the action callback. /// public class TypeSafeAccountPlugin : Plugin { public bool UpdateExecuted { get; private set; } - public PreImage LastPreImage { get; private set; } - public PostImage LastPostImage { get; private set; } + public TypeSafeAccountPluginAccountUpdatePreOperationPreImage LastPreImage { get; private set; } + public TypeSafeAccountPluginAccountUpdatePreOperationPostImage LastPostImage { get; private set; } public TypeSafeAccountPlugin() { @@ -35,7 +31,7 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle return base.OnBeforeBuildServiceProvider(services); } - internal void SetExecutionResult(PreImage preImage, PostImage postImage) + internal void SetExecutionResult(TypeSafeAccountPluginAccountUpdatePreOperationPreImage preImage, TypeSafeAccountPluginAccountUpdatePreOperationPostImage postImage) { UpdateExecuted = true; LastPreImage = preImage; diff --git a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountService.cs b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountService.cs index 6b827e1..6d8dcc9 100644 --- a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountService.cs +++ b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeAccountService.cs @@ -1,18 +1,16 @@ using System; -// Import the generated PreImage/PostImage from the namespace -using XrmPluginCore.Tests.TestPlugins.TypeSafe.PluginRegistrations.TypeSafeAccountPlugin.AccountUpdatePreOperation; - namespace XrmPluginCore.Tests.TestPlugins.TypeSafe; /// -/// Service for TypeSafeAccountPlugin that receives images directly +/// Service for TypeSafeAccountPlugin that receives images directly. The image wrapper types are +/// generated into this namespace, named after the registration. /// public class TypeSafeAccountService(TypeSafeAccountPlugin plugin) { private readonly TypeSafeAccountPlugin plugin = plugin ?? throw new ArgumentNullException(nameof(plugin)); - public void HandleUpdate(PreImage preImage, PostImage postImage) + public void HandleUpdate(TypeSafeAccountPluginAccountUpdatePreOperationPreImage preImage, TypeSafeAccountPluginAccountUpdatePreOperationPostImage postImage) { if (preImage != null) { diff --git a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactPlugin.cs b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactPlugin.cs index f5c6731..58c6380 100644 --- a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactPlugin.cs +++ b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactPlugin.cs @@ -2,20 +2,17 @@ using Microsoft.Xrm.Sdk; using XrmPluginCore.Enums; -// Import the generated PreImage from the namespace -using XrmPluginCore.Tests.TestPlugins.TypeSafe.PluginRegistrations.TypeSafeContactPlugin.ContactCreatePostOperation; - namespace XrmPluginCore.Tests.TestPlugins.TypeSafe; /// /// Test plugin using the type-safe image API. -/// PreImage wrapper is generated by the source generator -/// and passed directly to the action callback. +/// The PreImage wrapper is generated by the source generator (into this namespace, named after the +/// registration) and passed directly to the action callback. /// public class TypeSafeContactPlugin : Plugin { public bool CreateExecuted { get; private set; } - public PreImage LastPreImage { get; private set; } + public TypeSafeContactPluginContactCreatePostOperationPreImage LastPreImage { get; private set; } public TypeSafeContactPlugin() { @@ -32,7 +29,7 @@ protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceColle return base.OnBeforeBuildServiceProvider(services); } - internal void SetExecutionResult(PreImage preImage) + internal void SetExecutionResult(TypeSafeContactPluginContactCreatePostOperationPreImage preImage) { CreateExecuted = true; LastPreImage = preImage; diff --git a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactService.cs b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactService.cs index fc2412f..3856955 100644 --- a/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactService.cs +++ b/XrmPluginCore.Tests/TestPlugins/TypeSafe/TypeSafeContactService.cs @@ -1,12 +1,10 @@ using System; -// Import the generated PreImage from the namespace -using XrmPluginCore.Tests.TestPlugins.TypeSafe.PluginRegistrations.TypeSafeContactPlugin.ContactCreatePostOperation; - namespace XrmPluginCore.Tests.TestPlugins.TypeSafe; /// -/// Service for TypeSafeContactPlugin that receives PreImage directly +/// Service for TypeSafeContactPlugin that receives PreImage directly. The image wrapper type is +/// generated into this namespace, named after the registration. /// public class TypeSafeContactService { @@ -17,7 +15,7 @@ public TypeSafeContactService(TypeSafeContactPlugin plugin) this.plugin = plugin ?? throw new ArgumentNullException(nameof(plugin)); } - public void HandleCreate(PreImage preImage) + public void HandleCreate(TypeSafeContactPluginContactCreatePostOperationPreImage preImage) { if (preImage != null) { diff --git a/XrmPluginCore/CHANGELOG.md b/XrmPluginCore/CHANGELOG.md index f2381a1..bd5a0ff 100644 --- a/XrmPluginCore/CHANGELOG.md +++ b/XrmPluginCore/CHANGELOG.md @@ -1,4 +1,5 @@ ### v1.5.0 - Unreleased +* Breaking: Type-safe plugin image wrappers are now generated into the plugin's own namespace with registration-derived names (`{PluginClass}{Entity}{Operation}{Stage}PreImage`/`PostImage`) instead of a per-registration `...PluginRegistrations...` namespace containing bare `PreImage`/`PostImage` types. Handler signatures should reference the new type names (the XPC4002/XPC4003 code fix rewrites existing signatures for you), and any `using ...PluginRegistrations...;` directives can be removed. This mirrors the Custom API request/response naming and removes the aliased-usings machinery entirely. * Add: Error XPC2002: a plugin that registers the same entity/operation/stage combination more than once (via `RegisterStep` or `RegisterPluginStep`) is now reported at compile time. Each combination may be registered at most once per plugin. * Add: Info XPC3007: when a Custom API declares `AddRequestParameter`/`AddResponseProperty` but registers with the action-based `RegisterAPI(name, Action)` overload, it now suggests the type-safe `RegisterAPI(name, nameof(...))` overload that generates strongly-typed request/response DTOs. The accompanying code fix converts `s => s.Handle()` to `nameof(TService.Handle)`, and only offers the fix when the lambda calls a method directly on the service parameter (so `s => Other.Handle()` reports the suggestion without a misleading fix). * Fix: The `nameof(...)` code fix (XPC3001 and XPC3007) now produces valid syntax when the service type is written with a namespace or generic arguments (e.g. `RegisterAPI`), instead of emitting a malformed single identifier. diff --git a/XrmPluginCore/Plugin.cs b/XrmPluginCore/Plugin.cs index 83bf7b6..307561b 100644 --- a/XrmPluginCore/Plugin.cs +++ b/XrmPluginCore/Plugin.cs @@ -332,11 +332,12 @@ protected PluginStepConfigBuilder RegisterStep( { var builder = new PluginStepConfigBuilder(eventOperation, executionStage); - // Compute the generated ActionWrapper type name up front. Must match the namespace the source - // generator emits: {Namespace}.PluginRegistrations.{PluginClassName}.{Entity}{Operation}{Stage}. + // Compute the generated ActionWrapper type name up front. Must match what the source generator + // emits: a registration-derived, sanitized type prefix in the plugin's own namespace, i.e. + // {Namespace}.{Sanitize(PluginClass + Entity + Operation + Stage)}ActionWrapper. var wrapperTypeName = - $"{ChildClassNamespace}.PluginRegistrations.{ChildClassShortName}." + - $"{typeof(TEntity).Name}{eventOperation}{executionStage}.ActionWrapper"; + $"{ChildClassNamespace}." + + $"{IdentifierSanitizer.Sanitize($"{ChildClassShortName}{typeof(TEntity).Name}{eventOperation}{executionStage}")}ActionWrapper"; var registration = new PluginStepRegistration( pluginStepConfig: builder, From 1434f4dfc3c050e9f60d48d4ad04dfea5297e56e Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Fri, 3 Jul 2026 10:56:58 +0200 Subject: [PATCH 3/5] Update docs for flat image generation (v1.5.0) Rewrite the Type-Safe Images section of CLAUDE.md for the new scheme: generated wrappers live in the plugin namespace with registration-derived names, no per-registration namespace or using aliases. Reframe the shared image interfaces as an escape hatch (raw Entity / attribute-agnostic logic) rather than the recommended parameter type, since they bypass the generated typed properties. Record progress and the manual-migration decision in the plan. Co-Authored-By: Claude via Conducktor --- CLAUDE.md | 54 +++++++++++++++++--------- docs/plans/flatten-image-generation.md | 16 +++++++- 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 501a33b..8983a46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -144,9 +144,9 @@ RegisterStep( - Pre/Post image attributes from `WithPreImage()`/`WithPostImage()`/`AddImage()` calls - Method reference from the action delegate -3. **Code Generation**: Generates wrapper classes in isolated namespaces: - - Namespace: `{Namespace}.PluginRegistrations.{PluginClassName}.{Entity}{Operation}{Stage}` - - Classes: `PreImage`, `PostImage`, `ActionWrapper` (simple names, no prefixes) +3. **Code Generation**: Generates wrapper classes in the **plugin's own namespace**, named after the registration: + - Namespace: `{PluginNamespace}` (same namespace as the plugin class) + - Classes: `{PluginClass}{Entity}{Operation}{Stage}PreImage` / `...PostImage` / `...ActionWrapper` (the registration-derived prefix is sanitized into a valid identifier). Uniqueness within the namespace is guaranteed by XPC2002 (one registration per entity/operation/stage per plugin) plus distinct plugin class names — so no per-registration namespace or `using` alias is needed. 4. **Signature Validation**: The source generator validates that the handler method signature matches the registered images and emits compile-time diagnostics if there is a mismatch. @@ -156,9 +156,11 @@ RegisterStep( #### Example Usage -```csharp -using MyNamespace.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation; +The generated image types live in the plugin's own namespace, so when the handler is in that same +namespace no `using` is needed. (When the service lives elsewhere, reference the types by their +fully-qualified name — the code fix emits that form for you.) +```csharp public class AccountPlugin : Plugin { public AccountPlugin() @@ -181,8 +183,11 @@ public class AccountPlugin : Plugin public class AccountService { - // Handler signature MUST match registered images (enforced by source generator diagnostics) - public void HandleUpdate(PreImage preImage, PostImage postImage) + // Handler signature MUST match registered images (enforced by source generator diagnostics). + // The image types are named after the registration: {PluginClass}{Entity}{Operation}{Stage}{Kind}. + public void HandleUpdate( + AccountPluginAccountUpdatePostOperationPreImage preImage, + AccountPluginAccountUpdatePostOperationPostImage postImage) { var previousName = preImage.Name; // Type-safe, IntelliSense works var previousRevenue = preImage.Revenue; @@ -193,15 +198,17 @@ public class AccountService #### Generated Code Example -The source generator creates wrapper classes in isolated namespaces. Each wrapper holds the strongly-typed entity and implements the shared image interfaces (see [Image Interfaces](#image-interfaces) below): +The source generator creates wrapper classes in the plugin's own namespace, named after the +registration. Each wrapper holds the strongly-typed entity and implements the shared image interfaces +(see [Image Interfaces](#image-interfaces) below): ```csharp -// Generated in: {Namespace}.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation -namespace YourNamespace.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation +// Generated in the plugin's namespace, named {PluginClass}{Entity}{Operation}{Stage}{Kind} +namespace YourNamespace { - public sealed class PreImage : IPluginPreImage + public sealed class AccountPluginAccountUpdatePostOperationPreImage : IPluginPreImage { - public PreImage(Entity entity) + public AccountPluginAccountUpdatePostOperationPreImage(Entity entity) { Entity = entity.ToEntity(); } @@ -217,9 +224,9 @@ namespace YourNamespace.PluginRegistrations.AccountPlugin.AccountUpdatePostOpera public decimal? Revenue => Entity.Revenue; } - public sealed class PostImage : IPluginPostImage + public sealed class AccountPluginAccountUpdatePostOperationPostImage : IPluginPostImage { - public PostImage(Entity entity) + public AccountPluginAccountUpdatePostOperationPostImage(Entity entity) { Entity = entity.ToEntity(); } @@ -250,13 +257,22 @@ Every generated image implements a small interface hierarchy declared in `XrmPlu The non-generic `IPluginImage` also exposes the members that are always available on any entity image, regardless of which attributes were registered: `Id` (`Guid`, the primary key) and `LogicalName` (`string`). -Because each registration generates its **own** `PreImage`/`PostImage` type in its own namespace, these interfaces let you write shared logic that works across multiple registrations. A handler method may declare its parameters using any of the matching interfaces instead of the concrete generated type — the source generator accepts them and validates the kind (pre vs post) and, for the generic variants, the entity type: +Each registration generates its **own** concrete `...PreImage`/`...PostImage` type. Prefer those +concrete types — they expose the strongly-typed, per-attribute properties that are the whole point of +the feature. The interfaces are an **escape hatch** for the narrow case where you only need the raw +`Entity` (or genuinely shared, attribute-agnostic logic across registrations); declaring a parameter +as an interface throws away the generated typed properties. A handler method may declare its +parameters using any of the matching interfaces instead of the concrete generated type — the source +generator accepts them and validates the kind (pre vs post) and, for the generic variants, the entity +type: ```csharp public class AccountService { - // Concrete generated types (most specific) - public void HandleUpdate(PreImage pre, PostImage post) { } + // Concrete generated types (preferred — full typed property access) + public void HandleUpdate( + AccountPluginAccountUpdatePostOperationPreImage pre, + AccountPluginAccountUpdatePostOperationPostImage post) { } } public static class AuditHelper @@ -286,8 +302,8 @@ All three methods are valid and supported. `WithPreImage` and `WithPostImage` ar - **IntelliSense support**: Auto-completion for available image attributes - **No runtime overhead**: Simple property accessors, no reflection at access time - **Null safety**: Missing attributes return null instead of throwing exceptions -- **Namespace isolation**: Each step gets its own namespace, preventing naming conflicts -- **Shared interfaces**: `IPluginImage`/`IPluginPreImage`/`IPluginPostImage` (and generic variants) let handler methods share logic across the per-registration concrete image types +- **No namespace juggling**: types are emitted into the plugin's own namespace with registration-derived names, so there are no `using` aliases to manage +- **Shared interfaces** (escape hatch): `IPluginImage`/`IPluginPreImage`/`IPluginPostImage` (and generic variants) let handler methods share attribute-agnostic logic across the per-registration concrete image types when you only need the raw `Entity` ### Type-Safe Custom API Request/Response diff --git a/docs/plans/flatten-image-generation.md b/docs/plans/flatten-image-generation.md index 5a5e511..eee7926 100644 --- a/docs/plans/flatten-image-generation.md +++ b/docs/plans/flatten-image-generation.md @@ -1,7 +1,19 @@ # Plan: Flatten plugin image generation to eliminate the namespace/aliasing subsystem -> Status: **Approved — implementing.** Ships as a new minor **v1.5.0** that folds in the pending -> (unreleased) v1.4.2 XPC3007 work; nothing is released before this change. +> Status: **In progress.** Ships as a new minor **v1.5.0** that folds in the pending (unreleased) +> v1.4.2 XPC3007 work; nothing is released before this change. +> +> Progress: +> - ✅ PR #1 — XPC2002 duplicate-registration guard (merged, #21). +> - ✅ PR #2 — generator + runtime rename + code-fix convergence + aliased-usings deletion +> (committed `3a0064f` on `flatten-image-generation`). PR #3's code-fix work was folded in here +> because it's inseparable from the rename. +> - ⬜ PR #4 — docs (CLAUDE.md + migration note). +> +> Decisions: camel-concat names (no separator); interfaces are a documented escape hatch, not +> recommended; **migration = documented manual step** — the XPC4002/4003 code fix rewrites handler +> signatures to the new fully-qualified names, then the developer deletes the now-invalid +> `using ...PluginRegistrations...;` (surfaces as CS0234). No auto-removal of the stale using. ## Goal From 98d601adc0f73e37d592993fc970b3973d8408a5 Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Fri, 3 Jul 2026 12:28:45 +0200 Subject: [PATCH 4/5] Address Copilot review on PR #22 CreateHandlerMethodCodeFixProvider now bails out when a registered image's generated wrapper type name couldn't be resolved (HasPreImage/HasPostImage is set but the type-name property is missing), rather than creating a param-less, permanently mismatched handler method. Co-Authored-By: Claude via Conducktor --- .../CreateHandlerMethodCodeFixProvider.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs index 446ae50..e23c373 100644 --- a/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs +++ b/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs @@ -45,6 +45,20 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) diagnostic.Properties.TryGetValue(Constants.PropertyPreImageType, out var preImageType); diagnostic.Properties.TryGetValue(Constants.PropertyPostImageType, out var postImageType); + // The analyzer reports HasPreImage/HasPostImage unconditionally but only supplies the generated + // wrapper type name when it can resolve it. If an image is registered but its type name is + // missing, we can't emit a correct signature — don't offer a fix that would create a + // permanently mismatched, param-less method. + diagnostic.Properties.TryGetValue(Constants.PropertyHasPreImage, out var expectsPreImageStr); + diagnostic.Properties.TryGetValue(Constants.PropertyHasPostImage, out var expectsPostImageStr); + var expectsPreImage = bool.TryParse(expectsPreImageStr, out var ep) && ep; + var expectsPostImage = bool.TryParse(expectsPostImageStr, out var eq) && eq; + if ((expectsPreImage && string.IsNullOrEmpty(preImageType)) || + (expectsPostImage && string.IsNullOrEmpty(postImageType))) + { + return; + } + var hasPreImage = !string.IsNullOrEmpty(preImageType); var hasPostImage = !string.IsNullOrEmpty(postImageType); From cefe69240d216c1efefb88a3a3f8461eeb96f9b6 Mon Sep 17 00:00:00 2001 From: Morten Holt Date: Fri, 3 Jul 2026 12:31:03 +0200 Subject: [PATCH 5/5] Remove design plan/docs scratch folder The flatten-image-generation plan was a working reference for the v1.5.0 effort; the work is complete, so it no longer needs to live in the repo. Co-Authored-By: Claude via Conducktor --- docs/plans/flatten-image-generation.md | 211 ------------------------- 1 file changed, 211 deletions(-) delete mode 100644 docs/plans/flatten-image-generation.md diff --git a/docs/plans/flatten-image-generation.md b/docs/plans/flatten-image-generation.md deleted file mode 100644 index eee7926..0000000 --- a/docs/plans/flatten-image-generation.md +++ /dev/null @@ -1,211 +0,0 @@ -# Plan: Flatten plugin image generation to eliminate the namespace/aliasing subsystem - -> Status: **In progress.** Ships as a new minor **v1.5.0** that folds in the pending (unreleased) -> v1.4.2 XPC3007 work; nothing is released before this change. -> -> Progress: -> - ✅ PR #1 — XPC2002 duplicate-registration guard (merged, #21). -> - ✅ PR #2 — generator + runtime rename + code-fix convergence + aliased-usings deletion -> (committed `3a0064f` on `flatten-image-generation`). PR #3's code-fix work was folded in here -> because it's inseparable from the rename. -> - ⬜ PR #4 — docs (CLAUDE.md + migration note). -> -> Decisions: camel-concat names (no separator); interfaces are a documented escape hatch, not -> recommended; **migration = documented manual step** — the XPC4002/4003 code fix rewrites handler -> signatures to the new fully-qualified names, then the developer deletes the now-invalid -> `using ...PluginRegistrations...;` (surfaces as CS0234). No auto-removal of the stale using. - -## Goal - -Generate `PreImage`/`PostImage`/`ActionWrapper` the same way Custom APIs generate -`Request`/`Response`/`ActionWrapper`: **uniquely-named types in the plugin's own namespace**, -referenced by **fully-qualified name** in generated handler signatures. This removes the reason the -per-registration namespace exists, which lets us delete the entire aliased-usings subsystem. - -A new **XPC2002** error makes the uniqueness precondition explicit at compile time (mirroring the -existing runtime block), so generation can never collide. - ---- - -## 1. New naming scheme - -Today (`PluginStepMetadata.cs:52`, `RegisterStepHelper.cs:46`, runtime `Plugin.cs:335-339`): - -``` -namespace: {PluginNamespace}.PluginRegistrations.{PluginClass}.{Entity}{Operation}{Stage} -types: PreImage, PostImage, ActionWrapper -``` - -Proposed (mirrors Custom API at `Plugin.cs:474`): - -``` -namespace: {PluginNamespace} -prefix: Sanitize("{PluginClass}{Entity}{Operation}{Stage}") -types: {prefix}PreImage, {prefix}PostImage, {prefix}ActionWrapper -``` - -- **Why the plugin-class part is required (and Custom API doesn't need one):** a Custom API name is - globally unique by construction; an image's identity is the derived tuple `Entity+Operation+Stage`, - which repeats across plugin classes in the same namespace. Plugin class names are unique within a - namespace, and — per the registration rule, enforced by XPC2002 below — the tuple is unique within a - plugin class. So `{PluginClass}{Entity}{Operation}{Stage}` is unique within the namespace. No - index/disambiguator needed. -- **Sanitization:** route the whole prefix through the existing `IdentifierSanitizer.Sanitize` - (already used for Custom API names at `Plugin.cs:474`). This also hardens the - custom-message-operation case (`EventOperation` can be an arbitrary string like - `"contextand_DoThing"`), which the current raw-concatenation scheme handles only by luck. The - generator and the runtime must use the identical sanitizer so the reflected type name matches. -- **Verbosity is the accepted trade-off**, identical in spirit to Custom API's `SomeApiRequest`. We do - **not** steer handlers toward the shared interfaces to avoid the long name — declaring - `IPluginImage` throws away the generated typed property wrappers, which are the entire point - of the feature. The interfaces stay a documented escape hatch for when you genuinely only need the - raw `Entity`; the docs mention they exist but do not recommend them as the default parameter type. - ---- - -## 2. New diagnostic: XPC2002 — duplicate step registration - -- **Descriptor** (`DiagnosticDescriptors.cs`, category XPC2xxx "plugin class structure", **Error**): - "Plugin '{0}' registers {Entity}/{Operation}/{Stage} more than once. Each entity/operation/stage - combination may be registered at most once per plugin." -- **Analyzer** `Analyzers/DuplicateStepRegistrationAnalyzer.cs`: register a symbol/compilation-level - action over each `Plugin`-derived class, collect all `RegisterStep`/`RegisterPluginStep` - invocations in its constructor, key each by `(sanitized entity, sanitized operation, stage)`, and - report on the 2nd+ occurrence. Reuse `RegisterStepHelper` for extraction. -- **Rationale:** codifies the existing runtime block at compile time and *guarantees* the naming - scheme in section 1 is collision-free, so generation never breaks. Error severity because it's - already illegal. -- Docs: `rules/XPC2002.md`; entry in `AnalyzerReleases.Unshipped.md`. - ---- - -## 3. Source-generator changes - -- `Models/PluginStepMetadata.cs:52` — replace `RegistrationNamespace` with - `TypePrefix => Sanitize($"{PluginClassName}{EntityTypeName}{EventOperation}{ExecutionStage}")`; the - emit namespace becomes `Namespace` (plugin namespace). -- `Helpers/RegisterStepHelper.cs:46` — update/remove the duplicated namespace formula to match. -- `CodeGeneration/WrapperClassGenerator.cs:29` — emit into `metadata.Namespace`; prefix the three - class names with `metadata.TypePrefix`. Update per-file hint names to use the prefix so they stay - unique. -- Confirm the `IActionWrapper` implementation and interface list on the image classes are unchanged - (only names/namespace move). - -## 4. Runtime changes - -- `Plugin.cs:335-339` — build the image wrapper type name as - `$"{ChildClassNamespace}.{IdentifierSanitizer.Sanitize(ChildClassShortName + typeof(TEntity).Name + eventOperation + executionStage)}ActionWrapper"`, - exactly paralleling the Custom API path at `Plugin.cs:474`. This is the only runtime discovery - point; it must use the same sanitizer/format as the generator. - -## 5. Deletions & simplifications (the payoff) - -- **Delete** `CodeFixes/AliasedImageUsingsFixAllProvider.cs`. -- **Remove from `Helpers/SyntaxFactoryHelper.cs`:** `ApplyAliasedImageUsings` (both overloads), - `ConvertToAliasedUsingsAndQualifyRefs` (all overloads), `ImageAmbiguityRewriter`, - `DetectImageAmbiguity`, `GetAliasForImageNamespace`, `IsImageRegistrationNamespace`, and the - `qualifier` overload of `CreateImageParameterList`. Keep `CreateNameofExpression`, - `BuildSignatureDescription`, and a simplified `CreateImageParameterList` that takes a - **fully-qualified namespace** (see section 7). -- `AddUsingDirectiveIfMissing` — likely droppable for images (we go fully-qualified instead of adding - usings); keep only if still used elsewhere. - -## 6. Analyzer changes - -- `HandlerSignatureMismatchAnalyzer` (XPC4002/4003): update the *expected* image type resolution to - the new namespace + prefixed names. Comparison stays semantic (by symbol), so a user handler that - imports the type with a short name still matches. **Also make an unresolved/error-typed image - parameter count as a mismatch** so post-migration handlers (whose old `PreImage` type no longer - resolves) trigger the existing code fix rather than only a raw CS0246 — this is the migration hook - (see section 8). -- `ImageWithoutMethodReferenceAnalyzer` (XPC3003) and `FullEntityImageAnalyzer` (XPC3005): - naming-independent, no change expected — add a regression test to confirm. - -## 7. Code-fix changes (converge on the Custom API pattern) - -- `CreateHandlerMethodCodeFixProvider` and `FixHandlerSignatureCodeFixProvider`: - - Emit **fully-qualified** generated type names (`{PluginNamespace}.{prefix}PreImage`) in the - handler signature — exactly what the Custom API fixes already do (their tests assert - `TestNamespace.SomeApiResponse Handle(TestNamespace.SomeApiRequest request)`). No `using`, no - alias. - - Swap `GetFixAllProvider()` from `AliasedImageUsingsFixAllProvider` to - `WellKnownFixAllProviders.BatchFixer` — identical to the Custom API code fixes now. -- `PreferNameofCodeFixProvider`: unaffected by naming; verify. - -## 8. Migration strategy (breaking change) - -Existing handlers reference short `PreImage`/`PostImage` + `using {Ns}.PluginRegistrations.…;`. After -a rebuild the old namespace is gone, so those break (CS0234 on the using, CS0246 on the params). -Mitigation (pending decision — see "Open questions"): - -- **Auto-fix via enhanced XPC4002/4003** (section 6): once the old param type fails to resolve, the - signature analyzer flags a mismatch and its code fix rewrites the signature to the new - fully-qualified type; the stale `using` becomes an unused-using (CS8019/IDE0005), cleaned by - standard tooling. **OR** -- **Documented manual edit only**: rely on the compiler errors + a `docs` migration note. - -Note: we deliberately do **not** offer "switch to `IPluginImage`" as the migration —that -discards the generated typed properties (see section 1). Call the change out prominently as breaking -in the CHANGELOG with a short migration note either way. - -## 9. Tests - -- `GenerationTests/WrapperClassGenerationTests.cs` — update expected namespace + prefixed names; add a - case with **two registrations in one plugin** and **two plugins in one namespace** asserting unique, - collision-free names with no aliasing. -- `DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs`, - `CreateHandlerMethodCodeFixProviderTests.cs` — assert fully-qualified names; **delete** - aliasing/multi-registration-alias tests. -- New `DiagnosticTests/DuplicateStepRegistrationAnalyzerTests.cs` for XPC2002 (fires on dup; not on - distinct tuples; not across different plugins). -- New migration regression test: a handler with an unresolved old-style image param triggers XPC4002 - and the fix produces the new fully-qualified signature. -- Full `dotnet test` on both test projects; the whole suite (currently 119 in the SG tests) must stay - green minus the intentionally-removed aliasing tests. - -## 10. Docs & release tracking - -- **CLAUDE.md** "Type-Safe Images" section: rewrite the generated-code example to the flat scheme, - drop the "namespace isolation" bullet, and update the "How It Works" namespace line. Add XPC2002 to - the diagnostics coverage. Keep the "Image Interfaces" subsection as a documented escape hatch (only - when you need the raw `Entity`) — mention the interfaces exist but do not present them as the - recommended handler parameter type. -- `rules/XPC2002.md` new; `AnalyzerReleases.Unshipped.md` add XPC2002. -- **CHANGELOG** (`XrmPluginCore/CHANGELOG.md`): add a `Breaking:` line (the project already uses - `Breaking:` lines, e.g. v1.3.0). - -## 11. Versioning - -This changes generated type names + namespaces = breaking for consumers. Ship as **v1.5.0**, and -**rename the pending `### v1.4.2 - Unreleased` heading to `### v1.5.0 - Unreleased`**, folding the -XPC3007 bullets in alongside the new image bullets (1.4.2 is never released on its own). The repo -expresses breaking changes as `Breaking:` lines within minor bumps (precedent: v1.3.0), so add a -prominent `Breaking:` line + migration link under the 1.5.0 heading. - -Analyzer release tracking: XPC3007 currently sits in `AnalyzerReleases.Unshipped.md` alongside the -new XPC2002 — both move to `AnalyzerReleases.Shipped.md` under `## Release 1.5.0` when 1.5.0 ships. - -## 12. Suggested PR sequence - -1. **XPC2002** analyzer + rule doc + tests (independently valuable; establishes the invariant). -2. **Generator + runtime rename** behind the same discovery contract (sections 3-4) + generation - tests. -3. **Code-fix convergence + subsystem deletion** (sections 5, 7) + analyzer update (section 6) + - migration hook + test updates. -4. **Docs + CHANGELOG + release tracking** (sections 10-11). - -Splitting this way keeps each PR reviewable and green; step 2 is the only one that must land -atomically (generator and runtime must agree on the name format in one commit). - -## 13. Open questions / risks - -- **Concatenation ambiguity** (`"AB"+"CD…"` vs `"ABC"+"D…"`): theoretically possible with - separator-less concatenation, but it's the *same* risk the current `{Entity}{Operation}{Stage}` - scheme already accepts, and now bounded further by XPC2002 + unique class names. If we want it fully - eliminated, insert an unambiguous separator that's still a valid identifier char (e.g. `_`) between - segments — cheap, at the cost of an underscore in the type name. **Recommend adding the separator** - since we're touching this anyway. -- **Custom-message operations**: sanitization must be identical on generator and runtime; covered by - reusing `IdentifierSanitizer`, but worth an explicit shared test. -- **Non-image registrations**: plugins that register steps *without* images generate no wrapper types - today; confirm the rename doesn't accidentally start emitting types for them.