Fixed analyzer diagnostic message. (#28)

* Sadly default verification doesn't consider the arguments of a message
    - Analyzer for nullable + required properties was using the wrong message argument.
* Added common verifier function for validating the message arguments
* Added custom verification to include the common verifier and the help URI analyzer

Author: smaillet

src/Ubiquity.NET.CommandLine.SrcGen.UT/CommandAnalyzerTests.cs

@@ -29,8 +29,8 @@ public async Task Option_attribute_without_command_triggers_diagnostic( TestRunt
             analyzerTest.ExpectedDiagnostics.AddRange(
                [
                    new DiagnosticResult("UNC001", DiagnosticSeverity.Error)
-                       .WithSpan(9, 6, 9, 93)
-                       .WithArguments("OptionAttribute"),
+                       .WithArguments("OptionAttribute")
+                       .WithSpan(9, 6, 9, 93),
                ]
            );
 
@@ -75,12 +75,12 @@ public async Task FolderValidation_attribute_without_command_triggers_diagnostic
             analyzerTest.ExpectedDiagnostics.AddRange(
                [
                    new DiagnosticResult("UNC001", DiagnosticSeverity.Error)
-                       .WithSpan(11, 6, 11, 55)
-                       .WithArguments("FolderValidationAttribute"),
+                       .WithArguments("FolderValidationAttribute")
+                       .WithSpan(11, 6, 11, 55),
 
                    new DiagnosticResult("UNC002", DiagnosticSeverity.Error)
-                       .WithSpan(11, 6, 11, 55)
-                       .WithArguments("FolderValidationAttribute"),
+                       .WithArguments("FolderValidationAttribute")
+                       .WithSpan(11, 6, 11, 55),
                ]
              );
 
@@ -110,6 +110,7 @@ public async Task FileValidation_with_wrong_type_produces_UNC002( TestRuntime te
             analyzerTest.ExpectedDiagnostics.AddRange(
                [
                    new DiagnosticResult("UNC003", DiagnosticSeverity.Error)
+                      .WithArguments("FileValidationAttribute", "System.IO.FileInfo")
                       .WithSpan(9, 6, 9, 51),
                ]
              );
@@ -129,8 +130,8 @@ public async Task Required_nullable_types_produce_diagnostic( TestRuntime testRu
             analyzerTest.ExpectedDiagnostics.AddRange(
                [
                    new DiagnosticResult("UNC004", DiagnosticSeverity.Warning)
-                      .WithSpan(9, 6, 9, 127)
-                      .WithArguments("bool?", "OptionAttribute"),
+                      .WithArguments("bool?", "Thing1")
+                      .WithSpan(9, 6, 9, 127),
                ]
              );
 
@@ -165,15 +166,29 @@ private AnalyzerTest<MsTestVerifier> CreateTestRunner( SourceText source, TestRu
                 // Allow ALL diagnostics for testing, input source should contain valid C# code
                 // but might otherwise trigger the tested analyzer.
                 CompilerDiagnostics = CompilerDiagnostics.All,
+
+                // Add custom verification to handle cases not covered in default verifications
+                DiagnosticVerifier = DiagnosticVerifier,
             };
         }
 
-        // Sadly, at this time the test infrastructure doesn't provide support for
-        // testing the help URI
-        //private static string FormatHelpUri( string id )
-        //{
-        //    return $"https://ubiquitydotnet.github.io/Ubiquity.NET.Utils/CommandLine/diagnostics/{id}.html";
-        //}
+        /// <summary>Custom diagnostic verifier</summary>
+        /// <param name="diagnostic">diagnostic to verify (actual)</param>
+        /// <param name="result">Diagnostic result containing the expected results</param>
+        /// <param name="verifier">Implementation of <see cref="IVerifier"/> to use for verification</param>
+        /// <seealso href="https://github.com/dotnet/roslyn-sdk/issues/1246"/>
+        private void DiagnosticVerifier( Diagnostic diagnostic, DiagnosticResult result, IVerifier verifier )
+        {
+            DiagnosticVerifiers.VerifyMessageArguments( diagnostic, result, verifier );
+
+            // verify the help URI matches the expected form
+            verifier.EqualOrDiff( diagnostic.Descriptor.HelpLinkUri, FormatHelpUri( diagnostic.Id ) );
+        }
+
+        private static string FormatHelpUri( string id )
+        {
+            return $"https://ubiquitydotnet.github.io/Ubiquity.NET.Utils/CommandLine/diagnostics/{id}.html";
+        }
 
         private static SourceText GetSourceText( params string[] nameParts )
         {

src/Ubiquity.NET.CommandLine.SrcGen/CommandLineAnalyzer.cs

@@ -115,7 +115,7 @@ EquatableAttributeDataCollection attributes
             VerifyCommandAttribute( context, attribLoc, Constants.OptionAttribute.SimpleName );
 
             EquatableAttributeData attribute = attributes[Constants.OptionAttribute];
-            VerifyNotNullableRequired( context, symbol, attribute, attribLoc, Constants.OptionAttribute.SimpleName );
+            VerifyNotNullableRequired( context, symbol, attribute, attribLoc );
 
             // Additional validations...
         }
@@ -131,7 +131,7 @@ EquatableAttributeDataCollection attribs
             VerifyCommandAttribute( context, attribLoc, Constants.FileValidationAttribute.SimpleName );
 
             // Verify an Option property (or maybe an argument attribute once supported) exists
-            VerifyHasConstrainedAttribute( context, attribLoc, attribs );
+            VerifyHasConstrainedAttribute( context, attribLoc, attribs, Constants.FileValidationAttribute.SimpleName );
 
             // Verify type of the property is System.IO.FileInfo.
             VerifyRequiredPropertyType( context, symbol, Constants.FileInfo, attribLoc, Constants.FileValidationAttribute.SimpleName );
@@ -150,9 +150,9 @@ EquatableAttributeDataCollection attribs
             VerifyCommandAttribute( context, attribLoc, Constants.FolderValidationAttribute.SimpleName );
 
             // Verify an Option property (or maybe an argument attribute once supported) exists
-            VerifyHasConstrainedAttribute( context, attribLoc, attribs );
+            VerifyHasConstrainedAttribute( context, attribLoc, attribs, Constants.FolderValidationAttribute.SimpleName );
 
-            // Verify type of the property is System.IO.FileInfo.
+            // Verify type of the property is System.IO.DirectoryInfo.
             VerifyRequiredPropertyType( context, symbol, Constants.DirectoryInfo, attribLoc, Constants.FolderValidationAttribute.SimpleName );
 
             // Additional validations...
@@ -183,18 +183,26 @@ private static void VerifyCommandAttribute( SymbolAnalysisContext context, Locat
         /// <param name="context">Context to use for reporting diagnostics</param>
         /// <param name="attribLoc">Location to use for any diagnostics reported</param>
         /// <param name="attribs">Set of attributes to check</param>
+        /// <param name="typeConstraintName">Name of the attribute that requires a constraint (For diagnostic message)</param>
         /// <remarks>
         /// The <paramref name="attribLoc"/> is used to report diagnostics that normally references the attribute
         /// that is missing the constrained attribute. (ex. The `FileValidation` or `FolderValidation` that does NOT
         /// have an `OptionAttribute` that it validates.
         /// </remarks>
-        private static void VerifyHasConstrainedAttribute( SymbolAnalysisContext context, Location? attribLoc, EquatableAttributeDataCollection attribs )
+        private static void VerifyHasConstrainedAttribute(
+            SymbolAnalysisContext context,
+            Location? attribLoc,
+            EquatableAttributeDataCollection attribs,
+            string typeConstraintName
+            )
         {
-            // Verify an Option property (or maybe an argument attribute once supported) exists
+            // Verify an Option property
             if(!attribs.TryGetValue( Constants.OptionAttribute, out _ ))
             {
-                ReportDiagnostic( context, Diagnostics.MissingConstraintAttribute, attribLoc, Constants.FileValidationAttribute.SimpleName );
+                ReportDiagnostic( context, Diagnostics.MissingConstraintAttribute, attribLoc, typeConstraintName );
             }
+
+            // TODO: validate an Argument attribute or Option attribute
         }
 
         /// <summary>Verifies a property has an expected type</summary>
@@ -226,17 +234,15 @@ private static void VerifyNotNullableRequired(
             SymbolAnalysisContext context,
             IPropertySymbol property,
             EquatableAttributeData attribute,
-            Location? attribLoc,
-            string simpleName
-            )
+            Location? attribLoc )
         {
             if(attribute.NamedArguments.TryGetValue( Constants.CommonAttributeNamedArgs.Required, out StructurallyEquatableTypedConstant tc ))
             {
                 bool isRequired = !tc.IsNull && (bool)tc.Value!;
                 NamespaceQualifiedTypeName propType = property.Type.GetNamespaceQualifiedName();
                 if(isRequired && propType.IsNullable)
                 {
-                    ReportDiagnostic( context, Diagnostics.RequiredNullableType, attribLoc, $"{propType:A}", simpleName );
+                    ReportDiagnostic( context, Diagnostics.RequiredNullableType, attribLoc, $"{propType:A}", property.Name );
                 }
             }
         }

src/Ubiquity.NET.CommandLine.SrcGen/Diagnostics.cs

@@ -32,6 +32,7 @@ internal static ImmutableArray<DiagnosticDescriptor> CommandLineAnalyzerDiagnost
                 RequiredNullableType,
             ];
 
+        // Exception message is: '{0}'
         internal static readonly DiagnosticDescriptor InternalError = new(
             id: IDs.InternalError,
             title: Localized(nameof(Resources.InternalError_Title)),
@@ -44,6 +45,7 @@ internal static ImmutableArray<DiagnosticDescriptor> CommandLineAnalyzerDiagnost
             WellKnownDiagnosticTags.NotConfigurable
             );
 
+        // Property attribute '{0}' is only allowed on a property in a type attributed with a command attribute. This use will be ignored by the generator.
         internal static readonly DiagnosticDescriptor MissingCommandAttribute = new(
             id: IDs.MissingCommandAttribute,
             title: Localized(nameof(Resources.MissingCommandAttribute_Title)),
@@ -57,6 +59,7 @@ internal static ImmutableArray<DiagnosticDescriptor> CommandLineAnalyzerDiagnost
             WellKnownDiagnosticTags.Compiler
             );
 
+        // Property attribute '{0}' is not allowed on a property independent of a qualifying attribute such as OptionAttribute.
         internal static readonly DiagnosticDescriptor MissingConstraintAttribute = new(
             id: IDs.MissingConstraintAttribute,
             title: Localized(nameof(Resources.MissingConstraintAttribute_Title)),
@@ -69,6 +72,7 @@ internal static ImmutableArray<DiagnosticDescriptor> CommandLineAnalyzerDiagnost
             WellKnownDiagnosticTags.Compiler
             );
 
+        // Property attribute '{0}' requires a property of type '{1}'.
         internal static readonly DiagnosticDescriptor IncorrectPropertyType = new(
             id: IDs.IncorrectPropertyType,
             title: Localized(nameof(Resources.IncorrectPropertyType_Title)),

src/Ubiquity.NET.SourceGenerator.Test.Utils/CSharp/SourceAnalyzerTest.cs

@@ -13,7 +13,8 @@ namespace Ubiquity.NET.SourceGenerator.Test.Utils.CSharp
     /// <summary>Source analyzer tests for C# that allows specification of the language</summary>
     /// <typeparam name="TAnalyzer">Analyzer type</typeparam>
     /// <typeparam name="TVerifier">Verifier type</typeparam>
-    public class SourceAnalyzerTest<TAnalyzer, TVerifier> : AnalyzerTest<TVerifier>
+    public class SourceAnalyzerTest<TAnalyzer, TVerifier>
+        : AnalyzerTest<TVerifier>
         where TAnalyzer : DiagnosticAnalyzer, new()
         where TVerifier : IVerifier, new()
     {

src/Ubiquity.NET.SourceGenerator.Test.Utils/DiagnosticVerifiers.cs

@@ -0,0 +1,59 @@
+// Copyright (c) Ubiquity.NET Contributors. All rights reserved.
+// Licensed under the Apache-2.0 WITH LLVM-exception license. See the LICENSE.md file in the project root for full license information.
+
+using System.Diagnostics;
+using System.Globalization;
+using System.Text;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Testing;
+
+namespace Ubiquity.NET.SourceGenerator.Test.Utils
+{
+    /// <summary>Utility class to provide additional verification of a <see cref="Diagnostic"/></summary>
+    /// <remarks>
+    /// <see cref="AnalyzerTest{TVerifier}.DiagnosticVerifier"/> uses only a single delegate AND the default
+    /// handling of a test doesn't cover some common scenarios. This class provides those in a form that is
+    /// designed to make compossible verifiers as needed.
+    /// </remarks>
+    public static class DiagnosticVerifiers
+    {
+        /// <summary>Custom diagnostic verifier to validate the message arguments of a <see cref="Diagnostic"/></summary>
+        /// <param name="diagnostic">diagnostic to verify (actual)</param>
+        /// <param name="result">Diagnostic result containing the expected results</param>
+        /// <param name="verifier">Implementation of <see cref="IVerifier"/> to use for verification</param>
+        /// <remarks>
+        /// Default verification provided by <see cref="AnalyzerTest{TVerifier}"/> use an undocumented set
+        /// of heuristics to verify a given <see cref="Diagnostic"/> is "roughly equivalent" to an expected
+        /// <see cref="DiagnosticResult"/>. Unfortunately, the heuristics used are not documented and ultimately
+        /// don't include verification of the message args (which a diagnostic producer might get wrong). This
+        /// method will verify the arguments by formatting a message using the expected arguments and comparing
+        /// that to the message for the <paramref name="diagnostic"/>.
+        /// </remarks>
+        /// <seealso href="https://github.com/dotnet/roslyn-sdk/issues/1246"/>
+        public static void VerifyMessageArguments( Diagnostic diagnostic, DiagnosticResult result, IVerifier verifier )
+        {
+            var parsedFormat = CompositeFormat.Parse(diagnostic.Descriptor.MessageFormat.ToString());
+            int expectedMessageCount = result.MessageArguments?.Length ?? 0;
+            int actualMessageCount = parsedFormat.MinimumArgumentCount;
+
+            verifier.Equal(
+                parsedFormat.MinimumArgumentCount,
+                expectedMessageCount,
+                $"Missing test arguments; The number of expected arguments ({expectedMessageCount}) does not match the actual message {actualMessageCount}. [This is a debug only verification]"
+            );
+
+            // only test diagnostic if there are expected message args to validate
+            // There's no direct way to access the arguments of a diagnostic. (It's there but "internal" and
+            // therefore can not be relied upon). So, get the message format from the descriptor and format a message
+            // with the expected arguments. Then test the result of that against the message reported by the diagnostic.
+            // This will ultimately test that the arguments match expectations.
+            if(expectedMessageCount > 0 && parsedFormat.MinimumArgumentCount > 0 && result.MessageArguments is not null)
+            {
+                string expectedMessage = string.Format(CultureInfo.CurrentCulture, parsedFormat.Format, result.MessageArguments!);
+                string actualMessage = diagnostic.GetMessage( CultureInfo.CurrentCulture );
+                verifier.Equal( expectedMessage, actualMessage, "Message arguments should be the same" );
+            }
+        }
+    }
+}