Skip to content

Commit c96a6bf

Browse files
CopilotYunchuWangCopilot
authored
Add TimeProvider support to orchestration analyzer (#573)
* Initial plan * Update global.json to use available .NET SDK version Co-authored-by: YunchuWang <[email protected]> * Add TimeProvider support to DateTimeOrchestrationAnalyzer Co-authored-by: YunchuWang <[email protected]> * Update comment to include TimeProvider in visitor description Co-authored-by: YunchuWang <[email protected]> * Update global.json Co-authored-by: Copilot <[email protected]> * Address review comments: fix XML docs, remove redundant ToString(), combine if statements, restore test files Co-authored-by: YunchuWang <[email protected]> * Fix code fixer not showing for TimeProvider diagnostics by finding parent InvocationExpressionSyntax Co-Authored-By: YunchuWang <[email protected]> * Revert "Make TimeProvider code fixer more lenient for conditional compilation scenarios" This reverts commit c48f1d8. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: YunchuWang <[email protected]> Co-authored-by: wangbill <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent fe0bb2e commit c96a6bf

File tree

6 files changed

+315
-63
lines changed

6 files changed

+315
-63
lines changed

‎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? timeProvider;
2627
INamedTypeSymbol? iLogger;
2728

2829
/// <summary>
@@ -81,4 +82,9 @@ public sealed partial class KnownTypeSymbols
8182
/// Gets an ILogger type symbol.
8283
/// </summary>
8384
public INamedTypeSymbol? ILogger => this.GetOrResolveFullyQualifiedType("Microsoft.Extensions.Logging.ILogger", ref this.iLogger);
85+
86+
/// <summary>
87+
/// Gets a TimeProvider type symbol.
88+
/// </summary>
89+
public INamedTypeSymbol? TimeProvider => this.GetOrResolveFullyQualifiedType("System.TimeProvider", ref this.timeProvider);
8490
}

‎src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
namespace Microsoft.DurableTask.Analyzers.Orchestration;
1111

1212
/// <summary>
13-
/// Analyzer that reports a warning when a non-deterministic DateTime or DateTimeOffset property is used in an orchestration method.
13+
/// Analyzer that reports a warning when a non-deterministic DateTime, DateTimeOffset, or TimeProvider method is used in an orchestration method.
1414
/// </summary>
1515
[DiagnosticAnalyzer(LanguageNames.CSharp)]
1616
public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer<DateTimeOrchestrationVisitor>
@@ -36,18 +36,20 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer<DateTi
3636
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
3737

3838
/// <summary>
39-
/// Visitor that inspects the method body for DateTime and DateTimeOffset properties.
39+
/// Visitor that inspects the method body for DateTime and DateTimeOffset properties, and TimeProvider method invocations.
4040
/// </summary>
4141
public sealed class DateTimeOrchestrationVisitor : MethodProbeOrchestrationVisitor
4242
{
4343
INamedTypeSymbol systemDateTimeSymbol = null!;
4444
INamedTypeSymbol? systemDateTimeOffsetSymbol;
45+
INamedTypeSymbol? systemTimeProviderSymbol;
4546

4647
/// <inheritdoc/>
4748
public override bool Initialize()
4849
{
4950
this.systemDateTimeSymbol = this.Compilation.GetSpecialType(SpecialType.System_DateTime);
5051
this.systemDateTimeOffsetSymbol = this.Compilation.GetTypeByMetadataName("System.DateTimeOffset");
52+
this.systemTimeProviderSymbol = this.Compilation.GetTypeByMetadataName("System.TimeProvider");
5153
return true;
5254
}
5355

@@ -86,6 +88,33 @@ protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode meth
8688
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, methodSymbol.Name, property.ToString(), orchestrationName));
8789
}
8890
}
91+
92+
// Check for TimeProvider method invocations
93+
if (this.systemTimeProviderSymbol is not null)
94+
{
95+
foreach (IInvocationOperation operation in methodOperation.Descendants().OfType<IInvocationOperation>())
96+
{
97+
IMethodSymbol invokedMethod = operation.TargetMethod;
98+
99+
// Check if the method is called on TimeProvider type
100+
bool isTimeProvider = invokedMethod.ContainingType.Equals(this.systemTimeProviderSymbol, SymbolEqualityComparer.Default);
101+
102+
if (!isTimeProvider)
103+
{
104+
continue;
105+
}
106+
107+
// Check for non-deterministic TimeProvider methods: GetUtcNow, GetLocalNow, GetTimestamp
108+
bool isNonDeterministicMethod = invokedMethod.Name is "GetUtcNow" or "GetLocalNow" or "GetTimestamp";
109+
110+
if (isNonDeterministicMethod)
111+
{
112+
// e.g.: "The method 'Method1' uses 'System.TimeProvider.GetUtcNow()' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'"
113+
string timeProviderMethodName = $"{invokedMethod.ContainingType}.{invokedMethod.Name}()";
114+
reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, methodSymbol.Name, timeProviderMethodName, orchestrationName));
115+
}
116+
}
117+
}
89118
}
90119
}
91120
}

