Skip to content

Commit cbff2ac

Browse files
CopilotYunchuWang
andcommitted
Add LoggerOrchestrationAnalyzer implementation and tests
Co-authored-by: YunchuWang <[email protected]>
1 parent 58fe068 commit cbff2ac

File tree

6 files changed

+402
-0
lines changed

6 files changed

+402
-0
lines changed

src/Analyzers/AnalyzerReleases.Unshipped.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@
55

66
Rule ID | Category | Severity | Notes
77
--------|----------|----------|-------
8+
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.
89
DURABLE2003 | Activity | Warning | **FunctionNotFoundAnalyzer**: Warns when an activity function call references a name that does not match any defined activity in the compilation.
910
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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public sealed partial class KnownTypeSymbols
2323
INamedTypeSymbol? cancellationToken;
2424
INamedTypeSymbol? environment;
2525
INamedTypeSymbol? httpClient;
26+
INamedTypeSymbol? iLogger;
2627

2728
/// <summary>
2829
/// Gets a Guid type symbol.
@@ -75,4 +76,9 @@ public sealed partial class KnownTypeSymbols
7576
/// Gets an HttpClient type symbol.
7677
/// </summary>
7778
public INamedTypeSymbol? HttpClient => this.GetOrResolveFullyQualifiedType(typeof(HttpClient).FullName, ref this.httpClient);
79+
80+
/// <summary>
81+
/// Gets an ILogger type symbol.
82+
/// </summary>
83+
public INamedTypeSymbol? ILogger => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Logging.ILogger", ref this.iLogger);
7884
}
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
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.LoggerOrchestrationAnalyzer;
9+
10+
namespace Microsoft.DurableTask.Analyzers.Orchestration;
11+
12+
/// <summary>
13+
/// Analyzer that reports a warning when a non-contextual ILogger is used in an orchestration method.
14+
/// </summary>
15+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
16+
public sealed class LoggerOrchestrationAnalyzer : OrchestrationAnalyzer<LoggerOrchestrationVisitor>
17+
{
18+
/// <summary>
19+
/// Diagnostic ID supported for the analyzer.
20+
/// </summary>
21+
public const string DiagnosticId = "DURABLE0009";
22+
23+
static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.LoggerOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
24+
static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.LoggerOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));
25+
26+
static readonly DiagnosticDescriptor Rule = new(
27+
DiagnosticId,
28+
Title,
29+
MessageFormat,
30+
AnalyzersCategories.Orchestration,
31+
DiagnosticSeverity.Warning,
32+
isEnabledByDefault: true);
33+
34+
/// <inheritdoc/>
35+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
36+
37+
/// <summary>
38+
/// Visitor that inspects the method body for ILogger usage.
39+
/// </summary>
40+
public sealed class LoggerOrchestrationVisitor : MethodProbeOrchestrationVisitor
41+
{
42+
INamedTypeSymbol? iLoggerSymbol;
43+
44+
/// <inheritdoc/>
45+
public override bool Initialize()
46+
{
47+
this.iLoggerSymbol = this.KnownTypeSymbols.ILogger;
48+
if (this.iLoggerSymbol == null)
49+
{
50+
return false;
51+
}
52+
53+
return true;
54+
}
55+
56+
/// <inheritdoc/>
57+
protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode methodSyntax, IMethodSymbol methodSymbol, string orchestrationName, Action<Diagnostic> reportDiagnostic)
58+
{
59+
IOperation? methodOperation = semanticModel.GetOperation(methodSyntax);
60+
if (methodOperation is null)
61+
{
62+
return;
63+
}
64+
65+
// Check for ILogger parameters in the method signature
66+
foreach (IParameterSymbol parameter in methodSymbol.Parameters)
67+
{
68+
if (this.IsILoggerType(parameter.Type))
69+
{
70+
// Found an ILogger parameter - report diagnostic at the parameter location
71+
if (parameter.DeclaringSyntaxReferences.Length > 0)
72+
{
73+
SyntaxNode parameterSyntax = parameter.DeclaringSyntaxReferences[0].GetSyntax();
74+
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, parameterSyntax, methodSymbol.Name, orchestrationName));
75+
}
76+
}
77+
}
78+
79+
// Check for ILogger field or property references
80+
foreach (IOperation descendant in methodOperation.Descendants())
81+
{
82+
ITypeSymbol? typeToCheck = null;
83+
SyntaxNode? syntaxNode = null;
84+
85+
switch (descendant)
86+
{
87+
case IFieldReferenceOperation fieldRef:
88+
typeToCheck = fieldRef.Field.Type;
89+
syntaxNode = fieldRef.Syntax;
90+
break;
91+
case IPropertyReferenceOperation propRef:
92+
typeToCheck = propRef.Property.Type;
93+
syntaxNode = propRef.Syntax;
94+
break;
95+
case IParameterReferenceOperation paramRef:
96+
typeToCheck = paramRef.Parameter.Type;
97+
syntaxNode = paramRef.Syntax;
98+
break;
99+
}
100+
101+
if (typeToCheck != null && syntaxNode != null && this.IsILoggerType(typeToCheck))
102+
{
103+
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, syntaxNode, methodSymbol.Name, orchestrationName));
104+
}
105+
}
106+
}
107+
108+
bool IsILoggerType(ITypeSymbol type)
109+
{
110+
if (this.iLoggerSymbol == null)
111+
{
112+
return false;
113+
}
114+
115+
// Check if the type is ILogger or ILogger<T>
116+
if (type is INamedTypeSymbol namedType)
117+
{
118+
INamedTypeSymbol originalDefinition = namedType.OriginalDefinition;
119+
if (SymbolEqualityComparer.Default.Equals(originalDefinition, this.iLoggerSymbol))
120+
{
121+
return true;
122+
}
123+
}
124+
125+
return SymbolEqualityComparer.Default.Equals(type, this.iLoggerSymbol);
126+
}
127+
}
128+
}

src/Analyzers/Resources.resx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,12 @@
183183
<data name="ThreadTaskOrchestrationAnalyzerTitle" xml:space="preserve">
184184
<value>Thread and Task calls must be deterministic inside an orchestrator function</value>
185185
</data>
186+
<data name="LoggerOrchestrationAnalyzerMessageFormat" xml:space="preserve">
187+
<value>The method '{0}' uses a non-contextual logger that may cause unexpected behavior when invoked from orchestration '{1}'. Use 'context.CreateReplaySafeLogger()' instead.</value>
188+
</data>
189+
<data name="LoggerOrchestrationAnalyzerTitle" xml:space="preserve">
190+
<value>Orchestrations should use replay-safe loggers created from the orchestration context</value>
191+
</data>
186192
<data name="InputArgumentTypeMismatchAnalyzerMessageFormat" xml:space="preserve">
187193
<value>CallActivityAsync is passing the incorrect type '{0}' instead of '{1}' to the activity function '{2}'</value>
188194
</data>

0 commit comments

Comments
 (0)