diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/MessageTemplateExtractor.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/MessageTemplateExtractor.cs index 25efe1a6142..9eaa4ce288b 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/MessageTemplateExtractor.cs +++ b/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); diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/IMessageTemplateCheck.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/IMessageTemplateCheck.cs index 9a1d33ab29d..90a767604f6 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/IMessageTemplateCheck.cs +++ b/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); } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs index 9163baf7974..f16f8f7c67a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/MessageTemplateAnalyzer.cs +++ b/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) { diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/NamedPlaceholdersShouldBeUnique.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/NamedPlaceholdersShouldBeUnique.cs index e9038b4d891..ff958b21d42 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/NamedPlaceholdersShouldBeUnique.cs +++ b/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} diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/UsePascalCaseForNamedPlaceHolders.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/UsePascalCaseForNamedPlaceHolders.cs index df988b8a2c2..844af0844d5 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplates/UsePascalCaseForNamedPlaceHolders.cs +++ b/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) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplatesShouldBeCorrect.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplatesShouldBeCorrect.cs index 77d3808a508..e5b15fe0442 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/MessageTemplatesShouldBeCorrect.cs +++ b/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) { } } diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplates.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplatesParser.cs similarity index 98% rename from analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplates.cs rename to analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplatesParser.cs index d6bc8420ee1..ddf329b6904 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/MessageTemplates.cs +++ b/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})"; diff --git a/analyzers/tests/SonarAnalyzer.Test/Helpers/MessageTemplatesTest.cs b/analyzers/tests/SonarAnalyzer.Test/Helpers/MessageTemplatesTest.cs index 7c15ec8bff5..3e4a2ccbad9 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Helpers/MessageTemplatesTest.cs +++ b/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);