Refactor image registration analyzer and expand XPC3002

README.md

@@ -190,6 +190,20 @@ public string AccountNumber => Entity.AccountNumber;
 
 **Note:** This is optional. Without it, wrapper properties work normally but won't have XML documentation tooltips.
 
+#### Analyzer Diagnostics
+
+The source generator includes analyzers that help catch common issues at compile time:
+
+| ID | Severity | Title |
+|----|----------|-------|
+| [XPC2001](XrmPluginCore.SourceGenerator/rules/XPC2001.md) | Warning | No parameterless constructor found |
+| [XPC3001](XrmPluginCore.SourceGenerator/rules/XPC3001.md) | Warning | Prefer nameof over string literal for handler method |
+| [XPC3002](XrmPluginCore.SourceGenerator/rules/XPC3002.md) | Info | Consider using modern image registration API |
+| [XPC3003](XrmPluginCore.SourceGenerator/rules/XPC3003.md) | Warning | Image registration without method reference |
+| [XPC4001](XrmPluginCore.SourceGenerator/rules/XPC4001.md) | Error | Handler method not found |
+| [XPC4002](XrmPluginCore.SourceGenerator/rules/XPC4002.md) | Warning | Handler signature does not match registered images |
+| [XPC4003](XrmPluginCore.SourceGenerator/rules/XPC4003.md) | Error | Handler signature does not match registered images |
+
 ### Using the LocalPluginContext wrapper (Legacy)
 
 **NOTE**: This is only supported for legacy DAXIF/XrmFramework style plugins. It is recommended to use dependency injection based plugins instead.

XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs

@@ -658,9 +658,10 @@ public void DoSomething() { }
 	}
 
 	[Fact]
