diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index 7eefeb675..d65554814 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -10,7 +10,7 @@ namespace Microsoft.DurableTask.Analyzers.Orchestration; /// -/// Analyzer that reports a warning when a non-deterministic DateTime property is used in an orchestration method. +/// Analyzer that reports a warning when a non-deterministic DateTime or DateTimeOffset property is used in an orchestration method. /// [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer @@ -35,16 +35,18 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer SupportedDiagnostics => [Rule]; /// - /// Visitor that inspects the method body for DateTime properties. + /// Visitor that inspects the method body for DateTime and DateTimeOffset properties. /// public sealed class DateTimeOrchestrationVisitor : MethodProbeOrchestrationVisitor { INamedTypeSymbol systemDateTimeSymbol = null!; + INamedTypeSymbol? systemDateTimeOffsetSymbol; /// public override bool Initialize() { this.systemDateTimeSymbol = this.Compilation.GetSpecialType(SpecialType.System_DateTime); + this.systemDateTimeOffsetSymbol = this.Compilation.GetTypeByMetadataName("System.DateTimeOffset"); return true; } @@ -61,14 +63,25 @@ protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode meth { IPropertySymbol property = operation.Property; - if (!property.ContainingSymbol.Equals(this.systemDateTimeSymbol, SymbolEqualityComparer.Default)) + bool isDateTime = property.ContainingSymbol.Equals(this.systemDateTimeSymbol, SymbolEqualityComparer.Default); + bool isDateTimeOffset = this.systemDateTimeOffsetSymbol is not null && + property.ContainingSymbol.Equals(this.systemDateTimeOffsetSymbol, SymbolEqualityComparer.Default); + + if (!isDateTime && !isDateTimeOffset) { - return; + continue; } - if (property.Name is nameof(DateTime.Now) or nameof(DateTime.UtcNow) or nameof(DateTime.Today)) + // Check for non-deterministic properties + // DateTime has: Now, UtcNow, Today + // DateTimeOffset has: Now, UtcNow (but not Today) + bool isNonDeterministic = property.Name is nameof(DateTime.Now) or nameof(DateTime.UtcNow) || + (isDateTime && property.Name == nameof(DateTime.Today)); + + if (isNonDeterministic) { - // e.g.: "The method 'Method1' uses 'System.Date.Now' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" + // e.g.: "The method 'Method1' uses 'System.DateTime.Now' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" + // e.g.: "The method 'Method1' uses 'System.DateTimeOffset.Now' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, methodSymbol.Name, property.ToString(), orchestrationName)); } } diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs index 5b32857c7..fd233d1bb 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs @@ -26,7 +26,7 @@ public sealed class DateTimeOrchestrationFixer : OrchestrationContextFixer /// protected override void RegisterCodeFixes(CodeFixContext context, OrchestrationCodeFixContext orchestrationContext) { - // Parses the syntax node to see if it is a member access expression (e.g. DateTime.Now) + // Parses the syntax node to see if it is a member access expression (e.g. DateTime.Now or DateTimeOffset.Now) if (orchestrationContext.SyntaxNodeWithDiagnostic is not MemberAccessExpressionSyntax dateTimeExpression) { return; @@ -35,12 +35,30 @@ protected override void RegisterCodeFixes(CodeFixContext context, OrchestrationC // Gets the name of the TaskOrchestrationContext parameter (e.g. "context" or "ctx") string contextParameterName = orchestrationContext.TaskOrchestrationContextSymbol.Name; + // Use semantic analysis to determine if this is a DateTimeOffset expression + SemanticModel semanticModel = orchestrationContext.SemanticModel; + ITypeSymbol? typeSymbol = semanticModel.GetTypeInfo(dateTimeExpression.Expression).Type; + bool isDateTimeOffset = typeSymbol?.ToDisplayString() == "System.DateTimeOffset"; + bool isDateTimeToday = dateTimeExpression.Name.ToString() == "Today"; - string dateTimeTodaySuffix = isDateTimeToday ? ".Date" : string.Empty; - string recommendation = $"{contextParameterName}.CurrentUtcDateTime{dateTimeTodaySuffix}"; + + // Build the recommendation text + string recommendation; + if (isDateTimeOffset) + { + // For DateTimeOffset, we always just cast CurrentUtcDateTime + recommendation = $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime"; + } + else + { + // For DateTime, we may need to add .Date for Today + string dateTimeTodaySuffix = isDateTimeToday ? ".Date" : string.Empty; + recommendation = $"{contextParameterName}.CurrentUtcDateTime{dateTimeTodaySuffix}"; + } // e.g: "Use 'context.CurrentUtcDateTime' instead of 'DateTime.Now'" // e.g: "Use 'context.CurrentUtcDateTime.Date' instead of 'DateTime.Today'" + // e.g: "Use '(DateTimeOffset)context.CurrentUtcDateTime' instead of 'DateTimeOffset.Now'" string title = string.Format( CultureInfo.InvariantCulture, Resources.UseInsteadFixerTitle, @@ -50,15 +68,15 @@ protected override void RegisterCodeFixes(CodeFixContext context, OrchestrationC context.RegisterCodeFix( CodeAction.Create( title: title, - createChangedDocument: c => ReplaceDateTime(context.Document, orchestrationContext.Root, dateTimeExpression, contextParameterName, isDateTimeToday), + createChangedDocument: c => ReplaceDateTime(context.Document, orchestrationContext.Root, dateTimeExpression, contextParameterName, isDateTimeToday, isDateTimeOffset), equivalenceKey: title), // This key is used to prevent duplicate code fixes. context.Diagnostics); } - static Task ReplaceDateTime(Document document, SyntaxNode oldRoot, MemberAccessExpressionSyntax incorrectDateTimeSyntax, string contextParameterName, bool isDateTimeToday) + static Task ReplaceDateTime(Document document, SyntaxNode oldRoot, MemberAccessExpressionSyntax incorrectDateTimeSyntax, string contextParameterName, bool isDateTimeToday, bool isDateTimeOffset) { // Builds a 'context.CurrentUtcDateTime' syntax node - MemberAccessExpressionSyntax correctDateTimeSyntax = + ExpressionSyntax correctDateTimeSyntax = MemberAccessExpression( SyntaxKind.SimpleMemberAccessExpression, IdentifierName(contextParameterName), @@ -73,6 +91,15 @@ static Task ReplaceDateTime(Document document, SyntaxNode oldRoot, Mem IdentifierName("Date")); } + // If the original expression was DateTimeOffset, we need to cast the DateTime to DateTimeOffset + // This is done using a CastExpression: (DateTimeOffset)context.CurrentUtcDateTime + if (isDateTimeOffset) + { + correctDateTimeSyntax = CastExpression( + IdentifierName("DateTimeOffset"), + correctDateTimeSyntax); + } + // Replaces the old local declaration with the new local declaration. SyntaxNode newRoot = oldRoot.ReplaceNode(incorrectDateTimeSyntax, correctDateTimeSyntax); Document newDocument = document.WithSyntaxRoot(newRoot); diff --git a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs index 0f5fd842c..4c5a1889e 100644 --- a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs +++ b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs @@ -377,6 +377,82 @@ await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix, test => } + [Theory] + [InlineData("DateTimeOffset.Now")] + [InlineData("DateTimeOffset.UtcNow")] + public async Task DurableFunctionOrchestrationUsingDateTimeOffsetNonDeterministicPropertiesHasDiag(string expression) + { + string code = Wrapper.WrapDurableFunctionOrchestration($@" +[Function(""Run"")] +DateTimeOffset Run([OrchestrationTrigger] TaskOrchestrationContext context) +{{ + return {{|#0:{expression}|}}; +}} +"); + + string fix = Wrapper.WrapDurableFunctionOrchestration($@" +[Function(""Run"")] +DateTimeOffset Run([OrchestrationTrigger] TaskOrchestrationContext context) +{{ + return (DateTimeOffset)context.CurrentUtcDateTime; +}} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", $"System.{expression}", "Run"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + + [Fact] + public async Task TaskOrchestratorUsingDateTimeOffsetHasDiag() + { + string code = Wrapper.WrapTaskOrchestrator(@" +public class MyOrchestrator : TaskOrchestrator +{ + public override Task RunAsync(TaskOrchestrationContext context, string input) + { + return Task.FromResult({|#0:DateTimeOffset.Now|}); + } +} +"); + + string fix = Wrapper.WrapTaskOrchestrator(@" +public class MyOrchestrator : TaskOrchestrator +{ + public override Task RunAsync(TaskOrchestrationContext context, string input) + { + return Task.FromResult((DateTimeOffset)context.CurrentUtcDateTime); + } +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("RunAsync", "System.DateTimeOffset.Now", "MyOrchestrator"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + + [Fact] + public async Task FuncOrchestratorWithDateTimeOffsetHasDiag() + { + string code = Wrapper.WrapFuncOrchestrator(@" +tasks.AddOrchestratorFunc(""HelloSequence"", context => +{ + return {|#0:DateTimeOffset.UtcNow|}; +}); +"); + + string fix = Wrapper.WrapFuncOrchestrator(@" +tasks.AddOrchestratorFunc(""HelloSequence"", context => +{ + return (DateTimeOffset)context.CurrentUtcDateTime; +}); +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Main", "System.DateTimeOffset.UtcNow", "HelloSequence"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + static DiagnosticResult BuildDiagnostic() { return VerifyCS.Diagnostic(DateTimeOrchestrationAnalyzer.DiagnosticId);