Task Delay/Thread Sleep Roslyn Analyzer (#297)

src/Analyzers/AnalyzerReleases.Unshipped.md

@@ -6,4 +6,5 @@
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
 DURABLE0001 | Orchestration | Warning | DateTimeOrchestrationAnalyzer
-DURABLE0002 | Orchestration | Warning | GuidOrchestrationAnalyzer
+DURABLE0002 | Orchestration | Warning | GuidOrchestrationAnalyzer
+DURABLE0003 | Orchestration | Warning | DelayOrchestrationAnalyzer

src/Analyzers/KnownTypeSymbols.cs

@@ -21,6 +21,9 @@ sealed class KnownTypeSymbols(Compilation compilation)
     INamedTypeSymbol? taskOrchestratorBaseClass;
     INamedTypeSymbol? durableTaskRegistry;
     INamedTypeSymbol? guid;
+    INamedTypeSymbol? thread;
+    INamedTypeSymbol? task;
+    INamedTypeSymbol? taskT;
 
     /// <summary>
     /// Gets an OrchestrationTriggerAttribute type symbol.
@@ -52,6 +55,21 @@ sealed class KnownTypeSymbols(Compilation compilation)
     /// </summary>
     public INamedTypeSymbol? GuidType => this.GetOrResolveFullyQualifiedType(typeof(Guid).FullName, ref this.guid);
 
+    /// <summary>
+    /// Gets a Thread type symbol.
+    /// </summary>
+    public INamedTypeSymbol? Thread => this.GetOrResolveFullyQualifiedType(typeof(Thread).FullName, ref this.thread);
+
+    /// <summary>
+    /// Gets a Task type symbol.
+    /// </summary>
+    public INamedTypeSymbol? Task => this.GetOrResolveFullyQualifiedType(typeof(Task).FullName, ref this.task);
+
+    /// <summary>
+    /// Gets a Task&lt;T&gt; type symbol.
+    /// </summary>
+    public INamedTypeSymbol? TaskT => this.GetOrResolveFullyQualifiedType(typeof(Task<>).FullName, ref this.taskT);
+
     INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref INamedTypeSymbol? field)
     {
         if (field != null)

src/Analyzers/Orchestration/DelayOrchestrationAnalyzer.cs

@@ -0,0 +1,109 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Collections.Concurrent;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.DurableTask.Analyzers.Orchestration;
+
+/// <summary>
+/// Analyzer that reports a warning when Task.Delay or Thread.Sleep is used in an orchestration method.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class DelayOrchestrationAnalyzer : OrchestrationAnalyzer
+{
+    /// <summary>
+    /// Diagnostic ID supported for the analyzer.
+    /// </summary>
+    public const string DiagnosticId = "DURABLE0003";
+
+    static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.DelayOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
+    static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.DelayOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));
+
+    static readonly DiagnosticDescriptor Rule = new(
+        DiagnosticId,
+        Title,
+        MessageFormat,
+        AnalyzersCategories.Orchestration,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    /// <inheritdoc/>
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
+
+    /// <inheritdoc/>
+    protected override void RegisterAdditionalCompilationStartAction(CompilationStartAnalysisContext context, OrchestrationAnalysisResult orchestrationAnalysisResult)
+    {
+        var knownSymbols = new KnownTypeSymbols(context.Compilation);
+
+        if (knownSymbols.Thread == null || knownSymbols.Task == null || knownSymbols.TaskT == null)
+        {
+            return;
+        }
+
+        ConcurrentBag<(ISymbol Symbol, IInvocationOperation Invocation)> delayUsage = [];
+
+        context.RegisterOperationAction(
+            ctx =>
+            {
+                ctx.CancellationToken.ThrowIfCancellationRequested();
+
+                IInvocationOperation invocation = (IInvocationOperation)ctx.Operation;
+
+                if (!invocation.TargetMethod.ContainingType.Equals(knownSymbols.Thread, SymbolEqualityComparer.Default))
+                {
+                    return;
+                }
+
+                if (invocation.TargetMethod.Name is nameof(Thread.Sleep))
+                {
+                    ISymbol method = ctx.ContainingSymbol;
+                    delayUsage.Add((method, invocation));
+                }
+            },
+            OperationKind.Invocation);
+
+        context.RegisterOperationAction(
+            ctx =>
+            {
+                ctx.CancellationToken.ThrowIfCancellationRequested();
+
+                IInvocationOperation invocation = (IInvocationOperation)ctx.Operation;
+
+                if (!invocation.TargetMethod.ContainingType.Equals(knownSymbols.Task, SymbolEqualityComparer.Default) &&
+                    !invocation.TargetMethod.ContainingType.Equals(knownSymbols.TaskT, SymbolEqualityComparer.Default))
+                {
+                    return;
+                }
+
+                if (invocation.TargetMethod.Name is nameof(Task.Delay))
+                {
+                    ISymbol method = ctx.ContainingSymbol;
+                    delayUsage.Add((method, invocation));
+                }
+            },
+            OperationKind.Invocation);
+
+        context.RegisterCompilationEndAction(ctx =>
+        {
+            foreach ((ISymbol symbol, IInvocationOperation operation) in delayUsage)
+            {
+                if (symbol is IMethodSymbol method)
+                {
+                    if (orchestrationAnalysisResult.OrchestrationsByMethod.TryGetValue(method, out ConcurrentBag<AnalyzedOrchestration> orchestrations))
+                    {
+                        string methodName = symbol.Name;
+                        string invocationName = operation.TargetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);
+                        string orchestrationNames = string.Join(", ", orchestrations.Select(o => o.Name).OrderBy(n => n));
+
+                        // e.g.: "The method 'Method1' uses 'Thread.Sleep(int)' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'"
+                        ctx.ReportDiagnostic(Rule, operation, methodName, invocationName, orchestrationNames);
+                    }
+                }
+            }
+        });
+    }
+}

