Refactor image registration analyzer and expand XPC3002 (#4)

* Refactor image registration analyzer and expand XPC3002

Refactored the image registration analyzer to always report XPC3002 for any AddImage usage, regardless of handler syntax (nameof, method reference, or lambda).
XPC3003 is now only reported for lambda invocation with the modern API.

Added comprehensive unit tests for all scenarios and updated XPC3002 documentation to reflect the broader rule scope
 Removed obsolete enum and clarified analyzer logic and comments.

Author: mkholt

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