-	public async Task Should_Report_XPC3003_Not_XPC3002_When_WithPreImage_Used_Even_With_AddImage()
+	public async Task Should_Report_Both_XPC3002_And_XPC3003_When_AddImage_And_WithPreImage_Used_With_Invocation()
 	{
-		// Arrange - Both WithPreImage (modern) and AddImage (legacy) used - should report XPC3003 since modern takes precedence
+		// Arrange - Both WithPreImage (modern) and AddImage (legacy) used with lambda invocation
+		// Should report both: XPC3002 for AddImage, XPC3003 for lambda invocation with modern API
 		const string pluginSource = """
 
 			using XrmPluginCore;
@@ -703,7 +704,7 @@ public void DoSomething() { }
 		// Act - Run analyzer instead of generator
 		var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new ImageWithoutMethodReferenceAnalyzer());
 
-		// Assert - Should report XPC3003 (modern API takes precedence)
+		// Assert - Should report both diagnostics
 		var xpc3003Diagnostics = diagnostics
 			.Where(d => d.Id == "XPC3003")
 			.ToArray();
@@ -712,8 +713,126 @@ public void DoSomething() { }
 			.Where(d => d.Id == "XPC3002")
 			.ToArray();
 
-		xpc3003Diagnostics.Should().NotBeEmpty("XPC3003 should be reported when modern API (WithPreImage) is used");
-		xpc3002Diagnostics.Should().BeEmpty("XPC3002 should NOT be reported when modern API is also present");
+		xpc3003Diagnostics.Should().NotBeEmpty("XPC3003 should be reported when modern API is used with lambda invocation");
+		xpc3002Diagnostics.Should().NotBeEmpty("XPC3002 should be reported for AddImage usage");
+	}
+
+	[Fact]
+	public async Task Should_Report_XPC3002_When_AddImage_Used_With_Nameof()
+	{
+		// Arrange - AddImage used with nameof() - should still suggest migration to modern API
+		const string pluginSource = """
+
+			using XrmPluginCore;
+			using XrmPluginCore.Enums;
+			using Microsoft.Extensions.DependencyInjection;
+			using TestNamespace;
+
+			namespace TestNamespace
+			{
+			    public class TestPlugin : Plugin
+			    {
+			        public TestPlugin()
+			        {
+			            RegisterStep<Account, ITestService>(EventOperation.Update, ExecutionStage.PostOperation,
+			                nameof(ITestService.DoSomething))
+			                .AddImage(ImageType.PreImage, x => x.Name);
+			        }
+
+			        protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services)
+			        {
+			            return services.AddScoped<ITestService, TestService>();
+			        }
+			    }
+
+			    public interface ITestService
+			    {
+			        void DoSomething();
+			    }
+
+			    public class TestService : ITestService
+			    {
+			        public void DoSomething() { }
+			    }
+			}
+			""";
+
+		var source = TestFixtures.GetCompleteSource(pluginSource);
+
+		// Act - Run analyzer instead of generator
+		var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new ImageWithoutMethodReferenceAnalyzer());
+
+		// Assert - Should report XPC3002 for AddImage, but NOT XPC3003 (no lambda invocation)
+		var xpc3002Diagnostics = diagnostics
+			.Where(d => d.Id == "XPC3002")
+			.ToArray();
+
+		var xpc3003Diagnostics = diagnostics
+			.Where(d => d.Id == "XPC3003")
+			.ToArray();
+
+		xpc3002Diagnostics.Should().NotBeEmpty("XPC3002 should be reported when AddImage is used, even with nameof()");
+		xpc3002Diagnostics.Should().OnlyContain(d => d.Severity == DiagnosticSeverity.Info);
+		xpc3003Diagnostics.Should().BeEmpty("XPC3003 should NOT be reported when using nameof()");
+	}
+
+	[Fact]
+	public async Task Should_Report_XPC3002_When_AddImage_Used_With_MethodReference()
+	{
+		// Arrange - AddImage used with method reference syntax (s => s.DoSomething without parentheses)
+		const string pluginSource = """
+
+			using XrmPluginCore;
+			using XrmPluginCore.Enums;
+			using Microsoft.Extensions.DependencyInjection;
+			using TestNamespace;
+
+			namespace TestNamespace
+			{
+			    public class TestPlugin : Plugin
+			    {
+			        public TestPlugin()
+			        {
+			            RegisterStep<Account, ITestService>(EventOperation.Update, ExecutionStage.PostOperation,
+			                s => s.DoSomething)
+			                .AddImage(ImageType.PreImage, x => x.Name);
+			        }
+
+			        protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services)
+			        {
+			            return services.AddScoped<ITestService, TestService>();
+			        }
+			    }
+
+			    public interface ITestService
+			    {
+			        void DoSomething();
+			    }
+
+			    public class TestService : ITestService
+			    {
+			        public void DoSomething() { }
+			    }
+			}
+			""";
+
+		var source = TestFixtures.GetCompleteSource(pluginSource);
+
+		// Act - Run analyzer instead of generator
+		var diagnostics = await GetAnalyzerDiagnosticsAsync(source, new ImageWithoutMethodReferenceAnalyzer());
+
+		// Assert - Should report XPC3002 for AddImage, but NOT XPC3003 (method reference, not invocation)
+		var xpc3002Diagnostics = diagnostics
+			.Where(d => d.Id == "XPC3002")
+			.ToArray();
+
+		var xpc3003Diagnostics = diagnostics
+			.Where(d => d.Id == "XPC3003")
+			.ToArray();
+
+		xpc3002Diagnostics.Should().NotBeEmpty("XPC3002 should be reported when AddImage is used, even with method reference");
+		xpc3002Diagnostics.Should().OnlyContain(d => d.Severity == DiagnosticSeverity.Info);
+		xpc3003Diagnostics.Should().BeEmpty("XPC3003 should NOT be reported when using method reference syntax");
 	}
 
 	private static async Task<ImmutableArray<Diagnostic>> GetAnalyzerDiagnosticsAsync(string source, DiagnosticAnalyzer analyzer)

XrmPluginCore.SourceGenerator/Analyzers/ImageWithoutMethodReferenceAnalyzer.cs

@@ -2,25 +2,20 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using XrmPluginCore.SourceGenerator.Helpers;
 
 namespace XrmPluginCore.SourceGenerator.Analyzers;
 
 /// <summary>
-/// Analyzer that warns when lambda invocation syntax (s => s.Method()) is used with image registrations
-/// instead of method reference syntax (nameof(Service.Method)).
+/// Analyzer that reports:
+/// - XPC3002: When AddImage is used (suggesting migration to WithPreImage/WithPostImage)
+/// - XPC3003: When lambda invocation syntax (s => s.Method()) is used with modern API (WithPreImage/WithPostImage)
 /// </summary>
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 public class ImageWithoutMethodReferenceAnalyzer : DiagnosticAnalyzer
 {
-	private enum ImageRegistrationType
-	{
-		None,
-		Modern,  // WithPreImage, WithPostImage
-		Legacy   // AddImage
-	}
-
 	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
 		ImmutableArray.Create(
 			DiagnosticDescriptors.ImageWithoutMethodReference,
@@ -58,39 +53,36 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
 
 		var handlerArgument = arguments[2].Expression;
 
-		// Check if the 3rd argument is a lambda with an invocation body (s => s.Method())
-		if (!IsLambdaWithInvocation(handlerArgument, out var methodName, out var hasArguments))
-		{
-			return;
-		}
+		// Get image registration info
+		var (hasModernApi, addImageLocations) = GetImageRegistrationInfo(invocation);
 
-		// Check if the call chain has image registration and which type
-		var imageType = GetImageRegistrationType(invocation);
-		if (imageType == ImageRegistrationType.None)
+		// XPC3002: Report for any AddImage usage (suggesting migration to modern API)
+		foreach (var addImageLocation in addImageLocations)
 		{
-			return;
-		}
+			var diagnostic = Diagnostic.Create(
+				DiagnosticDescriptors.LegacyImageRegistration,
+				addImageLocation);
 
-		// Get the service type name (TService)
-		var serviceType = genericName.TypeArgumentList.Arguments[1].ToString();
+			context.ReportDiagnostic(diagnostic);
+		}
 
-		// Create diagnostic properties for the code fix
-		var properties = ImmutableDictionary.CreateBuilder<string, string>();
-		properties.Add("ServiceType", serviceType);
-		properties.Add("MethodName", methodName);
-		properties.Add("HasArguments", hasArguments.ToString());
+		// XPC3003: Report when modern API is used with lambda invocation syntax
+		if (hasModernApi && IsLambdaWithInvocation(handlerArgument, out var methodName, out var hasArguments))
+		{
+			var serviceType = genericName.TypeArgumentList.Arguments[1].ToString();
 
-		// Select appropriate diagnostic based on API type
-		var descriptor = imageType == ImageRegistrationType.Modern
-			? DiagnosticDescriptors.ImageWithoutMethodReference
-			: DiagnosticDescriptors.LegacyImageRegistration;
+			var properties = ImmutableDictionary.CreateBuilder<string, string>();
+			properties.Add("ServiceType", serviceType);
+			properties.Add("MethodName", methodName ?? string.Empty);
+			properties.Add("HasArguments", hasArguments.ToString());
 
-		var diagnostic = Diagnostic.Create(
-			descriptor,
-			handlerArgument.GetLocation(),
-			properties.ToImmutable());
+			var diagnostic = Diagnostic.Create(
+				DiagnosticDescriptors.ImageWithoutMethodReference,
+				handlerArgument.GetLocation(),
+				properties.ToImmutable());
 
-		context.ReportDiagnostic(diagnostic);
+			context.ReportDiagnostic(diagnostic);
+		}
 	}
 
 	private static bool IsLambdaWithInvocation(ExpressionSyntax expression, out string methodName, out bool hasArguments)
@@ -125,11 +117,18 @@ private static bool IsLambdaWithInvocation(ExpressionSyntax expression, out stri
 		return false;
 	}
 
-	private static ImageRegistrationType GetImageRegistrationType(InvocationExpressionSyntax registerStepInvocation)
+	/// <summary>
+	/// Checks the call chain for image registrations.
+	/// Returns whether modern API is used and the locations of all AddImage calls.
+	/// </summary>
+	private static (bool hasModernApi, List<Location> addImageLocations) GetImageRegistrationInfo(
+		InvocationExpressionSyntax registerStepInvocation)
 	{
+		var hasModernApi = false;
+		var addImageLocations = new List<Location>();
+
 		// Walk up to find the full fluent call chain
 		var current = registerStepInvocation.Parent;
-		var result = ImageRegistrationType.None;
 
 		while (current != null)
 		{
@@ -140,21 +139,19 @@ private static ImageRegistrationType GetImageRegistrationType(InvocationExpressi
 				if (methodName == Constants.WithPreImageMethodName ||
 					methodName == Constants.WithPostImageMethodName)
 				{
-					// Modern API takes precedence
-					return ImageRegistrationType.Modern;
+					hasModernApi = true;
 				}
-
-				if (methodName == Constants.AddImageMethodName)
+				else if (methodName == Constants.AddImageMethodName)
 				{
-					// Track legacy, but keep looking for modern
-					result = ImageRegistrationType.Legacy;
+					// Collect the location of the AddImage identifier for the diagnostic
+					addImageLocations.Add(memberAccess.Name.GetLocation());
 				}
 			}
 
 			// Move up the syntax tree
 			current = current.Parent;
 		}
 
-		return result;
+		return (hasModernApi, addImageLocations);
 	}
 }

XrmPluginCore.SourceGenerator/rules/XPC3002.md

@@ -6,16 +6,35 @@ Info (Suggestion)
 
 ## Description
 
-This rule reports when `AddImage()` is used with a lambda invocation expression (e.g., `s => s.HandleUpdate()`). While this is valid code, the modern `WithPreImage()` and `WithPostImage()` APIs with `nameof()` provide better type-safety and compile-time validation.
+This rule reports when `AddImage()` is used for image registration. While `AddImage()` is fully functional (especially when combined with `nameof()`), the modern `WithPreImage()` and `WithPostImage()` APIs provide a cleaner syntax and express intent more clearly.
 
-## ❌ Example of code that triggers this suggestion
+## ❌ Examples of code that triggers this suggestion
+
+### Using AddImage with nameof()
+
+```csharp
+public class AccountPlugin : Plugin
+{
+    public AccountPlugin()
+    {
+        // XPC3002 reported on AddImage
+        RegisterStep<Account, IAccountService>(
+            EventOperation.Update,
+            ExecutionStage.PostOperation,
+            nameof(IAccountService.HandleUpdate))
+            .AddImage(ImageType.PreImage, x => x.Name);
+    }
+}
+```
+
+### Using AddImage with lambda invocation
 
 ```csharp
 public class AccountPlugin : Plugin
 {
     public AccountPlugin()
     {
-        // XPC3002: Consider using modern image registration API
+        // XPC3002 reported on AddImage
         RegisterStep<Account, IAccountService>(
             EventOperation.Update,
             ExecutionStage.PostOperation,
@@ -27,7 +46,7 @@ public class AccountPlugin : Plugin
 
 ## ✅ How to fix
 
-Convert to the modern `WithPreImage()`/`WithPostImage()` API with `nameof()`:
+Convert `AddImage(ImageType.PreImage, ...)` to `WithPreImage(...)` and `AddImage(ImageType.PostImage, ...)` to `WithPostImage(...)`:
 
 ```csharp
 public class AccountPlugin : Plugin
@@ -45,18 +64,17 @@ public class AccountPlugin : Plugin
 
 ## Why this matters
 
-1. **Type-safe wrapper generation**: Using `WithPreImage()`/`WithPostImage()` with `nameof()` enables the source generator to create strongly-typed `PreImage` and `PostImage` wrapper classes. These provide IntelliSense support and compile-time safety.
+1. **Cleaner API**: `WithPreImage()` and `WithPostImage()` are more readable and express intent more clearly than the generic `AddImage(ImageType.PreImage/PostImage, ...)` pattern.
 
-2. **Signature validation**: The modern API enables compile-time validation that your handler method signature matches the registered images (XPC4002/XPC4003).
+2. **Type-safe wrapper generation**: Both APIs support type-safe wrapper generation when used with `nameof()` or method reference syntax.
 
-3. **Cleaner API**: `WithPreImage()` and `WithPostImage()` are more readable and express intent more clearly than the generic `AddImage()` method.
+3. **Consistency**: Using the modern API ensures consistency across your codebase.
 
 ## When to suppress
 
 This is an informational suggestion. You may choose to keep using `AddImage()` if:
 - You're maintaining legacy code and don't want to refactor
-- You intentionally don't need the type-safe wrappers
-- You're passing arguments to the handler method that would be lost in conversion
+- You prefer the explicit `ImageType` parameter
 
 ## See also