Add analyzer to suggest input parameter binding over GetInput()

src/Analyzers/AnalyzerReleases.Unshipped.md

@@ -4,4 +4,5 @@
 ### New Rules
 
 Rule ID | Category | Severity | Notes
---------|----------|----------|-------
+--------|----------|----------|-------
+DURABLE0009 | Orchestration | Info | **GetInputOrchestrationAnalyzer**: Suggests using input parameter binding instead of ctx.GetInput<T>() in orchestration methods.

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;
+
+/// <summary>
+/// Analyzer that reports an informational diagnostic when ctx.GetInput() is used in an orchestration method,
+/// suggesting the use of input parameter binding instead.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class GetInputOrchestrationAnalyzer : OrchestrationAnalyzer<GetInputOrchestrationVisitor>
+{
+    /// <summary>
+    /// Diagnostic ID supported for the analyzer.
+    /// </summary>
+    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);
+
+    /// <inheritdoc/>
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
+
+    /// <summary>
+    /// Visitor that inspects the method body for GetInput calls.
+    /// </summary>
+    public sealed class GetInputOrchestrationVisitor : MethodProbeOrchestrationVisitor
+    {
+        /// <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 operation in methodOperation.Descendants().OfType<IInvocationOperation>())
+            {
+                IMethodSymbol? method = operation.TargetMethod;
+                if (method == null)
+                {
+                    continue;
+                }
+
+                // Check if this is a call to GetInput<T>() 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<T>()' in orchestration 'MyOrchestrator'"
+                reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, orchestrationName));
+            }
+        }
+    }
+}

src/Analyzers/Resources.resx

@@ -210,4 +210,10 @@
   <data name="SubOrchestrationNotFoundAnalyzerTitle" xml:space="preserve">
     <value>Sub-orchestration not found</value>
   </data>
+  <data name="GetInputOrchestrationAnalyzerMessageFormat" xml:space="preserve">
+    <value>Consider using an input parameter instead of 'GetInput&lt;T&gt;()' in orchestration '{0}'</value>
+  </data>
+  <data name="GetInputOrchestrationAnalyzerTitle" xml:space="preserve">
+    <value>Input parameter binding can be used instead of GetInput</value>
+  </data>
 </root>

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<Microsoft.DurableTask.Analyzers.Orchestration.GetInputOrchestrationAnalyzer>;
+
+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<int>()|};
+    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<int, int>
+{
+    public override Task<int> 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<int, int>
+{
+    public override Task<int> RunAsync(TaskOrchestrationContext context, int input)
+    {
+        // Even though input parameter exists, GetInput is still flagged as not recommended
+        int value = {|#0:context.GetInput<int>()|};
+        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<int>()|};
+    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<int>()|};
+    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>()|};
+    int input2 = {|#1:context.GetInput<int>()|};
+    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);
+    }
+}