Skip to content

Commit

Permalink
Orchestration Banned APIs Roslyn analyzer (#309)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
allantargino authored May 13, 2024
1 parent 27d0443 commit 7eef072
Show file tree
Hide file tree
Showing 13 changed files with 555 additions and 2 deletions.
3 changes: 3 additions & 0 deletions src/Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 70 additions & 0 deletions src/Analyzers/KnownTypeSymbols.Azure.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using Microsoft.CodeAnalysis;

namespace Microsoft.DurableTask.Analyzers;

/// <summary>
/// Provides a set of well-known types that are used by the analyzers.
/// Inspired by KnownTypeSymbols class in
/// <see href="https://github.com/dotnet/runtime/blob/2a846acb1a92e811427babe3ff3f047f98c5df02/src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs">System.Text.Json.SourceGeneration</see> 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.
/// </summary>
public sealed partial class KnownTypeSymbols
{
INamedTypeSymbol? blobServiceClient;
INamedTypeSymbol? blobContainerClient;
INamedTypeSymbol? blobClient;
INamedTypeSymbol? queueServiceClient;
INamedTypeSymbol? queueClient;
INamedTypeSymbol? tableServiceClient;
INamedTypeSymbol? tableClient;
INamedTypeSymbol? cosmosClient;
INamedTypeSymbol? sqlConnection;

/// <summary>
/// Gets a BlobServiceClient type symbol.
/// </summary>
public INamedTypeSymbol? BlobServiceClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Blobs.BlobServiceClient", ref this.blobServiceClient);

/// <summary>
/// Gets a BlobContainerClient type symbol.
/// </summary>
public INamedTypeSymbol? BlobContainerClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Blobs.BlobContainerClient", ref this.blobContainerClient);

/// <summary>
/// Gets a BlobClient type symbol.
/// </summary>
public INamedTypeSymbol? BlobClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Blobs.BlobClient", ref this.blobClient);

/// <summary>
/// Gets a QueueServiceClient type symbol.
/// </summary>
public INamedTypeSymbol? QueueServiceClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Queues.QueueServiceClient", ref this.queueServiceClient);

/// <summary>
/// Gets a QueueClient type symbol.
/// </summary>
public INamedTypeSymbol? QueueClient => this.GetOrResolveFullyQualifiedType("Azure.Storage.Queues.QueueClient", ref this.queueClient);

/// <summary>
/// Gets a TableServiceClient type symbol.
/// </summary>
public INamedTypeSymbol? TableServiceClient => this.GetOrResolveFullyQualifiedType("Azure.Data.Tables.TableServiceClient", ref this.tableServiceClient);

/// <summary>
/// Gets a TableClient type symbol.
/// </summary>
public INamedTypeSymbol? TableClient => this.GetOrResolveFullyQualifiedType("Azure.Data.Tables.TableClient", ref this.tableClient);

/// <summary>
/// Gets a CosmosClient type symbol.
/// </summary>
public INamedTypeSymbol? CosmosClient => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Cosmos.CosmosClient", ref this.cosmosClient);

/// <summary>
/// Gets a SqlConnection type symbol.
/// </summary>
public INamedTypeSymbol? SqlConnection => this.GetOrResolveFullyQualifiedType("Microsoft.Data.SqlClient.SqlConnection", ref this.sqlConnection);
}
32 changes: 32 additions & 0 deletions src/Analyzers/KnownTypeSymbols.Net.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/// <summary>
/// Gets a Guid type symbol.
Expand All @@ -39,8 +44,35 @@ public sealed partial class KnownTypeSymbols
/// </summary>
public INamedTypeSymbol? TaskT => this.GetOrResolveFullyQualifiedType(typeof(Task<>).FullName, ref this.taskT);

/// <summary>
/// Gets a TaskFactory type symbol.
/// </summary>
public INamedTypeSymbol? TaskFactory => this.GetOrResolveFullyQualifiedType(typeof(TaskFactory).FullName, ref this.taskFactory);

/// <summary>
/// Gets a TaskFactory&lt;T&gt; type symbol.
/// </summary>
public INamedTypeSymbol? TaskFactoryT => this.GetOrResolveFullyQualifiedType(typeof(TaskFactory<>).FullName, ref this.taskFactoryT);

/// <summary>
/// Gets a TaskContinuationOptions type symbol.
/// </summary>
public INamedTypeSymbol? TaskContinuationOptions => this.GetOrResolveFullyQualifiedType(typeof(TaskContinuationOptions).FullName, ref this.taskContinuationOptions);

/// <summary>
/// Gets a CancellationToken type symbol.
/// </summary>
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.
/// <summary>
/// Gets an Environment type symbol.
/// </summary>
public INamedTypeSymbol? Environment => this.GetOrResolveFullyQualifiedType(typeof(Environment).FullName, ref this.environment);
#pragma warning restore RS1035

/// <summary>
/// Gets an HttpClient type symbol.
/// </summary>
public INamedTypeSymbol? HttpClient => this.GetOrResolveFullyQualifiedType(typeof(HttpClient).FullName, ref this.httpClient);
}
74 changes: 74 additions & 0 deletions src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -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.

