Skip to content

Commit 85dea61

Browse files
CopilotYunchuWang
Copilot
and
YunchuWang
committed
Extend environment analyzer to flag configuration access
Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
1 parent bc33408 commit 85dea61

File tree

3 files changed

+181
-10
lines changed

3 files changed

+181
-10
lines changed

‎src/Analyzers/KnownTypeSymbols.Net.cs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ public sealed partial class KnownTypeSymbols
2424
INamedTypeSymbol? environment;
2525
INamedTypeSymbol? httpClient;
2626
INamedTypeSymbol? iLogger;
27+
INamedTypeSymbol? iConfiguration;
28+
INamedTypeSymbol? iOptions;
29+
INamedTypeSymbol? iOptionsSnapshot;
30+
INamedTypeSymbol? iOptionsMonitor;
2731

2832
/// <summary>
2933
/// Gets a Guid type symbol.
@@ -81,4 +85,24 @@ public sealed partial class KnownTypeSymbols
8185
/// Gets an ILogger type symbol.
8286
/// </summary>
8387
public INamedTypeSymbol? ILogger => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Logging.ILogger", ref this.iLogger);
88+
89+
/// <summary>
90+
/// Gets an IConfiguration type symbol.
91+
/// </summary>
92+
public INamedTypeSymbol? IConfiguration => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Configuration.IConfiguration", ref this.iConfiguration);
93+
94+
/// <summary>
95+
/// Gets an IOptions type symbol.
96+
/// </summary>
97+
public INamedTypeSymbol? IOptions => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Options.IOptions`1", ref this.iOptions);
98+
99+
/// <summary>
100+
/// Gets an IOptionsSnapshot type symbol.
101+
/// </summary>
102+
public INamedTypeSymbol? IOptionsSnapshot => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Options.IOptionsSnapshot`1", ref this.iOptionsSnapshot);
103+
104+
/// <summary>
105+
/// Gets an IOptionsMonitor type symbol.
106+
/// </summary>
107+
public INamedTypeSymbol? IOptionsMonitor => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Options.IOptionsMonitor`1", ref this.iOptionsMonitor);
84108
}

‎src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs‎

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ public sealed class EnvironmentOrchestrationVisitor : MethodProbeOrchestrationVi
4545
/// <inheritdoc/>
4646
public override bool Initialize()
4747
{
48-
return this.KnownTypeSymbols.Environment != null;
48+
return this.KnownTypeSymbols.Environment != null
49+
|| this.KnownTypeSymbols.IConfiguration != null
50+
|| this.KnownTypeSymbols.IOptions != null
51+
|| this.KnownTypeSymbols.IOptionsSnapshot != null
52+
|| this.KnownTypeSymbols.IOptionsMonitor != null;
4953
}
5054

5155
/// <inheritdoc/>
@@ -57,19 +61,106 @@ protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode meth
5761
return;
5862
}
5963

60-
foreach (IInvocationOperation invocation in methodOperation.Descendants().OfType<IInvocationOperation>())
64+
foreach (IOperation operation in methodOperation.Descendants())
6165
{
62-
IMethodSymbol targetMethod = invocation.TargetMethod;
66+
this.CheckOperationForEnvironmentVariableAccess(operation, methodSymbol, orchestrationName, reportDiagnostic);
67+
}
68+
}
6369

64-
if (targetMethod.ContainingType.Equals(this.KnownTypeSymbols.Environment, SymbolEqualityComparer.Default) &&
65-
targetMethod.Name is nameof(Environment.GetEnvironmentVariable) or nameof(Environment.GetEnvironmentVariables) or nameof(Environment.ExpandEnvironmentVariables))
66-
{
67-
string invocationName = targetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);
70+
void CheckOperationForEnvironmentVariableAccess(IOperation operation, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> reportDiagnostic)
71+
{
72+
switch (operation)
73+
{
74+
case IInvocationOperation invocation when this.IsEnvironmentInvocation(invocation.TargetMethod):
75+
this.Report(operation, invocation.TargetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), methodSymbol, orchestrationName, reportDiagnostic);
76+
break;
77+
case IInvocationOperation invocation when this.IsConfigurationOrOptionsInvocation(invocation):
78+
this.Report(operation, invocation.TargetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), methodSymbol, orchestrationName, reportDiagnostic);
79+
break;
80+
case IPropertyReferenceOperation propertyReference when this.IsConfigurationOrOptionsPropertyReference(propertyReference):
81+
this.Report(operation, propertyReference.Property.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), methodSymbol, orchestrationName, reportDiagnostic);
82+
break;
83+
}
84+
}
85+
86+
void Report(IOperation operation, string memberName, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> reportDiagnostic)
87+
{
88+
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation, methodSymbol.Name, memberName, orchestrationName));
89+
}
90+
91+
bool IsEnvironmentInvocation(IMethodSymbol targetMethod)
92+
{
93+
return this.KnownTypeSymbols.Environment != null &&
94+
targetMethod.ContainingType.Equals(this.KnownTypeSymbols.Environment, SymbolEqualityComparer.Default) &&
95+
targetMethod.Name is nameof(Environment.GetEnvironmentVariable) or nameof(Environment.GetEnvironmentVariables) or nameof(Environment.ExpandEnvironmentVariables);
96+
}
97+
98+
bool IsConfigurationOrOptionsInvocation(IInvocationOperation invocation)
99+
{
100+
if (this.IsConfigurationOrOptionsType(invocation.Instance?.Type))
101+
{
102+
return true;
103+
}
68104

69-
// e.g.: "The method 'Method1' uses environment variables through 'Environment.GetEnvironmentVariable()' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'"
70-
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, invocation, methodSymbol.Name, invocationName, orchestrationName));
105+
if (invocation.TargetMethod.IsExtensionMethod)
106+
{
107+
ITypeSymbol? receiverType = invocation.TargetMethod.ReducedFrom?.Parameters.FirstOrDefault()?.Type ?? invocation.TargetMethod.Parameters.FirstOrDefault()?.Type;
108+
if (this.IsConfigurationOrOptionsType(receiverType))
109+
{
110+
return true;
71111
}
72112
}
113+
114+
return false;
115+
}
116+
117+
bool IsConfigurationOrOptionsPropertyReference(IPropertyReferenceOperation propertyReference)
118+
{
119+
if (this.IsConfigurationOrOptionsType(propertyReference.Instance?.Type))
120+
{
121+
return true;
122+
}
123+
124+
return this.IsConfigurationOrOptionsType(propertyReference.Property.ContainingType);
125+
}
126+
127+
bool IsConfigurationOrOptionsType(ITypeSymbol? type)
128+
{
129+
if (type is null)
130+
{
131+
return false;
132+
}
133+
134+
if (this.IsConfigurationType(type))
135+
{
136+
return true;
137+
}
138+
139+
if (type is INamedTypeSymbol namedType && this.IsOptionsType(namedType))
140+
{
141+
return true;
142+
}
143+
144+
return type.AllInterfaces.Any(this.IsConfigurationType) ||
145+
(type is INamedTypeSymbol typeSymbol && typeSymbol.AllInterfaces.Any(this.IsOptionsType));
146+
}
147+
148+
bool IsConfigurationType(ITypeSymbol type)
149+
{
150+
return this.KnownTypeSymbols.IConfiguration != null &&
151+
SymbolEqualityComparer.Default.Equals(type, this.KnownTypeSymbols.IConfiguration);
152+
}
153+
154+
bool IsOptionsType(INamedTypeSymbol type)
155+
{
156+
return this.IsOptionsType(type, this.KnownTypeSymbols.IOptions)
157+
|| this.IsOptionsType(type, this.KnownTypeSymbols.IOptionsSnapshot)
158+
|| this.IsOptionsType(type, this.KnownTypeSymbols.IOptionsMonitor);
159+
}
160+
161+
bool IsOptionsType(INamedTypeSymbol type, INamedTypeSymbol? optionsSymbol)
162+
{
163+
return optionsSymbol != null && SymbolEqualityComparer.Default.Equals(type.OriginalDefinition, optionsSymbol);
73164
}
74165
}
75166
}

‎test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,64 @@ void Method([OrchestrationTrigger] TaskOrchestrationContext context)
4242
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
4343
}
4444

45+
[Fact]
46+
public async Task AccessingConfigurationIsNotAllowedInAzureFunctionsOrchestrations()
47+
{
48+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
49+
[Function(""Run"")]
50+
void Method([OrchestrationTrigger] TaskOrchestrationContext context, Microsoft.Extensions.Configuration.IConfiguration configuration)
51+
{
52+
_ = {|#0:configuration[""PATH""]|};
53+
_ = {|#1:configuration.GetSection(""Section"")|};
54+
}
55+
");
56+
string[] members = [
57+
"IConfiguration.this[string]",
58+
"IConfiguration.GetSection(string)",
59+
];
60+
61+
DiagnosticResult[] expected = members.Select(
62+
(member, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", member, "Run")).ToArray();
63+
64+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
65+
}
66+
67+
[Fact]
68+
public async Task AccessingOptionsIsNotAllowedInAzureFunctionsOrchestrations()
69+
{
70+
string code = Wrapper.WrapDurableFunctionOrchestration(@"
71+
class MyOptions
72+
{
73+
public string? Value { get; set; }
74+
}
75+
76+
[Function(""Run"")]
77+
void Method(
78+
[OrchestrationTrigger] TaskOrchestrationContext context,
79+
Microsoft.Extensions.Options.IOptions<MyOptions> options,
80+
Microsoft.Extensions.Options.IOptionsSnapshot<MyOptions> snapshot,
81+
Microsoft.Extensions.Options.IOptionsMonitor<MyOptions> monitor)
82+
{
83+
_ = {|#0:options.Value|};
84+
_ = {|#1:snapshot.Value|};
85+
_ = {|#2:monitor.CurrentValue|};
86+
}
87+
");
88+
89+
string[] members = [
90+
"IOptions<Orchestrator.MyOptions>.Value",
91+
"IOptions<Orchestrator.MyOptions>.Value",
92+
"IOptionsMonitor<Orchestrator.MyOptions>.CurrentValue",
93+
];
94+
95+
DiagnosticResult[] expected = members.Select(
96+
(member, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", member, "Run")).ToArray();
97+
98+
await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected);
99+
}
100+
45101
static DiagnosticResult BuildDiagnostic()
46102
{
47103
return VerifyCS.Diagnostic(EnvironmentOrchestrationAnalyzer.DiagnosticId);
48104
}
49-
}
105+
}

0 commit comments

Comments
 (0)