Review 2

analyzers/src/SonarAnalyzer.CSharp/Helpers/MessageTemplateExtractor.cs

@@ -27,7 +27,7 @@ internal static class MessageTemplateExtractor
     public static ArgumentSyntax TemplateArgument(InvocationExpressionSyntax invocation, SemanticModel model) =>
        TemplateArgument(invocation, model, KnownType.Microsoft_Extensions_Logging_LoggerExtensions, MicrosoftExtensionsLogging, "message")
        ?? TemplateArgument(invocation, model, KnownType.Serilog_Log, Serilog, "messageTemplate")
-       ?? TemplateArgument(invocation, model, KnownType.Serilog_ILogger, Serilog, "messageTemplate")
+       ?? TemplateArgument(invocation, model, KnownType.Serilog_ILogger, Serilog, "messageTemplate", checkDerivedTypes: true)
        ?? TemplateArgument(invocation, model, KnownType.NLog_ILoggerExtensions, NLogLoggingMethods, "message")
        ?? TemplateArgument(invocation, model, KnownType.NLog_ILogger, NLogLoggingMethods, "message", checkDerivedTypes: true)
        ?? TemplateArgument(invocation, model, KnownType.NLog_ILoggerBase, NLogILoggerBase, "message", checkDerivedTypes: true);

analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/IMessageTemplateCheck.cs

@@ -24,5 +24,5 @@ public interface IMessageTemplateCheck
 {
     DiagnosticDescriptor Rule { get; }
 
-    void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressionSyntax invocation, ArgumentSyntax templateArgument, Helpers.MessageTemplates.Placeholder[] placeholders);
+    void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressionSyntax invocation, ArgumentSyntax templateArgument, MessageTemplatesParser.Placeholder[] placeholders);
 }

analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs

