diff --git a/README.md b/README.md index 9609b80..caabcfd 100644 --- a/README.md +++ b/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. diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs index eb0068f..e177fae 100644 --- a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs +++ b/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(EventOperation.Update, ExecutionStage.PostOperation, + nameof(ITestService.DoSomething)) + .AddImage(ImageType.PreImage, x => x.Name); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + { + return services.AddScoped(); + } + } + + 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(EventOperation.Update, ExecutionStage.PostOperation, + s => s.DoSomething) + .AddImage(ImageType.PreImage, x => x.Name); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + { + return services.AddScoped(); + } + } + + 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> GetAnalyzerDiagnosticsAsync(string source, DiagnosticAnalyzer analyzer) diff --git a/XrmPluginCore.SourceGenerator/Analyzers/ImageWithoutMethodReferenceAnalyzer.cs b/XrmPluginCore.SourceGenerator/Analyzers/ImageWithoutMethodReferenceAnalyzer.cs index 77d6d36..cf46ec0 100644 --- a/XrmPluginCore.SourceGenerator/Analyzers/ImageWithoutMethodReferenceAnalyzer.cs +++ b/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; /// -/// 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) /// [DiagnosticAnalyzer(LanguageNames.CSharp)] public class ImageWithoutMethodReferenceAnalyzer : DiagnosticAnalyzer { - private enum ImageRegistrationType - { - None, - Modern, // WithPreImage, WithPostImage - Legacy // AddImage - } - public override ImmutableArray 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(); - 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(); + 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) + /// + /// Checks the call chain for image registrations. + /// Returns whether modern API is used and the locations of all AddImage calls. + /// + private static (bool hasModernApi, List addImageLocations) GetImageRegistrationInfo( + InvocationExpressionSyntax registerStepInvocation) { + var hasModernApi = false; + var addImageLocations = new List(); + // Walk up to find the full fluent call chain var current = registerStepInvocation.Parent; - var result = ImageRegistrationType.None; while (current != null) { @@ -140,14 +139,12 @@ 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()); } } @@ -155,6 +152,6 @@ private static ImageRegistrationType GetImageRegistrationType(InvocationExpressi current = current.Parent; } - return result; + return (hasModernApi, addImageLocations); } } diff --git a/XrmPluginCore.SourceGenerator/rules/XPC3002.md b/XrmPluginCore.SourceGenerator/rules/XPC3002.md index a00b64a..1226472 100644 --- a/XrmPluginCore.SourceGenerator/rules/XPC3002.md +++ b/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( + 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( 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