Skip to content

Commit

Permalink
Attribute binding mismatching Roslyn code fixers (#318)
Browse files Browse the repository at this point in the history
Introduces Roslyn code fixers for the attribute binding mismatching diagnostics.
They essentially change the wrong type for the expected type. The code fixers are for `TaskOrchestrationContext`, `TaskEntityDispatcher` and `DurableTaskClient`. Besides those 3 code fixers, I had to do some small refactoring to our test infrastructure to allow the fixers to be tested.
  • Loading branch information
allantargino authored Jun 5, 2024
1 parent 90bd3fe commit 85ea79d
Show file tree
Hide file tree
Showing 13 changed files with 237 additions and 16 deletions.
1 change: 1 addition & 0 deletions src/Analyzers/Analyzers.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.9.2" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.9.2" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.ResxSourceGenerator" Version="3.11.0-beta1.24165.2" PrivateAssets="all" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Collections.Immutable;
using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;

namespace Microsoft.DurableTask.Analyzers.Functions.AttributeBinding;

/// <summary>
/// Code fixer for the <see cref="DurableClientBindingAnalyzer"/>.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MatchingAttributeBindingFixer))]
[Shared]
public sealed class DurableClientBindingFixer : MatchingAttributeBindingFixer
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => [DurableClientBindingAnalyzer.DiagnosticId];

/// <inheritdoc/>
public override string ExpectedType => "DurableTaskClient";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Collections.Immutable;
using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;

namespace Microsoft.DurableTask.Analyzers.Functions.AttributeBinding;

/// <summary>
/// Code fixer for the <see cref="EntityTriggerBindingAnalyzer"/>.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MatchingAttributeBindingFixer))]
[Shared]
public sealed class EntityTriggerBindingFixer : MatchingAttributeBindingFixer
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => [EntityTriggerBindingAnalyzer.DiagnosticId];

/// <inheritdoc/>
public override string ExpectedType => "TaskEntityDispatcher";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Globalization;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Microsoft.DurableTask.Analyzers.Functions.AttributeBinding;

/// <summary>
/// Base class for code fixers that fix the type of a parameter to match the expected type.
/// </summary>
public abstract class MatchingAttributeBindingFixer : CodeFixProvider
{
/// <summary>
/// Gets the expected type to be used during the code fix.
/// </summary>
public abstract string ExpectedType { get; }

/// <inheritdoc/>
public override FixAllProvider? GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

/// <inheritdoc/>
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken);
if (root == null)
{
return;
}

// Find the parameter syntax node that is causing the diagnostic.
if (root.FindNode(context.Span) is not ParameterSyntax parameterSyntax)
{
return;
}

TypeSyntax? incorrectTypeSyntax = parameterSyntax.Type;
if (incorrectTypeSyntax == null)
{
return;
}

// e.g: "Use 'TaskOrchestrationContext' instead of 'string'"
string title = string.Format(
CultureInfo.InvariantCulture,
Resources.UseInsteadFixerTitle,
this.ExpectedType,
incorrectTypeSyntax.ToString());

context.RegisterCodeFix(
CodeAction.Create(
title: title,
createChangedDocument: _ => ReplaceMismatchedType(context.Document, root, incorrectTypeSyntax, this.ExpectedType),
equivalenceKey: title), // This key is used to prevent duplicate code fixes.
context.Diagnostics);
}

