Skip to content

Commit 459c32a

Browse files
authored
fix(SourceGenerator): resolve hint name collision and improve DX for new registrations (#5)
* fix(SourceGenerator): resolve hint name collision and improve DX for new registrations - Include namespace in UniqueId to prevent collision when same-named plugins exist in different namespaces - Remove HandlerMethodValidator that blocked generation when handler method was missing - types are now always generated so developers can reference PreImage/PostImage when creating new handler methods - Remove unused HasValidationError property from PluginStepMetadata - Add GeneratedSourceInfo to test helper for hint name verification Fixes chicken-and-egg problem where developers couldn't create handler methods with correct signatures because the generated types didn't exist. * Fix: Checked non-existant property
1 parent 841c6a8 commit 459c32a

File tree

5 files changed

+244
-64
lines changed

5 files changed

+244
-64
lines changed

XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,235 @@ public void DoSomething() { }
835835
xpc3003Diagnostics.Should().BeEmpty("XPC3003 should NOT be reported when using method reference syntax");
836836
}
837837

838+
[Fact]
839+
public void Should_Generate_Types_Even_When_Handler_Method_Not_Found()
840+
{
841+
// Arrange - method reference points to NonExistentMethod but types should still be generated
842+
// This enables a better DX where developers can create the method with correct signature
843+
// using the generated PreImage/PostImage types
844+
const string pluginSource = """
845+
846+
using XrmPluginCore;
847+
using XrmPluginCore.Enums;
848+
using Microsoft.Extensions.DependencyInjection;
849+
using TestNamespace;
850+
851+
namespace TestNamespace
852+
{
853+
public class TestPlugin : Plugin
854+
{
855+
public TestPlugin()
856+
{
857+
RegisterStep<Account, ITestService>(EventOperation.Update, ExecutionStage.PostOperation,
858+
service => service.NonExistentMethod)
859+
.WithPreImage(x => x.Name);
860+
}
861+
862+
protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services)
863+
{
864+
return services.AddScoped<ITestService, TestService>();
865+
}
866+
}
867+
868+
public interface ITestService
869+
{
870+
void Process(); // Different method, NonExistentMethod doesn't exist
871+
}
872+
873+
public class TestService : ITestService
874+
{
875+
public void Process() { }
876+
}
877+
}
878+
""";
879+
880+
var source = TestFixtures.GetCompleteSource(pluginSource);
881+
882+
// Act
883+
var result = GeneratorTestHelper.RunGenerator(
884+
CompilationHelper.CreateCompilation(source));
885+
886+
// Assert - Types should be generated even though handler method doesn't exist
887+
result.GeneratedTrees.Should().NotBeEmpty(
888+
"PreImage/PostImage types should be generated even when handler method doesn't exist");
889+
890+
// Verify PreImage class is generated
891+
var generatedSource = result.GeneratedTrees.First().ToString();
892+
generatedSource.Should().Contain("public sealed class PreImage",
893+
"PreImage class should be generated to allow developers to create the handler method with correct signature");
894+
}
895+
896+
[Fact]
897+
public void Should_Generate_Types_Even_When_Handler_Method_Wrong_Signature()
898+
{
899+
// Arrange - method reference points to MethodWithoutImage but types should still be generated
900+
// This enables a better DX where developers can create the method with correct signature
901+
// using the generated PreImage/PostImage types
902+
const string pluginSource = """
903+
904+
using XrmPluginCore;
905+
using XrmPluginCore.Enums;
906+
using Microsoft.Extensions.DependencyInjection;
907+
using TestNamespace;
908+
909+
namespace TestNamespace
910+
{
911+
public class TestPlugin : Plugin
912+
{
913+
public TestPlugin()
914+
{
915+
RegisterStep<Account, ITestService>(EventOperation.Update, ExecutionStage.PostOperation,
916+
service => service.MethodWithoutImage)
917+
.WithPreImage(x => x.Name);
918+
}
919+
920+
protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services)
921+
{
922+
return services.AddScoped<ITestService, TestService>();
923+
}
924+
}
925+
926+
public interface ITestService
927+
{
928+
void Process(); // Different method
929+
void MethodWithoutImage(); // Method exists but wrong signature (missing PreImage parameter)
930+
}
931+
932+
public class TestService : ITestService
933+
{
934+
public void Process() { }
935+
public void MethodWithoutImage() { }
936+
}
937+
}
938+
""";
939+
940+
var source = TestFixtures.GetCompleteSource(pluginSource);
941+
942+
// Act
943+
var result = GeneratorTestHelper.RunGenerator(
944+
CompilationHelper.CreateCompilation(source));
945+
946+
// Assert - Types should be generated even though handler method doesn't exist
947+
result.GeneratedTrees.Should().NotBeEmpty(
948+
"PreImage/PostImage types should be generated even when handler method doesn't exist");
949+
950+
// Verify PreImage class is generated
951+
var generatedSource = result.GeneratedTrees.First().ToString();
952+
generatedSource.Should().Contain("public sealed class PreImage",
953+
"PreImage class should be generated to allow developers to create the handler method with correct signature");
954+
955+
generatedSource.Should().Contain("namespace TestNamespace.PluginRegistrations.TestPlugin.AccountUpdatePostOperation",
956+
"Generated types should be in the correct namespace");
957+
}
958+
959+
[Fact]
960+
public void Should_Generate_Unique_Files_For_Same_Named_Plugins_In_Different_Namespaces()
961+
{
962+
// Arrange - Two plugins with the same class name but in different namespaces
963+
// Both register the same entity/operation/stage combination
964+
// Previously this would cause a hint name collision
965+
// Note: We don't use GetCompleteSource here because it strips namespaces
966+
const string source = """
967+
using System;
968+
using Microsoft.Xrm.Sdk;
969+
using XrmPluginCore;
970+
using XrmPluginCore.Enums;
971+
using Microsoft.Extensions.DependencyInjection;
972+
using XrmPluginCore.Tests.Context.BusinessDomain;
973+
974+
namespace Namespace1
975+
{
976+
public class AccountPlugin : Plugin
977+
{
978+
public AccountPlugin()
979+
{
980+
RegisterStep<Account, ITestService>(EventOperation.Update, ExecutionStage.PostOperation,
981+
service => service.HandleUpdate)
982+
.WithPreImage(x => x.Name);
983+
}
984+
985+
protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services)
986+
{
987+
return services.AddScoped<ITestService, TestService>();
988+
}
989+
}
990+
991+
public interface ITestService
992+
{
993+
void HandleUpdate();
994+
}
995+
996+
public class TestService : ITestService
997+
{
998+
public void HandleUpdate() { }
999+
}
1000+
}
1001+
1002+
namespace Namespace2
1003+
{
1004+
public class AccountPlugin : Plugin
1005+
{
1006+
public AccountPlugin()
1007+
{
1008+
RegisterStep<Account, ITestService>(EventOperation.Update, ExecutionStage.PostOperation,
1009+
service => service.HandleUpdate)
1010+
.WithPreImage(x => x.AccountNumber);
1011+
}
1012+
1013+
protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services)
1014+
{
1015+
return services.AddScoped<ITestService, TestService>();
1016+
}
1017+
}
1018+
1019+
public interface ITestService
1020+
{
1021+
void HandleUpdate();
1022+
}
1023+
1024+
public class TestService : ITestService
1025+
{
1026+
public void HandleUpdate() { }
1027+
}
1028+
}
1029+
""";
1030+
1031+
// Act
1032+
var result = GeneratorTestHelper.RunGenerator(
1033+
CompilationHelper.CreateCompilation(source));
1034+
1035+
// Assert - Both plugins should generate separate files with unique hint names
1036+
result.GeneratedSources.Should().HaveCount(2,
1037+
"both plugins should generate code without hint name collision");
1038+
1039+
// Index sources by hint name for precise verification
1040+
var sourcesByHintName = result.GeneratedSources.ToDictionary(gs => gs.HintName, gs => gs.SourceText);
1041+
1042+
// Find the hint names for each namespace
1043+
var namespace1HintName = sourcesByHintName.Keys.Single(h => h.Contains("Namespace1_"));
1044+
var namespace2HintName = sourcesByHintName.Keys.Single(h => h.Contains("Namespace2_"));
1045+
1046+
// Verify Namespace1 source: correct namespace AND correct property (Name)
1047+
var namespace1Source = sourcesByHintName[namespace1HintName];
1048+
namespace1Source.Should().Contain("namespace Namespace1.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation",
1049+
"Namespace1 hint name should map to Namespace1 generated namespace");
1050+
namespace1Source.Should().Contain("public string Name =>",
1051+
"Namespace1 plugin registered Name attribute");
1052+
1053+
// Verify Namespace2 source: correct namespace AND correct property (AccountNumber)
1054+
var namespace2Source = sourcesByHintName[namespace2HintName];
1055+
namespace2Source.Should().Contain("namespace Namespace2.PluginRegistrations.AccountPlugin.AccountUpdatePostOperation",
1056+
"Namespace2 hint name should map to Namespace2 generated namespace");
1057+
namespace2Source.Should().Contain("public string AccountNumber =>",
1058+
"Namespace2 plugin registered AccountNumber attribute");
1059+
1060+
// Verify each source only contains its own namespace (not cross-contaminated)
1061+
namespace1Source.Should().NotContain("namespace Namespace2",
1062+
"Namespace1 source should not contain Namespace2 namespace declaration");
1063+
namespace2Source.Should().NotContain("namespace Namespace1",
1064+
"Namespace2 source should not contain Namespace1 namespace declaration");
1065+
}
1066+
8381067
private static async Task<ImmutableArray<Diagnostic>> GetAnalyzerDiagnosticsAsync(string source, DiagnosticAnalyzer analyzer)
8391068
{
8401069
var compilation = CompilationHelper.CreateCompilation(source);

XrmPluginCore.SourceGenerator.Tests/Helpers/GeneratorTestHelper.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,17 @@ public static GeneratorRunResult RunGenerator(CSharpCompilation compilation)
3535
.Where(tree => !compilation.SyntaxTrees.Contains(tree))
3636
.ToArray();
3737

38+
// Get generated sources with hint names from the run result
39+
var generatedSources = runResult.Results[0].GeneratedSources
40+
.Select(gs => new GeneratedSourceInfo(gs.HintName, gs.SourceText.ToString()))
41+
.ToArray();
42+
3843
return new GeneratorRunResult
3944
{
4045
OutputCompilation = (CSharpCompilation)outputCompilation,
4146
Diagnostics = [.. diagnostics],
4247
GeneratedTrees = generatedTrees,
48+
GeneratedSources = generatedSources,
4349
GeneratorDiagnostics = [.. runResult.Results[0].Diagnostics]
4450
};
4551
}
@@ -105,9 +111,15 @@ public class GeneratorRunResult
105111
public required CSharpCompilation OutputCompilation { get; init; }
106112
public required Diagnostic[] Diagnostics { get; init; }
107113
public required SyntaxTree[] GeneratedTrees { get; init; }
114+
public required GeneratedSourceInfo[] GeneratedSources { get; init; }
108115
public required Diagnostic[] GeneratorDiagnostics { get; init; }
109116
}
110117