src/Analyzers/Resources.resx

@@ -129,4 +129,10 @@
   <data name="GuidOrchestrationAnalyzerTitle" xml:space="preserve">
     <value>System.Guid calls must be deterministic inside an orchestration</value>
   </data>
+  <data name="DelayOrchestrationAnalyzerMessageFormat" xml:space="preserve">
+    <value>The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}'</value>
+  </data>
+  <data name="DelayOrchestrationAnalyzerTitle" xml:space="preserve">
+    <value>Thread.Sleep and Task.Delay calls are not allowed inside an orchestrator</value>
+  </data>
 </root>

test/Analyzers.Tests/Orchestration/DelayOrchestrationAnalyzerTests.cs

@@ -0,0 +1,108 @@
+// 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.DelayOrchestrationAnalyzer>;
+
+namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration;
+
+public class DelayOrchestrationAnalyzerTests
+{
+    [Fact]
+    public async Task EmptyCodeWithSymbolsAvailableHasNoDiag()
+    {
+        string code = @"";
+
+        await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
+    }
+
+    [Fact]
+    public async Task DurableFunctionOrchestrationUsingThreadSleepHasDiag()
+    {
+        string code = Wrapper.WrapDurableFunctionOrchestration(@"
+[Function(""Run"")]
+void Method([OrchestrationTrigger] TaskOrchestrationContext context)
+{
+    {|#0:Thread.Sleep(1000)|};
+}
+");
+        DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "Thread.Sleep(int)", "Run");
+
+        await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
+    }
+
+    [Fact]
+    public async Task DurableFunctionOrchestrationUsingTaskDelayHasDiag()
+    {
+        string code = Wrapper.WrapDurableFunctionOrchestration(@"
+[Function(""Run"")]
+async Task Method([OrchestrationTrigger] TaskOrchestrationContext context)
+{
+    await {|#0:Task.Delay(1000)|};
+}
+");
+        DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "Task.Delay(int)", "Run");
+
+        await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
+    }
+
+    [Fact]
+    public async Task DurableFunctionOrchestrationUsingTaskTDelayHasDiag()
+    {
+        string code = Wrapper.WrapDurableFunctionOrchestration(@"
+[Function(""Run"")]
+async Task Method([OrchestrationTrigger] TaskOrchestrationContext context)
+{
+    await {|#0:Task<object>.Delay(1000)|};
+}
+");
+        DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "Task.Delay(int)", "Run");
+
+        await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
+    }
+
+    [Fact]
+    public async Task TaskOrchestratorUsingThreadSleepHasDiag()
+    {
+        string code = Wrapper.WrapTaskOrchestrator(@"
+public class MyOrchestrator : TaskOrchestrator<object, object>
+{
+    public override Task<object> RunAsync(TaskOrchestrationContext context, object input)
+    {
+        Method();
+        return Task.FromResult(input);
+    }
+
+    private void Method() {
+        {|#0:Thread.Sleep(1000)|};
+    }
+}
+");
+
+        DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "Thread.Sleep(int)", "MyOrchestrator");
+
+        await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
+    }
+
+    [Fact]
+    public async Task FuncOrchestratorUsingThreadSleepHasDiag()
+    {
+        string code = Wrapper.WrapFuncOrchestrator(@"
+tasks.AddOrchestratorFunc(""HelloSequence"", context =>
+{
+    {|#0:Thread.Sleep(1000)|};
+});
+");
+
+        DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Main", "Thread.Sleep(int)", "HelloSequence");
+
+        await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
+    }
+
+    static DiagnosticResult BuildDiagnostic()
+    {
+        return VerifyCS.Diagnostic(DelayOrchestrationAnalyzer.DiagnosticId);
+    }
+}