From 7837d8cd4a606a4fcfa15c710ab9f2fbb3a37c80 Mon Sep 17 00:00:00 2001 From: Sarah Vu Date: Fri, 1 Sep 2023 14:31:46 -0700 Subject: [PATCH 1/4] add diag error log and info --- docs/analyzer-rules/AZFW0013.md | 25 +++++++++++++++++++ sdk/Sdk.Generators/DiagnosticDescriptors.cs | 7 ++++++ ...unctionMetadataProviderGenerator.Parser.cs | 4 +-- 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 docs/analyzer-rules/AZFW0013.md diff --git a/docs/analyzer-rules/AZFW0013.md b/docs/analyzer-rules/AZFW0013.md new file mode 100644 index 000000000..c09223c31 --- /dev/null +++ b/docs/analyzer-rules/AZFW0013.md @@ -0,0 +1,25 @@ +# AZFW0013: Unable to parse binding argument + +| | Value | +|-|-| +| **Rule ID** |AZFW0013| +| **Category** |[AzureFunctionsSyntax]| +| **Severity** |Error| + +## Cause + +This rule is triggered when a binding attribute argument is an invalid or null value. + +## Rule description + +[Attributes](https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/reflection-and-attributes/) are used to define bindings, and the function metadata generator parses the arguments passed into these attributes to generate binding information during start up. + +If the arguments passed in are invalid for any reason (such as being null), this rule is enforced. + +## How to fix violations + +Review the binding attribute. + +## When to suppress warnings + +This rule should not be suppressed because this error will prevent your functions from running. diff --git a/sdk/Sdk.Generators/DiagnosticDescriptors.cs b/sdk/Sdk.Generators/DiagnosticDescriptors.cs index 2ebf9408a..9aaca2a6b 100644 --- a/sdk/Sdk.Generators/DiagnosticDescriptors.cs +++ b/sdk/Sdk.Generators/DiagnosticDescriptors.cs @@ -62,5 +62,12 @@ private static DiagnosticDescriptor Create(string id, string title, string messa 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); + public static DiagnosticDescriptor InvalidBindingAttributeArgument { get; } + = Create(id: "AZFW0013", + title: "Invalid argument in binding attribute.", + messageFormat: "Invalid argument passed in binding attribute. Check that the argument is not null.", + category: "FunctionMetadataGeneration", + severity: DiagnosticSeverity.Error); + } } diff --git a/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs b/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs index 5ba6d02c7..27e55cd2d 100644 --- a/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs +++ b/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs @@ -632,7 +632,7 @@ private bool TryGetAttributeProperties(AttributeData attributeData, Location? at } else { - // TODO: Log diagnostic error + _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, Location.None)); } } } @@ -690,7 +690,7 @@ private bool TryLoadConstructorArguments(AttributeData attributeData, IDictionar } else { - // TODO: Log diagnostic error + _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, Location.None)); } } From 0f072212197a6b10a3206ea7979ee3b03af866e2 Mon Sep 17 00:00:00 2001 From: Sarah Vu Date: Wed, 6 Sep 2023 14:40:23 -0700 Subject: [PATCH 2/4] last fix, add test --- ...unctionMetadataProviderGenerator.Parser.cs | 11 +++++- .../DiagnosticResultTests.cs | 38 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs b/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs index 27e55cd2d..e83321939 100644 --- a/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs +++ b/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs @@ -632,10 +632,16 @@ private bool TryGetAttributeProperties(AttributeData attributeData, Location? at } else { - _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, Location.None)); + _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, attribLocation)); + return false; } } } + else + { + _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, attribLocation)); + return false; + } } // some properties have default values, so if these properties were not already defined in constructor or named arguments, we will auto-add them here @@ -690,7 +696,8 @@ private bool TryLoadConstructorArguments(AttributeData attributeData, IDictionar } else { - _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, Location.None)); + _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, attributeLocation)); + return false; } } diff --git a/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs b/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs index 8478337b2..4e57db6e3 100644 --- a/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs +++ b/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs @@ -231,6 +231,44 @@ await TestHelpers.RunTestAsync( expectedOutput, expectedDiagnosticResults); } + + [Fact] + public async void InvalidBindingArgument() + { + 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(""QueueFunction"")] + public string QueueFunc( + [QueueTrigger(""queueName"", Connection = null)] string queuePayload) + { + throw new NotImplementedException(); + } + } + }"; + + string? expectedGeneratedFileName = null; + string? expectedOutput = null; + + var expectedDiagnosticResults = new List + { + new DiagnosticResult(DiagnosticDescriptors.InvalidBindingAttributeArgument) + .WithSpan(12, 79, 12, 91) + }; + + await TestHelpers.RunTestAsync( + referencedExtensionAssemblies, + inputCode, + expectedGeneratedFileName, + expectedOutput, + expectedDiagnosticResults); + } } } } From 75396169c0a50d7d88f0fc85f50d9e89f583661d Mon Sep 17 00:00:00 2001 From: Sarah Vu Date: Wed, 6 Sep 2023 14:42:01 -0700 Subject: [PATCH 3/4] add release notes --- sdk/release_notes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/release_notes.md b/sdk/release_notes.md index 5a99a0a84..7ba2857c9 100644 --- a/sdk/release_notes.md +++ b/sdk/release_notes.md @@ -14,4 +14,5 @@ ### Microsoft.Azure.Functions.Worker.Sdk.Generators -- Parse named arguments by type (#1877) \ No newline at end of file +- Parse named arguments by type (#1877) +- Add diagnostic descriptor logs for parsing binding arguments in source gen (#1882) From 6c27339a5b85f89419d7ea94368ee0bd040fce2b Mon Sep 17 00:00:00 2001 From: Sarah Vu Date: Wed, 6 Sep 2023 15:51:58 -0700 Subject: [PATCH 4/4] bug fix --- ...unctionMetadataProviderGenerator.Parser.cs | 5 --- .../DiagnosticResultTests.cs | 38 ------------------- 2 files changed, 43 deletions(-) diff --git a/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs b/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs index e83321939..8c0d1caf6 100644 --- a/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs +++ b/sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs @@ -637,11 +637,6 @@ private bool TryGetAttributeProperties(AttributeData attributeData, Location? at } } } - else - { - _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.InvalidBindingAttributeArgument, attribLocation)); - return false; - } } // some properties have default values, so if these properties were not already defined in constructor or named arguments, we will auto-add them here diff --git a/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs b/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs index 4e57db6e3..8478337b2 100644 --- a/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs +++ b/test/Sdk.Generator.Tests/FunctionMetadataProviderGeneratorTests/DiagnosticResultTests.cs @@ -231,44 +231,6 @@ await TestHelpers.RunTestAsync( expectedOutput, expectedDiagnosticResults); } - - [Fact] - public async void InvalidBindingArgument() - { - 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(""QueueFunction"")] - public string QueueFunc( - [QueueTrigger(""queueName"", Connection = null)] string queuePayload) - { - throw new NotImplementedException(); - } - } - }"; - - string? expectedGeneratedFileName = null; - string? expectedOutput = null; - - var expectedDiagnosticResults = new List - { - new DiagnosticResult(DiagnosticDescriptors.InvalidBindingAttributeArgument) - .WithSpan(12, 79, 12, 91) - }; - - await TestHelpers.RunTestAsync( - referencedExtensionAssemblies, - inputCode, - expectedGeneratedFileName, - expectedOutput, - expectedDiagnosticResults); - } } } }