static Task<Document> ReplaceMismatchedType(Document document, SyntaxNode oldRoot, TypeSyntax incorrectTypeSyntax, string expectedType)
{
// Create the correct type syntax by using the identifier name provided by the derived class.
TypeSyntax correctTypeSyntax = SyntaxFactory.IdentifierName(expectedType);

// Replace the old local declaration with the new local declaration.
SyntaxNode newRoot = oldRoot.ReplaceNode(incorrectTypeSyntax, correctTypeSyntax);
Document newDocument = document.WithSyntaxRoot(newRoot);

return Task.FromResult(newDocument);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Collections.Immutable;
using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;

namespace Microsoft.DurableTask.Analyzers.Functions.AttributeBinding;

/// <summary>
/// Code fixer for the <see cref="OrchestrationTriggerBindingAnalyzer"/>.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MatchingAttributeBindingFixer))]
[Shared]
public sealed class OrchestrationTriggerBindingFixer : MatchingAttributeBindingFixer
{
/// <inheritdoc/>
public override ImmutableArray<string> FixableDiagnosticIds => [OrchestrationTriggerBindingAnalyzer.DiagnosticId];

/// <inheritdoc/>
public override string ExpectedType => "TaskOrchestrationContext";
}
3 changes: 3 additions & 0 deletions src/Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,7 @@
<data name="OutputArgumentTypeMismatchAnalyzerTitle" xml:space="preserve">
<value>Activity function call return type doesn't match the function definition return type</value>
</data>
<data name="UseInsteadFixerTitle" xml:space="preserve">
<value>Use '{0}' instead of '{1}'</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding;

public class DurableClientBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests<DurableClientBindingAnalyzer>
public class DurableClientBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests<DurableClientBindingAnalyzer, DurableClientBindingFixer>
{
protected override string ExpectedDiagnosticId => DurableClientBindingAnalyzer.DiagnosticId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding;

public class EntityTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests<EntityTriggerBindingAnalyzer>
public class EntityTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests<EntityTriggerBindingAnalyzer, EntityTriggerBindingFixer>
{
protected override string ExpectedDiagnosticId => EntityTriggerBindingAnalyzer.DiagnosticId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding;

public abstract class MatchingAttributeBindingSpecificationTests<TAnalyzer> where TAnalyzer : MatchingAttributeBindingAnalyzer, new()
public abstract class MatchingAttributeBindingSpecificationTests<TAnalyzer, TCodeFix>
where TAnalyzer : MatchingAttributeBindingAnalyzer, new()
where TCodeFix : MatchingAttributeBindingFixer, new()
{
protected abstract string ExpectedDiagnosticId { get; }
protected abstract string ExpectedAttribute { get; }
Expand Down Expand Up @@ -53,20 +55,31 @@ public async Task ExpectedAttributeWithWrongTypeHasDiag()
void Method({{|#0:{this.ExpectedAttribute} {this.WrongType} paramName|}})
{{
}}
");

string fix = Wrapper.WrapDurableFunctionOrchestration($@"
void Method({{|#0:{this.ExpectedAttribute} {this.ExpectedType} paramName|}})
{{
}}
");

DiagnosticResult expected = this.BuildDiagnostic().WithLocation(0).WithArguments(this.WrongType);

await VerifyAsync(code, expected);
await VerifyCodeFixAsync(code, expected, fix);
}

static async Task VerifyAsync(string source, params DiagnosticResult[] expected)
{
await CSharpAnalyzerVerifier<TAnalyzer>.VerifyDurableTaskAnalyzerAsync(source, expected);
await CSharpCodeFixVerifier<TAnalyzer, TCodeFix>.VerifyDurableTaskAnalyzerAsync(source, expected);
}

static async Task VerifyCodeFixAsync(string source, DiagnosticResult expected, string fix)
{
await CSharpCodeFixVerifier<TAnalyzer, TCodeFix>.VerifyDurableTaskCodeFixAsync(source, expected, fix);
}

DiagnosticResult BuildDiagnostic()
{
return CSharpAnalyzerVerifier<TAnalyzer>.Diagnostic(this.ExpectedDiagnosticId);
return CSharpCodeFixVerifier<TAnalyzer, TCodeFix>.Diagnostic(this.ExpectedDiagnosticId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding;

public class OrchestrationTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests<OrchestrationTriggerBindingAnalyzer>
public class OrchestrationTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests<OrchestrationTriggerBindingAnalyzer, OrchestrationTriggerBindingFixer>
{
protected override string ExpectedDiagnosticId => OrchestrationTriggerBindingAnalyzer.DiagnosticId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,7 @@ public static async Task VerifyDurableTaskAnalyzerAsync(string source, Action<Te
Test test = new()
{
TestCode = source,
ReferenceAssemblies = ReferenceAssemblies.Net.Net60.AddPackages([
new PackageIdentity("Azure.Storage.Blobs", "12.17.0"),
new PackageIdentity("Azure.Storage.Queues", "12.17.0"),
new PackageIdentity("Azure.Data.Tables", "12.8.3"),
new PackageIdentity("Microsoft.Azure.Cosmos", "3.39.1"),
new PackageIdentity("Microsoft.Azure.Functions.Worker", "1.21.0"),
new PackageIdentity("Microsoft.Azure.Functions.Worker.Extensions.DurableTask", "1.1.1"),
new PackageIdentity("Microsoft.Data.SqlClient", "5.2.0"),
]),
ReferenceAssemblies = References.CommonAssemblies,
};

test.ExpectedDiagnostics.AddRange(expected);
Expand Down
47 changes: 47 additions & 0 deletions test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.Durable.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Testing;

namespace Microsoft.DurableTask.Analyzers.Tests.Verifiers;

public static partial class CSharpCodeFixVerifier<TAnalyzer, TCodeFix>
where TAnalyzer : DiagnosticAnalyzer, new()
where TCodeFix : CodeFixProvider, new()
{
public static Task VerifyDurableTaskAnalyzerAsync(string source, params DiagnosticResult[] expected)
{
return VerifyDurableTaskAnalyzerAsync(source, null, expected);
}

public static async Task VerifyDurableTaskAnalyzerAsync(string source, Action<Test>? configureTest = null, params DiagnosticResult[] expected)
{
await RunAsync(expected, new Test()
{
TestCode = source,
}, configureTest);
}

public static Task VerifyDurableTaskCodeFixAsync(string source, DiagnosticResult expected, string fixedSource)
{
return VerifyDurableTaskCodeFixAsync(source, [expected], fixedSource);
}

public static async Task VerifyDurableTaskCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource)
{
await RunAsync(expected, new Test()
{
TestCode = source,
FixedCode = fixedSource,
});
}

static async Task RunAsync(DiagnosticResult[] expected, Test test, Action<Test>? configureTest = null)
{
test.ReferenceAssemblies = References.CommonAssemblies;
test.ExpectedDiagnostics.AddRange(expected);

configureTest?.Invoke(test);

await test.RunAsync(CancellationToken.None);
}
}
23 changes: 23 additions & 0 deletions test/Analyzers.Tests/Verifiers/References.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using Microsoft.CodeAnalysis.Testing;

namespace Microsoft.DurableTask.Analyzers.Tests.Verifiers;

public static class References
{
static readonly Lazy<ReferenceAssemblies> durableAssemblyReferences = new(() => BuildReferenceAssemblies());

public static ReferenceAssemblies CommonAssemblies => durableAssemblyReferences.Value;

static ReferenceAssemblies BuildReferenceAssemblies() => ReferenceAssemblies.Net.Net60.AddPackages([
new PackageIdentity("Azure.Storage.Blobs", "12.17.0"),
new PackageIdentity("Azure.Storage.Queues", "12.17.0"),
new PackageIdentity("Azure.Data.Tables", "12.8.3"),
new PackageIdentity("Microsoft.Azure.Cosmos", "3.39.1"),
new PackageIdentity("Microsoft.Azure.Functions.Worker", "1.21.0"),
new PackageIdentity("Microsoft.Azure.Functions.Worker.Extensions.DurableTask", "1.1.1"),
new PackageIdentity("Microsoft.Data.SqlClient", "5.2.0"),
]);
}

0 comments on commit 85ea79d

Please sign in to comment.