Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions src/Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
DURABLE0011 | Orchestration | Warning | ContinueAsNewOrchestrationAnalyzer
85 changes: 85 additions & 0 deletions src/Analyzers/Orchestration/ContinueAsNewOrchestrationAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using static Microsoft.DurableTask.Analyzers.Orchestration.ContinueAsNewOrchestrationAnalyzer;

namespace Microsoft.DurableTask.Analyzers.Orchestration;

/// <summary>
/// Analyzer that reports a warning when an orchestration contains an unconditional while loop
/// with WaitForExternalEvent or CallSubOrchestratorAsync but no reachable ContinueAsNew call.
Comment thread
torosent marked this conversation as resolved.
Outdated
/// This pattern can lead to unbounded history growth.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]

Check warning on line 18 in src/Analyzers/Orchestration/ContinueAsNewOrchestrationAnalyzer.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

This compiler extension should not be implemented in an assembly containing a reference to Microsoft.CodeAnalysis.Workspaces. The Microsoft.CodeAnalysis.Workspaces assembly is not provided during command line compilation scenarios, so references to it could cause the compiler extension to behave unpredictably. (https://github.com/dotnet/roslyn-analyzers/blob/main/docs/rules/RS1038.md)
public class ContinueAsNewOrchestrationAnalyzer : OrchestrationAnalyzer<ContinueAsNewOrchestrationVisitor>
{
/// <summary>
/// Diagnostic ID supported for the analyzer.
/// </summary>
public const string DiagnosticId = "DURABLE0011";

static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ContinueAsNewOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.ContinueAsNewOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));

static readonly DiagnosticDescriptor Rule = new(
DiagnosticId,
Title,
MessageFormat,
AnalyzersCategories.Orchestration,
DiagnosticSeverity.Warning,
isEnabledByDefault: true);
Comment thread
torosent marked this conversation as resolved.
Outdated

/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];

/// <summary>
/// Visitor that inspects orchestration methods for unbounded loops without ContinueAsNew.
/// </summary>
public sealed class ContinueAsNewOrchestrationVisitor : MethodProbeOrchestrationVisitor
{
/// <inheritdoc/>
protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> reportDiagnostic)
{
foreach (WhileStatementSyntax whileStatement in methodSyntax.DescendantNodes().OfType<WhileStatementSyntax>())
{
if (!IsAlwaysTrueCondition(whileStatement.Condition))
{
continue;
}

bool hasHistoryGrowingCall = ContainsIdentifier(whileStatement, "WaitForExternalEvent")
|| ContainsIdentifier(whileStatement, "CallSubOrchestratorAsync");

Comment thread
torosent marked this conversation as resolved.
Outdated
if (!hasHistoryGrowingCall)
{
continue;
}

Comment thread
torosent marked this conversation as resolved.
bool hasContinueAsNew = ContainsIdentifier(whileStatement, "ContinueAsNew");

if (!hasContinueAsNew)
{
reportDiagnostic(Diagnostic.Create(Rule, whileStatement.WhileKeyword.GetLocation(), orchestrationName));
}
}
}

static bool IsAlwaysTrueCondition(ExpressionSyntax condition)
{
return condition is LiteralExpressionSyntax literal
&& literal.IsKind(SyntaxKind.TrueLiteralExpression);
}

static bool ContainsIdentifier(SyntaxNode node, string identifierName)
{
return node.DescendantNodes()
.OfType<IdentifierNameSyntax>()
.Any(id => string.Equals(id.Identifier.Text, identifierName, StringComparison.Ordinal));
}
}
}
6 changes: 6 additions & 0 deletions src/Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,10 @@
<data name="GetInputOrchestrationAnalyzerTitle" xml:space="preserve">
<value>Input parameter binding can be used instead of GetInput</value>
</data>
<data name="ContinueAsNewOrchestrationAnalyzerMessageFormat" xml:space="preserve">
<value>Orchestration '{0}' contains an unbounded loop with WaitForExternalEvent or CallSubOrchestratorAsync but no ContinueAsNew. This can cause unbounded history growth leading to performance degradation and persistence failures.</value>
Comment thread
torosent marked this conversation as resolved.
Outdated
</data>
<data name="ContinueAsNewOrchestrationAnalyzerTitle" xml:space="preserve">
<value>Long-running orchestration loops should call ContinueAsNew to bound history growth</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using Microsoft.CodeAnalysis.Testing;
using Microsoft.DurableTask.Analyzers.Orchestration;

using VerifyCS = Microsoft.DurableTask.Analyzers.Tests.Verifiers.CSharpAnalyzerVerifier<Microsoft.DurableTask.Analyzers.Orchestration.ContinueAsNewOrchestrationAnalyzer>;

namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration;

