diff --git a/src/Dapr.Workflow.Analyzers/AnalyzerReleases.Unshipped.md b/src/Dapr.Workflow.Analyzers/AnalyzerReleases.Unshipped.md index 6ff5c0f5b..6d0050326 100644 --- a/src/Dapr.Workflow.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Dapr.Workflow.Analyzers/AnalyzerReleases.Unshipped.md @@ -7,3 +7,4 @@ Rule ID | Category | Severity | Notes --------|----------|----------|-------------------- DAPR1303 | Usage | Warning | The provided input type does not match the target workflow or activity input type DAPR1304 | Usage | Warning | The requested output type does not match the target workflow or activity output type +DAPR1305 | Usage | Warning | Workflow implementations do not support dependency injection via constructors diff --git a/src/Dapr.Workflow.Analyzers/CompilationExtensions.cs b/src/Dapr.Workflow.Analyzers/CompilationExtensions.cs index a4cf449e3..078343022 100644 --- a/src/Dapr.Workflow.Analyzers/CompilationExtensions.cs +++ b/src/Dapr.Workflow.Analyzers/CompilationExtensions.cs @@ -9,6 +9,7 @@ internal static class CompilationExtensions private const string WorkflowContextMetadataName = "Dapr.Workflow.WorkflowContext"; private const string WorkflowBaseMetadataName = "Dapr.Workflow.Workflow`2"; private const string WorkflowActivityBaseMetadataName = "Dapr.Workflow.WorkflowActivity`2"; + private const string WorkflowAbstractionsAssemblyName = "Dapr.Workflow.Abstractions"; internal static INamedTypeSymbol? GetDaprWorkflowClientType(this Compilation compilation) => compilation.GetTypeByMetadataName(DaprWorkflowClientMetadataName); @@ -19,8 +20,22 @@ internal static class CompilationExtensions internal static INamedTypeSymbol? GetWorkflowContextType(this Compilation compilation) => compilation.GetTypeByMetadataName(WorkflowContextMetadataName); - internal static INamedTypeSymbol? GetWorkflowBaseType(this Compilation compilation) => - compilation.GetTypeByMetadataName(WorkflowBaseMetadataName); + /// + /// Gets the Dapr.Workflow.Workflow<,> base type, verifying it originates from the + /// Dapr.Workflow.Abstractions assembly to avoid false positives against user-defined + /// types that share the same fully-qualified name. + /// + internal static INamedTypeSymbol? GetWorkflowBaseType(this Compilation compilation) + { + var type = compilation.GetTypeByMetadataName(WorkflowBaseMetadataName); + if (type is not null && + !string.Equals(type.ContainingAssembly.Name, WorkflowAbstractionsAssemblyName, StringComparison.Ordinal)) + { + return null; + } + + return type; + } internal static INamedTypeSymbol? GetWorkflowActivityBaseType(this Compilation compilation) => compilation.GetTypeByMetadataName(WorkflowActivityBaseMetadataName); diff --git a/src/Dapr.Workflow.Analyzers/Resources.Designer.cs b/src/Dapr.Workflow.Analyzers/Resources.Designer.cs index 5566fe834..495330104 100644 --- a/src/Dapr.Workflow.Analyzers/Resources.Designer.cs +++ b/src/Dapr.Workflow.Analyzers/Resources.Designer.cs @@ -130,5 +130,23 @@ internal static string DAPR1304Title { return ResourceManager.GetString("DAPR1304Title", resourceCulture); } } + + /// + /// Looks up a localized string similar to Workflow '{0}' has constructor parameter '{1}' of type '{2}', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead. + /// + internal static string DAPR1305MessageFormat { + get { + return ResourceManager.GetString("DAPR1305MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Workflow implementations do not support dependency injection via constructors. + /// + internal static string DAPR1305Title { + get { + return ResourceManager.GetString("DAPR1305Title", resourceCulture); + } + } } } diff --git a/src/Dapr.Workflow.Analyzers/Resources.resx b/src/Dapr.Workflow.Analyzers/Resources.resx index f332492bf..a9c3e4c9b 100644 --- a/src/Dapr.Workflow.Analyzers/Resources.resx +++ b/src/Dapr.Workflow.Analyzers/Resources.resx @@ -42,4 +42,10 @@ The requested output type '{0}' does not match the declared output type '{1}' for {2} '{3}' + + Workflow implementations do not support dependency injection via constructors + + + Workflow '{0}' has constructor parameter '{1}' of type '{2}', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead. + diff --git a/src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionAnalyzer.cs b/src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionAnalyzer.cs new file mode 100644 index 000000000..3dad2e69a --- /dev/null +++ b/src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionAnalyzer.cs @@ -0,0 +1,144 @@ +// ------------------------------------------------------------------------ +// Copyright 2025 The Dapr Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ------------------------------------------------------------------------ + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Dapr.Workflow.Analyzers; + +/// +/// Analyzes whether a Workflow implementation attempts to use constructor-based dependency +/// injection, which is not supported by the Dapr workflow runtime because workflow code must +/// be deterministic and is replayed multiple times. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class WorkflowDependencyInjectionAnalyzer : DiagnosticAnalyzer +{ + internal static readonly DiagnosticDescriptor WorkflowDependencyInjectionDescriptor = new( + id: "DAPR1305", + title: new LocalizableResourceString(nameof(Resources.DAPR1305Title), Resources.ResourceManager, typeof(Resources)), + messageFormat: new LocalizableResourceString(nameof(Resources.DAPR1305MessageFormat), Resources.ResourceManager, typeof(Resources)), + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// Gets the diagnostics supported by this analyzer. + /// + public override ImmutableArray SupportedDiagnostics => + [ + WorkflowDependencyInjectionDescriptor + ]; + + /// + /// Initializes analyzer actions. + /// + /// The analysis context. + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(static compilationStartContext => + { + var workflowBaseType = compilationStartContext.Compilation.GetWorkflowBaseType(); + if (workflowBaseType is null) + { + return; + } + + compilationStartContext.RegisterSyntaxNodeAction( + nodeContext => AnalyzeClassDeclaration(nodeContext, workflowBaseType), + SyntaxKind.ClassDeclaration); + }); + } + + private static void AnalyzeClassDeclaration( + SyntaxNodeAnalysisContext context, + INamedTypeSymbol workflowBaseType) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + var classDeclaration = (ClassDeclarationSyntax)context.Node; + + if (context.SemanticModel.GetDeclaredSymbol(classDeclaration, context.CancellationToken) is not INamedTypeSymbol classSymbol) + { + return; + } + + if (!DerivesFromWorkflow(classSymbol, workflowBaseType)) + { + return; + } + + foreach (var constructor in classDeclaration.Members.OfType()) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + foreach (var parameter in constructor.ParameterList.Parameters) + { + ReportParameterDiagnostic(context, classSymbol, parameter); + } + } + + // Also check primary constructors (C# 12+), where the parameter list appears on the class declaration itself. + if (classDeclaration.ParameterList is { Parameters.Count: > 0 } primaryCtorParams) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + foreach (var parameter in primaryCtorParams.Parameters) + { + ReportParameterDiagnostic(context, classSymbol, parameter); + } + } + } + + private static void ReportParameterDiagnostic( + SyntaxNodeAnalysisContext context, + INamedTypeSymbol classSymbol, + ParameterSyntax parameter) + { + var parameterSymbol = context.SemanticModel + .GetDeclaredSymbol(parameter, context.CancellationToken); + + var typeName = parameterSymbol?.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat) + ?? parameter.Type?.ToString() + ?? "unknown"; + + var paramName = parameter.Identifier.Text; + + context.ReportDiagnostic(Diagnostic.Create( + WorkflowDependencyInjectionDescriptor, + parameter.GetLocation(), + classSymbol.Name, + paramName, + typeName)); + } + + private static bool DerivesFromWorkflow(INamedTypeSymbol classSymbol, INamedTypeSymbol workflowBaseType) + { + for (var current = classSymbol.BaseType; current is not null; current = current.BaseType) + { + if (current.IsGenericType && + SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, workflowBaseType)) + { + return true; + } + } + + return false; + } +} diff --git a/src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionCodeFixProvider.cs b/src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionCodeFixProvider.cs new file mode 100644 index 000000000..1d0d6328f --- /dev/null +++ b/src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionCodeFixProvider.cs @@ -0,0 +1,101 @@ +// ------------------------------------------------------------------------ +// Copyright 2025 The Dapr Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ------------------------------------------------------------------------ + +using System.Collections.Immutable; +using System.Composition; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Dapr.Workflow.Analyzers; + +/// +/// Provides a code fix for DAPR1305 by removing the offending constructor parameter +/// from either a regular constructor or a primary constructor. +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(WorkflowDependencyInjectionCodeFixProvider))] +[Shared] +public sealed class WorkflowDependencyInjectionCodeFixProvider : CodeFixProvider +{ + /// + /// Gets the diagnostic IDs that this provider can fix. + /// + public override ImmutableArray FixableDiagnosticIds => ["DAPR1305"]; + + /// + /// Registers the code fix for the diagnostic. + /// + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + const string title = "Remove injected constructor parameter"; + context.RegisterCodeFix( + CodeAction.Create( + title, + createChangedDocument: c => RemoveParameterAsync(context.Document, context.Diagnostics.First(), c), + equivalenceKey: title), + context.Diagnostics); + return Task.CompletedTask; + } + + private static async Task RemoveParameterAsync( + Document document, + Diagnostic diagnostic, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + { + return document; + } + + var diagnosticSpan = diagnostic.Location.SourceSpan; + + var parameter = root + .FindToken(diagnosticSpan.Start) + .Parent? + .AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + + if (parameter is null) + { + return document; + } + + var paramList = (ParameterListSyntax)parameter.Parent!; + var removedIndex = paramList.Parameters.IndexOf(parameter); + var newParameters = paramList.Parameters.Remove(parameter); + + // When the first parameter is removed, the next parameter inherits the leading + // whitespace trivia that was on the removed separator, producing "( Type b)" + // instead of "(Type b)". Strip it so the result is clean. + if (removedIndex == 0 && newParameters.Count > 0) + { + var firstParam = newParameters[0]; + var newFirstParam = firstParam.WithLeadingTrivia(SyntaxFactory.TriviaList()); + newParameters = newParameters.Replace(firstParam, newFirstParam); + } + + var newParamList = paramList.WithParameters(newParameters); + var newRoot = root.ReplaceNode(paramList, newParamList); + + return document.WithSyntaxRoot(newRoot); + } + + /// + /// Gets the FixAllProvider for this code fix provider. + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; +} diff --git a/test/Dapr.Analyzers.Common/VerifyCodeFix.cs b/test/Dapr.Analyzers.Common/VerifyCodeFix.cs index aac71aa9e..378b3bee8 100644 --- a/test/Dapr.Analyzers.Common/VerifyCodeFix.cs +++ b/test/Dapr.Analyzers.Common/VerifyCodeFix.cs @@ -22,19 +22,39 @@ namespace Dapr.Analyzers.Common; internal static class VerifyCodeFix { + public static Task RunTest( + string code, + string expectedChangedCode, + string assemblyLocation, + IReadOnlyList metadataReferences, + ImmutableArray analyzers) where T : CodeFixProvider, new() => + RunTest(code, expectedChangedCode, assemblyLocation, metadataReferences, analyzers, diagnosticIndex: 0); + public static async Task RunTest( string code, string expectedChangedCode, string assemblyLocation, IReadOnlyList metadataReferences, - ImmutableArray analyzers) where T : CodeFixProvider, new() + ImmutableArray analyzers, + int diagnosticIndex) where T : CodeFixProvider, new() { - var (diagnostics, document, workspace) = + var (allDiagnostics, document, workspace) = await TestUtilities.GetDiagnosticsAdvanced(code, assemblyLocation, metadataReferences, analyzers); - Assert.Single(diagnostics); + // Filter to only the diagnostics produced by the supplied analyzers, ignoring any + // compiler warnings/errors that are incidental to the test code (e.g. CS5001, CS9113). + var analyzerDiagnosticIds = analyzers + .SelectMany(a => a.SupportedDiagnostics) + .Select(d => d.Id) + .ToHashSet(); + + var diagnostics = allDiagnostics + .Where(d => analyzerDiagnosticIds.Contains(d.Id)) + .ToImmutableArray(); + + Assert.NotEmpty(diagnostics); - var diagnostic = diagnostics[0]; + var diagnostic = diagnostics[diagnosticIndex]; var codeFixProvider = new T(); diff --git a/test/Dapr.E2E.Test/DaprTestApp.cs b/test/Dapr.E2E.Test/DaprTestApp.cs index 90f88f46b..ecab50d89 100644 --- a/test/Dapr.E2E.Test/DaprTestApp.cs +++ b/test/Dapr.E2E.Test/DaprTestApp.cs @@ -41,10 +41,17 @@ public DaprTestApp(ITestOutputHelper output, string appId) public string AppId => this.appId; + public int AppPort { get; private set; } + public (string httpEndpoint, string grpcEndpoint) Start(DaprRunConfiguration configuration) { var (appPort, httpPort, grpcPort, metricsPort) = GetFreePorts(); + if (configuration.UseAppPort) + { + this.AppPort = appPort; + } + var resourcesPath = Combine(".", "..", "..", "..", "..", "..", "test", "Dapr.E2E.Test", "components"); var configPath = Combine(".", "..", "..", "..", "..", "..", "test", "Dapr.E2E.Test", "configuration", "featureconfig.yaml"); var arguments = new List() diff --git a/test/Dapr.E2E.Test/DaprTestAppLifecycle.cs b/test/Dapr.E2E.Test/DaprTestAppLifecycle.cs index d37d1d2f8..0e9076d4f 100644 --- a/test/Dapr.E2E.Test/DaprTestAppLifecycle.cs +++ b/test/Dapr.E2E.Test/DaprTestAppLifecycle.cs @@ -12,7 +12,9 @@ // ------------------------------------------------------------------------ using System; +using System.Net; using System.Net.Http; +using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; using Xunit; @@ -56,6 +58,15 @@ public async ValueTask InitializeAsync() var response = await client.GetAsync($"{HttpEndpoint}/v1.0/healthz"); if (response.IsSuccessStatusCode) { + // For gRPC apps, also wait for the app port to accept TCP connections before + // returning so that the Dapr sidecar can successfully proxy gRPC requests. + if (this.Configuration?.UseAppPort == true + && string.Equals(this.Configuration?.AppProtocol, "grpc", StringComparison.OrdinalIgnoreCase) + && (this.state.App?.AppPort ?? 0) > 0) + { + await WaitForAppPortAsync(this.state.App.AppPort); + } + return; } @@ -65,6 +76,41 @@ public async ValueTask InitializeAsync() throw new TimeoutException("Timed out waiting for daprd health check"); } + private static async Task WaitForAppPortAsync(int port) + { + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + while (!cts.IsCancellationRequested) + { + try + { + using (var tcpClient = new TcpClient()) + { + await tcpClient.ConnectAsync(IPAddress.Loopback, port); + return; + } + } + catch (SocketException) + { + // Port not yet accepting connections — retry after a short delay. + } + catch (Exception) + { + // Treat any other connection error as a transient failure and retry. + } + + try + { + await Task.Delay(TimeSpan.FromMilliseconds(250), cts.Token); + } + catch (OperationCanceledException) + { + break; + } + } + + throw new TimeoutException($"Timed out waiting for gRPC app to listen on port {port}"); + } + public ValueTask DisposeAsync() { return ValueTask.CompletedTask; diff --git a/test/Dapr.Workflow.Analyzers.Test/WorkflowDependencyInjectionAnalyzerTests.cs b/test/Dapr.Workflow.Analyzers.Test/WorkflowDependencyInjectionAnalyzerTests.cs new file mode 100644 index 000000000..8dfd7f9c2 --- /dev/null +++ b/test/Dapr.Workflow.Analyzers.Test/WorkflowDependencyInjectionAnalyzerTests.cs @@ -0,0 +1,387 @@ +// ------------------------------------------------------------------------ +// Copyright 2025 The Dapr Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ------------------------------------------------------------------------ + +using Dapr.Analyzers.Common; + +namespace Dapr.Workflow.Analyzers.Test; + +public sealed class WorkflowDependencyInjectionAnalyzerTests +{ + // ------------------------------------------------------------------------- + // Diagnostics should be reported + // ------------------------------------------------------------------------- + + [Fact] + public async Task VerifyDiagnostic_WhenWorkflowHasSingleConstructorParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IMyService service) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expected = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(8, 26, 8, 44) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'service' of type 'IMyService', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expected); + } + + [Fact] + public async Task VerifyDiagnostic_WhenWorkflowHasMultipleConstructorParameters() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IServiceA serviceA, IServiceB serviceB) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expectedA = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(9, 26, 9, 44) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'serviceA' of type 'IServiceA', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var expectedB = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(9, 46, 9, 64) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'serviceB' of type 'IServiceB', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expectedA, expectedB); + } + + [Fact] + public async Task VerifyDiagnostic_WhenWorkflowHasConcreteTypeConstructorParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public class MyConcreteService { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(MyConcreteService service) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expected = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(8, 26, 8, 51) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'service' of type 'MyConcreteService', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expected); + } + + [Fact] + public async Task VerifyDiagnostic_WhenIndirectWorkflowSubclassHasConstructorParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public abstract class BaseOrderWorkflow : Workflow { } + + public sealed class ConcreteOrderWorkflow : BaseOrderWorkflow + { + public ConcreteOrderWorkflow(IMyService service) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expected = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(10, 34, 10, 52) + .WithMessage("Workflow 'ConcreteOrderWorkflow' has constructor parameter 'service' of type 'IMyService', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expected); + } + + // ------------------------------------------------------------------------- + // No diagnostics should be reported + // ------------------------------------------------------------------------- + + [Fact] + public async Task NoDiagnostic_WhenWorkflowHasParameterlessConstructor() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow() { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task NoDiagnostic_WhenWorkflowHasNoExplicitConstructor() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public sealed class OrderWorkflow : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task NoDiagnostic_WhenActivityHasConstructorParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class NotifyActivity : WorkflowActivity + { + private readonly IMyService _service; + + public NotifyActivity(IMyService service) + { + _service = service; + } + + public override Task RunAsync(WorkflowActivityContext context, string input) + => Task.FromResult(input); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task NoDiagnostic_WhenNonWorkflowClassHasConstructorParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class SomeOtherClass + { + public SomeOtherClass(IMyService service) { } + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task NoDiagnostic_WhenWorkflowHasOnlyParameterlessConstructorAlongWithNoOtherConstructors() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow() { } + + public override Task RunAsync(WorkflowContext context, int input) + => Task.FromResult(input.ToString()); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task VerifyDiagnostic_WhenWorkflowUsesPrimaryConstructorWithParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class OrderWorkflow(IMyService service) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expected = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(6, 35, 6, 53) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'service' of type 'IMyService', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expected); + } + + [Fact] + public async Task VerifyDiagnostic_WhenWorkflowUsesPrimaryConstructorWithMultipleParameters() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow(IServiceA serviceA, IServiceB serviceB) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expectedA = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(7, 35, 7, 53) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'serviceA' of type 'IServiceA', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var expectedB = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(7, 55, 7, 73) + .WithMessage("Workflow 'OrderWorkflow' has constructor parameter 'serviceB' of type 'IServiceB', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expectedA, expectedB); + } + + [Fact] + public async Task NoDiagnostic_WhenWorkflowUsesPrimaryConstructorWithNoParameters() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public sealed class OrderWorkflow() : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task NoDiagnostic_WhenActivityUsesPrimaryConstructorWithParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class NotifyActivity(IMyService service) : WorkflowActivity + { + public override Task RunAsync(WorkflowActivityContext context, string input) + => Task.FromResult(input); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task NoDiagnostic_WhenNonDaprGenericBaseClassSubclassHasConstructorParameter() + { + // A class that derives from a user-defined generic base class — NOT Dapr.Workflow.Workflow<,> — + // should never trigger DAPR1305 even if the base class looks structurally similar. + const string testCode = """ + using System.Threading.Tasks; + + public interface IMyService { } + + // Custom generic base class unrelated to Dapr.Workflow + public abstract class Processor + { + public abstract Task RunAsync(TInput input); + } + + public sealed class OrderProcessor : Processor + { + public OrderProcessor(IMyService service) { } + + public override Task RunAsync(string input) => Task.FromResult(input); + } + """; + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode); + } + + [Fact] + public async Task VerifyDiagnostic_WhenIndirectWorkflowSubclassUsesPrimaryConstructorWithParameter() + { + const string testCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public abstract class BaseOrderWorkflow : Workflow { } + + public sealed class ConcreteOrderWorkflow(IMyService service) : BaseOrderWorkflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + """; + + var expected = VerifyAnalyzer.Diagnostic(WorkflowDependencyInjectionAnalyzer.WorkflowDependencyInjectionDescriptor) + .WithSpan(8, 43, 8, 61) + .WithMessage("Workflow 'ConcreteOrderWorkflow' has constructor parameter 'service' of type 'IMyService', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead."); + + var analyzer = new VerifyAnalyzer(Utilities.GetReferences()); + await analyzer.VerifyAnalyzerAsync(testCode, expected); + } +} diff --git a/test/Dapr.Workflow.Analyzers.Test/WorkflowDependencyInjectionCodeFixProviderTests.cs b/test/Dapr.Workflow.Analyzers.Test/WorkflowDependencyInjectionCodeFixProviderTests.cs new file mode 100644 index 000000000..25d4fbb76 --- /dev/null +++ b/test/Dapr.Workflow.Analyzers.Test/WorkflowDependencyInjectionCodeFixProviderTests.cs @@ -0,0 +1,448 @@ +// ------------------------------------------------------------------------ +// Copyright 2025 The Dapr Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ------------------------------------------------------------------------ + +using System.Collections.Immutable; +using Dapr.Analyzers.Common; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Dapr.Workflow.Analyzers.Test; + +public sealed class WorkflowDependencyInjectionCodeFixProviderTests +{ + private static ImmutableArray Analyzers => + [new WorkflowDependencyInjectionAnalyzer()]; + + // ------------------------------------------------------------------------- + // Regular constructor + // ------------------------------------------------------------------------- + + [Fact] + public async Task RemovesParameter_FromRegularConstructor_SingleParam() + { + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IMyService service) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow() { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } + + [Fact] + public async Task RemovesFirstParameter_FromRegularConstructor_MultipleParams() + { + // When there are multiple parameters the fix fires once per parameter. + // Each invocation removes exactly the one flagged parameter. + // This test targets the first parameter. + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IServiceA serviceA, IServiceB serviceB) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + // The code fix only removes the first flagged parameter; the second still requires its own fix. + // VerifyCodeFix.RunTest targets the first diagnostic (serviceA). + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IServiceB serviceB) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } + + [Fact] + public async Task RemovesSecondParameter_FromRegularConstructor_MultipleParams() + { + // Targets the second diagnostic (serviceB) to verify that removing a non-first + // parameter does not disturb the leading trivia of the remaining first parameter. + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IServiceA serviceA, IServiceB serviceB) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(IServiceA serviceA) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers, + diagnosticIndex: 1); + } + + [Fact] + public async Task RemovesParameter_FromRegularConstructor_ConcreteType() + { + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public class MyConcreteService { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow(MyConcreteService service) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public class MyConcreteService { } + + public sealed class OrderWorkflow : Workflow + { + public OrderWorkflow() { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } + + [Fact] + public async Task RemovesParameter_FromRegularConstructor_IndirectSubclass() + { + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public abstract class BaseOrderWorkflow : Workflow { } + + public sealed class ConcreteOrderWorkflow : BaseOrderWorkflow + { + public ConcreteOrderWorkflow(IMyService service) { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public abstract class BaseOrderWorkflow : Workflow { } + + public sealed class ConcreteOrderWorkflow : BaseOrderWorkflow + { + public ConcreteOrderWorkflow() { } + + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } + + // ------------------------------------------------------------------------- + // Primary constructor + // ------------------------------------------------------------------------- + + [Fact] + public async Task RemovesParameter_FromPrimaryConstructor_SingleParam() + { + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class OrderWorkflow(IMyService service) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public sealed class OrderWorkflow() : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } + + [Fact] + public async Task RemovesFirstParameter_FromPrimaryConstructor_MultipleParams() + { + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow(IServiceA serviceA, IServiceB serviceB) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow(IServiceB serviceB) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } + + [Fact] + public async Task RemovesSecondParameter_FromPrimaryConstructor_MultipleParams() + { + // Targets the second diagnostic (serviceB) to verify that removing a non-first + // primary constructor parameter does not disturb the first parameter's trivia. + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow(IServiceA serviceA, IServiceB serviceB) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IServiceA { } + public interface IServiceB { } + + public sealed class OrderWorkflow(IServiceA serviceA) : Workflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers, + diagnosticIndex: 1); + } + + [Fact] + public async Task RemovesParameter_FromPrimaryConstructor_IndirectSubclass() + { + const string code = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public abstract class BaseOrderWorkflow : Workflow { } + + public sealed class ConcreteOrderWorkflow(IMyService service) : BaseOrderWorkflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + const string expectedChangedCode = """ + using Dapr.Workflow; + using System.Threading.Tasks; + + public interface IMyService { } + + public abstract class BaseOrderWorkflow : Workflow { } + + public sealed class ConcreteOrderWorkflow() : BaseOrderWorkflow + { + public override Task RunAsync(WorkflowContext context, string input) + => Task.FromResult(input); + } + + public static class Program { public static void Main() { } } + """; + + await VerifyCodeFix.RunTest( + code, + expectedChangedCode, + typeof(object).Assembly.Location, + Utilities.GetReferences(), + Analyzers); + } +}