From 7eef072e11fa4d4f95bc4c198b7d24828c967525 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Mon, 13 May 2024 16:31:33 -0300 Subject: [PATCH] Orchestration Banned APIs Roslyn analyzer (#309) This PR adds 3 analyzers that verify whether orchestrations are using banned APIs through: - Task.Run/Thread.Start/TaskFactory (`DURABLE0004`) - Input/Output types (such as HttpClient, etc) (`DURABLE0005`) - Environment Variables (`DURABLE0006`) The list of I/O types is by no means comprehensive or complete, but rather an initial set of well-known types used in Azure Functions - we should augment that list soon. I tried to have feature parity with the [types covered by our previous analyzer](https://github.com/Azure/azure-functions-durable-extension/blob/eee1aee25d4609e00e113b73a711b262105945a3/src/WebJobs.Extensions.DurableTask.Analyzers/Analyzers/Orchestrator/IOTypesAnalyzer.cs#L53). --- src/Analyzers/AnalyzerReleases.Unshipped.md | 3 + src/Analyzers/KnownTypeSymbols.Azure.cs | 70 ++++++++++++++ src/Analyzers/KnownTypeSymbols.Net.cs | 32 +++++++ .../EnvironmentOrchestrationAnalyzer.cs | 74 +++++++++++++++ .../Orchestration/IOOrchestrationAnalyzer.cs | 91 +++++++++++++++++++ .../ThreadTaskOrchestrationAnalyzer.cs | 81 +++++++++++++++++ src/Analyzers/Resources.resx | 18 ++++ src/Analyzers/RoslynExtensions.cs | 3 +- .../EnvironmentOrchestrationAnalyzerTests.cs | 49 ++++++++++ .../Orchestration/IOOrchestrationTests.cs | 64 +++++++++++++ .../ThreadTaskOrchestrationAnalyzerTests.cs | 59 ++++++++++++ .../CSharpAnalyzerVerifier.Durable.cs | 7 +- test/Analyzers.Tests/Wrapper.cs | 6 ++ 13 files changed, 555 insertions(+), 2 deletions(-) create mode 100644 src/Analyzers/KnownTypeSymbols.Azure.cs create mode 100644 src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs create mode 100644 src/Analyzers/Orchestration/IOOrchestrationAnalyzer.cs create mode 100644 src/Analyzers/Orchestration/ThreadTaskOrchestrationAnalyzer.cs create mode 100644 test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs create mode 100644 test/Analyzers.Tests/Orchestration/IOOrchestrationTests.cs create mode 100644 test/Analyzers.Tests/Orchestration/ThreadTaskOrchestrationAnalyzerTests.cs diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index 690cd6a4..c2109dea 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -8,6 +8,9 @@ Rule ID | Category | Severity | Notes DURABLE0001 | Orchestration | Warning | DateTimeOrchestrationAnalyzer DURABLE0002 | Orchestration | Warning | GuidOrchestrationAnalyzer DURABLE0003 | Orchestration | Warning | DelayOrchestrationAnalyzer +DURABLE0004 | Orchestration | Warning | ThreadTaskOrchestrationAnalyzer +DURABLE0005 | Orchestration | Warning | IOOrchestrationAnalyzer +DURABLE0006 | Orchestration | Warning | EnvironmentOrchestrationAnalyzer DURABLE0007 | Orchestration | Warning | CancellationTokenOrchestrationAnalyzer DURABLE0008 | Orchestration | Warning | OtherBindingsOrchestrationAnalyzer DURABLE1001 | Attribute Binding | Error | OrchestrationTriggerBindingAnalyzer diff --git a/src/Analyzers/KnownTypeSymbols.Azure.cs b/src/Analyzers/KnownTypeSymbols.Azure.cs new file mode 100644 index 00000000..fb0ce41b --- /dev/null +++ b/src/Analyzers/KnownTypeSymbols.Azure.cs @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.DurableTask.Analyzers; + +/// +/// Provides a set of well-known types that are used by the analyzers. +/// Inspired by KnownTypeSymbols class in +/// System.Text.Json.SourceGeneration source code. +/// Lazy initialization is used to avoid the the initialization of all types during class construction, since not all symbols are used by all analyzers. +/// +public sealed partial class KnownTypeSymbols +{ + INamedTypeSymbol? blobServiceClient; + INamedTypeSymbol? blobContainerClient; + INamedTypeSymbol? blobClient; + INamedTypeSymbol? queueServiceClient; + INamedTypeSymbol? queueClient; + INamedTypeSymbol? tableServiceClient; + INamedTypeSymbol? tableClient; + INamedTypeSymbol? cosmosClient; + INamedTypeSymbol? sqlConnection; + + /// + /// Gets a BlobServiceClient type symbol. + /// + public INamedTypeSymbol? BlobServiceClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Blobs.BlobServiceClient", ref this.blobServiceClient); + + /// + /// Gets a BlobContainerClient type symbol. + /// + public INamedTypeSymbol? BlobContainerClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Blobs.BlobContainerClient", ref this.blobContainerClient); + + /// + /// Gets a BlobClient type symbol. + /// + public INamedTypeSymbol? BlobClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Blobs.BlobClient", ref this.blobClient); + + /// + /// Gets a QueueServiceClient type symbol. + /// + public INamedTypeSymbol? QueueServiceClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Queues.QueueServiceClient", ref this.queueServiceClient); + + /// + /// Gets a QueueClient type symbol. + /// + public INamedTypeSymbol? QueueClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Queues.QueueClient", ref this.queueClient); + + /// + /// Gets a TableServiceClient type symbol. + /// + public INamedTypeSymbol? TableServiceClient => this.GetOrResolveFullyQualifiedType("Azure.Data.Tables.TableServiceClient", ref this.tableServiceClient); + + /// + /// Gets a TableClient type symbol. + /// + public INamedTypeSymbol? TableClient => this.GetOrResolveFullyQualifiedType("Azure.Data.Tables.TableClient", ref this.tableClient); + + /// + /// Gets a CosmosClient type symbol. + /// + public INamedTypeSymbol? CosmosClient => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Cosmos.CosmosClient", ref this.cosmosClient); + + /// + /// Gets a SqlConnection type symbol. + /// + public INamedTypeSymbol? SqlConnection => this.GetOrResolveFullyQualifiedType("Microsoft.Data.SqlClient.SqlConnection", ref this.sqlConnection); +} diff --git a/src/Analyzers/KnownTypeSymbols.Net.cs b/src/Analyzers/KnownTypeSymbols.Net.cs index 1e2ebc4a..9803273a 100644 --- a/src/Analyzers/KnownTypeSymbols.Net.cs +++ b/src/Analyzers/KnownTypeSymbols.Net.cs @@ -17,7 +17,12 @@ public sealed partial class KnownTypeSymbols INamedTypeSymbol? thread; INamedTypeSymbol? task; INamedTypeSymbol? taskT; + INamedTypeSymbol? taskFactory; + INamedTypeSymbol? taskContinuationOptions; + INamedTypeSymbol? taskFactoryT; INamedTypeSymbol? cancellationToken; + INamedTypeSymbol? environment; + INamedTypeSymbol? httpClient; /// /// Gets a Guid type symbol. @@ -39,8 +44,35 @@ public sealed partial class KnownTypeSymbols /// public INamedTypeSymbol? TaskT => this.GetOrResolveFullyQualifiedType(typeof(Task<>).FullName, ref this.taskT); + /// + /// Gets a TaskFactory type symbol. + /// + public INamedTypeSymbol? TaskFactory => this.GetOrResolveFullyQualifiedType(typeof(TaskFactory).FullName, ref this.taskFactory); + + /// + /// Gets a TaskFactory<T> type symbol. + /// + public INamedTypeSymbol? TaskFactoryT => this.GetOrResolveFullyQualifiedType(typeof(TaskFactory<>).FullName, ref this.taskFactoryT); + + /// + /// Gets a TaskContinuationOptions type symbol. + /// + public INamedTypeSymbol? TaskContinuationOptions => this.GetOrResolveFullyQualifiedType(typeof(TaskContinuationOptions).FullName, ref this.taskContinuationOptions); + /// /// Gets a CancellationToken type symbol. /// public INamedTypeSymbol? CancellationToken => this.GetOrResolveFullyQualifiedType(typeof(CancellationToken).FullName, ref this.cancellationToken); + +#pragma warning disable RS1035 // Environment Variables are not supposed to be used in Analyzers, but here we just reference the API, never using it. + /// + /// Gets an Environment type symbol. + /// + public INamedTypeSymbol? Environment => this.GetOrResolveFullyQualifiedType(typeof(Environment).FullName, ref this.environment); +#pragma warning restore RS1035 + + /// + /// Gets an HttpClient type symbol. + /// + public INamedTypeSymbol? HttpClient => this.GetOrResolveFullyQualifiedType(typeof(HttpClient).FullName, ref this.httpClient); } diff --git a/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs new file mode 100644 index 00000000..3e4fb773 --- /dev/null +++ b/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using static Microsoft.DurableTask.Analyzers.Orchestration.EnvironmentOrchestrationAnalyzer; + +namespace Microsoft.DurableTask.Analyzers.Orchestration; + +#pragma warning disable RS1035 // Environment Variables are not supposed to be used in Analyzers, but here we just reference the API, never using it. + +/// +/// Analyzer that reports usage of APIs in orchestrations. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class EnvironmentOrchestrationAnalyzer : OrchestrationAnalyzer +{ + /// + /// Diagnostic ID supported for the analyzer. + /// + public const string DiagnosticId = "DURABLE0006"; + + static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.EnvironmentOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.EnvironmentOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); + + static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + Title, + MessageFormat, + AnalyzersCategories.Orchestration, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => [Rule]; + + /// + /// Visitor that inspects the method body for retrievals of Environment Variables through the type. + /// + public sealed class EnvironmentOrchestrationVisitor : MethodProbeOrchestrationVisitor + { + /// + public override bool Initialize() + { + return this.KnownTypeSymbols.Environment != null; + } + + /// + protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + IOperation? methodOperation = semanticModel.GetOperation(methodSyntax); + if (methodOperation is null) + { + return; + } + + foreach (IInvocationOperation invocation in methodOperation.Descendants().OfType()) + { + IMethodSymbol targetMethod = invocation.TargetMethod; + + if (targetMethod.ContainingType.Equals(this.KnownTypeSymbols.Environment, SymbolEqualityComparer.Default) && + targetMethod.Name is nameof(Environment.GetEnvironmentVariable) or nameof(Environment.GetEnvironmentVariables) or nameof(Environment.ExpandEnvironmentVariables)) + { + string invocationName = targetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + + // e.g.: "The method 'Method1' uses environment variables through 'Environment.GetEnvironmentVariable()' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, invocation, methodSymbol.Name, invocationName, orchestrationName)); + } + } + } + } +} diff --git a/src/Analyzers/Orchestration/IOOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/IOOrchestrationAnalyzer.cs new file mode 100644 index 00000000..6051ccc5 --- /dev/null +++ b/src/Analyzers/Orchestration/IOOrchestrationAnalyzer.cs @@ -0,0 +1,91 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +using static Microsoft.DurableTask.Analyzers.Orchestration.IOOrchestrationAnalyzer; + +namespace Microsoft.DurableTask.Analyzers.Orchestration; + +/// +/// Analyzer that reports usage of I/O APIs in orchestrations. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class IOOrchestrationAnalyzer : OrchestrationAnalyzer +{ + /// + /// Diagnostic ID supported for the analyzer. + /// + public const string DiagnosticId = "DURABLE0005"; + + static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.IOOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.IOOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); + + static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + Title, + MessageFormat, + AnalyzersCategories.Orchestration, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => [Rule]; + + /// + /// Visitor that inspects the method body for I/O operations by searching for specific types. + /// + public sealed class IOOrchestrationVisitor : MethodProbeOrchestrationVisitor + { + ImmutableArray bannedTypes; + + /// + public override bool Initialize() + { + List candidateSymbols = [ + this.KnownTypeSymbols.HttpClient, + this.KnownTypeSymbols.BlobServiceClient, + this.KnownTypeSymbols.BlobContainerClient, + this.KnownTypeSymbols.BlobClient, + this.KnownTypeSymbols.QueueServiceClient, + this.KnownTypeSymbols.QueueClient, + this.KnownTypeSymbols.TableServiceClient, + this.KnownTypeSymbols.TableClient, + this.KnownTypeSymbols.CosmosClient, + this.KnownTypeSymbols.SqlConnection, + ]; + + // filter out null values, since some of them may not be available during compilation: + this.bannedTypes = candidateSymbols.Where(s => s is not null).ToImmutableArray()!; + + return this.bannedTypes.Length > 0; + } + + /// + protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + IOperation? methodOperation = semanticModel.GetOperation(methodSyntax); + if (methodOperation is null) + { + return; + } + + foreach (IOperation operation in methodOperation.Descendants()) + { + if (operation.Type is not null) + { + if (this.bannedTypes.Contains(operation.Type, SymbolEqualityComparer.Default)) + { + string typeName = operation.Type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + + // e.g.: "The method 'Method1' performs I/O through 'HttpClient' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation, methodSymbol.Name, typeName, orchestrationName)); + } + } + } + } + } +} diff --git a/src/Analyzers/Orchestration/ThreadTaskOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/ThreadTaskOrchestrationAnalyzer.cs new file mode 100644 index 00000000..8d640ed1 --- /dev/null +++ b/src/Analyzers/Orchestration/ThreadTaskOrchestrationAnalyzer.cs @@ -0,0 +1,81 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using static Microsoft.DurableTask.Analyzers.Orchestration.ThreadTaskOrchestrationAnalyzer; + +namespace Microsoft.DurableTask.Analyzers.Orchestration; + +/// +/// Analyzer that detects usage of non-deterministic / operations in orchestrations. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ThreadTaskOrchestrationAnalyzer : OrchestrationAnalyzer +{ + /// + /// Diagnostic ID supported for the analyzer. + /// + public const string DiagnosticId = "DURABLE0004"; + + static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ThreadTaskOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.ThreadTaskOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); + + static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + Title, + MessageFormat, + AnalyzersCategories.Orchestration, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => [Rule]; + + /// + /// Visitor that inspects the orchestration methods for non-deterministic / operations. + /// + public sealed class ThreadTaskOrchestrationVisitor : MethodProbeOrchestrationVisitor + { + /// + public override bool Initialize() + { + return this.KnownTypeSymbols.Thread != null && + this.KnownTypeSymbols.Task != null && + this.KnownTypeSymbols.TaskT != null && + this.KnownTypeSymbols.TaskFactory != null; + } + + /// + protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + IOperation? methodOperation = semanticModel.GetOperation(methodSyntax); + if (methodOperation is null) + { + return; + } + + // reports usage of Thread.Start, Task.Run, Task.ContinueWith and Task.Factory.StartNew + foreach (IInvocationOperation invocation in methodOperation.Descendants().OfType()) + { + IMethodSymbol targetMethod = invocation.TargetMethod; + + if (targetMethod.IsEqualTo(this.KnownTypeSymbols.Thread, nameof(Thread.Start)) || + targetMethod.IsEqualTo(this.KnownTypeSymbols.Task, nameof(Task.Run)) || + targetMethod.IsEqualTo(this.KnownTypeSymbols.TaskT, nameof(Task.Run)) || + targetMethod.IsEqualTo(this.KnownTypeSymbols.Task, nameof(Task.ContinueWith)) || + targetMethod.IsEqualTo(this.KnownTypeSymbols.TaskT, nameof(Task.ContinueWith)) || + targetMethod.IsEqualTo(this.KnownTypeSymbols.TaskFactory, nameof(TaskFactory.StartNew)) || + targetMethod.IsEqualTo(this.KnownTypeSymbols.TaskFactoryT, nameof(TaskFactory.StartNew))) + { + string invocationName = targetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + + // e.g.: "The method 'Method1' uses non-deterministic Threads/Tasks operations by the invocation of 'Thread.Start' in orchestration 'MyOrchestrator'" + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, invocation, methodSymbol.Name, invocationName, orchestrationName)); + } + } + } + } +} diff --git a/src/Analyzers/Resources.resx b/src/Analyzers/Resources.resx index 7f3481a0..6af3d76f 100644 --- a/src/Analyzers/Resources.resx +++ b/src/Analyzers/Resources.resx @@ -165,4 +165,22 @@ OrchestrationTrigger methods must not use any other bindings + + The method '{0}' uses environment variables through '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}' + + + Environment variables must not be accessed inside an orchestrator function + + + The method '{0}' performs I/O through '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}' + + + I/O operations are not allowed inside an orchestrator function + + + The method '{0}' uses non-deterministic Threads/Tasks operations by the invocation of '{1}' in orchestration '{2}' + + + Thread and Task calls must be deterministic inside an orchestrator function + \ No newline at end of file diff --git a/src/Analyzers/RoslynExtensions.cs b/src/Analyzers/RoslynExtensions.cs index 9ecaba5f..e1920524 100644 --- a/src/Analyzers/RoslynExtensions.cs +++ b/src/Analyzers/RoslynExtensions.cs @@ -118,7 +118,8 @@ public static IEnumerable GetSyntaxNodes(this IMethodSy /// True if the method symbol is contained in the type symbol and has the method name, false otherwise. public static bool IsEqualTo(this IMethodSymbol methodSymbol, INamedTypeSymbol? typeSymbol, string methodName) { - return methodSymbol.ContainingType.Equals(typeSymbol, SymbolEqualityComparer.Default) && + return (methodSymbol.ContainingType.Equals(typeSymbol, SymbolEqualityComparer.Default) || + methodSymbol.ContainingType.OriginalDefinition.Equals(typeSymbol, SymbolEqualityComparer.Default)) && methodSymbol.Name.Equals(methodName, StringComparison.Ordinal); } diff --git a/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs new file mode 100644 index 00000000..55c2113a --- /dev/null +++ b/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs @@ -0,0 +1,49 @@ +// 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; + +namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration; + +public class EnvironmentOrchestrationAnalyzerTest +{ + [Fact] + public async Task EmptyCodeHasNoDiag() + { + string code = @""; + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task GettingEnvironmentVariablesAreNotAllowedInAzureFunctionsOrchestrations() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context) +{ + {|#0:Environment.GetEnvironmentVariable(""PATH"")|}; + {|#1:Environment.GetEnvironmentVariables()|}; + {|#2:Environment.ExpandEnvironmentVariables(""PATH"")|}; +} +"); + string[] methods = [ + "Environment.GetEnvironmentVariable(string)", + "Environment.GetEnvironmentVariables()", + "Environment.ExpandEnvironmentVariables(string)", + ]; + + DiagnosticResult[] expected = methods.Select( + (method, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", method, "Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + static DiagnosticResult BuildDiagnostic() + { + return VerifyCS.Diagnostic(EnvironmentOrchestrationAnalyzer.DiagnosticId); + } +} \ No newline at end of file diff --git a/test/Analyzers.Tests/Orchestration/IOOrchestrationTests.cs b/test/Analyzers.Tests/Orchestration/IOOrchestrationTests.cs new file mode 100644 index 00000000..249fd89f --- /dev/null +++ b/test/Analyzers.Tests/Orchestration/IOOrchestrationTests.cs @@ -0,0 +1,64 @@ +// 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; + +namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration; + +public class IOOrchestrationTests +{ + [Fact] + public async Task EmptyCodeHasNoDiag() + { + string code = @""; + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task IOTypesAreBannedWithinAzureFunctionOrchestrations() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context) +{ + var http1 = {|#0:new HttpClient()|}; + + var blob1 = {|#1:new BlobServiceClient(""test"")|}; + var blob2 = {|#2:new BlobContainerClient(""test"",""test"")|}; + var blob3 = {|#3:new BlobClient(""test"",""test"",""test"")|}; + + var queue1 = {|#4:new QueueServiceClient(""test"")|}; + var queue2 = {|#5:new QueueClient(""test"",""test"")|}; + + var table1 = {|#6:new TableServiceClient(""test"")|}; + var table2 = {|#7:new TableClient(""test"",""test"")|}; + + var cosmos1 = {|#8:new CosmosClient(""test"")|}; + + var sql1 = {|#9:new SqlConnection()|}; +} +"); + string[] types = [ + "HttpClient", + "BlobServiceClient", "BlobContainerClient", "BlobClient", + "QueueServiceClient", "QueueClient", + "TableServiceClient", "TableClient", + "CosmosClient", + "SqlConnection", + ]; + + DiagnosticResult[] expected = types.Select( + (type, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", type, "Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + static DiagnosticResult BuildDiagnostic() + { + return VerifyCS.Diagnostic(IOOrchestrationAnalyzer.DiagnosticId); + } +} diff --git a/test/Analyzers.Tests/Orchestration/ThreadTaskOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/ThreadTaskOrchestrationAnalyzerTests.cs new file mode 100644 index 00000000..0021e6e4 --- /dev/null +++ b/test/Analyzers.Tests/Orchestration/ThreadTaskOrchestrationAnalyzerTests.cs @@ -0,0 +1,59 @@ +// 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; + +namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration; + +public class ThreadTaskOrchestrationAnalyzerTests +{ + [Fact] + public async Task EmptyCodeHasNoDiag() + { + string code = @""; + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task StartingThreadsTasksAreBannedWithinAzureFunctionOrchestrations() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +async Task Method([OrchestrationTrigger] TaskOrchestrationContext context) +{ + {|#0:new Thread(() => { }).Start()|}; + + Task t1 = {|#1:Task.Run(() => 0)|}; + await {|#2:t1.ContinueWith(task => 0)|}; + await {|#3:Task.Factory.StartNew(() => 0)|}; + + Task t2 = {|#4:Task.Run(() => { })|}; + await {|#5:t2.ContinueWith(task => { })|}; + await {|#6:Task.Factory.StartNew(() => { })|}; +} +"); + string[] invocations = [ + "Thread.Start()", + "Task.Run(Func)", + "Task.ContinueWith(Func, int>)", + "TaskFactory.StartNew(Func)", + "Task.Run(Action)", + "Task.ContinueWith(Action)", + "TaskFactory.StartNew(Action)", + ]; + + DiagnosticResult[] expected = invocations.Select( + (invocation, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", invocation, "Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + static DiagnosticResult BuildDiagnostic() + { + return VerifyCS.Diagnostic(ThreadTaskOrchestrationAnalyzer.DiagnosticId); + } +} diff --git a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs index 6d035e77..71939778 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs @@ -22,8 +22,13 @@ public static async Task VerifyDurableTaskAnalyzerAsync(string source, Action