‎src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs‎

Lines changed: 121 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Immutable;
55
using System.Composition;
66
using System.Globalization;
7+
using System.Linq;
78
using Microsoft.CodeAnalysis;
89
using Microsoft.CodeAnalysis.CodeActions;
910
using Microsoft.CodeAnalysis.CodeFixes;
@@ -26,51 +27,89 @@ public sealed class DateTimeOrchestrationFixer : OrchestrationContextFixer
2627
/// <inheritdoc/>
2728
protected override void RegisterCodeFixes(CodeFixContext context, OrchestrationCodeFixContext orchestrationContext)
2829
{
29-
// Parses the syntax node to see if it is a member access expression (e.g. DateTime.Now or DateTimeOffset.Now)
30-
if (orchestrationContext.SyntaxNodeWithDiagnostic is not MemberAccessExpressionSyntax dateTimeExpression)
31-
{
32-
return;
33-
}
34-
3530
// Gets the name of the TaskOrchestrationContext parameter (e.g. "context" or "ctx")
3631
string contextParameterName = orchestrationContext.TaskOrchestrationContextSymbol.Name;
37-
38-
// Use semantic analysis to determine if this is a DateTimeOffset expression
3932
SemanticModel semanticModel = orchestrationContext.SemanticModel;
40-
ITypeSymbol? typeSymbol = semanticModel.GetTypeInfo(dateTimeExpression.Expression).Type;
41-
bool isDateTimeOffset = typeSymbol?.ToDisplayString() == "System.DateTimeOffset";
4233

43-
bool isDateTimeToday = dateTimeExpression.Name.ToString() == "Today";
44-
45-
// Build the recommendation text
46-
string recommendation;
47-
if (isDateTimeOffset)
34+
// Handle DateTime/DateTimeOffset property access (e.g. DateTime.Now or DateTimeOffset.Now)
35+
if (orchestrationContext.SyntaxNodeWithDiagnostic is MemberAccessExpressionSyntax dateTimeExpression)
4836
{
49-
// For DateTimeOffset, we always just cast CurrentUtcDateTime
50-
recommendation = $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime";
37+
// Use semantic analysis to determine if this is a DateTimeOffset expression
38+
ITypeSymbol? typeSymbol = semanticModel.GetTypeInfo(dateTimeExpression.Expression).Type;
39+
bool isDateTimeOffset = typeSymbol?.ToDisplayString() == "System.DateTimeOffset";
40+
41+
bool isDateTimeToday = dateTimeExpression.Name.ToString() == "Today";
42+
43+
// Build the recommendation text
44+
string recommendation;
45+
if (isDateTimeOffset)
46+
{
47+
// For DateTimeOffset, we always just cast CurrentUtcDateTime
48+
recommendation = $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime";
49+
}
50+
else
51+
{
52+
// For DateTime, we may need to add .Date for Today
53+
string dateTimeTodaySuffix = isDateTimeToday ? ".Date" : string.Empty;
54+
recommendation = $"{contextParameterName}.CurrentUtcDateTime{dateTimeTodaySuffix}";
55+
}
56+
57+
// e.g: "Use 'context.CurrentUtcDateTime' instead of 'DateTime.Now'"
58+
// e.g: "Use 'context.CurrentUtcDateTime.Date' instead of 'DateTime.Today'"
59+
// e.g: "Use '(DateTimeOffset)context.CurrentUtcDateTime' instead of 'DateTimeOffset.Now'"
60+
string title = string.Format(
61+
CultureInfo.InvariantCulture,
62+
Resources.UseInsteadFixerTitle,
63+
recommendation,
64+
dateTimeExpression);
65+
66+
context.RegisterCodeFix(
67+
CodeAction.Create(
68+
title: title,
69+
createChangedDocument: c => ReplaceDateTime(context.Document, orchestrationContext.Root, dateTimeExpression, contextParameterName, isDateTimeToday, isDateTimeOffset),
70+
equivalenceKey: title), // This key is used to prevent duplicate code fixes.
71+
context.Diagnostics);
72+
return;
5173
}
52-
else
74+
75+
// Handle TimeProvider method invocations (e.g. TimeProvider.System.GetUtcNow())
76+
// The node might be the invocation itself or a child node, so we need to find the InvocationExpressionSyntax
77+
InvocationExpressionSyntax? timeProviderInvocation = orchestrationContext.SyntaxNodeWithDiagnostic as InvocationExpressionSyntax
78+
?? orchestrationContext.SyntaxNodeWithDiagnostic.AncestorsAndSelf().OfType<InvocationExpressionSyntax>().FirstOrDefault();
79+
80+
if (timeProviderInvocation != null &&
81+
semanticModel.GetSymbolInfo(timeProviderInvocation).Symbol is IMethodSymbol methodSymbol)
5382
{
54-
// For DateTime, we may need to add .Date for Today
55-
string dateTimeTodaySuffix = isDateTimeToday ? ".Date" : string.Empty;
56-
recommendation = $"{contextParameterName}.CurrentUtcDateTime{dateTimeTodaySuffix}";
57-
}
83+
string methodName = methodSymbol.Name;
84+
85+
// Check if the method returns DateTimeOffset
86+
bool returnsDateTimeOffset = methodSymbol.ReturnType.ToDisplayString() == "System.DateTimeOffset";
87+
88+
// Build the recommendation based on the method name
89+
string recommendation = methodName switch
90+
{
91+
"GetUtcNow" when returnsDateTimeOffset => $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime",
92+
"GetUtcNow" => $"{contextParameterName}.CurrentUtcDateTime",
93+
"GetLocalNow" when returnsDateTimeOffset => $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime.ToLocalTime()",
94+
"GetLocalNow" => $"{contextParameterName}.CurrentUtcDateTime.ToLocalTime()",
95+
"GetTimestamp" => $"{contextParameterName}.CurrentUtcDateTime.Ticks",
96+
_ => $"{contextParameterName}.CurrentUtcDateTime",
97+
};
98+
99+
// e.g: "Use 'context.CurrentUtcDateTime' instead of 'TimeProvider.System.GetUtcNow()'"
100+
string title = string.Format(
101+
CultureInfo.InvariantCulture,
102+
Resources.UseInsteadFixerTitle,
103+
recommendation,
104+
timeProviderInvocation);
58105

59-
// e.g: "Use 'context.CurrentUtcDateTime' instead of 'DateTime.Now'"
60-
// e.g: "Use 'context.CurrentUtcDateTime.Date' instead of 'DateTime.Today'"
61-
// e.g: "Use '(DateTimeOffset)context.CurrentUtcDateTime' instead of 'DateTimeOffset.Now'"
62-
string title = string.Format(
63-
CultureInfo.InvariantCulture,
64-
Resources.UseInsteadFixerTitle,
65-
recommendation,
66-
dateTimeExpression.ToString());
67-
68-
context.RegisterCodeFix(
69-
CodeAction.Create(
70-
title: title,
71-
createChangedDocument: c => ReplaceDateTime(context.Document, orchestrationContext.Root, dateTimeExpression, contextParameterName, isDateTimeToday, isDateTimeOffset),
72-
equivalenceKey: title), // This key is used to prevent duplicate code fixes.
73-
context.Diagnostics);
106+
context.RegisterCodeFix(
107+
CodeAction.Create(
108+
title: title,
109+
createChangedDocument: c => ReplaceTimeProvider(context.Document, orchestrationContext.Root, timeProviderInvocation, contextParameterName, methodName, returnsDateTimeOffset),
110+
equivalenceKey: title),
111+
context.Diagnostics);
112+
}
74113
}
75114

