Add XPC2002: duplicate plugin step registration (error)

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

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;
+
+/// <summary>
+/// Reports when a plugin class registers the same entity/operation/stage combination more than once
+/// (via <c>RegisterStep</c> or the legacy <c>RegisterPluginStep</c>). 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 <c>partial</c> plugins are
+/// aggregated) and duplicates are evaluated at compilation end, where inherited base-plugin
+/// registrations are also considered. Only types deriving from <c>XrmPluginCore.Plugin</c> are analyzed.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class DuplicateStepRegistrationAnalyzer : DiagnosticAnalyzer
+{
+	public override ImmutableArray<DiagnosticDescriptor> 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<INamedTypeSymbol, ConcurrentBag<Registration>>(SymbolEqualityComparer.Default);
+
+		context.RegisterSyntaxNodeAction(
+			nodeContext => CollectRegistration(nodeContext, registrationsByType),
+			SyntaxKind.InvocationExpression);
+
+		context.RegisterCompilationEndAction(endContext => ReportDuplicates(endContext, registrationsByType));
+	}
+
+	private static void CollectRegistration(
+		SyntaxNodeAnalysisContext context,
+		ConcurrentDictionary<INamedTypeSymbol, ConcurrentBag<Registration>> registrationsByType)
+	{
+		var invocation = (InvocationExpressionSyntax)context.Node;
+
+		if (!RegisterStepHelper.IsStepRegistrationCall(invocation, out var genericName) ||
+			genericName.TypeArgumentList.Arguments.Count < 1)
+		{
+			return;
+		}
+
+		var classDeclaration = invocation.FirstAncestorOrSelf<ClassDeclarationSyntax>();
+		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<Registration>())
+			.Add(new Registration(entity, operation, stage, location));
+	}
+
+	private static void ReportDuplicates(
+		CompilationAnalysisContext context,
+		ConcurrentDictionary<INamedTypeSymbol, ConcurrentBag<Registration>> 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<string>();
+			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));
+				}
+			}
+		}
+	}
+
+	/// <summary>
+	/// Resolves an operation/stage argument to its comparison value, matching what the runtime and
+	/// generated wrappers use (the string value for custom messages, <c>value.ToString()</c> for enums).
+	/// Constant evaluation drives this so it works uniformly for literals, <c>const</c> locals/fields,
+	/// and member-access references (<c>EventOperation.Update</c>, <c>MyMessages.DoThing</c>):
+	/// <list type="bullet">
+	/// <item>a constant string (literal, <c>const string</c>, or a <c>const string</c> member) yields
+	/// that string value — so two members sharing a value compare equal;</item>
+	/// <item>an enum constant (member access or a <c>const</c> enum) is mapped from its underlying
+	/// integral value back to the enum member name.</item>
+	/// </list>
+	/// Returns null when the value can't be resolved to a concrete string.
+	/// </summary>
+	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<IFieldSymbol>())
+			{
+				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}";
+	}
+}

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<string> FixableDiagnosticIds =>
 		ImmutableArray.Create(DiagnosticDescriptors.LocalPluginContextAsService.Id);
 
@@ -82,7 +80,7 @@ private static async Task<Document> ReplaceWithRegisterPluginStepAsync(
 
 		// Build new generic name: RegisterPluginStep<TEntity>
 		var newGenericName = SyntaxFactory.GenericName(
-			SyntaxFactory.Identifier(RegisterPluginStepMethodName),
+			SyntaxFactory.Identifier(Constants.RegisterPluginStepMethodName),
 			newTypeArgList);
 
 		ExpressionSyntax newExpression;

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";

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",

XrmPluginCore.SourceGenerator/Helpers/RegisterStepHelper.cs

@@ -131,6 +131,19 @@ public static bool IsRegisterStepCall(InvocationExpressionSyntax invocation, out
 		return false;
 	}
 
+	/// <summary>
+	/// Checks if an invocation is a <c>RegisterStep</c> or legacy <c>RegisterPluginStep</c> call and
+	/// extracts the generic name. Both identify a plugin step whose (entity, operation, stage) tuple
+	/// must be unique within a plugin.
+	/// </summary>
+	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);
+	}
+
 	/// <summary>
 	/// 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

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<Account, IAccountService>(
+            EventOperation.Update, ExecutionStage.PostOperation, nameof(IAccountService.HandleA))
+            .WithPreImage(x => x.Name);
+
+        // XPC2002: Account/Update/PostOperation is already registered above
+        RegisterStep<Account, IAccountService>(
+            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<Account, IAccountService>(
+            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)

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<TService>(name, Action<TService>)` overload, it now suggests the type-safe `RegisterAPI<TService>(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<My.Namespace.Service>`), instead of emitting a malformed single identifier.