From 7ef6afdd177976bfd1db344e526f69a1f7ae2fa8 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Fri, 5 Apr 2024 11:47:40 -0300 Subject: [PATCH 01/20] datetime analyzer --- src/Analyzers/AnalyzerReleases.Shipped.md | 3 + src/Analyzers/AnalyzerReleases.Unshipped.md | 8 + src/Analyzers/Helpers/AnalyzersCategories.cs | 9 + src/Analyzers/Helpers/KnownTypeSymbols.cs | 44 +++ src/Analyzers/Helpers/RoslynExtensions.cs | 61 +++++ .../DateTimeOrchestrationAnalyzer.cs | 70 +++++ .../Orchestration/OrchestrationAnalyzer.cs | 131 +++++++++ .../DateTimeOrchestrationAnalyzerTests.cs | 251 ++++++++++++++++++ .../CSharpAnalyzerVerifier.Durable.cs | 28 ++ 9 files changed, 605 insertions(+) create mode 100644 src/Analyzers/AnalyzerReleases.Shipped.md create mode 100644 src/Analyzers/AnalyzerReleases.Unshipped.md create mode 100644 src/Analyzers/Helpers/AnalyzersCategories.cs create mode 100644 src/Analyzers/Helpers/KnownTypeSymbols.cs create mode 100644 src/Analyzers/Helpers/RoslynExtensions.cs create mode 100644 src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs create mode 100644 src/Analyzers/Orchestration/OrchestrationAnalyzer.cs create mode 100644 test/Analyzers.Test/Orchestration/DateTimeOrchestrationAnalyzerTests.cs create mode 100644 test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.Durable.cs diff --git a/src/Analyzers/AnalyzerReleases.Shipped.md b/src/Analyzers/AnalyzerReleases.Shipped.md new file mode 100644 index 000000000..60b59dd99 --- /dev/null +++ b/src/Analyzers/AnalyzerReleases.Shipped.md @@ -0,0 +1,3 @@ +; Shipped analyzer releases +; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md new file mode 100644 index 000000000..5f2748e4f --- /dev/null +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -0,0 +1,8 @@ +; Unshipped analyzer release +; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +DURABLE0001 | Orchestration | Warning | DateTimeOrchestrationAnalyzer \ No newline at end of file diff --git a/src/Analyzers/Helpers/AnalyzersCategories.cs b/src/Analyzers/Helpers/AnalyzersCategories.cs new file mode 100644 index 000000000..045cb3212 --- /dev/null +++ b/src/Analyzers/Helpers/AnalyzersCategories.cs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.DurableTask.Analyzers.Helpers; + +static class AnalyzersCategories +{ + public const string Orchestration = "Orchestration"; +} diff --git a/src/Analyzers/Helpers/KnownTypeSymbols.cs b/src/Analyzers/Helpers/KnownTypeSymbols.cs new file mode 100644 index 000000000..3ddb844cf --- /dev/null +++ b/src/Analyzers/Helpers/KnownTypeSymbols.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis; + +namespace Microsoft.DurableTask.Analyzers.Helpers; + +/// +/// Provides a set of well-known types that are used by the analyzers. +/// Inspired by KnownTypeSymbols class in +/// System.Text.Json.SourceGeneration source code. +/// Lazy initialization is used to avoid the the initialization of all types during class construction, since not all symbols are used by all analyzers. +/// +sealed class KnownTypeSymbols(Compilation compilation) +{ + readonly Compilation compilation = compilation; + + Cached orchestrationTriggerAttribute; + public INamedTypeSymbol? OrchestrationTriggerAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.orchestrationTriggerAttribute); + + Cached functionAttribute; + public INamedTypeSymbol? FunctionAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionAttribute); + + INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref Cached field) + { + if (field.HasValue) + { + return field.Value; + } + + INamedTypeSymbol? type = this.compilation.GetTypeByMetadataName(fullyQualifiedName); + field = new(type); + return type; + } + + // We could use Lazy here, but because we need to use the `compilation` variable instance, + // that would require us to initiate the Lazy lambdas in the constructor. + // Because not all analyzers use all symbols, we would be allocating unnecessary lambdas. + readonly struct Cached(T value) + { + public readonly bool HasValue = true; + public readonly T Value = value; + } +} diff --git a/src/Analyzers/Helpers/RoslynExtensions.cs b/src/Analyzers/Helpers/RoslynExtensions.cs new file mode 100644 index 000000000..f1f308ce4 --- /dev/null +++ b/src/Analyzers/Helpers/RoslynExtensions.cs @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis; + +namespace Microsoft.DurableTask.Analyzers.Helpers; + +static class RoslynExtensions +{ + public static bool TryGetSingleValueFromAttribute(this ISymbol? symbol, INamedTypeSymbol attributeSymbol, out T value) + { + if (symbol.TryGetConstructorArgumentsFromAttribute(attributeSymbol, out ImmutableArray constructorArguments)) + { + object? valueObj = constructorArguments.FirstOrDefault().Value; + if (valueObj != null) + { + value = (T)valueObj; + return true; + } + } + + value = default!; + return false; + } + + public static bool TryGetConstructorArgumentsFromAttribute(this ISymbol? symbol, INamedTypeSymbol attributeSymbol, out ImmutableArray constructorArguments) + { + if (symbol != null) + { + foreach (AttributeData attribute in symbol.GetAttributes()) + { + if (attributeSymbol.Equals(attribute.AttributeClass, SymbolEqualityComparer.Default)) + { + constructorArguments = attribute.ConstructorArguments; + return true; + } + } + } + + return false; + } + + public static bool ContainsAttributeInAnyMethodArguments(this IMethodSymbol methodSymbol, INamedTypeSymbol attributeSymbol) + { + return methodSymbol.Parameters + .SelectMany(p => p.GetAttributes()) + .Any(a => attributeSymbol.Equals(a.AttributeClass, SymbolEqualityComparer.Default)); + } + + public static void ReportDiagnostic(this CompilationAnalysisContext ctx, DiagnosticDescriptor descriptor, IOperation operation, params string[] messageArgs) + { + ctx.ReportDiagnostic(BuildDiagnostic(descriptor, operation.Syntax, messageArgs)); + } + + public static Diagnostic BuildDiagnostic(DiagnosticDescriptor descriptor, SyntaxNode syntaxNode, params string[] messageArgs) + { + return Diagnostic.Create(descriptor, syntaxNode.GetLocation(), messageArgs); + } +} diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs new file mode 100644 index 000000000..b54e22b55 --- /dev/null +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Concurrent; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis; +using Microsoft.DurableTask.Analyzers.Helpers; + +namespace Microsoft.DurableTask.Analyzers.Orchestration; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer +{ + public const string DiagnosticId = "DURABLE0001"; + + static readonly DiagnosticDescriptor Rule = new( + DiagnosticId, + "DateTime calls must be deterministic inside an orchestration function", + "The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from Orchestration Function '{2}'", + AnalyzersCategories.Orchestration, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public override ImmutableArray SupportedDiagnostics => [Rule]; + + protected override void RegisterAdditionalCompilationStartAction(CompilationStartAnalysisContext context, OrchestrationAnalysisResult orchestrationAnalysisResult) + { + INamedTypeSymbol systemDateTimeSymbol = context.Compilation.GetSpecialType(SpecialType.System_DateTime); + + ConcurrentBag<(ISymbol symbol, IPropertyReferenceOperation operation)> dateTimeUsage = []; + + // search for usages of DateTime.Now, DateTime.UtcNow, DateTime.Today and store them + context.RegisterOperationAction(ctx => + { + ctx.CancellationToken.ThrowIfCancellationRequested(); + + var operation = (IPropertyReferenceOperation)ctx.Operation; + IPropertySymbol property = operation.Property; + + if (property.ContainingSymbol.Equals(systemDateTimeSymbol, SymbolEqualityComparer.Default) && + property.Name is nameof(DateTime.Now) or nameof(DateTime.UtcNow) or nameof(DateTime.Today)) + { + ISymbol method = ctx.ContainingSymbol; + dateTimeUsage.Add((method, operation)); + } + }, OperationKind.PropertyReference); + + // compare whether the found DateTime usages occur in methods invoked by orchestrations + context.RegisterCompilationEndAction(ctx => + { + foreach ((ISymbol symbol, IPropertyReferenceOperation operation) in dateTimeUsage) + { + if (symbol is IMethodSymbol method) + { + if (orchestrationAnalysisResult.OrchestrationsByMethod.TryGetValue(method, out ConcurrentBag orchestrations)) + { + string methodName = symbol.Name; + string dateTimePropertyName = operation.Property.ToString(); + string functionsNames = string.Join(", ", orchestrations.Select(o => o.FunctionName).OrderBy(n => n)); + + // e.g.: "The method 'Method1' uses 'System.Date.Now' that may cause non-deterministic behavior when invoked from Orchestration Function 'Run1'" + ctx.ReportDiagnostic(Rule, operation, methodName, dateTimePropertyName, functionsNames); + } + } + } + }); + } +} diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs new file mode 100644 index 000000000..41e5ce727 --- /dev/null +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -0,0 +1,131 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Concurrent; +using System.Diagnostics; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.DurableTask.Analyzers.Helpers; + +namespace Microsoft.DurableTask.Analyzers.Orchestration; + +public abstract class OrchestrationAnalyzer : DiagnosticAnalyzer +{ + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(context => + { + var knownSymbols = new KnownTypeSymbols(context.Compilation); + + if (knownSymbols.OrchestrationTriggerAttribute == null || knownSymbols.FunctionAttribute == null) + { + // symbols not available in this compilation, skip analysis + return; + } + + OrchestrationAnalysisResult result = new(); + + context.RegisterSyntaxNodeAction(ctx => + { + ctx.CancellationToken.ThrowIfCancellationRequested(); + + // Checks whether the declared method is an orchestration, if not, returns + if (ctx.ContainingSymbol is not IMethodSymbol methodSymbol || + !methodSymbol.ContainsAttributeInAnyMethodArguments(knownSymbols.OrchestrationTriggerAttribute) || + !methodSymbol.TryGetSingleValueFromAttribute(knownSymbols.FunctionAttribute, out string functionName)) + { + return; + } + + var orchestration = new OrchestrationMethod(functionName, methodSymbol); + var methodSyntax = (MethodDeclarationSyntax)ctx.Node; + + FindInvokedMethods(ctx.SemanticModel, methodSyntax, methodSymbol, orchestration, result); + }, SyntaxKind.MethodDeclaration); + + // allows concrete implementations to register specific actions/analysis and then check if they happen in any of the orchestration methods + this.RegisterAdditionalCompilationStartAction(context, result); + }); + } + + // Recursively find all methods invoked by the orchestration method + static void FindInvokedMethods( + SemanticModel semanticModel, MethodDeclarationSyntax callerSyntax, IMethodSymbol callerSymbol, + OrchestrationMethod rootOrchestration, OrchestrationAnalysisResult result) + { + if (!TryTrackMethod(semanticModel, callerSyntax, callerSymbol, rootOrchestration, result)) + { + // previously tracked method, leave to avoid infinite recursion + return; + } + + foreach (InvocationExpressionSyntax invocationSyntax in callerSyntax.DescendantNodes().OfType()) + { + IOperation? operation = semanticModel.GetOperation(invocationSyntax); + if (operation == null || operation is not IInvocationOperation invocation) + { + continue; + } + + IMethodSymbol calleeMethodSymbol = invocation.TargetMethod; + if (calleeMethodSymbol == null) + { + continue; + } + + // iterating over multiple syntax references is needed because the same method can be declared in multiple places (e.g. partial classes) + IEnumerable calleeSyntaxes = calleeMethodSymbol.DeclaringSyntaxReferences.Select(r => r.GetSyntax()).OfType(); + foreach (MethodDeclarationSyntax calleeSyntax in calleeSyntaxes) + { + FindInvokedMethods(semanticModel, calleeSyntax, calleeMethodSymbol, rootOrchestration, result); + } + } + } + + static bool TryTrackMethod(SemanticModel semanticModel, MethodDeclarationSyntax callerSyntax, IMethodSymbol callerSymbol, + OrchestrationMethod rootOrchestration, OrchestrationAnalysisResult result) + { + ConcurrentBag orchestrations = result.OrchestrationsByMethod.GetOrAdd(callerSymbol, []); + if (orchestrations.Contains(rootOrchestration)) + { + return false; + } + + orchestrations.Add(rootOrchestration); + + return true; + } + + /// + /// Register additional actions to be executed after the compilation has started. + /// It is expected from a concrete implementation of to register a + /// + /// and then compare that any discovered violations happened in any of the symbols in . + /// + /// Context originally provided by + /// Collection of symbols referenced by orchestrations + protected abstract void RegisterAdditionalCompilationStartAction(CompilationStartAnalysisContext context, OrchestrationAnalysisResult orchestrationAnalysisResult); + + protected readonly struct OrchestrationAnalysisResult + { + public ConcurrentDictionary> OrchestrationsByMethod { get; } + + public OrchestrationAnalysisResult() + { + this.OrchestrationsByMethod = new(SymbolEqualityComparer.Default); + } + } + + [DebuggerDisplay("[{FunctionName}] {OrchestrationMethodSymbol.Name}")] + protected readonly struct OrchestrationMethod(string functionName, IMethodSymbol orchestrationMethodSymbol) + { + public string FunctionName { get; } = functionName; + public IMethodSymbol OrchestrationMethodSymbol { get; } = orchestrationMethodSymbol; + } +} diff --git a/test/Analyzers.Test/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Test/Orchestration/DateTimeOrchestrationAnalyzerTests.cs new file mode 100644 index 000000000..5ebefab64 --- /dev/null +++ b/test/Analyzers.Test/Orchestration/DateTimeOrchestrationAnalyzerTests.cs @@ -0,0 +1,251 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis.Testing; +using Microsoft.DurableTask.Analyzers.Orchestration; + +using VerifyCS = Microsoft.DurableTask.Analyzers.Test.Verifiers.CSharpAnalyzerVerifier; + +namespace Microsoft.DurableTask.Analyzers.Test.Orchestration; + +public class DateTimeOrchestrationAnalyzerTests +{ + [Fact] + public async Task OrchestrationUsingDateTimeNowHasDiag() + { + string code = Wrap(@" +[Function(""Run"")] +DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return {|#0:DateTime.Now|}; +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.DateTime.Now", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task OrchestrationUsingDateTimeUtcNowHasDiag() + { + string code = Wrap(@" +[Function(""Run"")] +DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return {|#0:DateTime.UtcNow|}; +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.DateTime.UtcNow", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task OrchestrationUsingDateTimeTodayNowHasDiag() + { + string code = Wrap(@" +[Function(""Run"")] +DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + return {|#0:DateTime.Today|}; +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.DateTime.Today", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task ChainedMethodsInvokedBySingleOrchestrationHasDiag() + { + string code = Wrap(@" +[Function(""Run"")] +long Run([OrchestrationTrigger] TaskOrchestrationContext context) => Level1(); + +long Level1() => Level2(); + +long Level2() => Level3(); + +long Level3() => {|#0:DateTime.Now|}.Ticks; +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Level3", "System.DateTime.Now", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task ChainedMethodsInvokedByMultipleOrchestrationsHasDiag() + { + string code = Wrap(@" +[Function(""Run1"")] +long Run1([OrchestrationTrigger] TaskOrchestrationContext context) => Level1(); + +[Function(""Run2"")] +long Run2([OrchestrationTrigger] TaskOrchestrationContext context) => Level1(); + +long Level1() => Level2(); + +long Level2() => Level3(); + +long Level3() => {|#0:DateTime.Now|}.Ticks; +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Level3", "System.DateTime.Now", "Run1, Run2"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task RecursiveMethodInvokedByOrchestrationHasSingleDiag() + { + string code = Wrap(@" +[Function(""Run"")] +long Run([OrchestrationTrigger] TaskOrchestrationContext context) => RecursiveMethod(0); + +long RecursiveMethod(int i){ + if (i == 10) return 1; + DateTime date = {|#0:DateTime.Now|}; + return date.Ticks + RecursiveMethod(i + 1); +} +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("RecursiveMethod", "System.DateTime.Now", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task MethodInvokedMultipleTimesByOrchestrationHasSingleDiag() + { + string code = Wrap(@" +[Function(""Run"")] +void Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + _ = Method(); + _ = Method(); +} + +DateTime Method() => {|#0:DateTime.Now|}; +"); + + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "System.DateTime.Now", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task OrchestrationUsingDateTimeInLambdasHasDiag() + { + string code = Wrap(@" +[Function(""Run"")] +void Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + static DateTime fn0() => {|#0:DateTime.Now|}; + Func fn1 = () => {|#1:DateTime.Now|}; + Func fn2 = days => {|#2:DateTime.Now|}.AddDays(days); + Action fn3 = days => Console.WriteLine({|#3:DateTime.Now|}.AddDays(days)); +} +"); + + DiagnosticResult[] expected = Enumerable.Range(0, 4).Select( + i => BuildDiagnostic().WithLocation(i).WithArguments($"Run", "System.DateTime.Now", "Run")).ToArray(); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task OrchestrationUsingAsyncInvocationsHasDiag() + { + string code = Wrap(@" +[Function(nameof(Run))] +async Task Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + _ = await ValueTaskInvocation(); + _ = await TaskInvocation(); +} + +static ValueTask ValueTaskInvocation() => ValueTask.FromResult({|#0:DateTime.Now|}); + +static Task TaskInvocation() => Task.FromResult({|#1:DateTime.Now|}); +"); + + DiagnosticResult valueTaskExpected = BuildDiagnostic().WithLocation(0).WithArguments("ValueTaskInvocation", "System.DateTime.Now", "Run"); + DiagnosticResult taskExpected = BuildDiagnostic().WithLocation(1).WithArguments("TaskInvocation", "System.DateTime.Now", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, valueTaskExpected, taskExpected); + } + + [Fact] + public async Task EmptyCodeWithNoSymbolsAvailableHasNoDiag() + { + string code = @""; + + // checks that empty code with no assembly references of Durable Functions has no diagnostics. + // this guarantees that if someone adds our analyzer to a project that doesn't use Durable Functions, + // the analyzer won't crash/they won't get any diagnostics + await VerifyCS.VerifyAnalyzerAsync(code); + } + + [Fact] + public async Task EmptyCodeWithSymbolsAvailableHasNoDiag() + { + string code = @""; + + // checks that empty code with access to assembly references of Durable Functions has no diagnostics + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task NonOrchestrationHasNoDiag() + { + string code = Wrap(@" +[Function(""Func"")] +void Func(){ + Console.WriteLine(DateTime.Now); +} + +void Method(){ + Console.WriteLine(DateTime.Now); +} +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task MethodNotCalledByOrchestrationHasNoDiag() + { + string code = Wrap(@" +[Function(""Run"")] +DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) => new DateTime(2024, 1, 1); + +DateTime NotCalled() => DateTime.Now; +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + static string Wrap(string code) + { + return $@" +using System; +using System.Threading.Tasks; +using Microsoft.Azure.Functions.Worker; +using Microsoft.DurableTask; + +class Orchestrator +{{ +{code} +}} +"; + } + + static DiagnosticResult BuildDiagnostic() + { + return VerifyCS.Diagnostic(DateTimeOrchestrationAnalyzer.DiagnosticId); + } +} diff --git a/test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.Durable.cs new file mode 100644 index 000000000..7aa7afa5a --- /dev/null +++ b/test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.Durable.cs @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; + +namespace Microsoft.DurableTask.Analyzers.Test.Verifiers; + +public static partial class CSharpAnalyzerVerifier + where TAnalyzer : DiagnosticAnalyzer, new() +{ + /// + public static async Task VerifyDurableTaskAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new Test() + { + TestCode = source, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60.AddPackages([ + new PackageIdentity("Microsoft.Azure.Functions.Worker", "1.21.0"), + new PackageIdentity("Microsoft.Azure.Functions.Worker.Extensions.DurableTask", "1.1.1") + ]), + }; + + test.ExpectedDiagnostics.AddRange(expected); + + await test.RunAsync(CancellationToken.None); + } +} From b107350ba1eb10a0550445506d5c5894b7a91419 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Mon, 8 Apr 2024 12:36:49 -0300 Subject: [PATCH 02/20] adding xunit package --- test/Analyzers.Test/Analyzers.Test.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Analyzers.Test/Analyzers.Test.csproj b/test/Analyzers.Test/Analyzers.Test.csproj index 22aaae2c8..02d715a7b 100644 --- a/test/Analyzers.Test/Analyzers.Test.csproj +++ b/test/Analyzers.Test/Analyzers.Test.csproj @@ -4,6 +4,7 @@ + From 74b6b3dbf8ced573821ca9f00cc44057bdc534a0 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Mon, 8 Apr 2024 14:22:34 -0300 Subject: [PATCH 03/20] renaming Analyzers.Test to Analyzers.Tests --- Microsoft.DurableTask.sln | 2 +- .../Analyzers.Tests.csproj} | 1 - .../Orchestration/DateTimeOrchestrationAnalyzerTests.cs | 0 test/{Analyzers.Test => Analyzers.Tests}/Usings.cs | 0 .../Verifiers/CSharpAnalyzerVerifier.Durable.cs | 0 .../Verifiers/CSharpAnalyzerVerifier.cs | 0 .../Verifiers/CSharpCodeFixVerifier.cs | 0 7 files changed, 1 insertion(+), 2 deletions(-) rename test/{Analyzers.Test/Analyzers.Test.csproj => Analyzers.Tests/Analyzers.Tests.csproj} (90%) rename test/{Analyzers.Test => Analyzers.Tests}/Orchestration/DateTimeOrchestrationAnalyzerTests.cs (100%) rename test/{Analyzers.Test => Analyzers.Tests}/Usings.cs (100%) rename test/{Analyzers.Test => Analyzers.Tests}/Verifiers/CSharpAnalyzerVerifier.Durable.cs (100%) rename test/{Analyzers.Test => Analyzers.Tests}/Verifiers/CSharpAnalyzerVerifier.cs (100%) rename test/{Analyzers.Test => Analyzers.Tests}/Verifiers/CSharpCodeFixVerifier.cs (100%) diff --git a/Microsoft.DurableTask.sln b/Microsoft.DurableTask.sln index bec404b26..f465bd667 100644 --- a/Microsoft.DurableTask.sln +++ b/Microsoft.DurableTask.sln @@ -67,7 +67,7 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Benchmarks", "test\Benchmar EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Analyzers", "src\Analyzers\Analyzers.csproj", "{998E9D97-BD36-4A9D-81FC-5DAC1CE40083}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Analyzers.Test", "test\Analyzers.Test\Analyzers.Test.csproj", "{541FCCCE-1059-4691-B027-F761CD80DE92}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Analyzers.Tests", "test\Analyzers.Tests\Analyzers.Tests.csproj", "{541FCCCE-1059-4691-B027-F761CD80DE92}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/test/Analyzers.Test/Analyzers.Test.csproj b/test/Analyzers.Tests/Analyzers.Tests.csproj similarity index 90% rename from test/Analyzers.Test/Analyzers.Test.csproj rename to test/Analyzers.Tests/Analyzers.Tests.csproj index 02d715a7b..22aaae2c8 100644 --- a/test/Analyzers.Test/Analyzers.Test.csproj +++ b/test/Analyzers.Tests/Analyzers.Tests.csproj @@ -4,7 +4,6 @@ - diff --git a/test/Analyzers.Test/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs similarity index 100% rename from test/Analyzers.Test/Orchestration/DateTimeOrchestrationAnalyzerTests.cs rename to test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs diff --git a/test/Analyzers.Test/Usings.cs b/test/Analyzers.Tests/Usings.cs similarity index 100% rename from test/Analyzers.Test/Usings.cs rename to test/Analyzers.Tests/Usings.cs diff --git a/test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs similarity index 100% rename from test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.Durable.cs rename to test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs diff --git a/test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs similarity index 100% rename from test/Analyzers.Test/Verifiers/CSharpAnalyzerVerifier.cs rename to test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs diff --git a/test/Analyzers.Test/Verifiers/CSharpCodeFixVerifier.cs b/test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs similarity index 100% rename from test/Analyzers.Test/Verifiers/CSharpCodeFixVerifier.cs rename to test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs From abe7f769077c7bab185fec5afa2a8554205fe3ad Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Tue, 9 Apr 2024 11:18:22 -0300 Subject: [PATCH 04/20] supporting class based orchestrators --- src/Analyzers/Helpers/KnownTypeSymbols.cs | 17 +- src/Analyzers/Helpers/RoslynExtensions.cs | 39 +++ .../DateTimeOrchestrationAnalyzer.cs | 12 +- .../Orchestration/OrchestrationAnalyzer.cs | 184 +++++++++--- .../DateTimeOrchestrationAnalyzerTests.cs | 268 ++++++++++++------ 5 files changed, 398 insertions(+), 122 deletions(-) diff --git a/src/Analyzers/Helpers/KnownTypeSymbols.cs b/src/Analyzers/Helpers/KnownTypeSymbols.cs index 3ddb844cf..165adef09 100644 --- a/src/Analyzers/Helpers/KnownTypeSymbols.cs +++ b/src/Analyzers/Helpers/KnownTypeSymbols.cs @@ -15,11 +15,20 @@ sealed class KnownTypeSymbols(Compilation compilation) { readonly Compilation compilation = compilation; - Cached orchestrationTriggerAttribute; - public INamedTypeSymbol? OrchestrationTriggerAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.orchestrationTriggerAttribute); + Cached functionOrchestrationAttribute; + public INamedTypeSymbol? FunctionOrchestrationAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.functionOrchestrationAttribute); - Cached functionAttribute; - public INamedTypeSymbol? FunctionAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionAttribute); + Cached functionNameAttribute; + public INamedTypeSymbol? FunctionNameAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionNameAttribute); + + Cached taskOrchestratorInterface; + public INamedTypeSymbol? TaskOrchestratorInterface => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.ITaskOrchestrator", ref this.taskOrchestratorInterface); + + Cached taskOrchestratorBaseClass; + public INamedTypeSymbol? TaskOrchestratorBaseClass => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.TaskOrchestrator`2", ref this.taskOrchestratorBaseClass); + + Cached durableTaskRegistry; + public INamedTypeSymbol? DurableTaskRegistry => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.DurableTaskRegistry", ref this.durableTaskRegistry); INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref Cached field) { diff --git a/src/Analyzers/Helpers/RoslynExtensions.cs b/src/Analyzers/Helpers/RoslynExtensions.cs index f1f308ce4..a361b5778 100644 --- a/src/Analyzers/Helpers/RoslynExtensions.cs +++ b/src/Analyzers/Helpers/RoslynExtensions.cs @@ -4,6 +4,8 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Operations; namespace Microsoft.DurableTask.Analyzers.Helpers; @@ -49,6 +51,43 @@ public static bool ContainsAttributeInAnyMethodArguments(this IMethodSymbol meth .Any(a => attributeSymbol.Equals(a.AttributeClass, SymbolEqualityComparer.Default)); } + public static bool ImplementsInterface(this INamedTypeSymbol symbol, ISymbol interfaceSymbol) + { + return symbol.AllInterfaces.Any(i => interfaceSymbol.Equals(i, SymbolEqualityComparer.Default)); + } + + public static bool InheritsFromOpenGeneric(this INamedTypeSymbol symbol, ITypeSymbol type) + { + INamedTypeSymbol? baseType = symbol.BaseType; + while (baseType != null) + { + if (baseType.ConstructedFrom.Equals(type, SymbolEqualityComparer.Default)) + { + return true; + } + baseType = baseType.BaseType; + } + return false; + } + + public static IMethodSymbol? GetOverridenMethod(this INamedTypeSymbol typeSymbol, IMethodSymbol methodSymbol) + { + IEnumerable methods = typeSymbol.GetMembers(methodSymbol.Name).OfType(); + return methods.FirstOrDefault(m => m.OverriddenMethod != null && m.OverriddenMethod.OriginalDefinition.Equals(methodSymbol, SymbolEqualityComparer.Default)); + } + + public static IEnumerable GetSyntaxNodes(this IMethodSymbol methodSymbol) + { + return methodSymbol.DeclaringSyntaxReferences.Select(r => r.GetSyntax()).OfType(); + } + + public static Optional GetConstantValueFromAttribute(this IArgumentOperation argumentOperation, SemanticModel semanticModel, CancellationToken cancellationToken) + { + LiteralExpressionSyntax? nameLiteralSyntax = argumentOperation.Syntax.DescendantNodes().OfType().FirstOrDefault(); + + return semanticModel.GetConstantValue(nameLiteralSyntax, cancellationToken); + } + public static void ReportDiagnostic(this CompilationAnalysisContext ctx, DiagnosticDescriptor descriptor, IOperation operation, params string[] messageArgs) { ctx.ReportDiagnostic(BuildDiagnostic(descriptor, operation.Syntax, messageArgs)); diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index b54e22b55..82e351055 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -17,8 +17,8 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer static readonly DiagnosticDescriptor Rule = new( DiagnosticId, - "DateTime calls must be deterministic inside an orchestration function", - "The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from Orchestration Function '{2}'", + "System.DateTime calls must be deterministic inside an orchestration", + "The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}'", AnalyzersCategories.Orchestration, DiagnosticSeverity.Warning, isEnabledByDefault: true); @@ -54,14 +54,14 @@ property.Name is nameof(DateTime.Now) or nameof(DateTime.UtcNow) or nameof(DateT { if (symbol is IMethodSymbol method) { - if (orchestrationAnalysisResult.OrchestrationsByMethod.TryGetValue(method, out ConcurrentBag orchestrations)) + if (orchestrationAnalysisResult.OrchestrationsByMethod.TryGetValue(method, out ConcurrentBag orchestrations)) { string methodName = symbol.Name; string dateTimePropertyName = operation.Property.ToString(); - string functionsNames = string.Join(", ", orchestrations.Select(o => o.FunctionName).OrderBy(n => n)); + string orchestrationNames = string.Join(", ", orchestrations.Select(o => o.Name).OrderBy(n => n)); - // e.g.: "The method 'Method1' uses 'System.Date.Now' that may cause non-deterministic behavior when invoked from Orchestration Function 'Run1'" - ctx.ReportDiagnostic(Rule, operation, methodName, dateTimePropertyName, functionsNames); + // e.g.: "The method 'Method1' uses 'System.Date.Now' that may cause non-deterministic behavior when invoked from orchestration 'MyOrchestrator'" + ctx.ReportDiagnostic(Rule, operation, methodName, dateTimePropertyName, orchestrationNames); } } } diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 41e5ce727..816c80184 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System.Collections.Concurrent; -using System.Diagnostics; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -23,47 +22,182 @@ public override void Initialize(AnalysisContext context) { var knownSymbols = new KnownTypeSymbols(context.Compilation); - if (knownSymbols.OrchestrationTriggerAttribute == null || knownSymbols.FunctionAttribute == null) + if (knownSymbols.FunctionOrchestrationAttribute == null || knownSymbols.FunctionNameAttribute == null || + knownSymbols.TaskOrchestratorInterface == null || knownSymbols.TaskOrchestratorBaseClass == null || + knownSymbols.DurableTaskRegistry == null) { // symbols not available in this compilation, skip analysis return; } + IMethodSymbol runAsyncTaskOrchestratorInterface = knownSymbols.TaskOrchestratorInterface.GetMembers("RunAsync").OfType().First(); + IMethodSymbol runAsyncTaskOrchestratorBase = knownSymbols.TaskOrchestratorBaseClass.GetMembers("RunAsync").OfType().First(); + OrchestrationAnalysisResult result = new(); + // look for Durable Functions Orchestrations context.RegisterSyntaxNodeAction(ctx => { ctx.CancellationToken.ThrowIfCancellationRequested(); - // Checks whether the declared method is an orchestration, if not, returns - if (ctx.ContainingSymbol is not IMethodSymbol methodSymbol || - !methodSymbol.ContainsAttributeInAnyMethodArguments(knownSymbols.OrchestrationTriggerAttribute) || - !methodSymbol.TryGetSingleValueFromAttribute(knownSymbols.FunctionAttribute, out string functionName)) + if (ctx.ContainingSymbol is not IMethodSymbol methodSymbol) { return; } - var orchestration = new OrchestrationMethod(functionName, methodSymbol); - var methodSyntax = (MethodDeclarationSyntax)ctx.Node; + if (!methodSymbol.ContainsAttributeInAnyMethodArguments(knownSymbols.FunctionOrchestrationAttribute)) + { + return; + } - FindInvokedMethods(ctx.SemanticModel, methodSyntax, methodSymbol, orchestration, result); + if (!methodSymbol.TryGetSingleValueFromAttribute(knownSymbols.FunctionNameAttribute, out string functionName)) + { + return; + } + + var orchestration = new AnalyzedOrchestration(functionName); + var rootMethodSyntax = (MethodDeclarationSyntax)ctx.Node; + + FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); }, SyntaxKind.MethodDeclaration); + // look for TaskOrchestrator`2 Orchestrations + context.RegisterSyntaxNodeAction(ctx => + { + ctx.CancellationToken.ThrowIfCancellationRequested(); + + if (ctx.ContainingSymbol is not INamedTypeSymbol classSymbol) + { + return; + } + + if (!classSymbol.InheritsFromOpenGeneric(knownSymbols.TaskOrchestratorBaseClass)) + { + return; + } + + // Get the method that overrides TaskOrchestrator.RunAsync + IMethodSymbol? methodSymbol = classSymbol.GetOverridenMethod(runAsyncTaskOrchestratorBase); + if (methodSymbol == null) + { + return; + } + + var orchestration = new AnalyzedOrchestration(classSymbol.Name); + + IEnumerable methodSyntaxes = methodSymbol.GetSyntaxNodes(); + foreach (MethodDeclarationSyntax rootMethodSyntax in methodSyntaxes) + { + FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); + } + }, SyntaxKind.ClassDeclaration); + + // look for ITaskOrchestrator Orchestrations + context.RegisterSyntaxNodeAction(ctx => + { + ctx.CancellationToken.ThrowIfCancellationRequested(); + + if (ctx.ContainingSymbol is not INamedTypeSymbol classSymbol) + { + return; + } + + // Gets the method that implements ITaskOrchestrator.RunAsync + if (classSymbol.FindImplementationForInterfaceMember(runAsyncTaskOrchestratorInterface) is not IMethodSymbol methodSymbol) + { + return; + } + + // Skip if the found method is implemented in TaskOrchestrator + if (methodSymbol.ContainingType.ConstructedFrom.Equals(knownSymbols.TaskOrchestratorBaseClass, SymbolEqualityComparer.Default)) + { + return; + } + + var orchestration = new AnalyzedOrchestration(classSymbol.Name); + + IEnumerable methodSyntaxes = methodSymbol.GetSyntaxNodes(); + foreach (MethodDeclarationSyntax rootMethodSyntax in methodSyntaxes) + { + FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); + } + }, SyntaxKind.ClassDeclaration); + + // look for OrchestratorFunc Orchestrations + context.RegisterOperationAction(ctx => + { + if (ctx.Operation is not IInvocationOperation invocation) + { + return; + } + + if (!SymbolEqualityComparer.Default.Equals(invocation.Type, knownSymbols.DurableTaskRegistry)) + { + return; + } + + // there are 8 AddOrchestratorFunc overloads + if (invocation.TargetMethod.Name != "AddOrchestratorFunc") + { + return; + } + + // all overloads have the parameter orchestrator, either as an Action or a Func + IArgumentOperation orchestratorArgument = invocation.Arguments.First(a => a.Parameter!.Name == "orchestrator"); + if (orchestratorArgument.Value is not IDelegateCreationOperation delegateCreationOperation) + { + return; + } + + IMethodSymbol? methodSymbol = null; + switch (delegateCreationOperation.Target) + { + case IAnonymousFunctionOperation lambdaOperation: + // use the containing symbol of the lambda (e.g. the class declaring it) as the root method symbol + methodSymbol = ctx.ContainingSymbol as IMethodSymbol; + break; + case IMethodReferenceOperation methodReferenceOperation: + // use the method reference as the root method symbol + methodSymbol = methodReferenceOperation.Method; + break; + default: + break; + } + + if (methodSymbol == null) + { + return; + } + + // try to get the name of the orchestration from the method call, otherwise use the containing type name + IArgumentOperation nameArgument = invocation.Arguments.First(a => a.Parameter!.Name == "name"); + Optional name = nameArgument.GetConstantValueFromAttribute(ctx.Operation.SemanticModel!, ctx.CancellationToken); + string orchestrationName = name.Value?.ToString() ?? methodSymbol.Name; + + var orchestration = new AnalyzedOrchestration(orchestrationName); + + SyntaxNode funcRootSyntax = delegateCreationOperation.Syntax; + + FindInvokedMethods(ctx.Operation.SemanticModel!, funcRootSyntax, methodSymbol, orchestration, result); + }, OperationKind.Invocation); + // allows concrete implementations to register specific actions/analysis and then check if they happen in any of the orchestration methods this.RegisterAdditionalCompilationStartAction(context, result); }); } - // Recursively find all methods invoked by the orchestration method + // Recursively find all methods invoked by the orchestration root method and call the appropriate visitor method static void FindInvokedMethods( - SemanticModel semanticModel, MethodDeclarationSyntax callerSyntax, IMethodSymbol callerSymbol, - OrchestrationMethod rootOrchestration, OrchestrationAnalysisResult result) + SemanticModel semanticModel, SyntaxNode callerSyntax, IMethodSymbol callerSymbol, + AnalyzedOrchestration rootOrchestration, OrchestrationAnalysisResult result) { - if (!TryTrackMethod(semanticModel, callerSyntax, callerSymbol, rootOrchestration, result)) + ConcurrentBag orchestrations = result.OrchestrationsByMethod.GetOrAdd(callerSymbol, []); + if (orchestrations.Contains(rootOrchestration)) { // previously tracked method, leave to avoid infinite recursion return; } + orchestrations.Add(rootOrchestration); foreach (InvocationExpressionSyntax invocationSyntax in callerSyntax.DescendantNodes().OfType()) { @@ -80,7 +214,7 @@ static void FindInvokedMethods( } // iterating over multiple syntax references is needed because the same method can be declared in multiple places (e.g. partial classes) - IEnumerable calleeSyntaxes = calleeMethodSymbol.DeclaringSyntaxReferences.Select(r => r.GetSyntax()).OfType(); + IEnumerable calleeSyntaxes = calleeMethodSymbol.GetSyntaxNodes(); foreach (MethodDeclarationSyntax calleeSyntax in calleeSyntaxes) { FindInvokedMethods(semanticModel, calleeSyntax, calleeMethodSymbol, rootOrchestration, result); @@ -88,20 +222,6 @@ static void FindInvokedMethods( } } - static bool TryTrackMethod(SemanticModel semanticModel, MethodDeclarationSyntax callerSyntax, IMethodSymbol callerSymbol, - OrchestrationMethod rootOrchestration, OrchestrationAnalysisResult result) - { - ConcurrentBag orchestrations = result.OrchestrationsByMethod.GetOrAdd(callerSymbol, []); - if (orchestrations.Contains(rootOrchestration)) - { - return false; - } - - orchestrations.Add(rootOrchestration); - - return true; - } - /// /// Register additional actions to be executed after the compilation has started. /// It is expected from a concrete implementation of to register a @@ -114,7 +234,7 @@ static bool TryTrackMethod(SemanticModel semanticModel, MethodDeclarationSyntax protected readonly struct OrchestrationAnalysisResult { - public ConcurrentDictionary> OrchestrationsByMethod { get; } + public ConcurrentDictionary> OrchestrationsByMethod { get; } public OrchestrationAnalysisResult() { @@ -122,10 +242,8 @@ public OrchestrationAnalysisResult() } } - [DebuggerDisplay("[{FunctionName}] {OrchestrationMethodSymbol.Name}")] - protected readonly struct OrchestrationMethod(string functionName, IMethodSymbol orchestrationMethodSymbol) + protected readonly struct AnalyzedOrchestration(string name) { - public string FunctionName { get; } = functionName; - public IMethodSymbol OrchestrationMethodSymbol { get; } = orchestrationMethodSymbol; + public string Name { get; } = name; } } diff --git a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs index 5ebefab64..ea0d22799 100644 --- a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs +++ b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs @@ -11,9 +11,42 @@ namespace Microsoft.DurableTask.Analyzers.Test.Orchestration; public class DateTimeOrchestrationAnalyzerTests { [Fact] - public async Task OrchestrationUsingDateTimeNowHasDiag() + public async Task EmptyCodeWithNoSymbolsAvailableHasNoDiag() + { + string code = @""; + + // checks that empty code with no assembly references of Durable Functions has no diagnostics. + // this guarantees that if someone adds our analyzer to a project that doesn't use Durable Functions, + // the analyzer won't crash/they won't get any diagnostics + await VerifyCS.VerifyAnalyzerAsync(code); + } + + [Fact] + public async Task EmptyCodeWithSymbolsAvailableHasNoDiag() + { + string code = @""; + + // checks that empty code with access to assembly references of Durable Functions has no diagnostics + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + [Fact] + public async Task NonOrchestrationHasNoDiag() { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" +void Method(){ + Console.WriteLine(DateTime.Now); +} +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + + [Fact] + public async Task DurableFunctionOrchestrationUsingDateTimeNowHasDiag() + { + string code = WrapDurableFunctionOrchestration(@" [Function(""Run"")] DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) { @@ -27,9 +60,9 @@ DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) } [Fact] - public async Task OrchestrationUsingDateTimeUtcNowHasDiag() + public async Task DurableFunctionOrchestrationUsingDateTimeUtcNowHasDiag() { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" [Function(""Run"")] DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) { @@ -43,9 +76,9 @@ DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) } [Fact] - public async Task OrchestrationUsingDateTimeTodayNowHasDiag() + public async Task DurableFunctionOrchestrationUsingDateTimeTodayNowHasDiag() { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" [Function(""Run"")] DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) { @@ -59,28 +92,9 @@ DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) } [Fact] - public async Task ChainedMethodsInvokedBySingleOrchestrationHasDiag() + public async Task DurableFunctionOrchestrationInvokingChainedMethodsHasDiag() { - string code = Wrap(@" -[Function(""Run"")] -long Run([OrchestrationTrigger] TaskOrchestrationContext context) => Level1(); - -long Level1() => Level2(); - -long Level2() => Level3(); - -long Level3() => {|#0:DateTime.Now|}.Ticks; -"); - - DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Level3", "System.DateTime.Now", "Run"); - - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); - } - - [Fact] - public async Task ChainedMethodsInvokedByMultipleOrchestrationsHasDiag() - { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" [Function(""Run1"")] long Run1([OrchestrationTrigger] TaskOrchestrationContext context) => Level1(); @@ -100,9 +114,9 @@ public async Task ChainedMethodsInvokedByMultipleOrchestrationsHasDiag() } [Fact] - public async Task RecursiveMethodInvokedByOrchestrationHasSingleDiag() + public async Task DurableFunctionOrchestrationInvokingRecursiveMethodsHasSingleDiag() { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" [Function(""Run"")] long Run([OrchestrationTrigger] TaskOrchestrationContext context) => RecursiveMethod(0); @@ -119,14 +133,15 @@ long RecursiveMethod(int i){ } [Fact] - public async Task MethodInvokedMultipleTimesByOrchestrationHasSingleDiag() + public async Task DurableFunctionOrchestrationInvokingMethodMultipleTimesHasSingleDiag() { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" [Function(""Run"")] void Run([OrchestrationTrigger] TaskOrchestrationContext context) { _ = Method(); _ = Method(); + _ = Method(); } DateTime Method() => {|#0:DateTime.Now|}; @@ -138,9 +153,31 @@ void Run([OrchestrationTrigger] TaskOrchestrationContext context) } [Fact] - public async Task OrchestrationUsingDateTimeInLambdasHasDiag() + public async Task DurableFunctionOrchestrationInvokingAsyncMethodsHasDiag() { - string code = Wrap(@" + string code = WrapDurableFunctionOrchestration(@" +[Function(nameof(Run))] +async Task Run([OrchestrationTrigger] TaskOrchestrationContext context) +{ + _ = await ValueTaskInvocation(); + _ = await TaskInvocation(); +} + +static ValueTask ValueTaskInvocation() => ValueTask.FromResult({|#0:DateTime.Now|}); + +static Task TaskInvocation() => Task.FromResult({|#1:DateTime.Now|}); +"); + + DiagnosticResult valueTaskExpected = BuildDiagnostic().WithLocation(0).WithArguments("ValueTaskInvocation", "System.DateTime.Now", "Run"); + DiagnosticResult taskExpected = BuildDiagnostic().WithLocation(1).WithArguments("TaskInvocation", "System.DateTime.Now", "Run"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, valueTaskExpected, taskExpected); + } + + [Fact] + public async Task DurableFunctionOrchestrationUsingLambdasHasDiag() + { + string code = WrapDurableFunctionOrchestration(@" [Function(""Run"")] void Run([OrchestrationTrigger] TaskOrchestrationContext context) { @@ -158,89 +195,162 @@ void Run([OrchestrationTrigger] TaskOrchestrationContext context) } [Fact] - public async Task OrchestrationUsingAsyncInvocationsHasDiag() + public async Task DurableFunctionOrchestrationNotInvokingMethodHasNoDiag() { - string code = Wrap(@" -[Function(nameof(Run))] -async Task Run([OrchestrationTrigger] TaskOrchestrationContext context) + string code = WrapDurableFunctionOrchestration(@" +[Function(""Run"")] +DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) => new DateTime(2024, 1, 1); + +DateTime NotCalled() => DateTime.Now; +"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + } + + + [Fact] + public async Task TaskOrchestratorHasDiag() + { + string code = WrapTaskOrchestrator(@" +public class MyOrchestrator : TaskOrchestrator { - _ = await ValueTaskInvocation(); - _ = await TaskInvocation(); + public override Task RunAsync(TaskOrchestrationContext context, string input) + { + return Task.FromResult(Method()); + } + + private DateTime Method() => {|#0:DateTime.Now|}; } +"); -static ValueTask ValueTaskInvocation() => ValueTask.FromResult({|#0:DateTime.Now|}); + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "System.DateTime.Now", "MyOrchestrator"); -static Task TaskInvocation() => Task.FromResult({|#1:DateTime.Now|}); + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); + } + + [Fact] + public async Task TaskOrchestratorImplementingInterfaceHasDiag() + { + string code = WrapTaskOrchestrator(@" +public class MyOrchestrator : ITaskOrchestrator +{ + public Type InputType => typeof(object); + public Type OutputType => typeof(object); + + public Task RunAsync(TaskOrchestrationContext context, object? input) + { + return Task.FromResult((object?)Method()); + } + + private DateTime Method() => {|#0:DateTime.Now|}; +} "); - DiagnosticResult valueTaskExpected = BuildDiagnostic().WithLocation(0).WithArguments("ValueTaskInvocation", "System.DateTime.Now", "Run"); - DiagnosticResult taskExpected = BuildDiagnostic().WithLocation(1).WithArguments("TaskInvocation", "System.DateTime.Now", "Run"); + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Method", "System.DateTime.Now", "MyOrchestrator"); - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, valueTaskExpected, taskExpected); + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); } + [Fact] - public async Task EmptyCodeWithNoSymbolsAvailableHasNoDiag() + public async Task FuncOrchestratorWithLambdaHasDiag() { - string code = @""; + string code = WrapFuncOrchestrator(@" +tasks.AddOrchestratorFunc(""HelloSequence"", context => +{ + return {|#0:DateTime.Now|}; +}); +"); - // checks that empty code with no assembly references of Durable Functions has no diagnostics. - // this guarantees that if someone adds our analyzer to a project that doesn't use Durable Functions, - // the analyzer won't crash/they won't get any diagnostics - await VerifyCS.VerifyAnalyzerAsync(code); + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Main", "System.DateTime.Now", "HelloSequence"); + + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); } [Fact] - public async Task EmptyCodeWithSymbolsAvailableHasNoDiag() + public async Task FuncOrchestratorWithMethodReferenceHasDiag() { - string code = @""; + string code = @" +using System; +using Microsoft.DurableTask; +using Microsoft.DurableTask.Worker; +using Microsoft.Extensions.DependencyInjection; - // checks that empty code with access to assembly references of Durable Functions has no diagnostics - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); +public class Program +{ + public static void Main() + { + new ServiceCollection().AddDurableTaskWorker(builder => + { + builder.AddTasks(tasks => + { + tasks.AddOrchestratorFunc(""MyRun"", MyRunAsync); + }); + }); } - [Fact] - public async Task NonOrchestrationHasNoDiag() + static DateTime MyRunAsync(TaskOrchestrationContext context) { - string code = Wrap(@" -[Function(""Func"")] -void Func(){ - Console.WriteLine(DateTime.Now); + return {|#0:DateTime.Now|}; + } } +"; -void Method(){ - Console.WriteLine(DateTime.Now); -} -"); + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("MyRunAsync", "System.DateTime.Now", "MyRun"); - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); } - [Fact] - public async Task MethodNotCalledByOrchestrationHasNoDiag() + + static string WrapDurableFunctionOrchestration(string code) { - string code = Wrap(@" -[Function(""Run"")] -DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) => new DateTime(2024, 1, 1); + return $@" +{Usings()} +class Orchestrator +{{ +{code} +}} +"; + } -DateTime NotCalled() => DateTime.Now; -"); + static string WrapTaskOrchestrator(string code) + { + return $@" +{Usings()} +{code} +"; + } - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); + static string WrapFuncOrchestrator(string code) + { + return $@" +{Usings()} + +public class Program +{{ + public static void Main() + {{ + new ServiceCollection().AddDurableTaskWorker(builder => + {{ + builder.AddTasks(tasks => + {{ + {code} + }}); + }}); + }} +}} +"; } - static string Wrap(string code) + static string Usings() { return $@" using System; using System.Threading.Tasks; using Microsoft.Azure.Functions.Worker; using Microsoft.DurableTask; - -class Orchestrator -{{ -{code} -}} +using Microsoft.DurableTask.Worker; +using Microsoft.Extensions.DependencyInjection; "; } From 4c029fb9943393f6efd2c6a9c1ba490f90ff0c44 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 10 Apr 2024 07:42:04 -0300 Subject: [PATCH 05/20] commenting implementation details --- src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs | 1 + src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index 82e351055..dca2528b4 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -29,6 +29,7 @@ protected override void RegisterAdditionalCompilationStartAction(CompilationStar { INamedTypeSymbol systemDateTimeSymbol = context.Compilation.GetSpecialType(SpecialType.System_DateTime); + // stores the symbols (such as methods) and the DateTime references used in them ConcurrentBag<(ISymbol symbol, IPropertyReferenceOperation operation)> dateTimeUsage = []; // search for usages of DateTime.Now, DateTime.UtcNow, DateTime.Today and store them diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 816c80184..32ceeacdc 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -191,6 +191,7 @@ static void FindInvokedMethods( SemanticModel semanticModel, SyntaxNode callerSyntax, IMethodSymbol callerSymbol, AnalyzedOrchestration rootOrchestration, OrchestrationAnalysisResult result) { + // add the visited method to the list of orchestrations ConcurrentBag orchestrations = result.OrchestrationsByMethod.GetOrAdd(callerSymbol, []); if (orchestrations.Contains(rootOrchestration)) { From 88ff4a0b46df55b3d77dd36b9593351f4f5290bc Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 10 Apr 2024 07:45:40 -0300 Subject: [PATCH 06/20] flatten helpers folder structure --- src/Analyzers/{Helpers => }/AnalyzersCategories.cs | 2 +- src/Analyzers/{Helpers => }/KnownTypeSymbols.cs | 2 +- src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs | 1 - src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 1 - src/Analyzers/{Helpers => }/RoslynExtensions.cs | 2 +- 5 files changed, 3 insertions(+), 5 deletions(-) rename src/Analyzers/{Helpers => }/AnalyzersCategories.cs (77%) rename src/Analyzers/{Helpers => }/KnownTypeSymbols.cs (98%) rename src/Analyzers/{Helpers => }/RoslynExtensions.cs (98%) diff --git a/src/Analyzers/Helpers/AnalyzersCategories.cs b/src/Analyzers/AnalyzersCategories.cs similarity index 77% rename from src/Analyzers/Helpers/AnalyzersCategories.cs rename to src/Analyzers/AnalyzersCategories.cs index 045cb3212..b1a51f73c 100644 --- a/src/Analyzers/Helpers/AnalyzersCategories.cs +++ b/src/Analyzers/AnalyzersCategories.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -namespace Microsoft.DurableTask.Analyzers.Helpers; +namespace Microsoft.DurableTask.Analyzers; static class AnalyzersCategories { diff --git a/src/Analyzers/Helpers/KnownTypeSymbols.cs b/src/Analyzers/KnownTypeSymbols.cs similarity index 98% rename from src/Analyzers/Helpers/KnownTypeSymbols.cs rename to src/Analyzers/KnownTypeSymbols.cs index 165adef09..e43186478 100644 --- a/src/Analyzers/Helpers/KnownTypeSymbols.cs +++ b/src/Analyzers/KnownTypeSymbols.cs @@ -3,7 +3,7 @@ using Microsoft.CodeAnalysis; -namespace Microsoft.DurableTask.Analyzers.Helpers; +namespace Microsoft.DurableTask.Analyzers; /// /// Provides a set of well-known types that are used by the analyzers. diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index dca2528b4..24f6487de 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -6,7 +6,6 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; using Microsoft.CodeAnalysis; -using Microsoft.DurableTask.Analyzers.Helpers; namespace Microsoft.DurableTask.Analyzers.Orchestration; diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 32ceeacdc..58d0917bd 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -7,7 +7,6 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; -using Microsoft.DurableTask.Analyzers.Helpers; namespace Microsoft.DurableTask.Analyzers.Orchestration; diff --git a/src/Analyzers/Helpers/RoslynExtensions.cs b/src/Analyzers/RoslynExtensions.cs similarity index 98% rename from src/Analyzers/Helpers/RoslynExtensions.cs rename to src/Analyzers/RoslynExtensions.cs index a361b5778..83b63fe5a 100644 --- a/src/Analyzers/Helpers/RoslynExtensions.cs +++ b/src/Analyzers/RoslynExtensions.cs @@ -7,7 +7,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Operations; -namespace Microsoft.DurableTask.Analyzers.Helpers; +namespace Microsoft.DurableTask.Analyzers; static class RoslynExtensions { From 9365704833d8f5e33f8441a7b623118d3fb05775 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 10 Apr 2024 07:53:29 -0300 Subject: [PATCH 07/20] removing cache wrapper from KnownTypeSymbols --- src/Analyzers/KnownTypeSymbols.cs | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/Analyzers/KnownTypeSymbols.cs b/src/Analyzers/KnownTypeSymbols.cs index e43186478..d76ab5977 100644 --- a/src/Analyzers/KnownTypeSymbols.cs +++ b/src/Analyzers/KnownTypeSymbols.cs @@ -15,39 +15,28 @@ sealed class KnownTypeSymbols(Compilation compilation) { readonly Compilation compilation = compilation; - Cached functionOrchestrationAttribute; + INamedTypeSymbol? functionOrchestrationAttribute; public INamedTypeSymbol? FunctionOrchestrationAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.functionOrchestrationAttribute); - Cached functionNameAttribute; + INamedTypeSymbol? functionNameAttribute; public INamedTypeSymbol? FunctionNameAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionNameAttribute); - Cached taskOrchestratorInterface; + INamedTypeSymbol? taskOrchestratorInterface; public INamedTypeSymbol? TaskOrchestratorInterface => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.ITaskOrchestrator", ref this.taskOrchestratorInterface); - Cached taskOrchestratorBaseClass; + INamedTypeSymbol? taskOrchestratorBaseClass; public INamedTypeSymbol? TaskOrchestratorBaseClass => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.TaskOrchestrator`2", ref this.taskOrchestratorBaseClass); - Cached durableTaskRegistry; + INamedTypeSymbol? durableTaskRegistry; public INamedTypeSymbol? DurableTaskRegistry => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.DurableTaskRegistry", ref this.durableTaskRegistry); - INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref Cached field) + INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref INamedTypeSymbol? field) { - if (field.HasValue) + if (field != null) { - return field.Value; + return field; } - INamedTypeSymbol? type = this.compilation.GetTypeByMetadataName(fullyQualifiedName); - field = new(type); - return type; - } - - // We could use Lazy here, but because we need to use the `compilation` variable instance, - // that would require us to initiate the Lazy lambdas in the constructor. - // Because not all analyzers use all symbols, we would be allocating unnecessary lambdas. - readonly struct Cached(T value) - { - public readonly bool HasValue = true; - public readonly T Value = value; + return field = this.compilation.GetTypeByMetadataName(fullyQualifiedName); } } From 838eef4ab4956c36f6b9f455c974c1c442c3919e Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 10 Apr 2024 11:04:02 -0300 Subject: [PATCH 08/20] moving strings to resources file --- src/Analyzers/Analyzers.csproj | 12 ++ .../DateTimeOrchestrationAnalyzer.cs | 7 +- src/Analyzers/Resources.Designer.cs | 81 +++++++++++ src/Analyzers/Resources.resx | 126 ++++++++++++++++++ 4 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 src/Analyzers/Resources.Designer.cs create mode 100644 src/Analyzers/Resources.resx diff --git a/src/Analyzers/Analyzers.csproj b/src/Analyzers/Analyzers.csproj index c49a7f0eb..4c5d0f26c 100644 --- a/src/Analyzers/Analyzers.csproj +++ b/src/Analyzers/Analyzers.csproj @@ -31,6 +31,18 @@ + + + ResXFileCodeGenerator + Resources.Designer.cs + + + True + True + Resources.resx + + + diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index 24f6487de..677134aab 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -14,10 +14,13 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer { public const string DiagnosticId = "DURABLE0001"; + static readonly LocalizableString title = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString messageFormat = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); + static readonly DiagnosticDescriptor Rule = new( DiagnosticId, - "System.DateTime calls must be deterministic inside an orchestration", - "The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}'", + title, + messageFormat, AnalyzersCategories.Orchestration, DiagnosticSeverity.Warning, isEnabledByDefault: true); diff --git a/src/Analyzers/Resources.Designer.cs b/src/Analyzers/Resources.Designer.cs new file mode 100644 index 000000000..8abcbfaa1 --- /dev/null +++ b/src/Analyzers/Resources.Designer.cs @@ -0,0 +1,81 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.42000 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace Microsoft.DurableTask.Analyzers { + using System; + + + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// + // This class was auto-generated by the StronglyTypedResourceBuilder + // class via a tool like ResGen or Visual Studio. + // To add or remove a member, edit your .ResX file then rerun ResGen + // with the /str option, or rebuild your VS project. + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class Resources { + + private static global::System.Resources.ResourceManager resourceMan; + + private static global::System.Globalization.CultureInfo resourceCulture; + + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal Resources() { + } + + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { + get { + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.DurableTask.Analyzers.Resources", typeof(Resources).Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + /// + /// Looks up a localized string similar to The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}'. + /// + internal static string DateTimeOrchestrationAnalyzerMessageFormat { + get { + return ResourceManager.GetString("DateTimeOrchestrationAnalyzerMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to System.DateTime calls must be deterministic inside an orchestration. + /// + internal static string DateTimeOrchestrationAnalyzerTitle { + get { + return ResourceManager.GetString("DateTimeOrchestrationAnalyzerTitle", resourceCulture); + } + } + } +} diff --git a/src/Analyzers/Resources.resx b/src/Analyzers/Resources.resx new file mode 100644 index 000000000..12e076813 --- /dev/null +++ b/src/Analyzers/Resources.resx @@ -0,0 +1,126 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}' + + + System.DateTime calls must be deterministic inside an orchestration + + \ No newline at end of file From e80a6c8dfb91549bace68ba758fd4f1b456913e6 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 10 Apr 2024 11:39:38 -0300 Subject: [PATCH 09/20] reusing test case --- .../Orchestration/OrchestrationAnalyzer.cs | 4 +- .../DateTimeOrchestrationAnalyzerTests.cs | 52 ++++--------------- 2 files changed, 13 insertions(+), 43 deletions(-) diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 58d0917bd..5455dcdc7 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -152,11 +152,11 @@ public override void Initialize(AnalysisContext context) switch (delegateCreationOperation.Target) { case IAnonymousFunctionOperation lambdaOperation: - // use the containing symbol of the lambda (e.g. the class declaring it) as the root method symbol + // use the containing symbol of the lambda (e.g. the class declaring it) as the method symbol methodSymbol = ctx.ContainingSymbol as IMethodSymbol; break; case IMethodReferenceOperation methodReferenceOperation: - // use the method reference as the root method symbol + // use the method reference as the method symbol methodSymbol = methodReferenceOperation.Method; break; default: diff --git a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs index ea0d22799..433d28a1c 100644 --- a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs +++ b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using Microsoft.CodeAnalysis.Testing; @@ -42,51 +42,21 @@ void Method(){ await VerifyCS.VerifyDurableTaskAnalyzerAsync(code); } - - [Fact] - public async Task DurableFunctionOrchestrationUsingDateTimeNowHasDiag() - { - string code = WrapDurableFunctionOrchestration(@" -[Function(""Run"")] -DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) -{ - return {|#0:DateTime.Now|}; -} -"); - - DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.DateTime.Now", "Run"); - - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); - } - - [Fact] - public async Task DurableFunctionOrchestrationUsingDateTimeUtcNowHasDiag() - { - string code = WrapDurableFunctionOrchestration(@" -[Function(""Run"")] -DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) -{ - return {|#0:DateTime.UtcNow|}; -} -"); - - DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.DateTime.UtcNow", "Run"); - - await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); - } - - [Fact] - public async Task DurableFunctionOrchestrationUsingDateTimeTodayNowHasDiag() + [Theory] + [InlineData("DateTime.Now")] + [InlineData("DateTime.UtcNow")] + [InlineData("DateTime.Today")] + public async Task DurableFunctionOrchestrationUsingDateTimeNonDeterministicPropertiesHasDiag(string expression) { - string code = WrapDurableFunctionOrchestration(@" + string code = WrapDurableFunctionOrchestration($@" [Function(""Run"")] DateTime Run([OrchestrationTrigger] TaskOrchestrationContext context) -{ - return {|#0:DateTime.Today|}; -} +{{ + return {{|#0:{expression}|}}; +}} "); - DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", "System.DateTime.Today", "Run"); + DiagnosticResult expected = BuildDiagnostic().WithLocation(0).WithArguments("Run", $"System.{expression}", "Run"); await VerifyCS.VerifyDurableTaskAnalyzerAsync(code, expected); } From a3d379947bd313d4c9bc66001032fbd862e05b31 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 10 Apr 2024 13:13:30 -0300 Subject: [PATCH 10/20] fix namespaces + small rewrite for better readability --- .../Orchestration/DateTimeOrchestrationAnalyzer.cs | 8 ++++++-- .../Orchestration/DateTimeOrchestrationAnalyzerTests.cs | 4 ++-- .../Verifiers/CSharpAnalyzerVerifier.Durable.cs | 2 +- test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs | 2 +- test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index 677134aab..ac0410b9b 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -42,8 +42,12 @@ protected override void RegisterAdditionalCompilationStartAction(CompilationStar var operation = (IPropertyReferenceOperation)ctx.Operation; IPropertySymbol property = operation.Property; - if (property.ContainingSymbol.Equals(systemDateTimeSymbol, SymbolEqualityComparer.Default) && - property.Name is nameof(DateTime.Now) or nameof(DateTime.UtcNow) or nameof(DateTime.Today)) + if (!property.ContainingSymbol.Equals(systemDateTimeSymbol, SymbolEqualityComparer.Default)) + { + return; + } + + if (property.Name is nameof(DateTime.Now) or nameof(DateTime.UtcNow) or nameof(DateTime.Today)) { ISymbol method = ctx.ContainingSymbol; dateTimeUsage.Add((method, operation)); diff --git a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs index 433d28a1c..a546f6447 100644 --- a/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs +++ b/test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs @@ -4,9 +4,9 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.DurableTask.Analyzers.Orchestration; -using VerifyCS = Microsoft.DurableTask.Analyzers.Test.Verifiers.CSharpAnalyzerVerifier; +using VerifyCS = Microsoft.DurableTask.Analyzers.Tests.Verifiers.CSharpAnalyzerVerifier; -namespace Microsoft.DurableTask.Analyzers.Test.Orchestration; +namespace Microsoft.DurableTask.Analyzers.Tests.Orchestration; public class DateTimeOrchestrationAnalyzerTests { diff --git a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs index 7aa7afa5a..ee25f4fca 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs @@ -4,7 +4,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; -namespace Microsoft.DurableTask.Analyzers.Test.Verifiers; +namespace Microsoft.DurableTask.Analyzers.Tests.Verifiers; public static partial class CSharpAnalyzerVerifier where TAnalyzer : DiagnosticAnalyzer, new() diff --git a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs index c3c8e074f..7e2a5c6b5 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.cs @@ -7,7 +7,7 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Testing.Verifiers; -namespace Microsoft.DurableTask.Analyzers.Test.Verifiers; +namespace Microsoft.DurableTask.Analyzers.Tests.Verifiers; public static partial class CSharpAnalyzerVerifier where TAnalyzer : DiagnosticAnalyzer, new() diff --git a/test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs b/test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs index dd6685271..87d51c9ee 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.cs @@ -8,7 +8,7 @@ using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Testing.Verifiers; -namespace Microsoft.DurableTask.Analyzers.Test.Verifiers; +namespace Microsoft.DurableTask.Analyzers.Tests.Verifiers; public static partial class CSharpCodeFixVerifier where TAnalyzer : DiagnosticAnalyzer, new() From 77a9be702981e157598cde5925033021566cb856 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Thu, 11 Apr 2024 11:49:59 -0300 Subject: [PATCH 11/20] renaming roslyn method for better clarity --- src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 2 +- src/Analyzers/RoslynExtensions.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 5455dcdc7..3f4d45673 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -70,7 +70,7 @@ public override void Initialize(AnalysisContext context) return; } - if (!classSymbol.InheritsFromOpenGeneric(knownSymbols.TaskOrchestratorBaseClass)) + if (!classSymbol.BaseTypeIsConstructedFrom(knownSymbols.TaskOrchestratorBaseClass)) { return; } diff --git a/src/Analyzers/RoslynExtensions.cs b/src/Analyzers/RoslynExtensions.cs index 83b63fe5a..c9cde3b01 100644 --- a/src/Analyzers/RoslynExtensions.cs +++ b/src/Analyzers/RoslynExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System.Collections.Immutable; @@ -56,7 +56,7 @@ public static bool ImplementsInterface(this INamedTypeSymbol symbol, ISymbol int return symbol.AllInterfaces.Any(i => interfaceSymbol.Equals(i, SymbolEqualityComparer.Default)); } - public static bool InheritsFromOpenGeneric(this INamedTypeSymbol symbol, ITypeSymbol type) + public static bool BaseTypeIsConstructedFrom(this INamedTypeSymbol symbol, ITypeSymbol type) { INamedTypeSymbol? baseType = symbol.BaseType; while (baseType != null) From ddfa2412ad3f2fdb1f593f4c51e8368bf320966b Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Thu, 11 Apr 2024 11:50:27 -0300 Subject: [PATCH 12/20] using explicit type + new convention --- src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 10 +++++----- src/Analyzers/RoslynExtensions.cs | 2 +- .../Verifiers/CSharpAnalyzerVerifier.Durable.cs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 3f4d45673..dcfffb9ce 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -19,7 +19,7 @@ public override void Initialize(AnalysisContext context) context.RegisterCompilationStartAction(context => { - var knownSymbols = new KnownTypeSymbols(context.Compilation); + KnownTypeSymbols knownSymbols = new(context.Compilation); if (knownSymbols.FunctionOrchestrationAttribute == null || knownSymbols.FunctionNameAttribute == null || knownSymbols.TaskOrchestratorInterface == null || knownSymbols.TaskOrchestratorBaseClass == null || @@ -54,7 +54,7 @@ public override void Initialize(AnalysisContext context) return; } - var orchestration = new AnalyzedOrchestration(functionName); + AnalyzedOrchestration orchestration = new(functionName); var rootMethodSyntax = (MethodDeclarationSyntax)ctx.Node; FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); @@ -82,7 +82,7 @@ public override void Initialize(AnalysisContext context) return; } - var orchestration = new AnalyzedOrchestration(classSymbol.Name); + AnalyzedOrchestration orchestration = new(classSymbol.Name); IEnumerable methodSyntaxes = methodSymbol.GetSyntaxNodes(); foreach (MethodDeclarationSyntax rootMethodSyntax in methodSyntaxes) @@ -113,7 +113,7 @@ public override void Initialize(AnalysisContext context) return; } - var orchestration = new AnalyzedOrchestration(classSymbol.Name); + AnalyzedOrchestration orchestration = new(classSymbol.Name); IEnumerable methodSyntaxes = methodSymbol.GetSyntaxNodes(); foreach (MethodDeclarationSyntax rootMethodSyntax in methodSyntaxes) @@ -173,7 +173,7 @@ public override void Initialize(AnalysisContext context) Optional name = nameArgument.GetConstantValueFromAttribute(ctx.Operation.SemanticModel!, ctx.CancellationToken); string orchestrationName = name.Value?.ToString() ?? methodSymbol.Name; - var orchestration = new AnalyzedOrchestration(orchestrationName); + AnalyzedOrchestration orchestration = new(orchestrationName); SyntaxNode funcRootSyntax = delegateCreationOperation.Syntax; diff --git a/src/Analyzers/RoslynExtensions.cs b/src/Analyzers/RoslynExtensions.cs index c9cde3b01..f7353dfbf 100644 --- a/src/Analyzers/RoslynExtensions.cs +++ b/src/Analyzers/RoslynExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System.Collections.Immutable; diff --git a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs index ee25f4fca..b570d3bc5 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs @@ -12,7 +12,7 @@ public static partial class CSharpAnalyzerVerifier /// public static async Task VerifyDurableTaskAnalyzerAsync(string source, params DiagnosticResult[] expected) { - var test = new Test() + Test test = new() { TestCode = source, ReferenceAssemblies = ReferenceAssemblies.Net.Net60.AddPackages([ From 90e9deff83092cf35e4654621642fefd23646f82 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Thu, 11 Apr 2024 11:55:54 -0300 Subject: [PATCH 13/20] guards in case RunAsync methods are not available in analyzer --- src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index dcfffb9ce..5b9a1e1d2 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -29,8 +29,12 @@ public override void Initialize(AnalysisContext context) return; } - IMethodSymbol runAsyncTaskOrchestratorInterface = knownSymbols.TaskOrchestratorInterface.GetMembers("RunAsync").OfType().First(); - IMethodSymbol runAsyncTaskOrchestratorBase = knownSymbols.TaskOrchestratorBaseClass.GetMembers("RunAsync").OfType().First(); + IMethodSymbol? runAsyncTaskOrchestratorInterface = knownSymbols.TaskOrchestratorInterface.GetMembers("RunAsync").OfType().FirstOrDefault(); + IMethodSymbol? runAsyncTaskOrchestratorBase = knownSymbols.TaskOrchestratorBaseClass.GetMembers("RunAsync").OfType().FirstOrDefault(); + if (runAsyncTaskOrchestratorInterface == null || runAsyncTaskOrchestratorBase == null) + { + return; + } OrchestrationAnalysisResult result = new(); From 4c97c965db857128f1e737f6888d7513c60d4e82 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Thu, 11 Apr 2024 13:47:46 -0300 Subject: [PATCH 14/20] Resource.Designer.cs created by code generation --- src/Analyzers/Analyzers.csproj | 11 +--- src/Analyzers/Resources.Designer.cs | 81 ----------------------------- 2 files changed, 2 insertions(+), 90 deletions(-) delete mode 100644 src/Analyzers/Resources.Designer.cs diff --git a/src/Analyzers/Analyzers.csproj b/src/Analyzers/Analyzers.csproj index 4c5d0f26c..e54d0f5f0 100644 --- a/src/Analyzers/Analyzers.csproj +++ b/src/Analyzers/Analyzers.csproj @@ -29,18 +29,11 @@ + - - ResXFileCodeGenerator - Resources.Designer.cs - - - True - True - Resources.resx - + diff --git a/src/Analyzers/Resources.Designer.cs b/src/Analyzers/Resources.Designer.cs deleted file mode 100644 index 8abcbfaa1..000000000 --- a/src/Analyzers/Resources.Designer.cs +++ /dev/null @@ -1,81 +0,0 @@ -//------------------------------------------------------------------------------ -// -// This code was generated by a tool. -// Runtime Version:4.0.30319.42000 -// -// Changes to this file may cause incorrect behavior and will be lost if -// the code is regenerated. -// -//------------------------------------------------------------------------------ - -namespace Microsoft.DurableTask.Analyzers { - using System; - - - /// - /// A strongly-typed resource class, for looking up localized strings, etc. - /// - // This class was auto-generated by the StronglyTypedResourceBuilder - // class via a tool like ResGen or Visual Studio. - // To add or remove a member, edit your .ResX file then rerun ResGen - // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] - [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] - [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] - internal class Resources { - - private static global::System.Resources.ResourceManager resourceMan; - - private static global::System.Globalization.CultureInfo resourceCulture; - - [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] - internal Resources() { - } - - /// - /// Returns the cached ResourceManager instance used by this class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Resources.ResourceManager ResourceManager { - get { - if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.DurableTask.Analyzers.Resources", typeof(Resources).Assembly); - resourceMan = temp; - } - return resourceMan; - } - } - - /// - /// Overrides the current thread's CurrentUICulture property for all - /// resource lookups using this strongly typed resource class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Globalization.CultureInfo Culture { - get { - return resourceCulture; - } - set { - resourceCulture = value; - } - } - - /// - /// Looks up a localized string similar to The method '{0}' uses '{1}' that may cause non-deterministic behavior when invoked from orchestration '{2}'. - /// - internal static string DateTimeOrchestrationAnalyzerMessageFormat { - get { - return ResourceManager.GetString("DateTimeOrchestrationAnalyzerMessageFormat", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to System.DateTime calls must be deterministic inside an orchestration. - /// - internal static string DateTimeOrchestrationAnalyzerTitle { - get { - return ResourceManager.GetString("DateTimeOrchestrationAnalyzerTitle", resourceCulture); - } - } - } -} From da050128a46130ad2c30406d6eec4be4c8361c8d Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Fri, 12 Apr 2024 10:36:50 -0300 Subject: [PATCH 15/20] enabling StyleCop analyzers and fixing style issues --- src/Analyzers/Analyzers.csproj | 1 + src/Analyzers/KnownTypeSymbols.cs | 9 +-- .../DateTimeOrchestrationAnalyzer.cs | 18 +++--- .../Orchestration/OrchestrationAnalyzer.cs | 56 +++++++++++-------- src/Analyzers/RoslynExtensions.cs | 4 +- 5 files changed, 53 insertions(+), 35 deletions(-) diff --git a/src/Analyzers/Analyzers.csproj b/src/Analyzers/Analyzers.csproj index e54d0f5f0..cfcac5beb 100644 --- a/src/Analyzers/Analyzers.csproj +++ b/src/Analyzers/Analyzers.csproj @@ -3,6 +3,7 @@ netstandard2.0 true + true false diff --git a/src/Analyzers/KnownTypeSymbols.cs b/src/Analyzers/KnownTypeSymbols.cs index d76ab5977..7d4f32641 100644 --- a/src/Analyzers/KnownTypeSymbols.cs +++ b/src/Analyzers/KnownTypeSymbols.cs @@ -16,18 +16,19 @@ sealed class KnownTypeSymbols(Compilation compilation) readonly Compilation compilation = compilation; INamedTypeSymbol? functionOrchestrationAttribute; + INamedTypeSymbol? functionNameAttribute; + INamedTypeSymbol? taskOrchestratorInterface; + INamedTypeSymbol? taskOrchestratorBaseClass; + INamedTypeSymbol? durableTaskRegistry; + public INamedTypeSymbol? FunctionOrchestrationAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.functionOrchestrationAttribute); - INamedTypeSymbol? functionNameAttribute; public INamedTypeSymbol? FunctionNameAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionNameAttribute); - INamedTypeSymbol? taskOrchestratorInterface; public INamedTypeSymbol? TaskOrchestratorInterface => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.ITaskOrchestrator", ref this.taskOrchestratorInterface); - INamedTypeSymbol? taskOrchestratorBaseClass; public INamedTypeSymbol? TaskOrchestratorBaseClass => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.TaskOrchestrator`2", ref this.taskOrchestratorBaseClass); - INamedTypeSymbol? durableTaskRegistry; public INamedTypeSymbol? DurableTaskRegistry => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.DurableTaskRegistry", ref this.durableTaskRegistry); INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref INamedTypeSymbol? field) diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index ac0410b9b..acc469a55 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -3,9 +3,9 @@ using System.Collections.Concurrent; using System.Collections.Immutable; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis; namespace Microsoft.DurableTask.Analyzers.Orchestration; @@ -14,13 +14,13 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer { public const string DiagnosticId = "DURABLE0001"; - static readonly LocalizableString title = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); - static readonly LocalizableString messageFormat = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); + static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources)); static readonly DiagnosticDescriptor Rule = new( DiagnosticId, - title, - messageFormat, + Title, + MessageFormat, AnalyzersCategories.Orchestration, DiagnosticSeverity.Warning, isEnabledByDefault: true); @@ -32,10 +32,11 @@ protected override void RegisterAdditionalCompilationStartAction(CompilationStar INamedTypeSymbol systemDateTimeSymbol = context.Compilation.GetSpecialType(SpecialType.System_DateTime); // stores the symbols (such as methods) and the DateTime references used in them - ConcurrentBag<(ISymbol symbol, IPropertyReferenceOperation operation)> dateTimeUsage = []; + ConcurrentBag<(ISymbol Symbol, IPropertyReferenceOperation Operation)> dateTimeUsage = []; // search for usages of DateTime.Now, DateTime.UtcNow, DateTime.Today and store them - context.RegisterOperationAction(ctx => + context.RegisterOperationAction( + ctx => { ctx.CancellationToken.ThrowIfCancellationRequested(); @@ -52,7 +53,8 @@ protected override void RegisterAdditionalCompilationStartAction(CompilationStar ISymbol method = ctx.ContainingSymbol; dateTimeUsage.Add((method, operation)); } - }, OperationKind.PropertyReference); + }, + OperationKind.PropertyReference); // compare whether the found DateTime usages occur in methods invoked by orchestrations context.RegisterCompilationEndAction(ctx => diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 5b9a1e1d2..bd949317f 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -39,7 +39,8 @@ public override void Initialize(AnalysisContext context) OrchestrationAnalysisResult result = new(); // look for Durable Functions Orchestrations - context.RegisterSyntaxNodeAction(ctx => + context.RegisterSyntaxNodeAction( + ctx => { ctx.CancellationToken.ThrowIfCancellationRequested(); @@ -62,10 +63,12 @@ public override void Initialize(AnalysisContext context) var rootMethodSyntax = (MethodDeclarationSyntax)ctx.Node; FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); - }, SyntaxKind.MethodDeclaration); + }, + SyntaxKind.MethodDeclaration); // look for TaskOrchestrator`2 Orchestrations - context.RegisterSyntaxNodeAction(ctx => + context.RegisterSyntaxNodeAction( + ctx => { ctx.CancellationToken.ThrowIfCancellationRequested(); @@ -93,10 +96,12 @@ public override void Initialize(AnalysisContext context) { FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); } - }, SyntaxKind.ClassDeclaration); + }, + SyntaxKind.ClassDeclaration); // look for ITaskOrchestrator Orchestrations - context.RegisterSyntaxNodeAction(ctx => + context.RegisterSyntaxNodeAction( + ctx => { ctx.CancellationToken.ThrowIfCancellationRequested(); @@ -124,10 +129,12 @@ public override void Initialize(AnalysisContext context) { FindInvokedMethods(ctx.SemanticModel, rootMethodSyntax, methodSymbol, orchestration, result); } - }, SyntaxKind.ClassDeclaration); + }, + SyntaxKind.ClassDeclaration); // look for OrchestratorFunc Orchestrations - context.RegisterOperationAction(ctx => + context.RegisterOperationAction( + ctx => { if (ctx.Operation is not IInvocationOperation invocation) { @@ -182,17 +189,31 @@ public override void Initialize(AnalysisContext context) SyntaxNode funcRootSyntax = delegateCreationOperation.Syntax; FindInvokedMethods(ctx.Operation.SemanticModel!, funcRootSyntax, methodSymbol, orchestration, result); - }, OperationKind.Invocation); + }, + OperationKind.Invocation); // allows concrete implementations to register specific actions/analysis and then check if they happen in any of the orchestration methods this.RegisterAdditionalCompilationStartAction(context, result); }); } + /// + /// Register additional actions to be executed after the compilation has started. + /// It is expected from a concrete implementation of to register a + /// + /// and then compare that any discovered violations happened in any of the symbols in . + /// + /// Context originally provided by . + /// Collection of symbols referenced by orchestrations. + protected abstract void RegisterAdditionalCompilationStartAction(CompilationStartAnalysisContext context, OrchestrationAnalysisResult orchestrationAnalysisResult); + // Recursively find all methods invoked by the orchestration root method and call the appropriate visitor method static void FindInvokedMethods( - SemanticModel semanticModel, SyntaxNode callerSyntax, IMethodSymbol callerSymbol, - AnalyzedOrchestration rootOrchestration, OrchestrationAnalysisResult result) + SemanticModel semanticModel, + SyntaxNode callerSyntax, + IMethodSymbol callerSymbol, + AnalyzedOrchestration rootOrchestration, + OrchestrationAnalysisResult result) { // add the visited method to the list of orchestrations ConcurrentBag orchestrations = result.OrchestrationsByMethod.GetOrAdd(callerSymbol, []); @@ -201,6 +222,7 @@ static void FindInvokedMethods( // previously tracked method, leave to avoid infinite recursion return; } + orchestrations.Add(rootOrchestration); foreach (InvocationExpressionSyntax invocationSyntax in callerSyntax.DescendantNodes().OfType()) @@ -226,24 +248,14 @@ static void FindInvokedMethods( } } - /// - /// Register additional actions to be executed after the compilation has started. - /// It is expected from a concrete implementation of to register a - /// - /// and then compare that any discovered violations happened in any of the symbols in . - /// - /// Context originally provided by - /// Collection of symbols referenced by orchestrations - protected abstract void RegisterAdditionalCompilationStartAction(CompilationStartAnalysisContext context, OrchestrationAnalysisResult orchestrationAnalysisResult); - protected readonly struct OrchestrationAnalysisResult { - public ConcurrentDictionary> OrchestrationsByMethod { get; } - public OrchestrationAnalysisResult() { this.OrchestrationsByMethod = new(SymbolEqualityComparer.Default); } + + public ConcurrentDictionary> OrchestrationsByMethod { get; } } protected readonly struct AnalyzedOrchestration(string name) diff --git a/src/Analyzers/RoslynExtensions.cs b/src/Analyzers/RoslynExtensions.cs index f7353dfbf..8f7b8c624 100644 --- a/src/Analyzers/RoslynExtensions.cs +++ b/src/Analyzers/RoslynExtensions.cs @@ -2,9 +2,9 @@ // Licensed under the MIT License. using System.Collections.Immutable; -using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; namespace Microsoft.DurableTask.Analyzers; @@ -65,8 +65,10 @@ public static bool BaseTypeIsConstructedFrom(this INamedTypeSymbol symbol, IType { return true; } + baseType = baseType.BaseType; } + return false; } From f0120f32e0df80fca59f48bedb4de97f01b8268b Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:11:05 -0300 Subject: [PATCH 16/20] updating stylecop analyzers versions the previous version wasn't recognizing the latest c# empty collection initializers linting rules --- src/Directory.Build.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Directory.Build.targets b/src/Directory.Build.targets index 15f3aa2fe..13d3e148b 100644 --- a/src/Directory.Build.targets +++ b/src/Directory.Build.targets @@ -18,7 +18,7 @@ - + From 1dfb48104478391bfeee082b7115410ba19467ac Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:11:23 -0300 Subject: [PATCH 17/20] documenting members --- src/Analyzers/AnalyzersCategories.cs | 6 ++ src/Analyzers/KnownTypeSymbols.cs | 15 ++++ .../DateTimeOrchestrationAnalyzer.cs | 8 ++ .../Orchestration/OrchestrationAnalyzer.cs | 20 +++++ src/Analyzers/RoslynExtensions.cs | 89 ++++++++++++++----- 5 files changed, 115 insertions(+), 23 deletions(-) diff --git a/src/Analyzers/AnalyzersCategories.cs b/src/Analyzers/AnalyzersCategories.cs index b1a51f73c..2f2b1a6bd 100644 --- a/src/Analyzers/AnalyzersCategories.cs +++ b/src/Analyzers/AnalyzersCategories.cs @@ -3,7 +3,13 @@ namespace Microsoft.DurableTask.Analyzers; +/// +/// Provides a set of well-known categories that are used by the analyzers diagnostics. +/// static class AnalyzersCategories { + /// + /// The category for the orchestration related analyzers. + /// public const string Orchestration = "Orchestration"; } diff --git a/src/Analyzers/KnownTypeSymbols.cs b/src/Analyzers/KnownTypeSymbols.cs index 7d4f32641..19c82d1b3 100644 --- a/src/Analyzers/KnownTypeSymbols.cs +++ b/src/Analyzers/KnownTypeSymbols.cs @@ -21,14 +21,29 @@ sealed class KnownTypeSymbols(Compilation compilation) INamedTypeSymbol? taskOrchestratorBaseClass; INamedTypeSymbol? durableTaskRegistry; + /// + /// Gets OrchestrationTriggerAttribute type symbol. + /// public INamedTypeSymbol? FunctionOrchestrationAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.functionOrchestrationAttribute); + /// + /// Gets FunctionNameAttribute type symbol. + /// public INamedTypeSymbol? FunctionNameAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionNameAttribute); + /// + /// Gets ITaskOrchestrator type symbol. + /// public INamedTypeSymbol? TaskOrchestratorInterface => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.ITaskOrchestrator", ref this.taskOrchestratorInterface); + /// + /// Gets TaskOrchestrator type symbol. + /// public INamedTypeSymbol? TaskOrchestratorBaseClass => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.TaskOrchestrator`2", ref this.taskOrchestratorBaseClass); + /// + /// Gets DurableTaskRegistry type symbol. + /// public INamedTypeSymbol? DurableTaskRegistry => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.DurableTaskRegistry", ref this.durableTaskRegistry); INamedTypeSymbol? GetOrResolveFullyQualifiedType(string fullyQualifiedName, ref INamedTypeSymbol? field) diff --git a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs index acc469a55..04ccdc213 100644 --- a/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/DateTimeOrchestrationAnalyzer.cs @@ -9,9 +9,15 @@ namespace Microsoft.DurableTask.Analyzers.Orchestration; +/// +/// Analyzer that reports a warning when a non-deterministic DateTime property is used in an orchestration method. +/// [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer { + /// + /// Diagnostic ID supported for the analyzer. + /// public const string DiagnosticId = "DURABLE0001"; static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.DateTimeOrchestrationAnalyzerTitle), Resources.ResourceManager, typeof(Resources)); @@ -25,8 +31,10 @@ public sealed class DateTimeOrchestrationAnalyzer : OrchestrationAnalyzer DiagnosticSeverity.Warning, isEnabledByDefault: true); + /// public override ImmutableArray SupportedDiagnostics => [Rule]; + /// protected override void RegisterAdditionalCompilationStartAction(CompilationStartAnalysisContext context, OrchestrationAnalysisResult orchestrationAnalysisResult) { INamedTypeSymbol systemDateTimeSymbol = context.Compilation.GetSpecialType(SpecialType.System_DateTime); diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index bd949317f..430c453a7 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -10,8 +10,12 @@ namespace Microsoft.DurableTask.Analyzers.Orchestration; +/// +/// Base class for analyzers that analyze orchestrations. +/// public abstract class OrchestrationAnalyzer : DiagnosticAnalyzer { + /// public override void Initialize(AnalysisContext context) { context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); @@ -248,18 +252,34 @@ static void FindInvokedMethods( } } + /// + /// Data structure to store the result of the orchestration methods analysis. + /// protected readonly struct OrchestrationAnalysisResult { + /// + /// Initializes a new instance of the struct. + /// public OrchestrationAnalysisResult() { this.OrchestrationsByMethod = new(SymbolEqualityComparer.Default); } + /// + /// Gets the orchestrations that invokes/reaches a given method. + /// public ConcurrentDictionary> OrchestrationsByMethod { get; } } + /// + /// Data structure to store the orchestration data. + /// + /// Name of the orchestration. protected readonly struct AnalyzedOrchestration(string name) { + /// + /// Gets the name of the orchestration. + /// public string Name { get; } = name; } } diff --git a/src/Analyzers/RoslynExtensions.cs b/src/Analyzers/RoslynExtensions.cs index 8f7b8c624..194449967 100644 --- a/src/Analyzers/RoslynExtensions.cs +++ b/src/Analyzers/RoslynExtensions.cs @@ -9,8 +9,19 @@ namespace Microsoft.DurableTask.Analyzers; +/// +/// Extension methods for working with Roslyn types. +/// static class RoslynExtensions { + /// + /// Tries to get the value of an attribute that has a single value. + /// + /// Convertion Type. + /// Symbol containing the annotation. + /// Attribute to look for. + /// Retrieved value from the attribute instance. + /// true if the method succeeded to retrieve the value, false otherwise. public static bool TryGetSingleValueFromAttribute(this ISymbol? symbol, INamedTypeSymbol attributeSymbol, out T value) { if (symbol.TryGetConstructorArgumentsFromAttribute(attributeSymbol, out ImmutableArray constructorArguments)) @@ -27,23 +38,12 @@ public static bool TryGetSingleValueFromAttribute(this ISymbol? symbol, IName return false; } - public static bool TryGetConstructorArgumentsFromAttribute(this ISymbol? symbol, INamedTypeSymbol attributeSymbol, out ImmutableArray constructorArguments) - { - if (symbol != null) - { - foreach (AttributeData attribute in symbol.GetAttributes()) - { - if (attributeSymbol.Equals(attribute.AttributeClass, SymbolEqualityComparer.Default)) - { - constructorArguments = attribute.ConstructorArguments; - return true; - } - } - } - - return false; - } - + /// + /// Determines whether a method has a parameter with the specified attribute. + /// + /// Method symbol. + /// Attribute class symbol. + /// True if the method has the parameter, false otherwise. public static bool ContainsAttributeInAnyMethodArguments(this IMethodSymbol methodSymbol, INamedTypeSymbol attributeSymbol) { return methodSymbol.Parameters @@ -51,11 +51,12 @@ public static bool ContainsAttributeInAnyMethodArguments(this IMethodSymbol meth .Any(a => attributeSymbol.Equals(a.AttributeClass, SymbolEqualityComparer.Default)); } - public static bool ImplementsInterface(this INamedTypeSymbol symbol, ISymbol interfaceSymbol) - { - return symbol.AllInterfaces.Any(i => interfaceSymbol.Equals(i, SymbolEqualityComparer.Default)); - } - + /// + /// Determines whether the base type of a symbol is constructed from a specified type. + /// + /// Constructed Type Symbol. + /// Contructed From Type Symbol. + /// True if the base type is constructed from the specified type, false otherwise. public static bool BaseTypeIsConstructedFrom(this INamedTypeSymbol symbol, ITypeSymbol type) { INamedTypeSymbol? baseType = symbol.BaseType; @@ -72,17 +73,35 @@ public static bool BaseTypeIsConstructedFrom(this INamedTypeSymbol symbol, IType return false; } + /// + /// Gets the method that overrides a type's method. + /// + /// Type symbol containing the methods to look for. + /// Method to look for in the type symbol. + /// The overriden method. public static IMethodSymbol? GetOverridenMethod(this INamedTypeSymbol typeSymbol, IMethodSymbol methodSymbol) { IEnumerable methods = typeSymbol.GetMembers(methodSymbol.Name).OfType(); return methods.FirstOrDefault(m => m.OverriddenMethod != null && m.OverriddenMethod.OriginalDefinition.Equals(methodSymbol, SymbolEqualityComparer.Default)); } + /// + /// Gets the syntax nodes of a method symbol. + /// + /// Method symbol. + /// The collection of syntax nodes of a given method symbol. public static IEnumerable GetSyntaxNodes(this IMethodSymbol methodSymbol) { return methodSymbol.DeclaringSyntaxReferences.Select(r => r.GetSyntax()).OfType(); } + /// + /// Gets the literal value of an argument operation. + /// + /// Argument operation. + /// Semantical model. + /// Cancellation Token. + /// The literal value of the argument. public static Optional GetConstantValueFromAttribute(this IArgumentOperation argumentOperation, SemanticModel semanticModel, CancellationToken cancellationToken) { LiteralExpressionSyntax? nameLiteralSyntax = argumentOperation.Syntax.DescendantNodes().OfType().FirstOrDefault(); @@ -90,13 +109,37 @@ public static IEnumerable GetSyntaxNodes(this IMethodSy return semanticModel.GetConstantValue(nameLiteralSyntax, cancellationToken); } + /// + /// Reports a diagnostic for a given operation. + /// + /// Context for a compilation action. + /// Diagnostic Descriptor to be reported. + /// Operation which the location will be extracted. + /// Diagnostic message arguments to be reported. public static void ReportDiagnostic(this CompilationAnalysisContext ctx, DiagnosticDescriptor descriptor, IOperation operation, params string[] messageArgs) { ctx.ReportDiagnostic(BuildDiagnostic(descriptor, operation.Syntax, messageArgs)); } - public static Diagnostic BuildDiagnostic(DiagnosticDescriptor descriptor, SyntaxNode syntaxNode, params string[] messageArgs) + static Diagnostic BuildDiagnostic(DiagnosticDescriptor descriptor, SyntaxNode syntaxNode, params string[] messageArgs) { return Diagnostic.Create(descriptor, syntaxNode.GetLocation(), messageArgs); } + + static bool TryGetConstructorArgumentsFromAttribute(this ISymbol? symbol, INamedTypeSymbol attributeSymbol, out ImmutableArray constructorArguments) + { + if (symbol != null) + { + foreach (AttributeData attribute in symbol.GetAttributes()) + { + if (attributeSymbol.Equals(attribute.AttributeClass, SymbolEqualityComparer.Default)) + { + constructorArguments = attribute.ConstructorArguments; + return true; + } + } + } + + return false; + } } From 47f0fb9e6308c120f12a710b46b48fc4e1a2668c Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:13:02 -0300 Subject: [PATCH 18/20] removing resx embedded resource from analyzers --- src/Analyzers/Analyzers.csproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Analyzers/Analyzers.csproj b/src/Analyzers/Analyzers.csproj index cfcac5beb..f019c44c6 100644 --- a/src/Analyzers/Analyzers.csproj +++ b/src/Analyzers/Analyzers.csproj @@ -33,10 +33,6 @@ - - - - From d7274bd3c3c7679eca2d95552f8373d4d7b82a21 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 17 Apr 2024 07:42:29 -0300 Subject: [PATCH 19/20] Update src/Analyzers/Orchestration/OrchestrationAnalyzer.cs Co-authored-by: Varshitha Bachu --- src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 430c453a7..70af9674b 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -205,7 +205,7 @@ public override void Initialize(AnalysisContext context) /// Register additional actions to be executed after the compilation has started. /// It is expected from a concrete implementation of to register a /// - /// and then compare that any discovered violations happened in any of the symbols in . + /// and then compare that to any discovered violations happened in any of the symbols in . /// /// Context originally provided by . /// Collection of symbols referenced by orchestrations. From 6a2dd99058edcb2e9bbdeb5af46c039e4fc30849 Mon Sep 17 00:00:00 2001 From: Allan Targino <13934447+allantargino@users.noreply.github.com> Date: Wed, 17 Apr 2024 08:23:32 -0300 Subject: [PATCH 20/20] fixing comments grammar/additional comments --- src/Analyzers/KnownTypeSymbols.cs | 10 +++++----- src/Analyzers/Orchestration/OrchestrationAnalyzer.cs | 4 +++- .../Verifiers/CSharpAnalyzerVerifier.Durable.cs | 1 + 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Analyzers/KnownTypeSymbols.cs b/src/Analyzers/KnownTypeSymbols.cs index 19c82d1b3..81a45191e 100644 --- a/src/Analyzers/KnownTypeSymbols.cs +++ b/src/Analyzers/KnownTypeSymbols.cs @@ -22,27 +22,27 @@ sealed class KnownTypeSymbols(Compilation compilation) INamedTypeSymbol? durableTaskRegistry; /// - /// Gets OrchestrationTriggerAttribute type symbol. + /// Gets an OrchestrationTriggerAttribute type symbol. /// public INamedTypeSymbol? FunctionOrchestrationAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.OrchestrationTriggerAttribute", ref this.functionOrchestrationAttribute); /// - /// Gets FunctionNameAttribute type symbol. + /// Gets a FunctionNameAttribute type symbol. /// public INamedTypeSymbol? FunctionNameAttribute => this.GetOrResolveFullyQualifiedType("Microsoft.Azure.Functions.Worker.FunctionAttribute", ref this.functionNameAttribute); /// - /// Gets ITaskOrchestrator type symbol. + /// Gets an ITaskOrchestrator type symbol. /// public INamedTypeSymbol? TaskOrchestratorInterface => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.ITaskOrchestrator", ref this.taskOrchestratorInterface); /// - /// Gets TaskOrchestrator type symbol. + /// Gets a TaskOrchestrator type symbol. /// public INamedTypeSymbol? TaskOrchestratorBaseClass => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.TaskOrchestrator`2", ref this.taskOrchestratorBaseClass); /// - /// Gets DurableTaskRegistry type symbol. + /// Gets a DurableTaskRegistry type symbol. /// public INamedTypeSymbol? DurableTaskRegistry => this.GetOrResolveFullyQualifiedType("Microsoft.DurableTask.DurableTaskRegistry", ref this.durableTaskRegistry); diff --git a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs index 430c453a7..2a154cd8b 100644 --- a/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs +++ b/src/Analyzers/Orchestration/OrchestrationAnalyzer.cs @@ -18,6 +18,7 @@ public abstract class OrchestrationAnalyzer : DiagnosticAnalyzer /// public override void Initialize(AnalysisContext context) { + // this analyzer uses concurrent collections/operations, so we can enable actions concurrent execution to improve performance context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); @@ -156,13 +157,14 @@ public override void Initialize(AnalysisContext context) return; } - // all overloads have the parameter orchestrator, either as an Action or a Func + // all overloads have the parameter 'orchestrator', either as an Action or a Func IArgumentOperation orchestratorArgument = invocation.Arguments.First(a => a.Parameter!.Name == "orchestrator"); if (orchestratorArgument.Value is not IDelegateCreationOperation delegateCreationOperation) { return; } + // obtains the method symbol from the delegate creation operation IMethodSymbol? methodSymbol = null; switch (delegateCreationOperation.Target) { diff --git a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs index b570d3bc5..89a1178a8 100644 --- a/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs +++ b/test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs @@ -6,6 +6,7 @@ namespace Microsoft.DurableTask.Analyzers.Tests.Verifiers; +// Includes Durable Functions NuGet packages to an analyzer test and runs it public static partial class CSharpAnalyzerVerifier where TAnalyzer : DiagnosticAnalyzer, new() {