diff --git a/.gitignore b/.gitignore index e1a1752..a47f294 100644 --- a/.gitignore +++ b/.gitignore @@ -401,3 +401,6 @@ FodyWeavers.xsd # Claude local settings .claude/*.local.json + +# Conducktor local files +.conducktor/ diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CodeFixTestBase.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CodeFixTestBase.cs index 2c4ab8d..9d74ef3 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CodeFixTestBase.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CodeFixTestBase.cs @@ -3,6 +3,7 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; using XrmPluginCore.SourceGenerator.Tests.Helpers; namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; @@ -82,6 +83,75 @@ protected static async Task ApplyCodeFixAsync( return newText.ToString(); } + /// + /// Applies the code fix provider's FixAll provider across every matching diagnostic in the + /// document (Document scope), exercising the custom FixAllProvider rather than a single + /// per-diagnostic fix. + /// + protected static async Task ApplyFixAllAsync( + string source, + DiagnosticAnalyzer analyzer, + CodeFixProvider codeFixProvider, + string equivalenceKey, + params string[] diagnosticIds) + { + var document = CreateDocument(source); + + // Compute diagnostics against the document's own compilation so their locations reference the + // document's syntax tree (the FixAll provider maps locations back to documents). + var compilation = await document.Project.GetCompilationAsync(); + var compilationWithAnalyzers = compilation!.WithAnalyzers([analyzer]); + var analyzerDiagnostics = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + var diagnostics = analyzerDiagnostics.Where(d => diagnosticIds.Contains(d.Id)).ToImmutableArray(); + + if (diagnostics.IsEmpty) + { + return source; + } + + var fixAllProvider = codeFixProvider.GetFixAllProvider()!; + var fixAllContext = new FixAllContext( + document, + codeFixProvider, + FixAllScope.Document, + equivalenceKey, + diagnosticIds, + new FixAllDiagnosticProvider(diagnostics), + CancellationToken.None); + + var codeAction = await fixAllProvider.GetFixAsync(fixAllContext); + if (codeAction == null) + { + return source; + } + + var operations = await codeAction.GetOperationsAsync(CancellationToken.None); + var changedSolution = operations.OfType().Single().ChangedSolution; + var changedDocument = changedSolution.GetDocument(document.Id); + var newText = await changedDocument!.GetTextAsync(); + + return newText.ToString(); + } + + private sealed class FixAllDiagnosticProvider : FixAllContext.DiagnosticProvider + { + private readonly ImmutableArray _diagnostics; + + public FixAllDiagnosticProvider(ImmutableArray diagnostics) + { + _diagnostics = diagnostics; + } + + public override Task> GetDocumentDiagnosticsAsync(Document document, CancellationToken cancellationToken) + => Task.FromResult>(_diagnostics); + + public override Task> GetAllDiagnosticsAsync(Project project, CancellationToken cancellationToken) + => Task.FromResult>(_diagnostics); + + public override Task> GetProjectDiagnosticsAsync(Project project, CancellationToken cancellationToken) + => Task.FromResult>(Enumerable.Empty()); + } + protected static async Task> GetCodeActionsAsync( string source, DiagnosticAnalyzer analyzer, diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs index a2462b1..b11b814 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/CreateHandlerMethodCodeFixProviderTests.cs @@ -1,6 +1,7 @@ using FluentAssertions; using XrmPluginCore.SourceGenerator.Analyzers; using XrmPluginCore.SourceGenerator.CodeFixes; +using XrmPluginCore.SourceGenerator.Tests.Helpers; using Xunit; namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; @@ -55,11 +56,11 @@ public class TestService : ITestService // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Method created with PreImage parameter - fixedSource.Should().Contain("void HandleUpdate(PreImage preImage)"); + // Assert - Method created with alias-qualified PreImage parameter + fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - // Assert - Using directive added - fixedSource.Should().Contain("using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + // Assert - Aliased using directive added + fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); } [Fact] @@ -107,11 +108,11 @@ public class TestService : ITestService // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Method created with both image parameters - fixedSource.Should().Contain("void HandleUpdate(PreImage preImage, PostImage postImage)"); + // Assert - Method created with both alias-qualified image parameters + fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage, AccountUpdatePostOperation.PostImage postImage)"); - // Assert - Using directive added - fixedSource.Should().Contain("using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + // Assert - Aliased using directive added + fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); } [Fact] @@ -165,9 +166,11 @@ public class TestService : ITestService } [Fact] - public async Task Should_Avoid_Ambiguous_Usings() + public async Task Should_Use_Aliased_Usings_For_Multiple_Registrations() { - // Arrange - Using directive already exists for Update registration, method missing for Delete + // 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. const string source = """ using System; using System.ComponentModel; @@ -211,22 +214,191 @@ public class TestService : ITestService public void HandleUpdate(PreImage preImage) { } } } + + 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 - New method created with qualified type + // Assert - New method created with its own alias-qualified type fixedSource.Should().Contain("void HandleDelete(AccountDeletePostOperation.PreImage preImage)"); - // Assert - Existing PreImage references qualified with alias + // Assert - Existing bare PreImage references requalified with the Update alias CountOccurrences(fixedSource, "AccountUpdatePostOperation.PreImage preImage").Should().BeGreaterOrEqualTo(1); - // Assert - Aliased usings + // Assert - Each namespace aliased exactly once CountOccurrences(fixedSource, "using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;") - .Should().Be(1, "the existing using should be converted to aliased form"); + .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"); + + AssertNoAmbiguousReferences(fixedSource); + } + + [Fact] + public async Task FixAll_Should_Create_Multiple_Methods_On_Same_Interface() + { + // Arrange - Two same-service registrations, both missing their handler methods. + 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") + .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(); + } + } + + public interface ITestService + { + } + + 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)"); + + // 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); + } + + [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 = """ + 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 + { + public TestPlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + "HandleUpdate") + .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 + { + } + } + + 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) @@ -244,4 +416,9 @@ private static int CountOccurrences(string source, string search) private static Task ApplyCodeFixAsync(string source) => ApplyCodeFixAsync(source, new HandlerMethodNotFoundAnalyzer(), new CreateHandlerMethodCodeFixProvider(), DiagnosticDescriptors.HandlerMethodNotFound.Id); + + private static Task ApplyFixAllAsync(string source) + => ApplyFixAllAsync(source, new HandlerMethodNotFoundAnalyzer(), new CreateHandlerMethodCodeFixProvider(), + nameof(CreateHandlerMethodCodeFixProvider), + DiagnosticDescriptors.HandlerMethodNotFound.Id); } diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs index cf56b34..881e6d8 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/FixHandlerSignatureCodeFixProviderTests.cs @@ -1,6 +1,7 @@ using FluentAssertions; using XrmPluginCore.SourceGenerator.Analyzers; using XrmPluginCore.SourceGenerator.CodeFixes; +using XrmPluginCore.SourceGenerator.Tests.Helpers; using Xunit; namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; @@ -57,11 +58,11 @@ public void HandleUpdate() { } // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed - fixedSource.Should().Contain("void HandleUpdate(PreImage preImage)"); + // Assert - Signature fixed (alias-qualified) + fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - // Assert - Using directive added - fixedSource.Should().Contain("using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + // Assert - Aliased using directive added + fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); } [Fact] @@ -110,11 +111,11 @@ public void HandleUpdate() { } // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed - fixedSource.Should().Contain("void HandleUpdate(PostImage postImage)"); + // Assert - Signature fixed (alias-qualified) + fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PostImage postImage)"); - // Assert - Using directive added - fixedSource.Should().Contain("using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + // Assert - Aliased using directive added + fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); } [Fact] @@ -164,11 +165,11 @@ public void HandleUpdate() { } // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed with both image parameters - fixedSource.Should().Contain("void HandleUpdate(PreImage preImage, PostImage postImage)"); + // Assert - Signature fixed with both image parameters (alias-qualified) + fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage, AccountUpdatePostOperation.PostImage postImage)"); - // Assert - Using directive added - fixedSource.Should().Contain("using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); + // Assert - Aliased using directive added + fixedSource.Should().Contain("using AccountUpdatePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"); } [Fact] @@ -218,19 +219,22 @@ public void HandleUpdate() { } // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed - fixedSource.Should().Contain("void HandleUpdate(PreImage preImage)"); + // Assert - Signature fixed (alias-qualified) + fixedSource.Should().Contain("void HandleUpdate(AccountUpdatePostOperation.PreImage preImage)"); - // Assert - Using directive not duplicated (count occurrences) - var usingDirective = "using TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation;"; - var count = CountOccurrences(fixedSource, usingDirective); - count.Should().Be(1, "the using directive should not be duplicated"); + // 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_Avoid_Ambiguous_Usings() + public async Task Should_Use_Aliased_Usings_For_Multiple_Registrations() { - // Arrange - Using directive already exists for Update registration + // 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; @@ -276,20 +280,450 @@ 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 { } + } """; // Act var fixedSource = await ApplyCodeFixAsync(source); - // Assert - Signature fixed + // 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 - Using directive not duplicated (count occurrences) + // 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); + } + + [Fact] + public async Task Should_Realias_CleanedUp_Plain_Using() + { + // 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. + 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 + { + 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(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, "the using directive should be de-ambiguified"); + .Should().Be(1); CountOccurrences(fixedSource, "using AccountDeletePostOperation = TestNamespace.PluginRegistrations.TestPlugin.AccountDeletePostOperation;") - .Should().Be(1, "the using directive should be de-ambiguified"); + .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(); + } + } + + public interface ITestService + { + void HandleUpdate(); + void HandleDelete(); + } + + public class TestService : ITestService + { + public void HandleUpdate() { } + 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); + } + + [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 = """ + 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 + { + 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 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) @@ -307,4 +741,9 @@ private static int CountOccurrences(string source, string search) private static Task ApplyCodeFixAsync(string source) => ApplyCodeFixAsync(source, new HandlerSignatureMismatchAnalyzer(), new FixHandlerSignatureCodeFixProvider(), DiagnosticDescriptors.HandlerSignatureMismatch.Id, DiagnosticDescriptors.HandlerSignatureMismatchError.Id); + + private static Task ApplyFixAllAsync(string source) + => ApplyFixAllAsync(source, new HandlerSignatureMismatchAnalyzer(), new FixHandlerSignatureCodeFixProvider(), + nameof(FixHandlerSignatureCodeFixProvider), + DiagnosticDescriptors.HandlerSignatureMismatch.Id, DiagnosticDescriptors.HandlerSignatureMismatchError.Id); } diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs new file mode 100644 index 0000000..383e8e3 --- /dev/null +++ b/XrmPluginCore.SourceGenerator/CodeFixes/AliasedImageUsingsFixAllProvider.cs @@ -0,0 +1,348 @@ +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 63d4ccc..a46538c 100644 --- a/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs +++ b/XrmPluginCore.SourceGenerator/CodeFixes/CreateHandlerMethodCodeFixProvider.cs @@ -22,7 +22,7 @@ public class CreateHandlerMethodCodeFixProvider : CodeFixProvider ImmutableArray.Create(DiagnosticDescriptors.HandlerMethodNotFound.Id); public sealed override FixAllProvider GetFixAllProvider() => - WellKnownFixAllProviders.BatchFixer; + AliasedImageUsingsFixAllProvider.Instance; public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -132,29 +132,45 @@ private static async Task CreateMethodAsync( return solution; } - // Detect ambiguity before creating the method - var (needsAlias, alias) = SyntaxFactoryHelper.DetectImageAmbiguity(interfaceRoot, imageNamespace); - - // Create the method declaration with qualifier if ambiguous - var methodDeclaration = CreateMethodDeclaration(methodName, hasPreImage, hasPostImage, needsAlias ? alias : null); - - var newInterface = interfaceDeclaration.AddMembers(methodDeclaration); - var newRoot = interfaceRoot.ReplaceNode(interfaceDeclaration, newInterface); - - // Handle usings - if (needsAlias) - { - newRoot = SyntaxFactoryHelper.ConvertToAliasedUsingsAndQualifyRefs(newRoot, imageNamespace); - } - else + // 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 = SyntaxFactoryHelper.AddUsingDirectiveIfMissing(newRoot, imageNamespace); + newRoot = newRoot.ReplaceNode(targetInterface, targetInterface.AddMembers(methodDeclaration)); } return solution.WithDocumentSyntaxRoot(interfaceDocument.Id, newRoot); } - private static MethodDeclarationSyntax CreateMethodDeclaration(string methodName, bool hasPreImage, bool hasPostImage, string qualifier = null) + internal static MethodDeclarationSyntax CreateMethodDeclaration(string methodName, bool hasPreImage, bool hasPostImage, string qualifier = null) { return SyntaxFactory.MethodDeclaration( SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.VoidKeyword)), @@ -165,7 +181,7 @@ private static MethodDeclarationSyntax CreateMethodDeclaration(string methodName .WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed); } - private static async Task FindInterfaceDeclarationAsync( + internal static async Task FindInterfaceDeclarationAsync( Solution solution, INamedTypeSymbol typeSymbol, CancellationToken cancellationToken) diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs index cb56481..c84966e 100644 --- a/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs +++ b/XrmPluginCore.SourceGenerator/CodeFixes/FixHandlerSignatureCodeFixProvider.cs @@ -25,7 +25,7 @@ public class FixHandlerSignatureCodeFixProvider : CodeFixProvider DiagnosticDescriptors.HandlerSignatureMismatchError.Id); public sealed override FixAllProvider GetFixAllProvider() => - WellKnownFixAllProviders.BatchFixer; + AliasedImageUsingsFixAllProvider.Instance; public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -195,9 +195,11 @@ private static async Task FixMethodDeclarationsAsync( continue; } - // Detect ambiguity - var ambiguity = SyntaxFactoryHelper.DetectImageAmbiguity(methodRoot, imageNamespace); - var newParameters = SyntaxFactoryHelper.CreateImageParameterList(hasPreImage, hasPostImage, ambiguity.needsAlias ? ambiguity.alias : null); + // 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)>(); @@ -211,8 +213,14 @@ private static async Task FixMethodDeclarationsAsync( } } + // 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 - SyntaxNode newRoot = methodRoot; foreach (var target in targets) { var current = newRoot.DescendantNodes().OfType() @@ -224,16 +232,6 @@ private static async Task FixMethodDeclarationsAsync( } } - // Handle usings - if (ambiguity.needsAlias) - { - newRoot = SyntaxFactoryHelper.ConvertToAliasedUsingsAndQualifyRefs(newRoot, imageNamespace); - } - else - { - newRoot = SyntaxFactoryHelper.AddUsingDirectiveIfMissing(newRoot, imageNamespace); - } - solution = solution.WithDocumentSyntaxRoot(methodDocument.Id, newRoot); } diff --git a/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs b/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs index 90fcaff..ec3f097 100644 --- a/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs +++ b/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs @@ -1,7 +1,9 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; using System.Linq; +using System.Threading; namespace XrmPluginCore.SourceGenerator.Helpers; @@ -109,6 +111,38 @@ public static bool IsRegisterStepCall(InvocationExpressionSyntax invocation, out return false; } + /// + /// Resolves the service type symbol (the second generic argument of a RegisterStep call) from a + /// diagnostic location. Returns null if no enclosing RegisterStep call is found or the type cannot + /// be resolved. + /// + public static INamedTypeSymbol ResolveServiceType( + SyntaxNode root, + SemanticModel semanticModel, + TextSpan diagnosticSpan, + CancellationToken cancellationToken) + { + var diagnosticNode = root.FindNode(diagnosticSpan); + var registerStepInvocation = diagnosticNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(i => IsRegisterStepCall(i, out _)); + + if (registerStepInvocation == null) + { + return null; + } + + var genericName = GetGenericName(registerStepInvocation); + if (genericName == null || genericName.TypeArgumentList.Arguments.Count < 2) + { + return null; + } + + var serviceTypeSyntax = genericName.TypeArgumentList.Arguments[1]; + var typeInfo = semanticModel.GetTypeInfo(serviceTypeSyntax, cancellationToken); + return typeInfo.Type as INamedTypeSymbol; + } + /// /// Gets the GenericNameSyntax from a RegisterStep call. /// diff --git a/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs b/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs index 0db1e3f..876e842 100644 --- a/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs +++ b/XrmPluginCore.SourceGenerator/Helpers/SyntaxFactoryHelper.cs @@ -145,11 +145,90 @@ public static (bool needsAlias, string alias) DetectImageAmbiguity(SyntaxNode ro 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. /// - public static SyntaxNode ConvertToAliasedUsingsAndQualifyRefs(SyntaxNode root, string newImageNamespace) + /// 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) { @@ -172,32 +251,54 @@ public static SyntaxNode ConvertToAliasedUsingsAndQualifyRefs(SyntaxNode root, s } } - // Also include the new namespace - if (!string.IsNullOrEmpty(newImageNamespace)) + // Also include each new namespace + foreach (var newImageNamespace in newImageNamespaces) { - plainNamespaceToAlias[newImageNamespace] = GetLastNamespaceSegment(newImageNamespace); + if (!string.IsNullOrEmpty(newImageNamespace)) + { + plainNamespaceToAlias[newImageNamespace] = GetLastNamespaceSegment(newImageNamespace); + } } // Rewrite the tree - var rewriter = new ImageAmbiguityRewriter(plainNamespaceToAlias); + var rewriter = new ImageAmbiguityRewriter(plainNamespaceToAlias, semanticModel); var newRoot = rewriter.Visit(root); - // Add the new aliased using if not already present - if (newRoot is CompilationUnitSyntax newCompUnit && !string.IsNullOrEmpty(newImageNamespace)) + // Add each new aliased using if not already present + if (newRoot is CompilationUnitSyntax newCompUnit) { - var newAlias = GetLastNamespaceSegment(newImageNamespace); - var alreadyExists = newCompUnit.Usings.Any(u => - u.Alias?.Name.ToString() == newAlias || - u.Name?.ToString() == newImageNamespace); - - if (!alreadyExists) + var usingsToAdd = new List(); + foreach (var newImageNamespace in newImageNamespaces) { - var aliasedUsing = SyntaxFactory.UsingDirective( - SyntaxFactory.NameEquals(SyntaxFactory.IdentifierName(newAlias)), - SyntaxFactory.ParseName(newImageNamespace)) - .WithTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed); + 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)); + } + } - newRoot = newCompUnit.AddUsings(aliasedUsing); + if (usingsToAdd.Count > 0) + { + newRoot = newCompUnit.AddUsings(usingsToAdd.ToArray()); } } @@ -235,16 +336,14 @@ private sealed class ImageAmbiguityRewriter : CSharpSyntaxRewriter { private readonly Dictionary _plainNamespaceToAlias; - // Reverse map: for each bare type name, which alias qualifies it - // Built from existing usings only (not the new one being added) - private readonly Dictionary _typeToExistingAlias; + // 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) + public ImageAmbiguityRewriter(Dictionary plainNamespaceToAlias, SemanticModel semanticModel) { _plainNamespaceToAlias = plainNamespaceToAlias; - - // Build type-to-alias map for qualifying existing bare references - _typeToExistingAlias = new Dictionary(); + _semanticModel = semanticModel; } public override SyntaxNode VisitUsingDirective(UsingDirectiveSyntax node) @@ -257,9 +356,6 @@ public override SyntaxNode VisitUsingDirective(UsingDirectiveSyntax node) var ns = node.Name?.ToString(); if (ns != null && _plainNamespaceToAlias.TryGetValue(ns, out var alias)) { - // Track which alias maps to which namespace for qualifying references - _typeToExistingAlias[ns] = alias; - // Convert to aliased using return SyntaxFactory.UsingDirective( SyntaxFactory.NameEquals(SyntaxFactory.IdentifierName(alias)), @@ -299,16 +395,13 @@ public override SyntaxNode VisitIdentifierName(IdentifierNameSyntax node) return base.VisitIdentifierName(node); } - // Find the alias for this type reference based on the existing usings that were converted - // We need to figure out which alias applies to this bare reference. - // The bare reference was valid under the old plain using, so find which converted namespace it belonged to. - string matchingAlias = null; - foreach (var kvp in _typeToExistingAlias) - { - matchingAlias = kvp.Value; - break; // There should be exactly one existing plain image using at this point - } - + // 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); @@ -323,5 +416,28 @@ public override SyntaxNode VisitIdentifierName(IdentifierNameSyntax node) .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; + } } }