From 85ea79d5f92171d9d7368469a106e9e88b2c5260 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 5 Jun 2024 11:36:30 -0300 Subject: [PATCH] Attribute binding mismatching Roslyn code fixers (#318) 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. --- src/Analyzers/Analyzers.csproj | 1 + .../DurableClientBindingFixer.cs | 23 ++++++ .../EntityTriggerBindingFixer.cs | 23 ++++++ .../MatchingAttributeBindingFixer.cs | 73 +++++++++++++++++++ .../OrchestrationTriggerBindingFixer.cs | 23 ++++++ src/Analyzers/Resources.resx | 3 + .../DurableClientBindingAnalyzerTests.cs | 2 +- .../EntityTriggerBindingAnalyzerTests.cs | 2 +- ...chingAttributeBindingSpecificationTests.cs | 21 +++++- ...rchestrationTriggerBindingAnalyzerTests.cs | 2 +- .../CSharpAnalyzerVerifier.Durable.cs | 10 +-- .../CSharpCodeFixVerifier.Durable.cs | 47 ++++++++++++ test/Analyzers.Tests/Verifiers/References.cs | 23 ++++++ 13 files changed, 237 insertions(+), 16 deletions(-) create mode 100644 src/Analyzers/Functions/AttributeBinding/DurableClientBindingFixer.cs create mode 100644 src/Analyzers/Functions/AttributeBinding/EntityTriggerBindingFixer.cs create mode 100644 src/Analyzers/Functions/AttributeBinding/MatchingAttributeBindingFixer.cs create mode 100644 src/Analyzers/Functions/AttributeBinding/OrchestrationTriggerBindingFixer.cs create mode 100644 test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.Durable.cs create mode 100644 test/Analyzers.Tests/Verifiers/References.cs diff --git a/src/Analyzers/Analyzers.csproj b/src/Analyzers/Analyzers.csproj index c9245547..20a462b6 100644 --- a/src/Analyzers/Analyzers.csproj +++ b/src/Analyzers/Analyzers.csproj @@ -31,6 +31,7 @@ + diff --git a/src/Analyzers/Functions/AttributeBinding/DurableClientBindingFixer.cs b/src/Analyzers/Functions/AttributeBinding/DurableClientBindingFixer.cs new file mode 100644 index 00000000..865fe691 --- /dev/null +++ b/src/Analyzers/Functions/AttributeBinding/DurableClientBindingFixer.cs @@ -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; + +/// +/// Code fixer for the . +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MatchingAttributeBindingFixer))] +[Shared] +public sealed class DurableClientBindingFixer : MatchingAttributeBindingFixer +{ + /// + public override ImmutableArray FixableDiagnosticIds => [DurableClientBindingAnalyzer.DiagnosticId]; + + /// + public override string ExpectedType => "DurableTaskClient"; +} diff --git a/src/Analyzers/Functions/AttributeBinding/EntityTriggerBindingFixer.cs b/src/Analyzers/Functions/AttributeBinding/EntityTriggerBindingFixer.cs new file mode 100644 index 00000000..1b80e75e --- /dev/null +++ b/src/Analyzers/Functions/AttributeBinding/EntityTriggerBindingFixer.cs @@ -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; + +/// +/// Code fixer for the . +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MatchingAttributeBindingFixer))] +[Shared] +public sealed class EntityTriggerBindingFixer : MatchingAttributeBindingFixer +{ + /// + public override ImmutableArray FixableDiagnosticIds => [EntityTriggerBindingAnalyzer.DiagnosticId]; + + /// + public override string ExpectedType => "TaskEntityDispatcher"; +} diff --git a/src/Analyzers/Functions/AttributeBinding/MatchingAttributeBindingFixer.cs b/src/Analyzers/Functions/AttributeBinding/MatchingAttributeBindingFixer.cs new file mode 100644 index 00000000..532dbe46 --- /dev/null +++ b/src/Analyzers/Functions/AttributeBinding/MatchingAttributeBindingFixer.cs @@ -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; + +/// +/// Base class for code fixers that fix the type of a parameter to match the expected type. +/// +public abstract class MatchingAttributeBindingFixer : CodeFixProvider +{ + /// + /// Gets the expected type to be used during the code fix. + /// + public abstract string ExpectedType { get; } + + /// + public override FixAllProvider? GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + 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 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); + } +} diff --git a/src/Analyzers/Functions/AttributeBinding/OrchestrationTriggerBindingFixer.cs b/src/Analyzers/Functions/AttributeBinding/OrchestrationTriggerBindingFixer.cs new file mode 100644 index 00000000..b788c819 --- /dev/null +++ b/src/Analyzers/Functions/AttributeBinding/OrchestrationTriggerBindingFixer.cs @@ -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; + +/// +/// Code fixer for the . +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MatchingAttributeBindingFixer))] +[Shared] +public sealed class OrchestrationTriggerBindingFixer : MatchingAttributeBindingFixer +{ + /// + public override ImmutableArray FixableDiagnosticIds => [OrchestrationTriggerBindingAnalyzer.DiagnosticId]; + + /// + public override string ExpectedType => "TaskOrchestrationContext"; +} diff --git a/src/Analyzers/Resources.resx b/src/Analyzers/Resources.resx index b8c51631..5f8d5219 100644 --- a/src/Analyzers/Resources.resx +++ b/src/Analyzers/Resources.resx @@ -195,4 +195,7 @@ Activity function call return type doesn't match the function definition return type + + Use '{0}' instead of '{1}' + \ No newline at end of file diff --git a/test/Analyzers.Tests/Functions/AttributeBinding/DurableClientBindingAnalyzerTests.cs b/test/Analyzers.Tests/Functions/AttributeBinding/DurableClientBindingAnalyzerTests.cs index a44842fb..bb2c8b06 100644 --- a/test/Analyzers.Tests/Functions/AttributeBinding/DurableClientBindingAnalyzerTests.cs +++ b/test/Analyzers.Tests/Functions/AttributeBinding/DurableClientBindingAnalyzerTests.cs @@ -5,7 +5,7 @@ namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding; -public class DurableClientBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests +public class DurableClientBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests { protected override string ExpectedDiagnosticId => DurableClientBindingAnalyzer.DiagnosticId; diff --git a/test/Analyzers.Tests/Functions/AttributeBinding/EntityTriggerBindingAnalyzerTests.cs b/test/Analyzers.Tests/Functions/AttributeBinding/EntityTriggerBindingAnalyzerTests.cs index bde399f4..2e5386c2 100644 --- a/test/Analyzers.Tests/Functions/AttributeBinding/EntityTriggerBindingAnalyzerTests.cs +++ b/test/Analyzers.Tests/Functions/AttributeBinding/EntityTriggerBindingAnalyzerTests.cs @@ -5,7 +5,7 @@ namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding; -public class EntityTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests +public class EntityTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests { protected override string ExpectedDiagnosticId => EntityTriggerBindingAnalyzer.DiagnosticId; diff --git a/test/Analyzers.Tests/Functions/AttributeBinding/MatchingAttributeBindingSpecificationTests.cs b/test/Analyzers.Tests/Functions/AttributeBinding/MatchingAttributeBindingSpecificationTests.cs index 35018340..2887f8eb 100644 --- a/test/Analyzers.Tests/Functions/AttributeBinding/MatchingAttributeBindingSpecificationTests.cs +++ b/test/Analyzers.Tests/Functions/AttributeBinding/MatchingAttributeBindingSpecificationTests.cs @@ -7,7 +7,9 @@ namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding; -public abstract class MatchingAttributeBindingSpecificationTests where TAnalyzer : MatchingAttributeBindingAnalyzer, new() +public abstract class MatchingAttributeBindingSpecificationTests + where TAnalyzer : MatchingAttributeBindingAnalyzer, new() + where TCodeFix : MatchingAttributeBindingFixer, new() { protected abstract string ExpectedDiagnosticId { get; } protected abstract string ExpectedAttribute { get; } @@ -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.VerifyDurableTaskAnalyzerAsync(source, expected); + await CSharpCodeFixVerifier.VerifyDurableTaskAnalyzerAsync(source, expected); + } + + static async Task VerifyCodeFixAsync(string source, DiagnosticResult expected, string fix) + { + await CSharpCodeFixVerifier.VerifyDurableTaskCodeFixAsync(source, expected, fix); } DiagnosticResult BuildDiagnostic() { - return CSharpAnalyzerVerifier.Diagnostic(this.ExpectedDiagnosticId); + return CSharpCodeFixVerifier.Diagnostic(this.ExpectedDiagnosticId); } } diff --git a/test/Analyzers.Tests/Functions/AttributeBinding/OrchestrationTriggerBindingAnalyzerTests.cs b/test/Analyzers.Tests/Functions/AttributeBinding/OrchestrationTriggerBindingAnalyzerTests.cs index f1d020f9..25b05598 100644 --- a/test/Analyzers.Tests/Functions/AttributeBinding/OrchestrationTriggerBindingAnalyzerTests.cs +++ b/test/Analyzers.Tests/Functions/AttributeBinding/OrchestrationTriggerBindingAnalyzerTests.cs @@ -5,7 +5,7 @@ namespace Microsoft.DurableTask.Analyzers.Tests.Functions.AttributeBinding; -public class OrchestrationTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests +public class OrchestrationTriggerBindingAnalyzerTests : MatchingAttributeBindingSpecificationTests { protected override string ExpectedDiagnosticId => OrchestrationTriggerBindingAnalyzer.DiagnosticId; diff --git a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs index 71939778..00253ef6 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs @@ -21,15 +21,7 @@ public static async Task VerifyDurableTaskAnalyzerAsync(string source, Action + 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? 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? configureTest = null) + { + test.ReferenceAssemblies = References.CommonAssemblies; + test.ExpectedDiagnostics.AddRange(expected); + + configureTest?.Invoke(test); + + await test.RunAsync(CancellationToken.None); + } +} \ No newline at end of file diff --git a/test/Analyzers.Tests/Verifiers/References.cs b/test/Analyzers.Tests/Verifiers/References.cs new file mode 100644 index 00000000..1df31e76 --- /dev/null +++ b/test/Analyzers.Tests/Verifiers/References.cs @@ -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 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"), + ]); +}