diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DuplicateStepRegistrationAnalyzerTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DuplicateStepRegistrationAnalyzerTests.cs new file mode 100644 index 0000000..e6bade8 --- /dev/null +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DuplicateStepRegistrationAnalyzerTests.cs @@ -0,0 +1,443 @@ +using FluentAssertions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using XrmPluginCore.SourceGenerator.Analyzers; +using XrmPluginCore.SourceGenerator.Tests.Helpers; +using Xunit; + +namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; + +/// +/// Tests for XPC2002 (duplicate plugin step registration for the same entity/operation/stage). +/// +public class DuplicateStepRegistrationAnalyzerTests +{ + [Fact] + public async Task Should_Report_XPC2002_For_Duplicate_Entity_Operation_Stage() + { + var source = WrapPlugin(""" + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + var diagnostic = diagnostics.Should().ContainSingle(d => d.Id == "XPC2002").Subject; + diagnostic.Severity.Should().Be(DiagnosticSeverity.Error); + diagnostic.GetMessage().Should().Contain("Account/Update/PostOperation"); + } + + [Fact] + public async Task Should_Not_Report_When_Stage_Differs() + { + var source = WrapPlugin(""" + RegisterStep(EventOperation.Update, ExecutionStage.PreOperation, + nameof(ITestService.HandleA)); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Not_Report_When_Operation_Differs() + { + var source = WrapPlugin(""" + RegisterStep(EventOperation.Create, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_Once_Per_Extra_Duplicate() + { + // Three registrations of the same tuple -> two duplicates reported (2nd and 3rd). + var source = WrapPlugin(""" + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleC)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Count(d => d.Id == "XPC2002").Should().Be(2); + } + + [Fact] + public async Task Should_Report_For_Duplicate_Custom_Message_String() + { + var source = WrapPlugin("""" + RegisterStep("custom_DoThing", ExecutionStage.PostOperation, s => { }); + RegisterStep("custom_DoThing", ExecutionStage.PostOperation, s => { }); + """"); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_For_Duplicate_RegisterPluginStep() + { + var source = WrapPlugin(""" + RegisterPluginStep(EventOperation.Update, ExecutionStage.PostOperation, ctx => { }); + RegisterPluginStep(EventOperation.Update, ExecutionStage.PostOperation, ctx => { }); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_For_Duplicate_Const_Message_Name() + { + var source = WrapPlugin(""" + const string Msg = "custom_DoThing"; + RegisterStep(Msg, ExecutionStage.PostOperation, s => { }); + RegisterStep(Msg, ExecutionStage.PostOperation, s => { }); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_For_Duplicate_Const_Enum_Operation() + { + var source = WrapPlugin(""" + const EventOperation Op = EventOperation.Update; + RegisterStep(Op, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep(Op, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Not_Report_When_Const_Enum_Operations_Differ() + { + var source = WrapPlugin(""" + const EventOperation Create = EventOperation.Create; + const EventOperation Update = EventOperation.Update; + RegisterStep(Create, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep(Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_When_Different_Const_Members_Share_Message_Value() + { + // Two distinct const string members with the same value, referenced via member access. The + // runtime/wrapper naming keys off the string value, so these are duplicates even though the + // member identifiers differ. + const string source = """ + using System; + using Microsoft.Xrm.Sdk; + using XrmPluginCore; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using XrmPluginCore.Tests.Context.BusinessDomain; + + namespace TestNamespace + { + public static class Msgs + { + public const string A = "custom_DoThing"; + public const string B = "custom_DoThing"; + } + + public class TestPlugin : Plugin + { + public TestPlugin() + { + RegisterStep(Msgs.A, ExecutionStage.PostOperation, s => { }); + RegisterStep(Msgs.B, ExecutionStage.PostOperation, s => { }); + } + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Not_Report_When_Const_Members_Have_Different_Message_Values() + { + const string source = """ + using System; + using Microsoft.Xrm.Sdk; + using XrmPluginCore; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using XrmPluginCore.Tests.Context.BusinessDomain; + + namespace TestNamespace + { + public static class Msgs + { + public const string A = "custom_DoThingA"; + public const string B = "custom_DoThingB"; + } + + public class TestPlugin : Plugin + { + public TestPlugin() + { + RegisterStep(Msgs.A, ExecutionStage.PostOperation, s => { }); + RegisterStep(Msgs.B, ExecutionStage.PostOperation, s => { }); + } + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_For_Duplicate_Undefined_Enum_Cast_Operation() + { + // An undefined enum value resolves via Enum.ToString() to its integral text ("999"), so two such + // registrations are still recognized as duplicates. + var source = WrapPlugin(""" + RegisterStep((EventOperation)999, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep((EventOperation)999, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Not_Report_When_Undefined_Enum_Cast_Operations_Differ() + { + var source = WrapPlugin(""" + RegisterStep((EventOperation)998, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterStep((EventOperation)999, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + """); + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_When_Duplicate_Split_Across_Partial_Declarations() + { + const string source = """ + using System; + using Microsoft.Xrm.Sdk; + using XrmPluginCore; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using XrmPluginCore.Tests.Context.BusinessDomain; + + namespace TestNamespace + { + public partial class TestPlugin : Plugin + { + public TestPlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + RegisterExtra(); + } + } + + public partial class TestPlugin + { + private void RegisterExtra() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + } + } + + public interface ITestService { void HandleA(); void HandleB(); } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().ContainSingle(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Report_When_Derived_Re_Registers_Inherited_Tuple() + { + const string source = """ + using System; + using Microsoft.Xrm.Sdk; + using XrmPluginCore; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using XrmPluginCore.Tests.Context.BusinessDomain; + + namespace TestNamespace + { + public class BasePlugin : Plugin + { + public BasePlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + } + } + + public class DerivedPlugin : BasePlugin + { + public DerivedPlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleB)); + } + } + + public interface ITestService { void HandleA(); void HandleB(); } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + + var diagnostic = diagnostics.Should().ContainSingle(d => d.Id == "XPC2002").Subject; + diagnostic.GetMessage().Should().Contain("DerivedPlugin"); + } + + [Fact] + public async Task Should_Not_Report_For_Non_Plugin_Class_With_RegisterStep_Method() + { + // A class that is not a Plugin but happens to have a generic RegisterStep method must not + // be flagged. + const string source = """ + using XrmPluginCore.Enums; + + namespace TestNamespace + { + public class NotAPlugin + { + public NotAPlugin() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation); + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation); + } + + private void RegisterStep(EventOperation op, ExecutionStage stage) { } + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + [Fact] + public async Task Should_Not_Report_Across_Different_Plugin_Classes() + { + const string source = """ + using System; + using Microsoft.Xrm.Sdk; + using XrmPluginCore; + using XrmPluginCore.Enums; + using Microsoft.Extensions.DependencyInjection; + using XrmPluginCore.Tests.Context.BusinessDomain; + + namespace TestNamespace + { + public class PluginA : Plugin + { + public PluginA() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + } + } + + public class PluginB : Plugin + { + public PluginB() + { + RegisterStep(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.HandleA)); + } + } + + public interface ITestService { void HandleA(); } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + + diagnostics.Should().NotContain(d => d.Id == "XPC2002"); + } + + private static string WrapPlugin(string registrations) => + $$""" + using System; + 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() + { + {{registrations}} + } + } + + public interface ITestService + { + void HandleA(); + void HandleB(); + void HandleC(); + } + } + """; + + private static async Task> GetDiagnosticsAsync(string source) + { + var compilation = CompilationHelper.CreateCompilation(source); + var compilationWithAnalyzers = compilation.WithAnalyzers([new DuplicateStepRegistrationAnalyzer()]); + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } +} diff --git a/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md b/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md index 6807015..171ae7b 100644 --- a/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md +++ b/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md @@ -2,6 +2,7 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- +XPC2002 | XrmPluginCore.SourceGenerator | Error | XPC2002 Duplicate plugin step registration for the same entity/operation/stage XPC3007 | XrmPluginCore.SourceGenerator | Info | XPC3007 Prefer the type-safe Custom API handler overload when request/response parameters are declared ### Removed Rules diff --git a/XrmPluginCore.SourceGenerator/Analyzers/DuplicateStepRegistrationAnalyzer.cs b/XrmPluginCore.SourceGenerator/Analyzers/DuplicateStepRegistrationAnalyzer.cs new file mode 100644 index 0000000..cc71aa4 --- /dev/null +++ b/XrmPluginCore.SourceGenerator/Analyzers/DuplicateStepRegistrationAnalyzer.cs @@ -0,0 +1,218 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using XrmPluginCore.SourceGenerator.Helpers; + +namespace XrmPluginCore.SourceGenerator.Analyzers; + +/// +/// Reports when a plugin class registers the same entity/operation/stage combination more than once +/// (via RegisterStep or the legacy RegisterPluginStep). Such a duplicate is rejected at +/// registration time and would make the source-generated image/wrapper type names — which are derived +/// from that tuple — ambiguous. +/// +/// Registrations are collected per plugin type across every syntax tree (so partial plugins are +/// aggregated) and duplicates are evaluated at compilation end, where inherited base-plugin +/// registrations are also considered. Only types deriving from XrmPluginCore.Plugin are analyzed. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class DuplicateStepRegistrationAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(DiagnosticDescriptors.DuplicateStepRegistration); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + // Accumulate registrations per plugin type across all trees (handles partial declarations). + var registrationsByType = new ConcurrentDictionary>(SymbolEqualityComparer.Default); + + context.RegisterSyntaxNodeAction( + nodeContext => CollectRegistration(nodeContext, registrationsByType), + SyntaxKind.InvocationExpression); + + context.RegisterCompilationEndAction(endContext => ReportDuplicates(endContext, registrationsByType)); + } + + private static void CollectRegistration( + SyntaxNodeAnalysisContext context, + ConcurrentDictionary> registrationsByType) + { + var invocation = (InvocationExpressionSyntax)context.Node; + + if (!RegisterStepHelper.IsStepRegistrationCall(invocation, out var genericName) || + genericName.TypeArgumentList.Arguments.Count < 1) + { + return; + } + + var classDeclaration = invocation.FirstAncestorOrSelf(); + if (classDeclaration == null || + context.SemanticModel.GetDeclaredSymbol(classDeclaration) is not INamedTypeSymbol typeSymbol || + !IsPluginClass(typeSymbol)) + { + return; + } + + var entity = context.SemanticModel.GetTypeInfo(genericName.TypeArgumentList.Arguments[0]).Type?.Name; + + var (operationExpr, stageExpr) = RegisterStepHelper.GetOperationAndStageArguments(invocation, context.SemanticModel); + if (entity == null || operationExpr == null || stageExpr == null) + { + return; + } + + var operation = ResolveTupleValue(operationExpr, context.SemanticModel); + var stage = ResolveTupleValue(stageExpr, context.SemanticModel); + + // Skip registrations whose tuple can't be resolved to concrete values: the generator can't + // derive a stable name from them either, so keying them would risk false positives. + if (operation == null || stage == null) + { + return; + } + + var location = (RegisterStepHelper.GetGenericName(invocation) ?? (SyntaxNode)invocation).GetLocation(); + registrationsByType + .GetOrAdd(typeSymbol, _ => new ConcurrentBag()) + .Add(new Registration(entity, operation, stage, location)); + } + + private static void ReportDuplicates( + CompilationAnalysisContext context, + ConcurrentDictionary> registrationsByType) + { + foreach (var entry in registrationsByType) + { + var type = entry.Key; + + // Seed with registrations inherited from source-defined base plugins so a derived class that + // re-registers an inherited tuple is caught. Base-internal duplicates are reported when the + // base itself is evaluated, so they are not re-reported here. + var seen = new HashSet(); + for (var baseType = type.BaseType; baseType != null && !IsFrameworkPluginBase(baseType); baseType = baseType.BaseType) + { + if (registrationsByType.TryGetValue(baseType, out var baseRegistrations)) + { + foreach (var registration in baseRegistrations) + { + seen.Add(registration.Key); + } + } + } + + // Order for deterministic reporting (ConcurrentBag is unordered; declarations may span trees). + var ordered = entry.Value + .OrderBy(r => r.Location.SourceTree?.FilePath, System.StringComparer.Ordinal) + .ThenBy(r => r.Location.SourceSpan.Start); + + foreach (var registration in ordered) + { + if (!seen.Add(registration.Key)) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.DuplicateStepRegistration, + registration.Location, + type.Name, + registration.Entity, + registration.Operation, + registration.Stage)); + } + } + } + } + + /// + /// Resolves an operation/stage argument to its comparison value, matching what the runtime and + /// generated wrappers use (the string value for custom messages, value.ToString() for enums). + /// Constant evaluation drives this so it works uniformly for literals, const locals/fields, + /// and member-access references (EventOperation.Update, MyMessages.DoThing): + /// + /// a constant string (literal, const string, or a const string member) yields + /// that string value — so two members sharing a value compare equal; + /// an enum constant (member access or a const enum) is mapped from its underlying + /// integral value back to the enum member name. + /// + /// Returns null when the value can't be resolved to a concrete string. + /// + private static string ResolveTupleValue(ExpressionSyntax expression, SemanticModel semanticModel) + { + var constant = semanticModel.GetConstantValue(expression); + if (!constant.HasValue) + { + return null; + } + + // Custom message name: the runtime and generated wrapper naming key off the string value, not the + // member identifier, so a const string member (MyMessages.DoThing) resolves to its value. + if (constant.Value is string stringValue) + { + return stringValue; + } + + // Enum value: GetConstantValue gives the underlying integral value, so map it back to the member + // name to match EventOperation.Update.ToString(). + if (semanticModel.GetTypeInfo(expression).Type is INamedTypeSymbol enumType && + enumType.TypeKind == TypeKind.Enum) + { + foreach (var field in enumType.GetMembers().OfType()) + { + if (field.HasConstantValue && Equals(field.ConstantValue, constant.Value)) + { + return field.Name; + } + } + + // An undefined value (e.g. (EventOperation)999): Enum.ToString() returns the integral text, + // so use that rather than skipping the registration. + return constant.Value?.ToString(); + } + + return null; + } + + private static bool IsPluginClass(INamedTypeSymbol type) + { + for (var baseType = type.BaseType; baseType != null; baseType = baseType.BaseType) + { + if (IsFrameworkPluginBase(baseType)) + { + return true; + } + } + + return false; + } + + private static bool IsFrameworkPluginBase(INamedTypeSymbol type) => + type.Name == Constants.PluginBaseClassName && + type.ContainingNamespace?.ToDisplayString() == Constants.PluginNamespace; + + private readonly struct Registration + { + public Registration(string entity, string operation, string stage, Location location) + { + Entity = entity; + Operation = operation; + Stage = stage; + Location = location; + } + + public string Entity { get; } + public string Operation { get; } + public string Stage { get; } + public Location Location { get; } + public string Key => $"{Entity}|{Operation}|{Stage}"; + } +} diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs index 144be48..48be8f8 100644 --- a/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs +++ b/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs @@ -18,8 +18,6 @@ namespace XrmPluginCore.SourceGenerator.CodeFixes; [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(LocalPluginContextAsServiceCodeFixProvider)), Shared] public class LocalPluginContextAsServiceCodeFixProvider : CodeFixProvider { - private const string RegisterPluginStepMethodName = "RegisterPluginStep"; - public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(DiagnosticDescriptors.LocalPluginContextAsService.Id); @@ -82,7 +80,7 @@ private static async Task ReplaceWithRegisterPluginStepAsync( // Build new generic name: RegisterPluginStep var newGenericName = SyntaxFactory.GenericName( - SyntaxFactory.Identifier(RegisterPluginStepMethodName), + SyntaxFactory.Identifier(Constants.RegisterPluginStepMethodName), newTypeArgList); ExpressionSyntax newExpression; diff --git a/XrmPluginCore.SourceGenerator/Constants.cs b/XrmPluginCore.SourceGenerator/Constants.cs index 4f59878..1e1fd28 100644 --- a/XrmPluginCore.SourceGenerator/Constants.cs +++ b/XrmPluginCore.SourceGenerator/Constants.cs @@ -12,6 +12,7 @@ internal static class Constants // Method names public const string RegisterStepMethodName = "RegisterStep"; + public const string RegisterPluginStepMethodName = "RegisterPluginStep"; public const string WithPreImageMethodName = "WithPreImage"; public const string WithPostImageMethodName = "WithPostImage"; public const string AddImageMethodName = "AddImage"; diff --git a/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs b/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs index cb5aefe..4ab2afa 100644 --- a/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs +++ b/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs @@ -33,6 +33,17 @@ public static class DiagnosticDescriptors isEnabledByDefault: true, helpLinkUri: $"{HelpLinkBaseUri}/XPC2001.md"); + public static readonly DiagnosticDescriptor DuplicateStepRegistration = new( + id: "XPC2002", + title: "Duplicate plugin step registration", + messageFormat: "Plugin '{0}' registers {1}/{2}/{3} more than once. Each entity/operation/stage combination may be registered at most once per plugin.", + category: Category, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "A plugin may register a given entity/operation/stage combination only once; the platform keys steps by that combination. A duplicate is rejected at registration time and would also make the source-generated image/wrapper type names ambiguous.", + helpLinkUri: $"{HelpLinkBaseUri}/XPC2002.md", + customTags: WellKnownDiagnosticTags.CompilationEnd); + public static readonly DiagnosticDescriptor PreferNameofOverStringLiteral = new( id: "XPC3001", title: "Prefer nameof over string literal for handler method", diff --git a/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs b/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs index 135c36c..e4ecdf0 100644 --- a/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs +++ b/XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs @@ -131,6 +131,19 @@ public static bool IsRegisterStepCall(InvocationExpressionSyntax invocation, out return false; } + /// + /// Checks if an invocation is a RegisterStep or legacy RegisterPluginStep call and + /// extracts the generic name. Both identify a plugin step whose (entity, operation, stage) tuple + /// must be unique within a plugin. + /// + public static bool IsStepRegistrationCall(InvocationExpressionSyntax invocation, out GenericNameSyntax genericName) + { + genericName = GetGenericName(invocation); + return genericName != null && + (genericName.Identifier.Text == Constants.RegisterStepMethodName || + genericName.Identifier.Text == Constants.RegisterPluginStepMethodName); + } + /// /// 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 diff --git a/XrmPluginCore.SourceGenerator/rules/XPC2002.md b/XrmPluginCore.SourceGenerator/rules/XPC2002.md new file mode 100644 index 0000000..b5db7d4 --- /dev/null +++ b/XrmPluginCore.SourceGenerator/rules/XPC2002.md @@ -0,0 +1,56 @@ +# XPC2002: Duplicate plugin step registration + +## Severity + +Error + +## Description + +A plugin may register a given **entity / operation / stage** combination at most once. The platform +keys plugin steps by that combination, so a duplicate is rejected at registration time. It also makes +the source-generated type-safe image and wrapper types ambiguous, because their names are derived from +`{PluginClass}{Entity}{Operation}{Stage}`. + +This applies to both `RegisterStep` and the legacy `RegisterPluginStep`. + +## ❌ Example of violation + +```csharp +public class AccountPlugin : Plugin +{ + public AccountPlugin() + { + RegisterStep( + EventOperation.Update, ExecutionStage.PostOperation, nameof(IAccountService.HandleA)) + .WithPreImage(x => x.Name); + + // XPC2002: Account/Update/PostOperation is already registered above + RegisterStep( + EventOperation.Update, ExecutionStage.PostOperation, nameof(IAccountService.HandleB)) + .WithPreImage(x => x.Revenue); + } +} +``` + +## ✅ How to fix + +Collapse the work into a single registration per entity/operation/stage, and let one handler do it: + +```csharp +public class AccountPlugin : Plugin +{ + public AccountPlugin() + { + RegisterStep( + EventOperation.Update, ExecutionStage.PostOperation, nameof(IAccountService.HandleUpdate)) + .WithPreImage(x => x.Name, x => x.Revenue); + } +} +``` + +If the two pieces of work are genuinely distinct, use a different stage (e.g. `PreOperation` vs +`PostOperation`) or a different message, so each registration has a unique combination. + +## See also + +- [XPC2001: No parameterless constructor found](XPC2001.md) diff --git a/XrmPluginCore/CHANGELOG.md b/XrmPluginCore/CHANGELOG.md index 5e0031b..f2381a1 100644 --- a/XrmPluginCore/CHANGELOG.md +++ b/XrmPluginCore/CHANGELOG.md @@ -1,4 +1,5 @@ -### v1.4.2 - Unreleased +### v1.5.0 - Unreleased +* 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.