@@ -46,7 +46,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
             if (enabledChecks.Length > 0
                 && MessageTemplateExtractor.TemplateArgument(invocation, c.SemanticModel) is { } argument
                 && HasValidExpression(argument)
-                && Helpers.MessageTemplates.Parse(argument.Expression.ToString()) is { Success: true } result)
+                && MessageTemplatesParser.Parse(argument.Expression.ToString()) is { Success: true } result)
             {
                 foreach (var check in enabledChecks)
                 {

analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/NamedPlaceholdersShouldBeUnique.cs

@@ -29,7 +29,7 @@ public sealed class NamedPlaceholdersShouldBeUnique : IMessageTemplateCheck
 
     public DiagnosticDescriptor Rule => S6677;
 
-    public void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressionSyntax invocation, ArgumentSyntax templateArgument, Helpers.MessageTemplates.Placeholder[] placeholders)
+    public void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressionSyntax invocation, ArgumentSyntax templateArgument, MessageTemplatesParser.Placeholder[] placeholders)
     {
         var duplicatedGroups = placeholders
             .Where(x => x.Name != "_" && !int.TryParse(x.Name, out _)) // exclude wildcard "_" and index placeholders like {42}

analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/UsePascalCaseForNamedPlaceHolders.cs

@@ -18,7 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-using static SonarAnalyzer.Helpers.MessageTemplates;
+using static SonarAnalyzer.Helpers.MessageTemplatesParser;
 
 namespace SonarAnalyzer.Rules.MessageTemplates;
 
@@ -31,7 +31,7 @@ public sealed class UsePascalCaseForNamedPlaceHolders : IMessageTemplateCheck
 
     public DiagnosticDescriptor Rule => S6678;
 
-    public void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressionSyntax invocation, ArgumentSyntax templateArgument, Helpers.MessageTemplates.Placeholder[] placeholders)
+    public void Execute(SonarSyntaxNodeReportingContext context, InvocationExpressionSyntax invocation, ArgumentSyntax templateArgument, Placeholder[] placeholders)
     {
         var nonPascalCasePlaceholders = placeholders.Where(x => char.IsLower(x.Name[0])).ToArray();
         if (nonPascalCasePlaceholders.Length > 0)

analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplatesShouldBeCorrect.cs

@@ -52,57 +52,51 @@ protected override void Initialize(SonarAnalysisContext context) =>
 
     private static class TemplateValidator
     {
+        private const int EmptyPlaceholderSize = 2; // "{}"
+
         private const string TextPattern = @"([^\{]|\{\{|\}\})+";
         private const string HolePattern = @"{(?<Placeholder>[^\}]*)}";
         private const string TemplatePattern = $"^({TextPattern}|{HolePattern})*$";
         // This is similar to the regex used for MessageTemplatesAnalyzer, but it is far more permissive.
         // The goal is to manually parse the placeholders, so that we can report more specific issues than just "malformed template".
         private static readonly Regex TemplateRegex = new(TemplatePattern, RegexOptions.Compiled, TimeSpan.FromMilliseconds(300));
-
         private static readonly Regex PlaceholderNameRegex = new("^[0-9a-zA-Z_]+$", RegexOptions.Compiled, RegexConstants.DefaultTimeout);
         private static readonly Regex PlaceholderAlignmentRegex = new("^-?[0-9]+$", RegexOptions.Compiled, RegexConstants.DefaultTimeout);
 
         public static bool ContainsErrors(string template, out List<ParsingError> errors)
         {
-            var result = Helpers.MessageTemplates.Parse(template, TemplateRegex);
+            var result = MessageTemplatesParser.Parse(template, TemplateRegex);
             errors = result.Success
                 ? result.Placeholders.Select(ParsePlaceholder).Where(x => x is not null).ToList()
                 : [new("should be syntactically correct", 0, template.Length)];
             return errors.Count > 0;
         }
 
-        private static ParsingError ParsePlaceholder(Helpers.MessageTemplates.Placeholder placeholder)
+        private static ParsingError ParsePlaceholder(MessageTemplatesParser.Placeholder placeholder)
         {
             if (placeholder.Length == 0)
             {
-                return new($"should not contain empty placeholder", placeholder.Start - 1, 2); // highlight both brackets '{}', since there is no content
+                return new("should not contain empty placeholder", placeholder.Start - 1, EmptyPlaceholderSize);
             }
-
             var parts = Split(placeholder.Name);
-            if (!PlaceholderNameRegex.SafeIsMatch(parts.Name))
-            {
-                return new($"placeholder '{parts.Name}' should only contain letters, numbers, and underscore", placeholder);
-            }
-            else if (parts.Alignment is not null && !PlaceholderAlignmentRegex.SafeIsMatch(parts.Alignment))
-            {
-                return new($"placeholder '{parts.Name}' should have numeric alignment instead of '{parts.Alignment}'", placeholder);
-            }
-            else if (parts.Format == string.Empty)
-            {
-                return new($"placeholder '{parts.Name}' should not have empty format", placeholder);
-            }
-            else
+            return "🔥" switch
             {
-                return null;
-            }
+                _ when !PlaceholderNameRegex.SafeIsMatch(parts.Name) =>
+                    new($"placeholder '{parts.Name}' should only contain letters, numbers, and underscore", placeholder),
+
+                _ when parts.Alignment is not null && !PlaceholderAlignmentRegex.SafeIsMatch(parts.Alignment) =>
+                    new($"placeholder '{parts.Name}' should have numeric alignment instead of '{parts.Alignment}'", placeholder),
+
+                _ when parts.Format == string.Empty =>
+                    new($"placeholder '{parts.Name}' should not have empty format", placeholder),
+
+                _ => null,
+            };
         }
 
         // pattern is: name[,alignment][:format]
         private static Parts Split(string placeholder)
         {
-            var start = placeholder[0] is '@' or '$' ? 1 : 0; // skip prefix
-
-            string name;
             string alignment = null;
             string format = null;
 
@@ -115,44 +109,33 @@ private static Parts Split(string placeholder)
                 alignmentIndex = -1;
             }
 
-            if (NotFound(alignmentIndex))
+            if (formatIndex == -1)
             {
-                if (NotFound(formatIndex))
-                {
-                    name = placeholder.Substring(start);
-                }
-                else
-                {
-                    name = placeholder.Substring(start, formatIndex - start);
-                    format = IsEmpty(formatIndex) ? string.Empty : placeholder.Substring(formatIndex + 1);
-                }
+                formatIndex = placeholder.Length;
             }
             else
             {
-                if (NotFound(formatIndex))
-                {
-                    name = placeholder.Substring(start, alignmentIndex - start);
-                    alignment = IsEmpty(alignmentIndex) ? string.Empty : placeholder.Substring(alignmentIndex + 1);
-                }
-                else
-                {
-                    name = placeholder.Substring(start, alignmentIndex - start);
-                    alignment = placeholder.Substring(alignmentIndex + 1, formatIndex - alignmentIndex - 1);
-                    format = placeholder.Substring(formatIndex + 1);
-                }
+                format = placeholder.Substring(formatIndex + 1);
+            }
+            if (alignmentIndex == -1)
+            {
+                alignmentIndex = formatIndex;
+            }
+            else
+            {
+                alignment = placeholder.Substring(alignmentIndex + 1, formatIndex - alignmentIndex - 1);
             }
 
+            var start = placeholder[0] is '@' or '$' ? 1 : 0; // skip prefix
+            var name = placeholder.Substring(start, alignmentIndex - start);
             return new(name, alignment, format);
-
-            bool NotFound(int index) => index == -1;
-            bool IsEmpty(int index) => index == placeholder.Length - 1;
         }
 
         private sealed record Parts(string Name, string Alignment, string Format);
 
         public sealed record ParsingError(string Message, int Start, int Length)
         {
-            public ParsingError(string message, Helpers.MessageTemplates.Placeholder placeholder)
+            public ParsingError(string message, MessageTemplatesParser.Placeholder placeholder)
                 : this(message, placeholder.Start, placeholder.Length)
             { }
         }

analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplates.cs → analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplatesParser.cs

@@ -25,7 +25,7 @@ namespace SonarAnalyzer.Helpers;
 /// <summary>
 /// Grammar can be found <a href="https://github.com/messagetemplates/grammar"> here</a>.
 /// </summary>
-public static class MessageTemplates
+public static class MessageTemplatesParser
 {
     private const string NamePattern = "[0-9a-zA-Z_]+";
     private const string PlaceholderPattern = $"(?<Placeholder>{NamePattern})";

analyzers/tests/SonarAnalyzer.Test/Helpers/MessageTemplatesTest.cs

@@ -35,7 +35,7 @@ public class MessageTemplatesTest
     [DataRow("{{hello {{world}}}}")]
     public void Parse_NoPlaceholder(string template)
     {
-        var result = MessageTemplates.Parse(template);
+        var result = MessageTemplatesParser.Parse(template);
         ShouldBeSuccess(result);
     }
 
@@ -61,7 +61,7 @@ public void Parse_NoPlaceholder(string template)
     [DataRow(""" "hello" + "{world}" + "!" """, "world", 13, 5)]
     public void Parse_Placeholder(string template, string placeholder, int start, int length)
     {
-        var result = MessageTemplates.Parse(template);
+        var result = MessageTemplatesParser.Parse(template);
         ShouldBeSuccess(result, 1);
         ShouldBe(result.Placeholders[0], placeholder, start, length);
     }
@@ -87,7 +87,7 @@ public void Parse_Placeholder(string template, string placeholder, int start, in
     [DataRow("hello {world:format,42}")] // semantically looks like a typo, format and alignment are reversed, but it's syntactically valid.
     public void Parse_Placeholder_Named_Alignment_Format(string template)
     {
-        var result = MessageTemplates.Parse(template);
+        var result = MessageTemplatesParser.Parse(template);
         ShouldBeSuccess(result, 1);
         ShouldBe(result.Placeholders[0], "world", 7, 5);
     }
@@ -104,7 +104,7 @@ public void Parse_Placeholder_Multiple()
             Logic {@_orchestrates42} the mind,
             Programmer's {ballet:_}.
             """;
-        var result = MessageTemplates.Parse(template);
+        var result = MessageTemplatesParser.Parse(template);
         ShouldBeSuccess(result, 6);
         ShouldBe(result.Placeholders[0], "silent", 12, 6);
         ShouldBe(result.Placeholders[1], "0", 56, 1);
@@ -129,20 +129,20 @@ public void Parse_Placeholder_Multiple()
     [DataRow(""" "hello {" + "world" + "}" """)]    // '+' and '"' is not allowed in placeholders
     public void Parse_Placeholder_Failure(string template)
     {
-        var result = MessageTemplates.Parse(template);
+        var result = MessageTemplatesParser.Parse(template);
         result.Should().NotBeNull();
         result.Success.Should().BeFalse();
         result.Placeholders.Should().BeNull();
     }
 
-    private static void ShouldBeSuccess(MessageTemplates.ParseResult actual, int placeholderCount = 0)
+    private static void ShouldBeSuccess(MessageTemplatesParser.ParseResult actual, int placeholderCount = 0)
     {
         actual.Should().NotBeNull();
         actual.Success.Should().BeTrue();
         actual.Placeholders.Should().NotBeNull().And.HaveCount(placeholderCount);
     }
 
-    private static void ShouldBe(MessageTemplates.Placeholder actual, string name, int start, int length)
+    private static void ShouldBe(MessageTemplatesParser.Placeholder actual, string name, int start, int length)
     {
         actual.Should().NotBeNull();
         actual.Name.Should().Be(name);