118+
/// <summary>
119+
/// Information about a generated source file including its hint name.
120+
/// </summary>
121+
public record GeneratedSourceInfo(string HintName, string SourceText);
122+
111123
/// <summary>
112124
/// Result from compiling generated code.
113125
/// </summary>

XrmPluginCore.SourceGenerator/Generators/PluginImageGenerator.cs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using XrmPluginCore.SourceGenerator.Helpers;
1010
using XrmPluginCore.SourceGenerator.Models;
1111
using XrmPluginCore.SourceGenerator.Parsers;
12-
using XrmPluginCore.SourceGenerator.Validation;
1312

1413
namespace XrmPluginCore.SourceGenerator.Generators;
1514

@@ -94,11 +93,6 @@ private static IEnumerable<PluginStepMetadata> TransformToMetadata(
9493
if (mergedMetadata is null)
9594
continue;
9695

97-
// Validate handler method signature
98-
HandlerMethodValidator.ValidateHandlerMethod(
99-
mergedMetadata,
100-
semanticModel.Compilation);
101-
10296
// Include if:
10397
// - Has method reference (for ActionWrapper generation)
10498
// - OR has images with attributes (for image wrapper generation)
@@ -185,10 +179,6 @@ private void GenerateSourceFromMetadata(
185179
}
186180
}
187181

