diff --git a/src/Analyzers/KnownTypeSymbols.Net.cs b/src/Analyzers/KnownTypeSymbols.Net.cs index c9bde3ed..b5fd4078 100644 --- a/src/Analyzers/KnownTypeSymbols.Net.cs +++ b/src/Analyzers/KnownTypeSymbols.Net.cs @@ -24,6 +24,10 @@ public sealed partial class KnownTypeSymbols INamedTypeSymbol? environment; INamedTypeSymbol? httpClient; INamedTypeSymbol? iLogger; + INamedTypeSymbol? iConfiguration; + INamedTypeSymbol? iOptions; + INamedTypeSymbol? iOptionsSnapshot; + INamedTypeSymbol? iOptionsMonitor; /// /// Gets a Guid type symbol. @@ -81,4 +85,24 @@ public sealed partial class KnownTypeSymbols /// Gets an ILogger type symbol. /// public INamedTypeSymbol? ILogger => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Logging.ILogger", ref this.iLogger); + + /// + /// Gets an IConfiguration type symbol. + /// + public INamedTypeSymbol? IConfiguration => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Configuration.IConfiguration", ref this.iConfiguration); + + /// + /// Gets an IOptions type symbol. + /// + public INamedTypeSymbol? IOptions => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Options.IOptions`1", ref this.iOptions); + + /// + /// Gets an IOptionsSnapshot type symbol. + /// + public INamedTypeSymbol? IOptionsSnapshot => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Options.IOptionsSnapshot`1", ref this.iOptionsSnapshot); + + /// + /// Gets an IOptionsMonitor type symbol. + /// + public INamedTypeSymbol? IOptionsMonitor => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Options.IOptionsMonitor`1", ref this.iOptionsMonitor); } diff --git a/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs index c64ea76b..996bc794 100644 --- a/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/EnvironmentOrchestrationAnalyzer.cs @@ -45,7 +45,11 @@ public sealed class EnvironmentOrchestrationVisitor : MethodProbeOrchestrationVi /// public override bool Initialize() { - return this.KnownTypeSymbols.Environment != null; + return this.KnownTypeSymbols.Environment != null + || this.KnownTypeSymbols.IConfiguration != null + || this.KnownTypeSymbols.IOptions != null + || this.KnownTypeSymbols.IOptionsSnapshot != null + || this.KnownTypeSymbols.IOptionsMonitor != null; } /// @@ -57,19 +61,106 @@ protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode meth return; } - foreach (IInvocationOperation invocation in methodOperation.Descendants().OfType()) + foreach (IOperation operation in methodOperation.Descendants()) { - IMethodSymbol targetMethod = invocation.TargetMethod; + this.CheckOperationForEnvironmentVariableAccess(operation, methodSymbol, orchestrationName, reportDiagnostic); + } + } - if (targetMethod.ContainingType.Equals(this.KnownTypeSymbols.Environment, SymbolEqualityComparer.Default) && - targetMethod.Name is nameof(Environment.GetEnvironmentVariable) or nameof(Environment.GetEnvironmentVariables) or nameof(Environment.ExpandEnvironmentVariables)) - { - string invocationName = targetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat); + void CheckOperationForEnvironmentVariableAccess(IOperation operation, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + switch (operation) + { + case IInvocationOperation invocation when this.IsEnvironmentInvocation(invocation.TargetMethod): + this.Report(operation, invocation.TargetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), methodSymbol, orchestrationName, reportDiagnostic); + break; + case IInvocationOperation invocation when this.IsConfigurationOrOptionsInvocation(invocation): + this.Report(operation, invocation.TargetMethod.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), methodSymbol, orchestrationName, reportDiagnostic); + break; + case IPropertyReferenceOperation propertyReference when this.IsConfigurationOrOptionsPropertyReference(propertyReference): + this.Report(operation, propertyReference.Property.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat), methodSymbol, orchestrationName, reportDiagnostic); + break; + } + } + + void Report(IOperation operation, string memberName, IMethodSymbol methodSymbol, string orchestrationName, Action reportDiagnostic) + { + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation, methodSymbol.Name, memberName, orchestrationName)); + } + + bool IsEnvironmentInvocation(IMethodSymbol targetMethod) + { + return this.KnownTypeSymbols.Environment != null && + targetMethod.ContainingType.Equals(this.KnownTypeSymbols.Environment, SymbolEqualityComparer.Default) && + targetMethod.Name is nameof(Environment.GetEnvironmentVariable) or nameof(Environment.GetEnvironmentVariables) or nameof(Environment.ExpandEnvironmentVariables); + } + + bool IsConfigurationOrOptionsInvocation(IInvocationOperation invocation) + { + if (this.IsConfigurationOrOptionsType(invocation.Instance?.Type)) + { + return true; + } - // e.g.: "The method 'Method1' uses environment variables through 'Environment.GetEnvironmentVariable()' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" - reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, invocation, methodSymbol.Name, invocationName, orchestrationName)); + if (invocation.TargetMethod.IsExtensionMethod) + { + ITypeSymbol? receiverType = invocation.TargetMethod.ReducedFrom?.Parameters.FirstOrDefault()?.Type ?? invocation.TargetMethod.Parameters.FirstOrDefault()?.Type; + if (this.IsConfigurationOrOptionsType(receiverType)) + { + return true; } } + + return false; + } + + bool IsConfigurationOrOptionsPropertyReference(IPropertyReferenceOperation propertyReference) + { + if (this.IsConfigurationOrOptionsType(propertyReference.Instance?.Type)) + { + return true; + } + + return this.IsConfigurationOrOptionsType(propertyReference.Property.ContainingType); + } + + bool IsConfigurationOrOptionsType(ITypeSymbol? type) + { + if (type is null) + { + return false; + } + + if (this.IsConfigurationType(type)) + { + return true; + } + + if (type is INamedTypeSymbol namedType && this.IsOptionsType(namedType)) + { + return true; + } + + return type.AllInterfaces.Any(this.IsConfigurationType) || + (type is INamedTypeSymbol typeSymbol && typeSymbol.AllInterfaces.Any(this.IsOptionsType)); + } + + bool IsConfigurationType(ITypeSymbol type) + { + return this.KnownTypeSymbols.IConfiguration != null && + SymbolEqualityComparer.Default.Equals(type, this.KnownTypeSymbols.IConfiguration); + } + + bool IsOptionsType(INamedTypeSymbol type) + { + return this.IsOptionsType(type, this.KnownTypeSymbols.IOptions) + || this.IsOptionsType(type, this.KnownTypeSymbols.IOptionsSnapshot) + || this.IsOptionsType(type, this.KnownTypeSymbols.IOptionsMonitor); + } + + bool IsOptionsType(INamedTypeSymbol type, INamedTypeSymbol? optionsSymbol) + { + return optionsSymbol != null && SymbolEqualityComparer.Default.Equals(type.OriginalDefinition, optionsSymbol); } } } diff --git a/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs index 55c2113a..61f09bf3 100644 --- a/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs +++ b/test/Analyzers.Tests/Orchestration/EnvironmentOrchestrationAnalyzerTests.cs @@ -42,8 +42,64 @@ void Method([OrchestrationTrigger] TaskOrchestrationContext context) await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); } + [Fact] + public async Task AccessingConfigurationIsNotAllowedInAzureFunctionsOrchestrations() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +void Method([OrchestrationTrigger] TaskOrchestrationContext context, Microsoft.Extensions.Configuration.IConfiguration configuration) +{ + _ = {|#0:configuration[""PATH""]|}; + _ = {|#1:configuration.GetSection(""Section"")|}; +} +"); + string[] members = [ + "IConfiguration.this[string]", + "IConfiguration.GetSection(string)", + ]; + + DiagnosticResult[] expected = members.Select( + (member, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", member, "Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task AccessingOptionsIsNotAllowedInAzureFunctionsOrchestrations() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +class MyOptions +{ + public string? Value { get; set; } +} + +[Function(""Run"")] +void Method( + [OrchestrationTrigger] TaskOrchestrationContext context, + Microsoft.Extensions.Options.IOptions options, + Microsoft.Extensions.Options.IOptionsSnapshot snapshot, + Microsoft.Extensions.Options.IOptionsMonitor monitor) +{ + _ = {|#0:options.Value|}; + _ = {|#1:snapshot.Value|}; + _ = {|#2:monitor.CurrentValue|}; +} +"); + + string[] members = [ + "IOptions.Value", + "IOptions.Value", + "IOptionsMonitor.CurrentValue", + ]; + + DiagnosticResult[] expected = members.Select( + (member, i) => BuildDiagnostic().WithLocation(i).WithArguments("Method", member, "Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + static DiagnosticResult BuildDiagnostic() { return VerifyCS.Diagnostic(EnvironmentOrchestrationAnalyzer.DiagnosticId); } -} \ No newline at end of file +}