Skip to content

Commit dfbbb20

Browse files
CopilotYunchuWang
andauthored
Add analyzer to suggest input parameter binding over GetInput() (#550)
* Initial plan * Add GetInput analyzer with Info severity and comprehensive tests Co-authored-by: YunchuWang <[email protected]> * Add null check for TargetMethod per code review feedback Co-authored-by: YunchuWang <[email protected]> * Use CSharpAnalyzerVerifier instead of CodeFixVerifier for consistency Co-authored-by: YunchuWang <[email protected]> * Merge main branch and resolve conflict in AnalyzerReleases.Unshipped.md Co-authored-by: YunchuWang <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: YunchuWang <[email protected]> Co-authored-by: wangbill <[email protected]>
1 parent f7732a6 commit dfbbb20

File tree

4 files changed

+260
-1
lines changed

4 files changed

+260
-1
lines changed

src/Analyzers/AnalyzerReleases.Unshipped.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@
44
### New Rules
55

66
Rule ID | Category | Severity | Notes
7-
--------|----------|----------|-------
7+
--------|----------|----------|-------
8+
DURABLE0009 | Orchestration | Info | **GetInputOrchestrationAnalyzer**: Suggests using input parameter binding instead of ctx.GetInput<T>() in orchestration methods.
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System.Collections.Immutable;
5+
using Microsoft.CodeAnalysis;
6+
using Microsoft.CodeAnalysis.Diagnostics;
7+
using Microsoft.CodeAnalysis.Operations;
8+
using static Microsoft.DurableTask.Analyzers.Orchestration.GetInputOrchestrationAnalyzer;
9+
10+
namespace Microsoft.DurableTask.Analyzers.Orchestration;
11+
12+
/// <summary>
13+
/// Analyzer that reports an informational diagnostic when ctx.GetInput() is used in an orchestration method,
14+
/// suggesting the use of input parameter binding instead.
15+
/// </summary>
16+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
17+
public sealed class GetInputOrchestrationAnalyzer : OrchestrationAnalyzer<GetInputOrchestrationVisitor>
18+
{
19+
/// <summary>
20+
/// Diagnostic ID supported for the analyzer.
21+
/// </summary>
22+
public const string DiagnosticId = "DURABLE0009";
23+
24+
static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.GetInputOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
25+
static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.GetInputOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));
26+
27+
static readonly DiagnosticDescriptor Rule = new(
28+
DiagnosticId,
29+
Title,
30+
MessageFormat,
31+
AnalyzersCategories.Orchestration,
32+
DiagnosticSeverity.Info,
33+
isEnabledByDefault: true);
34+
35+
/// <inheritdoc/>
36+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
37+
38+
/// <summary>
39+
/// Visitor that inspects the method body for GetInput calls.
40+
/// </summary>
41+
public sealed class GetInputOrchestrationVisitor : MethodProbeOrchestrationVisitor
42+
{
43+
/// <inheritdoc/>
44+
protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> reportDiagnostic)
45+
{
46+
IOperation? methodOperation = semanticModel.GetOperation(methodSyntax);
47+
if (methodOperation is null)
48+
{
49+
return;
50+
}
51+
52+
foreach (IInvocationOperation operation in methodOperation.Descendants().OfType<IInvocationOperation>())
53+
{
54+
IMethodSymbol? method = operation.TargetMethod;
55+
if (method == null)
56+
{
57+
continue;
58+
}
59+
60+
// Check if this is a call to GetInput<T>() on TaskOrchestrationContext
61+
if (method.Name != "GetInput" || !method.IsGenericMethod)
62+
{
63+
continue;
64+
}
65+
66+
// Verify the containing type is TaskOrchestrationContext
67+
if (!method.ContainingType.Equals(this.KnownTypeSymbols.TaskOrchestrationContext, SymbolEqualityComparer.Default))
68+
{
69+
continue;
70+
}
71+
72+
// e.g.: "Consider using an input parameter instead of 'GetInput<T>()' in orchestration 'MyOrchestrator'"
73+
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, orchestrationName));
74+
}
75+
}
76+
}
77+
}

