Add Roslyn analyzer for non-contextual logger usage in orchestrations (DURABLE0009)

src/Analyzers/AnalyzerReleases.Unshipped.md

@@ -5,5 +5,6 @@
 
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
+DURABLE0009 | Orchestration | Warning | **LoggerOrchestrationAnalyzer**: Warns when a non-contextual ILogger is used in an orchestration method. Orchestrations should use `context.CreateReplaySafeLogger()` instead of injecting ILogger directly.
 DURABLE2003 | Activity | Warning | **FunctionNotFoundAnalyzer**: Warns when an activity function call references a name that does not match any defined activity in the compilation.
 DURABLE2004 | Orchestration | Warning | **FunctionNotFoundAnalyzer**: Warns when a sub-orchestration call references a name that does not match any defined orchestrator in the compilation.

src/Analyzers/KnownTypeSymbols.Net.cs

@@ -23,6 +23,7 @@ public sealed partial class KnownTypeSymbols
     INamedTypeSymbol? cancellationToken;
     INamedTypeSymbol? environment;
     INamedTypeSymbol? httpClient;
+    INamedTypeSymbol? iLogger;
 
     /// <summary>
     /// Gets a Guid type symbol.
@@ -75,4 +76,9 @@ public sealed partial class KnownTypeSymbols
     /// Gets an HttpClient type symbol.
     /// </summary>
     public INamedTypeSymbol? HttpClient => this.GetOrResolveFullyQualifiedType(typeof(HttpClient).FullName, ref this.httpClient);
+
+    /// <summary>
+    /// Gets an ILogger type symbol.
+    /// </summary>
+    public INamedTypeSymbol? ILogger => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Logging.ILogger", ref this.iLogger);
 }

src/Analyzers/Orchestration/LoggerOrchestrationAnalyzer.cs

@@ -0,0 +1,146 @@
+// 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.LoggerOrchestrationAnalyzer;
+
+namespace Microsoft.DurableTask.Analyzers.Orchestration;
+
+/// <summary>
+/// Analyzer that reports a warning when a non-contextual ILogger is used in an orchestration method.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class LoggerOrchestrationAnalyzer : OrchestrationAnalyzer<LoggerOrchestrationVisitor>
+{
+    /// <summary>
+    /// Diagnostic ID supported for the analyzer.
+    /// </summary>
+    public const string DiagnosticId = "DURABLE0009";
+
+    static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.LoggerOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
+    static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.LoggerOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));
+
+    static readonly DiagnosticDescriptor Rule = new(
+        DiagnosticId,
+        Title,
+        MessageFormat,
+        AnalyzersCategories.Orchestration,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    /// <inheritdoc/>
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
+
+    /// <summary>
+    /// Visitor that inspects the method body for ILogger usage.
+    /// </summary>
+    public sealed class LoggerOrchestrationVisitor : MethodProbeOrchestrationVisitor
+    {
+        INamedTypeSymbol? iLoggerSymbol;
+
+        /// <inheritdoc/>
+        public override bool Initialize()
+        {
+            this.iLoggerSymbol = this.KnownTypeSymbols.ILogger;
+            if (this.iLoggerSymbol == null)
+            {
+                return false;
+            }
+
+            return true;
+        }
+
+        /// <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;
+            }
+
+            // Track which parameters we've already reported on to avoid duplicates
+            HashSet<IParameterSymbol> reportedParameters = new(SymbolEqualityComparer.Default);
+
+            // Check for ILogger parameters in the method signature
+            foreach (IParameterSymbol parameter in methodSymbol.Parameters)
+            {
+                if (this.IsILoggerType(parameter.Type))
+                {
+                    // Found an ILogger parameter - report diagnostic at the parameter location
+                    if (parameter.DeclaringSyntaxReferences.Length > 0)
+                    {
+                        SyntaxNode parameterSyntax = parameter.DeclaringSyntaxReferences[0].GetSyntax();
+                        reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, parameterSyntax, methodSymbol.Name, orchestrationName));
+                        reportedParameters.Add(parameter);
+                    }
+                }
+            }
+
+            // Check for ILogger field or property references (but not parameter references, as those were already reported)
+            foreach (IOperation descendant in methodOperation.Descendants())
+            {
+                ITypeSymbol? typeToCheck = null;
+                SyntaxNode? syntaxNode = null;
+
+                switch (descendant)
+                {
+                    case IFieldReferenceOperation fieldRef:
+                        typeToCheck = fieldRef.Field.Type;
+                        syntaxNode = fieldRef.Syntax;
+                        break;
+                    case IPropertyReferenceOperation propRef:
+                        typeToCheck = propRef.Property.Type;
+                        syntaxNode = propRef.Syntax;
+                        break;
+                    case IParameterReferenceOperation paramRef:
+                        // Skip parameter references that we already reported on in the parameter list
+                        if (reportedParameters.Contains(paramRef.Parameter))
+                        {
+                            continue;
+                        }
+
+                        typeToCheck = paramRef.Parameter.Type;
+                        syntaxNode = paramRef.Syntax;
+                        break;
+                }
+
+                if (typeToCheck != null && syntaxNode != null && this.IsILoggerType(typeToCheck))
+                {
+                    reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, syntaxNode, methodSymbol.Name, orchestrationName));
+                }
+            }
+        }
+
+        bool IsILoggerType(ITypeSymbol type)
+        {
+            if (this.iLoggerSymbol == null)
+            {
+                return false;
+            }
+
+            // First check for exact match with ILogger
+            if (SymbolEqualityComparer.Default.Equals(type, this.iLoggerSymbol))
+            {
+                return true;
+            }
+
+            // Check if the type implements ILogger interface (covers ILogger<T> case)
+            if (type is INamedTypeSymbol namedType)
+            {
+                foreach (INamedTypeSymbol interfaceType in namedType.AllInterfaces)
+                {
+                    if (SymbolEqualityComparer.Default.Equals(interfaceType, this.iLoggerSymbol))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+    }
+}

src/Analyzers/Resources.resx

@@ -183,6 +183,12 @@
   <data name="ThreadTaskOrchestrationAnalyzerTitle" xml:space="preserve">
     <value>Thread and Task calls must be deterministic inside an orchestrator function</value>
   </data>
+  <data name="LoggerOrchestrationAnalyzerMessageFormat" xml:space="preserve">
+    <value>The method '{0}' uses a non-contextual logger that may cause unexpected behavior when invoked from orchestration '{1}'. Use 'context.CreateReplaySafeLogger()' instead.</value>
+  </data>
+  <data name="LoggerOrchestrationAnalyzerTitle" xml:space="preserve">
+    <value>Orchestrations should use replay-safe loggers created from the orchestration context</value>
+  </data>
   <data name="InputArgumentTypeMismatchAnalyzerMessageFormat" xml:space="preserve">
     <value>CallActivityAsync is passing the incorrect type '{0}' instead of '{1}' to the activity function '{2}'</value>
   </data>