diff --git a/.claude/settings.local.json b/.claude/settings.json similarity index 100% rename from .claude/settings.local.json rename to .claude/settings.json diff --git a/.gitignore b/.gitignore index a4fe18b..e1a1752 100644 --- a/.gitignore +++ b/.gitignore @@ -398,3 +398,6 @@ FodyWeavers.xsd # JetBrains Rider *.sln.iml + +# Claude local settings +.claude/*.local.json diff --git a/README.md b/README.md index caabcfd..b16b516 100644 --- a/README.md +++ b/README.md @@ -200,6 +200,7 @@ The source generator includes analyzers that help catch common issues at compile | [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 | +| [XPC3004](XrmPluginCore.SourceGenerator/rules/XPC3004.md) | Error | Do not use LocalPluginContext as TService in RegisterStep | | [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 | diff --git a/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/LocalPluginContextAsServiceAnalyzerTests.cs b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/LocalPluginContextAsServiceAnalyzerTests.cs new file mode 100644 index 0000000..5ee43db --- /dev/null +++ b/XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/LocalPluginContextAsServiceAnalyzerTests.cs @@ -0,0 +1,277 @@ +using FluentAssertions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using XrmPluginCore.SourceGenerator.Analyzers; +using XrmPluginCore.SourceGenerator.CodeFixes; +using XrmPluginCore.SourceGenerator.Tests.Helpers; +using Xunit; + +namespace XrmPluginCore.SourceGenerator.Tests.DiagnosticTests; + +/// +/// Tests for LocalPluginContextAsServiceAnalyzer that errors when LocalPluginContext is used as TService in RegisterStep. +/// +public class LocalPluginContextAsServiceAnalyzerTests : CodeFixTestBase +{ + [Fact] + public async Task Should_Report_XPC3004_When_LocalPluginContext_Explicitly_Specified() + { + // Arrange + 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, + Execute); + } + + private void Execute(LocalPluginContext context) { } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + => services; + } +} +"""; + + var source = TestFixtures.GetCompleteSource(pluginSource); + var diagnostics = await GetDiagnosticsAsync(source); + + // Assert + diagnostics.Should().ContainSingle(d => d.Id == "XPC3004"); + var diagnostic = diagnostics.Single(d => d.Id == "XPC3004"); + diagnostic.Severity.Should().Be(DiagnosticSeverity.Error); + diagnostic.GetMessage().Should().Contain("Contact"); + diagnostic.GetMessage().Should().Contain("LocalPluginContext"); + } + + [Fact] + public async Task Should_Report_XPC3004_When_LocalPluginContext_Used_As_TService_With_Lambda() + { + // Arrange + 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, + ctx => ctx.TracingService.Trace("hello")); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + => services; + } +} +"""; + + var source = TestFixtures.GetCompleteSource(pluginSource); + var diagnostics = await GetDiagnosticsAsync(source); + + // Assert + diagnostics.Should().ContainSingle(d => d.Id == "XPC3004"); + } + + [Fact] + public async Task Should_Not_Report_XPC3004_When_DI_Service_Used() + { + // Arrange + 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.HandleUpdate)); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + => services.AddScoped(); + } + + public interface ITestService + { + void HandleUpdate(); + } + + public class TestService : ITestService + { + public void HandleUpdate() { } + } +} +"""; + + var source = TestFixtures.GetCompleteSource(pluginSource); + var diagnostics = await GetDiagnosticsAsync(source); + + // Assert + diagnostics.Should().NotContain(d => d.Id == "XPC3004"); + } + + [Fact] + public async Task Should_Not_Report_XPC3004_For_SingleTypeParam_Overload() + { + // Arrange — RegisterStep with a single type arg uses Action + 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, + sp => sp.GetRequiredService()); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + => services; + } +} +"""; + + var source = TestFixtures.GetCompleteSource(pluginSource); + var diagnostics = await GetDiagnosticsAsync(source); + + // Assert + diagnostics.Should().NotContain(d => d.Id == "XPC3004"); + } + + [Fact] + public async Task CodeFix_Should_Rewrite_To_RegisterPluginStep() + { + // Arrange + 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, + Execute); + } + + private void Execute(LocalPluginContext context) { } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + => services; + } +} +"""; + + var source = TestFixtures.GetCompleteSource(pluginSource); + + // Act + var fixedSource = await ApplyCodeFixAsync( + source, + new LocalPluginContextAsServiceAnalyzer(), + new LocalPluginContextAsServiceCodeFixProvider(), + DiagnosticDescriptors.LocalPluginContextAsService.Id); + + // Assert + fixedSource.Should().Contain("RegisterPluginStep"); + fixedSource.Should().NotContain("RegisterStep"); + } + + [Fact] + public async Task CodeFix_Should_Have_Correct_Title() + { + // Arrange + 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, + Execute); + } + + private void Execute(LocalPluginContext context) { } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + => services; + } +} +"""; + + var source = TestFixtures.GetCompleteSource(pluginSource); + + // Act + var codeActions = await GetCodeActionsAsync( + source, + new LocalPluginContextAsServiceAnalyzer(), + new LocalPluginContextAsServiceCodeFixProvider(), + DiagnosticDescriptors.LocalPluginContextAsService.Id); + + // Assert + codeActions.Should().ContainSingle(); + codeActions[0].Title.Should().Be("Use RegisterPluginStep instead"); + } + + private static async Task> GetDiagnosticsAsync(string source) + { + var compilation = CompilationHelper.CreateCompilation(source); + var analyzer = new LocalPluginContextAsServiceAnalyzer(); + + var compilationWithAnalyzers = compilation.WithAnalyzers( + [analyzer]); + + return await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(); + } +} diff --git a/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md b/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md index 6213dee..5ce08df 100644 --- a/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md +++ b/XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md @@ -2,6 +2,7 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- +XPC3004 | XrmPluginCore.SourceGenerator | Error | Do not use LocalPluginContext as TService in RegisterStep ### Removed Rules diff --git a/XrmPluginCore.SourceGenerator/Analyzers/LocalPluginContextAsServiceAnalyzer.cs b/XrmPluginCore.SourceGenerator/Analyzers/LocalPluginContextAsServiceAnalyzer.cs new file mode 100644 index 0000000..c5b4c7f --- /dev/null +++ b/XrmPluginCore.SourceGenerator/Analyzers/LocalPluginContextAsServiceAnalyzer.cs @@ -0,0 +1,59 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; +using XrmPluginCore.SourceGenerator.Helpers; + +namespace XrmPluginCore.SourceGenerator.Analyzers; + +/// +/// Analyzer that reports an error when LocalPluginContext is used as TService in RegisterStep calls. +/// This causes a runtime exception because LocalPluginContext is not registered in the DI container. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class LocalPluginContextAsServiceAnalyzer : DiagnosticAnalyzer +{ + private const string LocalPluginContextFullName = "XrmPluginCore.LocalPluginContext"; + + public override ImmutableArray SupportedDiagnostics => + ImmutableArray.Create(DiagnosticDescriptors.LocalPluginContextAsService); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression); + } + + private void AnalyzeInvocation(SyntaxNodeAnalysisContext context) + { + var invocation = (InvocationExpressionSyntax)context.Node; + + if (!RegisterStepHelper.IsRegisterStepCall(invocation, out var genericName)) + { + return; + } + + // Only fire for exactly 2 type args: RegisterStep + if (genericName.TypeArgumentList.Arguments.Count != 2) + { + return; + } + + // Use semantic model to check full type name (avoids false positives on user-defined LocalPluginContext) + var serviceTypeArg = genericName.TypeArgumentList.Arguments[1]; + var typeInfo = context.SemanticModel.GetTypeInfo(serviceTypeArg); + if (typeInfo.Type?.ToDisplayString() != LocalPluginContextFullName) + { + return; + } + + var entityTypeName = genericName.TypeArgumentList.Arguments[0].ToString(); + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.LocalPluginContextAsService, + invocation.GetLocation(), + entityTypeName)); + } +} diff --git a/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs b/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs new file mode 100644 index 0000000..144be48 --- /dev/null +++ b/XrmPluginCore.SourceGenerator/CodeFixes/LocalPluginContextAsServiceCodeFixProvider.cs @@ -0,0 +1,102 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using XrmPluginCore.SourceGenerator.Helpers; + +namespace XrmPluginCore.SourceGenerator.CodeFixes; + +/// +/// Code fix provider that rewrites RegisterStep<TEntity, LocalPluginContext> to RegisterPluginStep<TEntity>. +/// +[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); + + public sealed override FixAllProvider GetFixAllProvider() => + WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root == null) + { + return; + } + + var diagnostic = context.Diagnostics.First(); + var diagnosticSpan = diagnostic.Location.SourceSpan; + var invocationNode = root.FindNode(diagnosticSpan).FirstAncestorOrSelf(); + + if (invocationNode == null) + { + return; + } + + if (!RegisterStepHelper.IsRegisterStepCall(invocationNode, out var genericName)) + { + return; + } + + var entityTypeName = genericName.TypeArgumentList.Arguments[0].ToString(); + + context.RegisterCodeFix( + CodeAction.Create( + title: $"Use RegisterPluginStep<{entityTypeName}> instead", + createChangedDocument: c => ReplaceWithRegisterPluginStepAsync(context.Document, invocationNode, c), + equivalenceKey: nameof(LocalPluginContextAsServiceCodeFixProvider)), + diagnostic); + } + + private static async Task ReplaceWithRegisterPluginStepAsync( + Document document, + InvocationExpressionSyntax invocationNode, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root == null) + { + return document; + } + + if (!RegisterStepHelper.IsRegisterStepCall(invocationNode, out var genericName)) + { + return document; + } + + // Build new type argument list with only the first type arg (TEntity) + var entityTypeArg = genericName.TypeArgumentList.Arguments[0] + .WithoutTrivia(); + var newTypeArgList = SyntaxFactory.TypeArgumentList( + SyntaxFactory.SingletonSeparatedList(entityTypeArg)); + + // Build new generic name: RegisterPluginStep + var newGenericName = SyntaxFactory.GenericName( + SyntaxFactory.Identifier(RegisterPluginStepMethodName), + newTypeArgList); + + ExpressionSyntax newExpression; + if (invocationNode.Expression is MemberAccessExpressionSyntax memberAccess) + { + newExpression = memberAccess.WithName(newGenericName); + } + else + { + newExpression = newGenericName; + } + + var newInvocation = invocationNode.WithExpression(newExpression); + var newRoot = root.ReplaceNode(invocationNode, newInvocation); + return document.WithSyntaxRoot(newRoot); + } +} diff --git a/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs b/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs index 0805420..9b820da 100644 --- a/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs +++ b/XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs @@ -61,6 +61,16 @@ public static class DiagnosticDescriptors isEnabledByDefault: true, helpLinkUri: $"{HelpLinkBaseUri}/XPC3003.md"); + public static readonly DiagnosticDescriptor LocalPluginContextAsService = new( + id: "XPC3004", + title: "Do not use LocalPluginContext as TService in RegisterStep", + messageFormat: "RegisterStep<{0}, LocalPluginContext> will cause a runtime exception because LocalPluginContext is not registered in the DI container. Use RegisterPluginStep<{0}> instead, or migrate to a DI-registered service type.", + category: Category, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "LocalPluginContext cannot be resolved via GetRequiredService() in the scoped DI container. Use RegisterPluginStep which correctly wraps the action, or migrate to a DI-registered service.", + helpLinkUri: $"{HelpLinkBaseUri}/XPC3004.md"); + public static readonly DiagnosticDescriptor HandlerMethodNotFound = new( id: "XPC4001", title: "Handler method not found", diff --git a/XrmPluginCore.SourceGenerator/rules/XPC3004.md b/XrmPluginCore.SourceGenerator/rules/XPC3004.md new file mode 100644 index 0000000..1c4188d --- /dev/null +++ b/XrmPluginCore.SourceGenerator/rules/XPC3004.md @@ -0,0 +1,88 @@ +# XPC3004: Do not use LocalPluginContext as TService in RegisterStep + +## Severity + +Error + +## Description + +This rule reports when `LocalPluginContext` is used as the `TService` type argument in `RegisterStep()`. This causes a runtime exception because `LocalPluginContext` is not registered in the DI container. At runtime, the plugin will call `sp.GetRequiredService()`, which throws `InvalidOperationException`. + +This typically happens when migrating from the legacy `RegisterPluginStep` API and mistakenly using `RegisterStep` instead of the correct DI-based approach. + +## ❌ Example of violation + +```csharp +public class ContactPlugin : Plugin +{ + public ContactPlugin() + { + // XPC3004: LocalPluginContext is not in DI — causes runtime exception + RegisterStep( + EventOperation.Update, + ExecutionStage.PostOperation, + Execute); + } + + private void Execute(LocalPluginContext context) { } +} +``` + +## ✅ How to fix (interim — keep LocalPluginContext logic) + +Use `RegisterPluginStep` which correctly wraps the `LocalPluginContext`: + +```csharp +public class ContactPlugin : Plugin +{ + public ContactPlugin() + { + RegisterPluginStep( + EventOperation.Update, + ExecutionStage.PostOperation, + Execute); + } + + private void Execute(LocalPluginContext context) { } +} +``` + +## ✅ How to fix (recommended — migrate to DI service) + +Migrate to a DI-registered service type for full dependency injection support: + +```csharp +public class ContactPlugin : Plugin +{ + public ContactPlugin() + { + RegisterStep( + EventOperation.Update, + ExecutionStage.PostOperation, + nameof(IContactService.HandleUpdate)); + } + + protected override IServiceCollection OnBeforeBuildServiceProvider(IServiceCollection services) + { + return services.AddScoped(); + } +} + +public interface IContactService +{ + void HandleUpdate(); +} +``` + +## Why this matters + +`RegisterStep` calls `sp.GetRequiredService()` at runtime. `LocalPluginContext` is constructed manually (not registered in the DI container), so this call throws `InvalidOperationException`. The exception is then caught and re-thrown as `InvalidPluginExecutionException`, masking the original cause. + +## Code fix availability + +Yes — the code fix rewrites `RegisterStep(...)` to `RegisterPluginStep(...)`. + +## See also + +- [XPC3001: Prefer nameof over string literal for handler method](XPC3001.md) +- `RegisterPluginStep` (legacy API, accepts `Action`)