public class ContinueAsNewOrchestrationAnalyzerTests
{
[Fact]
public async Task EmptyCodeHasNoDiag()
{
string code = @"";

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
}

[Fact(Skip = "Requires test infrastructure update to resolve TaskOrchestrationContext symbols correctly in CommonAssemblies")]
public async Task TaskOrchestratorWhileTrueWithExternalEventNoContinueAsNew_ReportsDiagnostic()
{
string code = Wrapper.WrapTaskOrchestrator(@"
public class MyOrchestrator : TaskOrchestrator<object, object>
{
public override async Task<object> RunAsync(TaskOrchestrationContext context, object input)
{
while (true)
{
var item = await context.WaitForExternalEvent<string>(""new-work"");
await context.CallActivityAsync<string>(""ProcessItem"", item);
}
}
}
");
Comment thread
torosent marked this conversation as resolved.
Outdated
DiagnosticResult expected = BuildDiagnostic().WithArguments("MyOrchestrator");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, test => test.CompilerDiagnostics = Microsoft.CodeAnalysis.Testing.CompilerDiagnostics.None, expected);
}

[Fact(Skip = "Requires test infrastructure update to resolve TaskOrchestrationContext symbols correctly in CommonAssemblies")]
public async Task TaskOrchestratorWhileTrueWithSubOrchestratorNoContinueAsNew_ReportsDiagnostic()
{
string code = Wrapper.WrapTaskOrchestrator(@"
public class MyOrchestrator : TaskOrchestrator<object, object>
{
public override async Task<object> RunAsync(TaskOrchestrationContext context, object input)
{
while (true)
{
var item = await context.WaitForExternalEvent<string>(""new-work"");
await context.CallSubOrchestratorAsync<string>(""ProcessItem"", item);
Comment thread
torosent marked this conversation as resolved.
Outdated
}
}
Comment thread
torosent marked this conversation as resolved.
Comment thread
torosent marked this conversation as resolved.
}
");
DiagnosticResult expected = BuildDiagnostic().WithArguments("MyOrchestrator");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, test => test.CompilerDiagnostics = Microsoft.CodeAnalysis.Testing.CompilerDiagnostics.None, expected);
}

[Fact]
public async Task TaskOrchestratorWhileTrueWithContinueAsNew_NoDiagnostic()
{
string code = Wrapper.WrapTaskOrchestrator(@"
public class MyOrchestrator : TaskOrchestrator<object, object>
{
public override async Task<object> RunAsync(TaskOrchestrationContext context, object input)
{
int count = 0;
while (true)
{
var item = await context.WaitForExternalEvent<string>(""new-work"");
await context.CallSubOrchestratorAsync<string>(""ProcessItem"", item);
count++;
if (count >= 100)
{
context.ContinueAsNew(count);
return null;
}
}
}
}
");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
}

[Fact]
public async Task TaskOrchestratorWhileTrueWithOnlyActivitiesAndContinueAsNew_NoDiagnostic()
{
string code = Wrapper.WrapTaskOrchestrator(@"
public class MyOrchestrator : TaskOrchestrator<object, object>
{
public override async Task<object> RunAsync(TaskOrchestrationContext context, object input)
{
while (true)
{
await context.CallActivityAsync<string>(""DoWork"", ""input"");
await context.CreateTimer(TimeSpan.FromSeconds(30), CancellationToken.None);
context.ContinueAsNew(null);
return null;
}
}
}
");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
}

[Fact]
public async Task TaskOrchestratorNoWhileTrue_NoDiagnostic()
{
string code = Wrapper.WrapTaskOrchestrator(@"
public class MyOrchestrator : TaskOrchestrator<string, string>
{
public override async Task<string> RunAsync(TaskOrchestrationContext context, string input)
{
var result = await context.CallActivityAsync<string>(""DoWork"", input);
return result;
}
}
");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
}

[Fact]
public async Task TaskOrchestratorWhileTrueWithOnlyActivitiesNoExternalEvent_NoDiagnostic()
{
string code = Wrapper.WrapTaskOrchestrator(@"
public class MyOrchestrator : TaskOrchestrator<object, object>
{
public override async Task<object> RunAsync(TaskOrchestrationContext context, object input)
{
while (true)
{
await context.CallActivityAsync<string>(""DoWork"", ""input"");
await context.CreateTimer(TimeSpan.FromSeconds(30), CancellationToken.None);
}
}
}
");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
}

[Fact(Skip = "Requires test infrastructure update to resolve TaskOrchestrationContext symbols correctly in CommonAssemblies")]
public async Task DurableFunctionWhileTrueWithExternalEventNoContinueAsNew_ReportsDiagnostic()
{
string code = Wrapper.WrapDurableFunctionOrchestration(@"
[Function(""Run"")]
async Task<object> Method([OrchestrationTrigger] TaskOrchestrationContext context)
{
while (true)
{
var item = await context.WaitForExternalEvent<string>(""new-work"");
await context.CallActivityAsync<string>(""ProcessItem"", item);
}
}
");
DiagnosticResult expected = BuildDiagnostic().WithArguments("Run");

await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, test => test.CompilerDiagnostics = Microsoft.CodeAnalysis.Testing.CompilerDiagnostics.None, expected);
}

Comment thread
torosent marked this conversation as resolved.
static DiagnosticResult BuildDiagnostic()
{
return new DiagnosticResult(ContinueAsNewOrchestrationAnalyzer.DiagnosticId, Microsoft.CodeAnalysis.DiagnosticSeverity.Warning);
Comment thread
torosent marked this conversation as resolved.
Outdated
}
}
Loading