76115
static Task<Document> ReplaceDateTime(Document document, SyntaxNode oldRoot, MemberAccessExpressionSyntax incorrectDateTimeSyntax, string contextParameterName, bool isDateTimeToday, bool isDateTimeOffset)
@@ -106,4 +145,49 @@ static Task<Document> ReplaceDateTime(Document document, SyntaxNode oldRoot, Mem
106145

107146
return Task.FromResult(newDocument);
108147
}
148+
149+
static Task<Document> ReplaceTimeProvider(Document document, SyntaxNode oldRoot, InvocationExpressionSyntax incorrectTimeProviderSyntax, string contextParameterName, string methodName, bool returnsDateTimeOffset)
150+
{
151+
// Build the correct expression based on the method name
152+
ExpressionSyntax correctExpression = methodName switch
153+
{
154+
"GetUtcNow" => MemberAccessExpression(
155+
SyntaxKind.SimpleMemberAccessExpression,
156+
IdentifierName(contextParameterName),
157+
IdentifierName("CurrentUtcDateTime")),
158+
"GetLocalNow" => InvocationExpression(
159+
MemberAccessExpression(
160+
SyntaxKind.SimpleMemberAccessExpression,
161+
MemberAccessExpression(
162+
SyntaxKind.SimpleMemberAccessExpression,
163+
IdentifierName(contextParameterName),
164+
IdentifierName("CurrentUtcDateTime")),
165+
IdentifierName("ToLocalTime"))),
166+
"GetTimestamp" => MemberAccessExpression(
167+
SyntaxKind.SimpleMemberAccessExpression,
168+
MemberAccessExpression(
169+
SyntaxKind.SimpleMemberAccessExpression,
170+
IdentifierName(contextParameterName),
171+
IdentifierName("CurrentUtcDateTime")),
172+
IdentifierName("Ticks")),
173+
_ => MemberAccessExpression(
174+
SyntaxKind.SimpleMemberAccessExpression,
175+
IdentifierName(contextParameterName),
176+
IdentifierName("CurrentUtcDateTime")),
177+
};
178+
179+
// If the method returns DateTimeOffset, we need to cast the DateTime to DateTimeOffset
180+
if (returnsDateTimeOffset)
181+
{
182+
correctExpression = CastExpression(
183+
IdentifierName("DateTimeOffset"),
184+
correctExpression);
185+
}
186+
187+
// Replaces the old invocation with the new expression
188+
SyntaxNode newRoot = oldRoot.ReplaceNode(incorrectTimeProviderSyntax, correctExpression);
189+
Document newDocument = document.WithSyntaxRoot(newRoot);
190+
191+
return Task.FromResult(newDocument);
192+
}
109193
}

0 commit comments

Comments
 (0)