[Source Gen] Add retry options diagnostic descriptors (#1683)

Azure · Jun 27, 2023 · 6910409 · 6910409
1 parent a6322a3
commit 6910409
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 17 deletions.
diff --git a/docs/analyzer-rules/AZFW0012.md b/docs/analyzer-rules/AZFW0012.md
@@ -0,0 +1,30 @@
+# AZFW0012: Invalid Retry Options
+
+| | Value |
+|-|-|
+| **Rule ID** |AZFW0012|
+| **Category** |[AzureFunctionsSyntax]|
+| **Severity** |Error|
+
+## Cause
+
+This rule is triggered when a retry attribute is incorrectly used on an Azure Function.
+
+## Rule description
+
+This rule is triggered in two scenarios:
+
+- A retry strategy is not recognized (only Fixed Delay and Exponential Backoff are supported)
+- A retry attribute is used on a trigger type that does not support function-level retry.
+
+A list of triggers that support function-level retry can be found [here](https://learn.microsoft.com/en-us/azure/azure-functions/functions-bindings-error-pages?tabs=fixed-delay%2Cin-process&pivots=programming-language-csharp#retries).
+
+## How to fix violations
+
+Use the supported retry attributes on the correct trigger types. This information can be found [here](https://learn.microsoft.com/en-us/azure/azure-functions/functions-bindings-error-pages?tabs=fixed-delay%2Cin-process&pivots=programming-language-csharp#retries).
+
+Example functions using retry attributes can be found on the [extensions sample app](https://github.com/Azure/azure-functions-dotnet-worker/tree/main/samples/Extensions) in the source repository. There are examples on the CosmosDB function, the timer function, and more.
+
+## When to suppress warnings
+
+This rule should not be suppressed because this error will lead to unexpected retry behavior.
diff --git a/sdk/Sdk.Generators/DiagnosticDescriptors.cs b/sdk/Sdk.Generators/DiagnosticDescriptors.cs
@@ -51,8 +51,15 @@ private static DiagnosticDescriptor Create(string id, string title, string messa
 
         public static DiagnosticDescriptor InvalidCardinality { get; }
                   = Create(id: "AZFW0008",
-                    title: "Input or Trigger Binding Cardinality is invalid.",
-                    messageFormat: "The Cardinality of the Input or Trigger Binding on parameter '{0}' is invalid. IsBatched may be used incorrectly.",
+                    title: "Input or trigger binding cardinality is invalid.",
+                    messageFormat: "The cardinality of the input or trigger binding on parameter '{0}' is invalid. IsBatched may be used incorrectly.",
+                    category: "FunctionMetadataGeneration",
+                    severity: DiagnosticSeverity.Error);
+
+        public static DiagnosticDescriptor InvalidRetryOptions { get; }
+                    = Create(id: "AZFW0012",
+                    title: "Invalid operation with a retry attribute.",
+                    messageFormat: "Invalid use of a retry attribute. Check that the attribute is used on a trigger that supports function-level retry.",
                     category: "FunctionMetadataGeneration",
                     severity: DiagnosticSeverity.Error);
     }

diff --git a/...Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Emitter.cs b/...Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Emitter.cs
@@ -134,12 +134,12 @@ private StringBuilder BuildRetryOptions(GeneratorRetryOptions? retry)
                 else if (retry?.Strategy is RetryStrategy.ExponentialBackoff)
                 {
                     builder.AppendLine($$"""
-                                    Retry = new DefaultRetryOptions
-                                    {
-                                        MaxRetryCount = {{retry!.MaxRetryCount}},
-                                        MinimumInterval = TimeSpan.Parse("{{retry.MinimumInterval}}"),
-                                        MaximumInterval = TimeSpan.Parse("{{retry.MaximumInterval}}")
-                                    },
+                                     Retry = new DefaultRetryOptions
+                                     {
+                                         MaxRetryCount = {{retry!.MaxRetryCount}},
+                                         MinimumInterval = TimeSpan.Parse("{{retry.MinimumInterval}}"),
+                                         MaximumInterval = TimeSpan.Parse("{{retry.MaximumInterval}}")
+                                     },
                      """);
                 }
 

diff --git a/....Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs b/....Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs
@@ -6,6 +6,7 @@
 using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
+using System.Reflection;
 using System.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -114,13 +115,14 @@ private bool TryGetBindings(MethodDeclarationSyntax method, SemanticModel model,
 
                 if (retryOptions is not null)
                 {
-                    if (supportsRetryOptions && retryOptions is not null)
+                    if (supportsRetryOptions)
                     {
                         validatedRetryOptions = retryOptions;
                     }
-                    else if (!supportsRetryOptions && retryOptions is not null)
+                    else if (!supportsRetryOptions)
                     {
-                        // TODO: Log retry options related diagnostic error here
+                        _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidRetryOptions, method.GetLocation()));
+                        return false;
                     }
                 }
 
@@ -145,7 +147,7 @@ private bool TryGetMethodOutputBinding(MethodDeclarationSyntax method, SemanticM
                 {
                     if (SymbolEqualityComparer.Default.Equals(attribute.AttributeClass?.BaseType, _knownFunctionMetadataTypes.RetryAttribute))
                     {
-                        if (TryGetRetryOptionsFromAtttribute(attribute, out GeneratorRetryOptions? retryOptionsFromAttr))
+                        if (TryGetRetryOptionsFromAttribute(attribute, method.GetLocation(), out GeneratorRetryOptions? retryOptionsFromAttr))
                         {
                             retryOptions = retryOptionsFromAttr;
                         }
@@ -188,7 +190,7 @@ private bool TryGetMethodOutputBinding(MethodDeclarationSyntax method, SemanticM
                 return true;
             }
 
-            private bool TryGetRetryOptionsFromAtttribute(AttributeData attribute, out GeneratorRetryOptions? retryOptions)
+            private bool TryGetRetryOptionsFromAttribute(AttributeData attribute, Location location, out GeneratorRetryOptions? retryOptions)
             {
                 retryOptions = null;
 
@@ -224,12 +226,12 @@ private bool TryGetRetryOptionsFromAtttribute(AttributeData attribute, out Gener
 
                         if (attrProperties.TryGetValue(Constants.RetryConstants.MaximumIntervalKey, out object? maximumInterval)) // nonnullable constructor arg of attribute, wouldn't expect this to fail
                         {
-                            retryOptions.MinimumInterval = maximumInterval!.ToString();
+                            retryOptions.MaximumInterval = maximumInterval!.ToString();
                         }
                     }
                     else
                     {
-                        // TODO: Diagnostic error for retry options parsing failure
+                        _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidRetryOptions, location));
                         return false;
                     }
 

diff --git a/sdk/release_notes.md b/sdk/release_notes.md
@@ -17,3 +17,4 @@
 - Add support for retry options (#1548)
 - Bug fix for when DefaultValue is not present on an IsBatched prop (#1602).
 - SDK changes for .NET 8.0 support (#1643)
+- Add diagnostic descriptors for retry options (#1683)
diff --git a/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs b/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs
@@ -193,6 +193,44 @@ await TestHelpers.RunTestAsync<FunctionMetadataProviderGenerator>(
                     expectedOutput,
                     expectedDiagnosticResults);
             }
+
+            [Fact]
+            public async void InvalidRetryOptionsFailure()
+            {
+                var inputCode = @"using System;
+                using System.Threading.Tasks;
+                using Microsoft.Azure.Functions.Worker;
+                using Microsoft.Azure.Functions.Worker.Http;
+
+                namespace FunctionApp
+                {
+                    public class HttpFunction
+                    {
+                        [Function(""HttpFunction"")]
+                        [FixedDelayRetry(5, ""00:00:10"")]
+                        public string Run([HttpTrigger(""get"")] string req)
+                        {
+                            throw new NotImplementedException();
+                        }
+                    }
+                }";
+
+                string? expectedGeneratedFileName = null;
+                string? expectedOutput = null;
+
+                var expectedDiagnosticResults = new List<DiagnosticResult>
+                {
+                    new DiagnosticResult(DiagnosticDescriptors.InvalidRetryOptions)
+                    .WithSpan(10, 25, 15, 26)
+                };
+
+                await TestHelpers.RunTestAsync<FunctionMetadataProviderGenerator>(
+                    referencedExtensionAssemblies,
+                    inputCode,
+                    expectedGeneratedFileName,
+                    expectedOutput,
+                    expectedDiagnosticResults);
+            }
         }
     }
 }
diff --git a/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/RetryOptionsTests.cs b/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/RetryOptionsTests.cs
@@ -143,7 +143,7 @@ namespace SampleApp
                     public static class TimerFunction
                     {
                         [Function("TimerFunction")]
-                        [FixedDelayRetry(5, "00:00:10")]
+                        [ExponentialBackoffRetry(5, "00:00:04", "00:15:00")]
                         public static void Run([TimerTrigger("0 */5 * * * *")] TimerInfo timerInfo,
                             FunctionContext context)
                         {
@@ -184,7 +184,8 @@ public Task<ImmutableArray<IFunctionMetadata>> GetFunctionMetadataAsync(string d
                                 Retry = new DefaultRetryOptions
                                 {
                                     MaxRetryCount = 5,
-                                    DelayInterval = TimeSpan.Parse("00:00:10")
+                                    MinimumInterval = TimeSpan.Parse("00:00:04"),
+                                    MaximumInterval = TimeSpan.Parse("00:15:00")
                                 },
                                 ScriptFile = "TestProject.dll"
                             };