diff --git a/global.json b/global.json index 332b6c45b..4b560e684 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "10.0.101", + "version": "10.0.100", "rollForward": "latestFeature" }, "msbuild-sdks": { diff --git a/src/Analyzers/KnownTypeSymbols.Net.cs b/src/Analyzers/KnownTypeSymbols.Net.cs index 9803273ae..f39ce260e 100644 --- a/src/Analyzers/KnownTypeSymbols.Net.cs +++ b/src/Analyzers/KnownTypeSymbols.Net.cs @@ -23,6 +23,7 @@ public sealed partial class KnownTypeSymbols INamedTypeSymbol? cancellationToken; INamedTypeSymbol? environment; INamedTypeSymbol? httpClient; + INamedTypeSymbol? timeProvider; /// /// Gets a Guid type symbol. @@ -75,4 +76,9 @@ public sealed partial class KnownTypeSymbols /// Gets an HttpClient type symbol. /// public INamedTypeSymbol? HttpClient => this.GetOrResolveFullyQualifiedType(typeof(HttpClient).FullName, ref this.httpClient); + + /// + /// Gets a TimeProvider type symbol. + /// + public INamedTypeSymbol? TimeProvider => this.GetOrResolveFullyQualifiedType("System.TimeProvider", ref this.timeProvider); } diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index d65554814..94b30e901 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 or DateTimeOffset property is used in an orchestration method. +/// Analyzer that reports a warning when a non-deterministic DateTime, DateTimeOffset, or TimeProvider method is used in an orchestration method. /// [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer @@ -35,18 +35,20 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer SupportedDiagnostics => [Rule]; /// - /// Visitor that inspects the method body for DateTime and DateTimeOffset properties. + /// Visitor that inspects the method body for DateTime and DateTimeOffset properties, and TimeProvider method invocations. /// public sealed class DateTimeOrchestrationVisitor : MethodProbeOrchestrationVisitor { INamedTypeSymbol systemDateTimeSymbol = null!; INamedTypeSymbol? systemDateTimeOffsetSymbol; + INamedTypeSymbol? systemTimeProviderSymbol; /// public override bool Initialize() { this.systemDateTimeSymbol = this.Compilation.GetSpecialType(SpecialType.System_DateTime); this.systemDateTimeOffsetSymbol = this.Compilation.GetTypeByMetadataName("System.DateTimeOffset"); + this.systemTimeProviderSymbol = this.Compilation.GetTypeByMetadataName("System.TimeProvider"); return true; } @@ -85,6 +87,33 @@ protected override void VisitMethod(SemanticModel semanticModel, SyntaxNode meth reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, methodSymbol.Name, property.ToString(), orchestrationName)); } } + + // Check for TimeProvider method invocations + if (this.systemTimeProviderSymbol is not null) + { + foreach (IInvocationOperation operation in methodOperation.Descendants().OfType()) + { + IMethodSymbol invokedMethod = operation.TargetMethod; + + // Check if the method is called on TimeProvider type + bool isTimeProvider = invokedMethod.ContainingType.Equals(this.systemTimeProviderSymbol, SymbolEqualityComparer.Default); + + if (!isTimeProvider) + { + continue; + } + + // Check for non-deterministic TimeProvider methods: GetUtcNow, GetLocalNow, GetTimestamp + bool isNonDeterministicMethod = invokedMethod.Name is "GetUtcNow" or "GetLocalNow" or "GetTimestamp"; + + if (isNonDeterministicMethod) + { + // e.g.: "The method 'Method1' uses 'System.TimeProvider.GetUtcNow()' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" + string timeProviderMethodName = $"{invokedMethod.ContainingType}.{invokedMethod.Name}()"; + reportDiagnostic(RoslynExtensions.BuildDiagnostic(Rule, operation.Syntax, methodSymbol.Name, timeProviderMethodName, orchestrationName)); + } + } + } } } } diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs index fd233d1bb..6ea59d5e4 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationFixer.cs @@ -26,51 +26,87 @@ 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 or DateTimeOffset.Now) - if (orchestrationContext.SyntaxNodeWithDiagnostic is not MemberAccessExpressionSyntax dateTimeExpression) - { - return; - } - // 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"; - // Build the recommendation text - string recommendation; - if (isDateTimeOffset) + // Handle DateTime/DateTimeOffset property access (e.g. DateTime.Now or DateTimeOffset.Now) + if (orchestrationContext.SyntaxNodeWithDiagnostic is MemberAccessExpressionSyntax dateTimeExpression) { - // For DateTimeOffset, we always just cast CurrentUtcDateTime - recommendation = $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime"; + // Use semantic analysis to determine if this is a DateTimeOffset expression + ITypeSymbol? typeSymbol = semanticModel.GetTypeInfo(dateTimeExpression.Expression).Type; + bool isDateTimeOffset = typeSymbol?.ToDisplayString() == "System.DateTimeOffset"; + + bool isDateTimeToday = dateTimeExpression.Name.ToString() == "Today"; + + // 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, + recommendation, + dateTimeExpression.ToString()); + + context.RegisterCodeFix( + CodeAction.Create( + title: title, + createChangedDocument: c => ReplaceDateTime(context.Document, orchestrationContext.Root, dateTimeExpression, contextParameterName, isDateTimeToday, isDateTimeOffset), + equivalenceKey: title), // This key is used to prevent duplicate code fixes. + context.Diagnostics); } - else + + // Handle TimeProvider method invocations (e.g. TimeProvider.System.GetUtcNow()) + else if (orchestrationContext.SyntaxNodeWithDiagnostic is InvocationExpressionSyntax timeProviderInvocation) { - // For DateTime, we may need to add .Date for Today - string dateTimeTodaySuffix = isDateTimeToday ? ".Date" : string.Empty; - recommendation = $"{contextParameterName}.CurrentUtcDateTime{dateTimeTodaySuffix}"; - } + // Determine the method being called + if (semanticModel.GetSymbolInfo(timeProviderInvocation).Symbol is IMethodSymbol methodSymbol) + { + string methodName = methodSymbol.Name; + + // Check if the method returns DateTimeOffset + bool returnsDateTimeOffset = methodSymbol.ReturnType.ToDisplayString() == "System.DateTimeOffset"; - // 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, - recommendation, - dateTimeExpression.ToString()); - - context.RegisterCodeFix( - CodeAction.Create( - title: title, - createChangedDocument: c => ReplaceDateTime(context.Document, orchestrationContext.Root, dateTimeExpression, contextParameterName, isDateTimeToday, isDateTimeOffset), - equivalenceKey: title), // This key is used to prevent duplicate code fixes. - context.Diagnostics); + // Build the recommendation based on the method name + string recommendation = methodName switch + { + "GetUtcNow" when returnsDateTimeOffset => $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime", + "GetUtcNow" => $"{contextParameterName}.CurrentUtcDateTime", + "GetLocalNow" when returnsDateTimeOffset => $"(DateTimeOffset){contextParameterName}.CurrentUtcDateTime.ToLocalTime()", + "GetLocalNow" => $"{contextParameterName}.CurrentUtcDateTime.ToLocalTime()", + "GetTimestamp" => $"{contextParameterName}.CurrentUtcDateTime.Ticks", + _ => $"{contextParameterName}.CurrentUtcDateTime", + }; + + // e.g: "Use 'context.CurrentUtcDateTime' instead of 'TimeProvider.System.GetUtcNow()'" + string title = string.Format( + CultureInfo.InvariantCulture, + Resources.UseInsteadFixerTitle, + recommendation, + timeProviderInvocation.ToString()); + + context.RegisterCodeFix( + CodeAction.Create( + title: title, + createChangedDocument: c => ReplaceTimeProvider(context.Document, orchestrationContext.Root, timeProviderInvocation, contextParameterName, methodName, returnsDateTimeOffset), + equivalenceKey: title), + context.Diagnostics); + } + } } static Task ReplaceDateTime(Document document, SyntaxNode oldRoot, MemberAccessExpressionSyntax incorrectDateTimeSyntax, string contextParameterName, bool isDateTimeToday, bool isDateTimeOffset) @@ -106,4 +142,49 @@ static Task ReplaceDateTime(Document document, SyntaxNode oldRoot, Mem return Task.FromResult(newDocument); } + + static Task ReplaceTimeProvider(Document document, SyntaxNode oldRoot, InvocationExpressionSyntax incorrectTimeProviderSyntax, string contextParameterName, string methodName, bool returnsDateTimeOffset) + { + // Build the correct expression based on the method name + ExpressionSyntax correctExpression = methodName switch + { + "GetUtcNow" => MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(contextParameterName), + IdentifierName("CurrentUtcDateTime")), + "GetLocalNow" => InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(contextParameterName), + IdentifierName("CurrentUtcDateTime")), + IdentifierName("ToLocalTime"))), + "GetTimestamp" => MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(contextParameterName), + IdentifierName("CurrentUtcDateTime")), + IdentifierName("Ticks")), + _ => MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(contextParameterName), + IdentifierName("CurrentUtcDateTime")), + }; + + // If the method returns DateTimeOffset, we need to cast the DateTime to DateTimeOffset + if (returnsDateTimeOffset) + { + correctExpression = CastExpression( + IdentifierName("DateTimeOffset"), + correctExpression); + } + + // Replaces the old invocation with the new expression + SyntaxNode newRoot = oldRoot.ReplaceNode(incorrectTimeProviderSyntax, correctExpression); + Document newDocument = document.WithSyntaxRoot(newRoot); + + return Task.FromResult(newDocument); + } } diff --git a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs index 4c5a1889e..6989b86a3 100644 --- a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs +++ b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs @@ -453,6 +453,130 @@ public async Task FuncOrchestratorWithDateTimeOffsetHasDiag() await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); } + [Theory] + [InlineData("TimeProvider.System.GetUtcNow()")] + [InlineData("TimeProvider.System.GetLocalNow()")] + public async Task DurableFunctionOrchestrationUsingTimeProviderNonDeterministicMethodsHasDiag(string expression) + { + string code = Wrapper.WrapDurableFunctionOrchestration($@" +[Function(""Run"")] +DateTimeOffset Run([OrchestrationTrigger] TaskOrchestrationContext context) +{{ + return {{|#0:{expression}|}}; +}} +"); + + string expectedReplacement = expression.Contains("GetLocalNow") + ? "(DateTimeOffset)context.CurrentUtcDateTime.ToLocalTime()" + : "(DateTimeOffset)context.CurrentUtcDateTime"; + + string fix = Wrapper.WrapDurableFunctionOrchestration($@" +[Function(""Run"")] +DateTimeOffset Run([OrchestrationTrigger] TaskOrchestrationContext context) +{{ + return {expectedReplacement}; +}} +"); + + // The analyzer reports the method name as "System.TimeProvider.GetUtcNow()" or "System.TimeProvider.GetLocalNow()" + string methodName = expression.Contains("GetLocalNow") ? "System.TimeProvider.GetLocalNow()" : "System.TimeProvider.GetUtcNow()"; + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", methodName, "Run"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + + [Fact] + public async Task DurableFunctionOrchestrationUsingTimeProviderGetTimestampHasDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +long Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return {|#0:TimeProvider.System.GetTimestamp()|}; +} +"); + + string fix = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +long Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return context.CurrentUtcDateTime.Ticks; +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.TimeProvider.GetTimestamp()", "Run"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + + [Fact] + public async Task TaskOrchestratorUsingTimeProviderHasDiag() + { + string code = Wrapper.WrapTaskOrchestrator(@" +public class MyOrchestrator : TaskOrchestrator +{ + public override Task RunAsync(TaskOrchestrationContext context, string input) + { + return Task.FromResult({|#0:TimeProvider.System.GetUtcNow()|}); + } +} +"); + + 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.TimeProvider.GetUtcNow()", "MyOrchestrator"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + + [Fact] + public async Task FuncOrchestratorWithTimeProviderHasDiag() + { + string code = Wrapper.WrapFuncOrchestrator(@" +tasks.AddOrchestratorFunc(""HelloSequence"", context => +{ + return {|#0:TimeProvider.System.GetUtcNow()|}; +}); +"); + + string fix = Wrapper.WrapFuncOrchestrator(@" +tasks.AddOrchestratorFunc(""HelloSequence"", context => +{ + return (DateTimeOffset)context.CurrentUtcDateTime; +}); +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Main", "System.TimeProvider.GetUtcNow()", "HelloSequence"); + + await VerifyCS.VerifyDurableTaskCodeFixAsync(code, expected, fix); + } + + [Fact] + public async Task DurableFunctionOrchestrationInvokingMethodWithTimeProviderHasDiag() + { + string code = Wrapper.WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +DateTimeOffset Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return GetTime(); +} + +DateTimeOffset GetTime() => {|#0:TimeProvider.System.GetUtcNow()|}; +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("GetTime", "System.TimeProvider.GetUtcNow()", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + static DiagnosticResult BuildDiagnostic() { return VerifyCS.Diagnostic(DateTimeOrchestrationAnalyzer.DiagnosticId); diff --git a/test/Analyzers.Tests/Verifiers/References.cs b/test/Analyzers.Tests/Verifiers/References.cs index 1df31e762..b6b33d93c 100644 --- a/test/Analyzers.Tests/Verifiers/References.cs +++ b/test/Analyzers.Tests/Verifiers/References.cs @@ -11,7 +11,7 @@ public static class References public static ReferenceAssemblies CommonAssemblies => durableAssemblyReferences.Value; - static ReferenceAssemblies BuildReferenceAssemblies() => ReferenceAssemblies.Net.Net60.AddPackages([ + static ReferenceAssemblies BuildReferenceAssemblies() => ReferenceAssemblies.Net.Net80.AddPackages([ new PackageIdentity("Azure.Storage.Blobs", "12.17.0"), new PackageIdentity("Azure.Storage.Queues", "12.17.0"), new PackageIdentity("Azure.Data.Tables", "12.8.3"),