/// <summary>
/// Analyzer that reports usage of <see cref="System.Environment"/> APIs in orchestrations.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class EnvironmentOrchestrationAnalyzer : OrchestrationAnalyzer<EnvironmentOrchestrationVisitor>
{
/// <summary>
/// Diagnostic ID supported for the analyzer.
/// </summary>
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);

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

/// <summary>
/// Visitor that inspects the method body for retrievals of Environment Variables through the <see cref="System.Environment" /> type.
/// </summary>
public sealed class EnvironmentOrchestrationVisitor : MethodProbeOrchestrationVisitor
{
/// <inheritdoc/>
public override bool Initialize()
{
return this.KnownTypeSymbols.Environment != null;
}

/// <inheritdoc/>
protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> reportDiagnostic)
{
IOperation? methodOperation = semanticModel.GetOperation(methodSyntax);
if (methodOperation is null)
{
return;
}

foreach (IInvocationOperation invocation in methodOperation.Descendants().OfType<IInvocationOperation>())
{
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));
}
}
}
}
}
91 changes: 91 additions & 0 deletions src/Analyzers/Orchestration/IOOrchestrationAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// Analyzer that reports usage of I/O APIs in orchestrations.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class IOOrchestrationAnalyzer : OrchestrationAnalyzer<IOOrchestrationVisitor>
{
/// <summary>
/// Diagnostic ID supported for the analyzer.
/// </summary>
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);

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

/// <summary>
/// Visitor that inspects the method body for I/O operations by searching for specific types.
/// </summary>
public sealed class IOOrchestrationVisitor : MethodProbeOrchestrationVisitor
{
ImmutableArray<INamedTypeSymbol> bannedTypes;

/// <inheritdoc/>
public override bool Initialize()
{
List<INamedTypeSymbol?> 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;
}

/// <inheritdoc/>
protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> 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));
}
}
}
}
}
}
81 changes: 81 additions & 0 deletions src/Analyzers/Orchestration/ThreadTaskOrchestrationAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// Analyzer that detects usage of non-deterministic <see cref="Thread"/>/<see cref="Task"/> operations in orchestrations.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class ThreadTaskOrchestrationAnalyzer : OrchestrationAnalyzer<ThreadTaskOrchestrationVisitor>
{
/// <summary>
/// Diagnostic ID supported for the analyzer.
/// </summary>
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);

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

/// <summary>
/// Visitor that inspects the orchestration methods for non-deterministic <see cref="Thread"/>/<see cref="Task"/> operations.
/// </summary>
public sealed class ThreadTaskOrchestrationVisitor : MethodProbeOrchestrationVisitor
{
/// <inheritdoc/>
public override bool Initialize()
{
return this.KnownTypeSymbols.Thread != null &&
this.KnownTypeSymbols.Task != null &&
this.KnownTypeSymbols.TaskT != null &&
this.KnownTypeSymbols.TaskFactory != null;
}

/// <inheritdoc/>
protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> 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<IInvocationOperation>())
{
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));
}
}
}
}
}
Loading

0 comments on commit 7eef072

Please sign in to comment.