Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Orchestration Banned APIs Roslyn analyzer #309

Merged
merged 5 commits into from
May 13, 2024
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
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.
allantargino marked this conversation as resolved.
Show resolved Hide resolved
/// <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
allantargino marked this conversation as resolved.
Show resolved Hide resolved
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
Loading