diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index d9cf2bd7..0317d471 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -4,4 +4,5 @@ ### New Rules Rule ID | Category | Severity | Notes ---------|----------|----------|------- \ No newline at end of file +--------|----------|----------|------- +DURABLE0009 | Orchestration | Info | **GetInputOrchestrationAnalyzer**: Suggests using input parameter binding instead of ctx.GetInput() in orchestration methods. diff --git a/src/Analyzers/Orchestration/GetInputOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/GetInputOrchestrationAnalyzer.cs new file mode 100644 index 00000000..8e18d504 --- /dev/null +++ b/src/Analyzers/Orchestration/GetInputOrchestrationAnalyzer.cs @@ -0,0 +1,77 @@ +// 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.GetInputOrchestrationAnalyzer; + +namespace Microsoft.DurableTask.Analyzers.Orchestration; + +/// +/// Analyzer that reports an informational diagnostic when ctx.GetInput() is used in an orchestration method, +/// suggesting the use of input parameter binding instead. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class GetInputOrchestrationAnalyzer : OrchestrationAnalyzer +{ + /// + /// Diagnostic ID supported for the analyzer. + /// + public const string DiagnosticId = "DURABLE0009"; + + static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.GetInputOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.GetInputOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); + + static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + Title, + MessageFormat, + AnalyzersCategories.Orchestration, + DiagnosticSeverity.Info, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => [Rule]; + + /// + /// Visitor that inspects the method body for GetInput calls. + /// + public sealed class GetInputOrchestrationVisitor : MethodProbeOrchestrationVisitor + { + /// + protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + IOperation? methodOperation = semanticModel.GetOperation(methodSyntax); + if (methodOperation is null) + { + return; + } + + foreach (IInvocationOperation operation in methodOperation.Descendants().OfType()) + { + IMethodSymbol? method = operation.TargetMethod; + if (method == null) + { + continue; + } + + // Check if this is a call to GetInput() on TaskOrchestrationContext + if (method.Name != "GetInput" || !method.IsGenericMethod) + { + continue; + } + + // Verify the containing type is TaskOrchestrationContext + if (!method.ContainingType.Equals(this.KnownTypeSymbols.TaskOrchestrationContext, SymbolEqualityComparer.Default)) + { + continue; + } + + // e.g.: "Consider using an input parameter instead of 'GetInput()' in orchestration 'MyOrchestrator'" + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, orchestrationName)); + } + } + } +} diff --git a/src/Analyzers/Resources.resx b/src/Analyzers/Resources.resx index ee14969a..6b56987f 100644 --- a/src/Analyzers/Resources.resx +++ b/src/Analyzers/Resources.resx @@ -210,4 +210,10 @@ Sub-orchestration not found + + Consider using an input parameter instead of 'GetInput<T>()' in orchestration '{0}' + + + Input parameter binding can be used instead of GetInput + \ No newline at end of file diff --git a/test/Analyzers.Tests/Orchestration/GetInputOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/GetInputOrchestrationAnalyzerTests.cs new file mode 100644 index 00000000..ea2b3d3a --- /dev/null +++ b/test/Analyzers.Tests/Orchestration/GetInputOrchestrationAnalyzerTests.cs @@ -0,0 +1,175 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis.Testing; +using Microsoft.DurableTask.Analyzers.Orchestration; + +using VerifyCS = Microsoft.DurableTask.Analyzers.Tests.Verifiers.CSharpAnalyzerVerifier; + +namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration; + +public class GetInputOrchestrationAnalyzerTests +{ + [Fact] + public async Task EmptyCodeWithNoSymbolsAvailableHasNoDiag() + { + string code = @""; + + // checks that empty code with no assembly references of Durable Functions has no diagnostics. + // this guarantees that if someone adds our analyzer to a project that doesn't use Durable Functions, + // the analyzer won't crash/they won't get any diagnostics + await VerifyCS.VerifyAnalyzerAsync(code); + } + + [Fact] + public async Task EmptyCodeWithSymbolsAvailableHasNoDiag() + { + string code = @""; + + // checks that empty code with access to assembly references of Durable Functions has no diagnostics + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task NonOrchestrationHasNoDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +void Method(){ + // This is not an orchestration method, so no diagnostic +} +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task DurableFunctionOrchestrationUsingGetInputHasInfoDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +int Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + int input = {|#0:context.GetInput()|}; + return input; +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task DurableFunctionOrchestrationWithInputParameterHasNoDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +int Run([OrchestrationTrigger] TaskOrchestrationContext context, int input) +{ + // Using input parameter is the recommended approach + return input; +} +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task TaskOrchestratorWithInputParameterHasNoDiag() + { + string code = Wrapper.WrapTaskOrchestrator(@" +public class MyOrchestrator : TaskOrchestrator +{ + public override Task RunAsync(TaskOrchestrationContext context, int input) + { + // Using input parameter is the recommended approach + return Task.FromResult(input); + } +} +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task TaskOrchestratorUsingGetInputHasInfoDiag() + { + string code = Wrapper.WrapTaskOrchestrator(@" +public class MyOrchestrator : TaskOrchestrator +{ + public override Task RunAsync(TaskOrchestrationContext context, int input) + { + // Even though input parameter exists, GetInput is still flagged as not recommended + int value = {|#0:context.GetInput()|}; + return Task.FromResult(value); + } +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("MyOrchestrator"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task OrchestratorFuncUsingGetInputHasInfoDiag() + { + string code = Wrapper.WrapFuncOrchestrator(@" +tasks.AddOrchestratorFunc(""MyOrchestration"", (TaskOrchestrationContext context) => +{ + int input = {|#0:context.GetInput()|}; + return Task.FromResult(input); +}); +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("MyOrchestration"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task NestedMethodCallWithGetInputHasInfoDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +int Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return HelperMethod(context); +} + +int HelperMethod(TaskOrchestrationContext context) +{ + int input = {|#0:context.GetInput()|}; + return input; +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task MultipleGetInputCallsHaveMultipleDiags() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +int Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + int input1 = {|#0:context.GetInput()|}; + int input2 = {|#1:context.GetInput()|}; + return input1 + input2; +} +"); + + DiagnosticResult expected1 = BuildDiagnostic().WithLocation(0).WithArguments("Run"); + DiagnosticResult expected2 = BuildDiagnostic().WithLocation(1).WithArguments("Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected1, expected2); + } + + static DiagnosticResult BuildDiagnostic() + { + return VerifyCS.Diagnostic(GetInputOrchestrationAnalyzer.DiagnosticId); + } +}