188-
// Skip generation if validation failed (analyzer will report the error)
189-
if (metadata?.HasValidationError == true)
190-
return;
191-
192182
// Generate code if we have a handler method reference (ActionWrapper always needed)
193183
if (string.IsNullOrEmpty(metadata?.HandlerMethodName))
194184
return;

XrmPluginCore.SourceGenerator/Models/PluginStepMetadata.cs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,6 @@ internal sealed class PluginStepMetadata
4242
/// </summary>
4343
public List<DiagnosticInfo> Diagnostics { get; set; } = [];
4444

45-
/// <summary>
46-
/// If true, generation should be skipped for this registration due to validation errors.
47-
/// The analyzer will report the appropriate diagnostic. Not included in equality comparison.
48-
/// </summary>
49-
public bool HasValidationError { get; set; }
50-
5145
/// <summary>
5246
/// Gets the namespace for generated wrapper classes.
5347
/// Format: {OriginalNamespace}.PluginRegistrations.{PluginClassName}.{Entity}{Op}{Stage}
@@ -57,10 +51,11 @@ internal sealed class PluginStepMetadata
5751

5852
/// <summary>
5953
/// Gets a unique identifier for this registration.
60-
/// Includes plugin class name to differentiate multiple registrations for the same entity/operation/stage.
54+
/// Includes namespace and plugin class name to differentiate multiple registrations
55+
/// for the same entity/operation/stage across different namespaces.
6156
/// </summary>
6257
public string UniqueId =>
63-
$"{PluginClassName}_{EntityTypeName}_{EventOperation}_{ExecutionStage}";
58+
$"{Namespace?.Replace(".", "_")}_{PluginClassName}_{EntityTypeName}_{EventOperation}_{ExecutionStage}";
6459

6560
public override bool Equals(object obj)
6661
{

XrmPluginCore.SourceGenerator/Validation/HandlerMethodValidator.cs

Lines changed: 0 additions & 46 deletions
This file was deleted.

0 commit comments

Comments
 (0)