Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
19 changes: 17 additions & 2 deletions src/Dapr.Workflow.Analyzers/CompilationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
/// <summary>
/// Gets the <c>Dapr.Workflow.Workflow&lt;,&gt;</c> base type, verifying it originates from the
/// <c>Dapr.Workflow.Abstractions</c> assembly to avoid false positives against user-defined
/// types that share the same fully-qualified name.
/// </summary>
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);
Expand Down
18 changes: 18 additions & 0 deletions src/Dapr.Workflow.Analyzers/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Dapr.Workflow.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,10 @@
<data name="DAPR1304MessageFormat" xml:space="preserve">
<value>The requested output type '{0}' does not match the declared output type '{1}' for {2} '{3}'</value>
</data>
<data name="DAPR1305Title" xml:space="preserve">
<value>Workflow implementations do not support dependency injection via constructors</value>
</data>
<data name="DAPR1305MessageFormat" xml:space="preserve">
<value>Workflow '{0}' has constructor parameter '{1}' of type '{2}', but dependency injection is not supported in workflow implementations. Move dependencies to a WorkflowActivity instead.</value>
</data>
</root>
144 changes: 144 additions & 0 deletions src/Dapr.Workflow.Analyzers/WorkflowDependencyInjectionAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// 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.
/// </summary>
[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);

/// <summary>
/// Gets the diagnostics supported by this analyzer.
/// </summary>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
[
WorkflowDependencyInjectionDescriptor
];

/// <summary>
/// Initializes analyzer actions.
/// </summary>
/// <param name="context">The analysis context.</param>
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterCompilationStartAction(static compilationStartContext =>
{
var workflowBaseType = compilationStartContext.Compilation.GetWorkflowBaseType();
Comment thread
WhitWaldo marked this conversation as resolved.
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<ConstructorDeclarationSyntax>())
{
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;
}
}
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// Provides a code fix for DAPR1305 by removing the offending constructor parameter
/// from either a regular constructor or a primary constructor.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(WorkflowDependencyInjectionCodeFixProvider))]
[Shared]
public sealed class WorkflowDependencyInjectionCodeFixProvider : CodeFixProvider
{
/// <summary>
/// Gets the diagnostic IDs that this provider can fix.
/// </summary>
public override ImmutableArray<string> FixableDiagnosticIds => ["DAPR1305"];

/// <summary>
/// Registers the code fix for the diagnostic.
/// </summary>
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<Document> 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<ParameterSyntax>()
.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);
}

/// <summary>
/// Gets the FixAllProvider for this code fix provider.
/// </summary>
public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
}
28 changes: 24 additions & 4 deletions test/Dapr.Analyzers.Common/VerifyCodeFix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,39 @@ namespace Dapr.Analyzers.Common;

internal static class VerifyCodeFix
{
public static Task RunTest<T>(
string code,
string expectedChangedCode,
string assemblyLocation,
IReadOnlyList<MetadataReference> metadataReferences,
ImmutableArray<DiagnosticAnalyzer> analyzers) where T : CodeFixProvider, new() =>
RunTest<T>(code, expectedChangedCode, assemblyLocation, metadataReferences, analyzers, diagnosticIndex: 0);

public static async Task RunTest<T>(
string code,
string expectedChangedCode,
string assemblyLocation,
IReadOnlyList<MetadataReference> metadataReferences,
ImmutableArray<DiagnosticAnalyzer> analyzers) where T : CodeFixProvider, new()
ImmutableArray<DiagnosticAnalyzer> 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();

Expand Down
7 changes: 7 additions & 0 deletions test/Dapr.E2E.Test/DaprTestApp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>()
Expand Down
Loading
Loading