From c65608e41c6e8f841833ba48aaee85c4826e0fc9 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Mon, 6 May 2024 17:21:14 -0300 Subject: [PATCH] Function orchestration bindings analyzer (#304) Adds an analyzer that reports a warning when a Durable Function orchestration has parameters bindings other than `OrchestrationTrigger` (such as `DurableClient` or `EntityTrigger`). --- src/Analyzers/AnalyzerReleases.Unshipped.md | 1 + .../OtherBindingsOrchestrationAnalyzer.cs | 73 ++++++++++++++++ src/Analyzers/Resources.resx | 6 ++ .../OtherBindingsOrchestrationAnalyzer.cs | 86 +++++++++++++++++++ 4 files changed, 166 insertions(+) create mode 100644 src/Analyzers/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs create mode 100644 test/Analyzers.Tests/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index bf14e3b2..690cd6a4 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -9,6 +9,7 @@ DURABLE0001 | Orchestration | Warning | DateTimeOrchestrationAnalyzer DURABLE0002 | Orchestration | Warning | GuidOrchestrationAnalyzer DURABLE0003 | Orchestration | Warning | DelayOrchestrationAnalyzer DURABLE0007 | Orchestration | Warning | CancellationTokenOrchestrationAnalyzer +DURABLE0008 | Orchestration | Warning | OtherBindingsOrchestrationAnalyzer DURABLE1001 | Attribute Binding | Error | OrchestrationTriggerBindingAnalyzer DURABLE1002 | Attribute Binding | Error | DurableClientBindingAnalyzer DURABLE1003 | Attribute Binding | Error | EntityTriggerBindingAnalyzer \ No newline at end of file diff --git a/src/Analyzers/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs b/src/Analyzers/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs new file mode 100644 index 00000000..6cea911d --- /dev/null +++ b/src/Analyzers/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.DurableTask.Analyzers.Orchestration; +using static Microsoft.DurableTask.Analyzers.Functions.Orchestration.OtherBindingsOrchestrationAnalyzer; + +namespace Microsoft.DurableTask.Analyzers.Functions.Orchestration; + +/// +/// Analyzer that reports a warning when a Durable Function Orchestration has parameters bindings other than OrchestrationTrigger. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +class OtherBindingsOrchestrationAnalyzer : OrchestrationAnalyzer +{ + /// + /// Diagnostic ID supported for the analyzer. + /// + public const string DiagnosticId = "DURABLE0008"; + + static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.OtherBindingsOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.OtherBindingsOrchestrationAnalyzerMessageFormat), 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 Durable Functions' method signatures for parameters binding other than OrchestrationTrigger. + /// + public sealed class OtherBindingsOrchestrationOrchestrationVisitor : OrchestrationVisitor + { + ImmutableArray bannedBindings; + + /// + public override bool Initialize() + { + List candidateSymbols = [ + this.KnownTypeSymbols.DurableClientAttribute, + this.KnownTypeSymbols.EntityTriggerAttribute, + ]; + + // filter out null values, since some of them may not be available during compilation + this.bannedBindings = candidateSymbols.Where(s => s != null).ToImmutableArray()!; + + return this.bannedBindings.Length > 0; + } + + /// + public override void VisitDurableFunction(SemanticModel sm, MethodDeclarationSyntax methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + foreach (IParameterSymbol parameter in methodSymbol.Parameters) + { + IEnumerable attributesSymbols = parameter.GetAttributes().Select(att => att.AttributeClass); + + if (attributesSymbols.Any(att => att != null && this.bannedBindings.Contains(att, SymbolEqualityComparer.Default))) + { + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, parameter, orchestrationName)); + } + } + } + } +} diff --git a/src/Analyzers/Resources.resx b/src/Analyzers/Resources.resx index 5e5da2cf..7f3481a0 100644 --- a/src/Analyzers/Resources.resx +++ b/src/Analyzers/Resources.resx @@ -159,4 +159,10 @@ CancellationToken should not be used as an orchestrator function parameter + + Orchestration '{0}' is using multiple bindings + + + OrchestrationTrigger methods must not use any other bindings + \ No newline at end of file diff --git a/test/Analyzers.Tests/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs b/test/Analyzers.Tests/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs new file mode 100644 index 00000000..9c048eab --- /dev/null +++ b/test/Analyzers.Tests/Functions/Orchestration/OtherBindingsOrchestrationAnalyzer.cs @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis.Testing; +using Microsoft.DurableTask.Analyzers.Functions.Orchestration; +using VerifyCS = Microsoft.DurableTask.Analyzers.Tests.Verifiers.CSharpAnalyzerVerifier; + +namespace Microsoft.DurableTask.Analyzers.Tests.Functions.Orchestration; + +public class OtherBindingsOrchestrationAnalyzerTests +{ + [Fact] + public async Task EmptyCodeHasNoDiag() + { + string code = @""; + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task DurableFunctionOrchestrationWithNoBannedBindingHasNoDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context) +{ +} +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task DurableFunctionOrchestrationUsingDurableClientHasDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context, {|#0:[DurableClient] DurableTaskClient client|}) +{ +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task DurableFunctionOrchestrationUsingEntityTriggerHasDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context, {|#0:[EntityTrigger] TaskEntityDispatcher dispatcher|}) +{ +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + + [Fact] + public async Task DurableFunctionOrchestrationUsingMultipleBannedBindingsHasDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context, +{|#0:[EntityTrigger] TaskEntityDispatcher dispatcher|}, +{|#1:[DurableClient] DurableTaskClient client|}) +{ +} +"); + + DiagnosticResult[] expected = Enumerable.Range(0, 2).Select( + i => BuildDiagnostic().WithLocation(i).WithArguments("Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + static DiagnosticResult BuildDiagnostic() + { + return VerifyCS.Diagnostic(OtherBindingsOrchestrationAnalyzer.DiagnosticId); + } +}