src/Analyzers/Resources.resx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,4 +210,10 @@
210210
<data name="SubOrchestrationNotFoundAnalyzerTitle" xml:space="preserve">
211211
<value>Sub-orchestration not found</value>
212212
</data>
213+
<data name="GetInputOrchestrationAnalyzerMessageFormat" xml:space="preserve">
214+
<value>Consider using an input parameter instead of 'GetInput&lt;T&gt;()' in orchestration '{0}'</value>
215+
</data>
216+
<data name="GetInputOrchestrationAnalyzerTitle" xml:space="preserve">
217+
<value>Input parameter binding can be used instead of GetInput</value>
218+
</data>
213219
</root>
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.CodeAnalysis.Testing;
5+
using Microsoft.DurableTask.Analyzers.Orchestration;
6+
7+
using VerifyCS = Microsoft.DurableTask.Analyzers.Tests.Verifiers.CSharpAnalyzerVerifier<Microsoft.DurableTask.Analyzers.Orchestration.GetInputOrchestrationAnalyzer>;
8+
9+
namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration;
10+
11+
public class GetInputOrchestrationAnalyzerTests
12+
{
13+
[Fact]
14+
public async Task EmptyCodeWithNoSymbolsAvailableHasNoDiag()
15+
{
16+
string code = @"";
17+
18+
// checks that empty code with no assembly references of Durable Functions has no diagnostics.
19+
// this guarantees that if someone adds our analyzer to a project that doesn't use Durable Functions,
20+
// the analyzer won't crash/they won't get any diagnostics
21+
await VerifyCS.VerifyAnalyzerAsync(code);
22+
}
23+
24+
[Fact]
25+
public async Task EmptyCodeWithSymbolsAvailableHasNoDiag()
26+
{
27+
string code = @"";
28+
29+
// checks that empty code with access to assembly references of Durable Functions has no diagnostics
30+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
31+
}
32+
33+
[Fact]
34+
public async Task NonOrchestrationHasNoDiag()
35+
{
36+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
37+
void Method(){
38+
// This is not an orchestration method, so no diagnostic
39+
}
40+
");
41+
42+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
43+
}
44+
45+
[Fact]
46+
public async Task DurableFunctionOrchestrationUsingGetInputHasInfoDiag()
47+
{
48+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
49+
[Function(""Run"")]
50+
int Run([OrchestrationTrigger] TaskOrchestrationContext context)
51+
{
52+
int input = {|#0:context.GetInput<int>()|};
53+
return input;
54+
}
55+
");
56+
57+
DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run");
58+
59+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
60+
}
61+
62+
[Fact]
63+
public async Task DurableFunctionOrchestrationWithInputParameterHasNoDiag()
64+
{
65+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
66+
[Function(""Run"")]
67+
int Run([OrchestrationTrigger] TaskOrchestrationContext context, int input)
68+
{
69+
// Using input parameter is the recommended approach
70+
return input;
71+
}
72+
");
73+
74+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
75+
}
76+
77+
[Fact]
78+
public async Task TaskOrchestratorWithInputParameterHasNoDiag()
79+
{
80+
string code = Wrapper.WrapTaskOrchestrator(@"
81+
public class MyOrchestrator : TaskOrchestrator<int, int>
82+
{
83+
public override Task<int> RunAsync(TaskOrchestrationContext context, int input)
84+
{
85+
// Using input parameter is the recommended approach
86+
return Task.FromResult(input);
87+
}
88+
}
89+
");
90+
91+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code);
92+
}
93+
94+
[Fact]
95+
public async Task TaskOrchestratorUsingGetInputHasInfoDiag()
96+
{
97+
string code = Wrapper.WrapTaskOrchestrator(@"
98+
public class MyOrchestrator : TaskOrchestrator<int, int>
99+
{
100+
public override Task<int> RunAsync(TaskOrchestrationContext context, int input)
101+
{
102+
// Even though input parameter exists, GetInput is still flagged as not recommended
103+
int value = {|#0:context.GetInput<int>()|};
104+
return Task.FromResult(value);
105+
}
106+
}
107+
");
108+
109+
DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("MyOrchestrator");
110+
111+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
112+
}
113+
114+
[Fact]
115+
public async Task OrchestratorFuncUsingGetInputHasInfoDiag()
116+
{
117+
string code = Wrapper.WrapFuncOrchestrator(@"
118+
tasks.AddOrchestratorFunc(""MyOrchestration"", (TaskOrchestrationContext context) =>
119+
{
120+
int input = {|#0:context.GetInput<int>()|};
121+
return Task.FromResult(input);
122+
});
123+
");
124+
125+
DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("MyOrchestration");
126+
127+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
128+
}
129+
130+
[Fact]
131+
public async Task NestedMethodCallWithGetInputHasInfoDiag()
132+
{
133+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
134+
[Function(""Run"")]
135+
int Run([OrchestrationTrigger] TaskOrchestrationContext context)
136+
{
137+
return HelperMethod(context);
138+
}
139+
140+
int HelperMethod(TaskOrchestrationContext context)
141+
{
142+
int input = {|#0:context.GetInput<int>()|};
143+
return input;
144+
}
145+
");
146+
147+
DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run");
148+
149+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
150+
}
151+
152+
[Fact]
153+
public async Task MultipleGetInputCallsHaveMultipleDiags()
154+
{
155+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
156+
[Function(""Run"")]
157+
int Run([OrchestrationTrigger] TaskOrchestrationContext context)
158+
{
159+
int input1 = {|#0:context.GetInput<int>()|};
160+
int input2 = {|#1:context.GetInput<int>()|};
161+
return input1 + input2;
162+
}
163+
");
164+
165+
DiagnosticResult expected1 = BuildDiagnostic().WithLocation(0).WithArguments("Run");
166+
DiagnosticResult expected2 = BuildDiagnostic().WithLocation(1).WithArguments("Run");
167+
168+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected1, expected2);
169+
}
170+
171+
static DiagnosticResult BuildDiagnostic()
172+
{
173+
return VerifyCS.Diagnostic(GetInputOrchestrationAnalyzer.DiagnosticId);
174+
}
175+
}

0 commit comments

Comments
 (0)