Added analyser for invalid template parameter names

Author: Sviatoslav Bychkov

src/AutoLoggerMessageGenerator.UnitTests/Extractors/LogCallExtractorTests.Extract_WithLogMethodInvocationCode_ShouldTransformThemIntoLogCallObject.verified.txt

@@ -34,6 +34,7 @@
     {
       NativeType: global::System.String,
       Name: @message,
+      Type: Message,
       HasPropertiesToLog: false
     },
     {

src/AutoLoggerMessageGenerator.UnitTests/Extractors/LogMessageExtractorTests.cs renamed to src/AutoLoggerMessageGenerator.UnitTests/Extractors/LogCallMessageExtractorTests.cs

@@ -3,7 +3,7 @@
 
 namespace AutoLoggerMessageGenerator.UnitTests.Extractors;
 
-public class LogMessageExtractorTests : BaseSourceGeneratorTest
+public class LogCallMessageExtractorTests : BaseSourceGeneratorTest
 {
     [Theory]
     [InlineData("Hello world {1}, {2}!")]
@@ -13,7 +13,7 @@ public void Extract_WithPassingLiteralValues_ShouldReturnExpectedMessage(string
         var (compilation, syntaxTree) = CompileSourceCode($"""{LoggerName}.LogInformation("{expectedMessage}");""");
         var (invocationExpression, methodSymbol, semanticModel) = FindLoggerMethodInvocation(compilation, syntaxTree);
 
-        var result = LogMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
+        var result = LogCallMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
 
         result.Should().Be(expectedMessage);
     }
@@ -28,7 +28,7 @@ public void Extract_WithPassingConstClassMembers_ShouldReturnExpectedMessage()
         );
         var (invocationExpression, methodSymbol, semanticModel) = FindLoggerMethodInvocation(compilation, syntaxTree);
 
-        var result = LogMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
+        var result = LogCallMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
 
         result.Should().Be(expectedMessage);
     }
@@ -44,7 +44,7 @@ public void Extract_WithPassingLocalConstVariable_ShouldReturnExpectedMessage()
         );
         var (invocationExpression, methodSymbol, semanticModel) = FindLoggerMethodInvocation(compilation, syntaxTree);
 
-        var result = LogMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
+        var result = LogCallMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
 
         result.Should().Be(expectedMessage);
     }
@@ -55,7 +55,7 @@ public void Extract_WithPassingNullValues_ShouldReturnNull()
         var (compilation, syntaxTree) = CompileSourceCode($"{LoggerName}.LogInformation(null);");
         var (invocationExpression, methodSymbol, semanticModel) = FindLoggerMethodInvocation(compilation, syntaxTree);
 
-        var result = LogMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
+        var result = LogCallMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
 
         result.Should().Be(null);
     }
@@ -74,7 +74,7 @@ public void Extract_WithBinaryExpressions_ShouldReturnConcatenatedString()
         var (compilation, syntaxTree) = CompileSourceCode(sourceCode);
         var (invocationExpression, methodSymbol, semanticModel) = FindLoggerMethodInvocation(compilation, syntaxTree);
 
-        var result = LogMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
+        var result = LogCallMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
 
         result.Should().Be("Hello world! 42");
     }
@@ -93,7 +93,7 @@ public void Extract_WithBinaryNonConstantExpressions_ShouldReturnNull()
         var (compilation, syntaxTree) = CompileSourceCode(sourceCode);
         var (invocationExpression, methodSymbol, semanticModel) = FindLoggerMethodInvocation(compilation, syntaxTree);
 
-        var result = LogMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
+        var result = LogCallMessageExtractor.Extract(methodSymbol!, invocationExpression, semanticModel!);
 
         result.Should().Be(null);
     }

src/AutoLoggerMessageGenerator.UnitTests/Extractors/LogCallMessageParameterNamesExtractorTests.cs

@@ -0,0 +1,25 @@
+using AutoLoggerMessageGenerator.Extractors;
+using FluentAssertions;
+
+namespace AutoLoggerMessageGenerator.UnitTests.Extractors;
+
+public class LogCallMessageParameterNamesExtractorTests
+{
+    [Theory]
+    [InlineData("Hello world {1}, {2}!", new[] { "1", "2" })]
+    [InlineData("{param1} and {param2} are here", new[] { "param1", "param2" })]
+    [InlineData("No parameters in this string", new string[0])]
+    [InlineData("Edge case with empty braces {}", new[] { "" })]
+    [InlineData("{0}{1}{2}{3}", new[] { "0", "1", "2", "3" })]
+    [InlineData("{a} mixed with text {b}", new[] { "a", "b" })]
+    [InlineData("double {{escaped braces}}", new[] {"{escaped braces"})]
+    [InlineData("{name1}-{name2}-{name3}", new[] { "name1", "name2", "name3" })]
+    [InlineData("Duplicate {param} and {param} again", new[] { "param", "param" })]
+    [InlineData("", new string[0])]
+    [InlineData(null, new string[0])]
+    public void Extract_WithGivenMessage_ShouldReturnGivenMessageParameterNames(string message, params string[] expectedMessageParameters)
+    {
+        var result = LogCallMessageParameterNamesExtractor.Extract(message);
+        result.Should().BeEquivalentTo(expectedMessageParameters);
+    }
+}

src/AutoLoggerMessageGenerator.UnitTests/Extractors/LogCallParametersExtractorTests.cs

@@ -72,7 +72,7 @@ public void Extract_WithGivenMessageAndNoParameters_ShouldReturnOnlyMessageParam
     [Fact]
     public void Extract_WithUtilityParameters_ShouldExtractAllParameters()
     {
-        const string message = "The {message} was processed in {Time}ms";
+        const string message = "The {EventName} was processed in {Time}ms";
 
         string extensionDeclaration = $$"""
                                         private static void Log<T1, T2>(

src/AutoLoggerMessageGenerator/Analysers/InvalidTemplateParameterNameAnalyser.cs

@@ -0,0 +1,67 @@
+using System.Collections.Immutable;
+using System.Linq;
+using AutoLoggerMessageGenerator.Extractors;
+using AutoLoggerMessageGenerator.Filters;
+using AutoLoggerMessageGenerator.Mappers;
+using AutoLoggerMessageGenerator.Utilities;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace AutoLoggerMessageGenerator.Analysers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class InvalidTemplateParameterNameAnalyser : DiagnosticAnalyzer
+{
+    private static readonly DiagnosticDescriptor Rule = new(
+        id: "ALM001",
+        title: "Invalid template parameter name",
+        messageFormat: $"Template parameters ({{0}}) have invalid names ({IdentifierHelper.ValidCSharpParameterNameRegex}).",
+        category: "Naming",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: $"Template parameters in log messages must have valid names"
+    );
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze |
+                                               GeneratedCodeAnalysisFlags.ReportDiagnostics);
+
+        context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.InvocationExpression);
+    }
+
+    private void AnalyzeNode(SyntaxNodeAnalysisContext context)
+    {
+        var invocationExpression = (InvocationExpressionSyntax)context.Node;
+
+        if (!LogCallFilter.IsLogCallInvocation(invocationExpression, context.CancellationToken))
+            return;
+
+        var semanticModel = context.SemanticModel;
+        var methodSymbol = semanticModel.GetSymbolInfo(invocationExpression).Symbol as IMethodSymbol;
+        if (methodSymbol is null || !LogCallFilter.IsLoggerMethod(methodSymbol))
+            return;
+
+        var message = LogCallMessageExtractor.Extract(methodSymbol, invocationExpression, semanticModel);
+        if (message is null)
+            return;
+
+        var templateParameterNames = LogCallMessageParameterNamesExtractor.Extract(message)
+            .Where(parameterName => !IdentifierHelper.IsValidCSharpParameterName(parameterName))
+            .ToImmutableArray();
+
+        if (!templateParameterNames.Any())
+            return;
+
+        var location = LogCallLocationMapper.Map(invocationExpression);
+        if (location is null)
+            return;
+
+        context.ReportDiagnostic(Diagnostic.Create(Rule, location.Value.Context, string.Join(", ", templateParameterNames)));
+    }
+}

src/AutoLoggerMessageGenerator/Extractors/LogCallExtractor.cs

@@ -22,7 +22,7 @@ internal static class LogCallExtractor
         if (logLevel is null)
             return default;
 
-        var message = LogMessageExtractor.Extract(methodSymbol, invocationExpression, semanticModel);
+        var message = LogCallMessageExtractor.Extract(methodSymbol, invocationExpression, semanticModel);
         if (message is null)
             return default;
 

src/AutoLoggerMessageGenerator/Extractors/LogMessageExtractor.cs renamed to src/AutoLoggerMessageGenerator/Extractors/LogCallMessageExtractor.cs

@@ -5,7 +5,7 @@
 
 namespace AutoLoggerMessageGenerator.Extractors;
 
-internal static class LogMessageExtractor
+internal static class LogCallMessageExtractor
 {
     public static string? Extract(IMethodSymbol methodSymbol, InvocationExpressionSyntax invocationExpressionSyntax,
         SemanticModel semanticModel)

src/AutoLoggerMessageGenerator/Extractors/LogCallMessageParameterNamesExtractor.cs

@@ -0,0 +1,15 @@
+using System.Collections.Immutable;
+using System.Linq;
+using System.Text.RegularExpressions;
+
+namespace AutoLoggerMessageGenerator.Extractors;
+
+internal static partial class LogCallMessageParameterNamesExtractor
+{
+    public static ImmutableArray<string> Extract(string? message) =>
+        [..MessageArgumentRegex().Matches(message ?? string.Empty).Select(c => c.Groups[1].Value)];
+
+    [GeneratedRegex(@"\{(.*?)\}", RegexOptions.Compiled)]
+    private static partial Regex MessageArgumentRegex();
+}
+

src/AutoLoggerMessageGenerator/Extractors/LogCallParametersExtractor.cs

@@ -1,6 +1,5 @@
 using System.Collections.Immutable;
 using System.Linq;
-using System.Text.RegularExpressions;
 using AutoLoggerMessageGenerator.Import.Microsoft.Extensions.Telemetry.LoggerMessage;
 using AutoLoggerMessageGenerator.Models;
 using AutoLoggerMessageGenerator.Utilities;
@@ -10,16 +9,14 @@
 
 namespace AutoLoggerMessageGenerator.Extractors;
 
-internal partial class LogCallParametersExtractor(LogPropertiesCheck? logPropertiesCheck = null)
+internal class LogCallParametersExtractor(LogPropertiesCheck? logPropertiesCheck = null)
 {
     public ImmutableArray<LogCallParameter>? Extract(string message, IMethodSymbol methodSymbol)
     {
-        var matches = MessageArgumentRegex().Matches(message);
-        var templateParametersNames = matches.Select(c => c.Groups[1].Value)
-            .Select(TransformParameterName)
+        var templateParametersNames = LogCallMessageParameterNamesExtractor.Extract(message)
+            .Select(IdentifierHelper.AddAtPrefixIfNotExists)
             .ToArray();
 
-        // TODO: Report a diagnostic message in the separate analyzer that some parameter names are "invalid"
         if (!templateParametersNames.All(IdentifierHelper.IsValidCSharpParameterName))
             return null;
 
@@ -74,7 +71,4 @@ private static LogCallParameter CreateLogCallParameter(ITypeSymbol @nativeType,
 
     private static string TransformParameterName(string parameterName) =>
         parameterName.StartsWith('@') ? parameterName : '@' + parameterName;
-
-    [GeneratedRegex(@"\{(.*?)\}", RegexOptions.Compiled)]
-    private static partial Regex MessageArgumentRegex();
 }

src/AutoLoggerMessageGenerator/Utilities/IdentifierHelper.cs

@@ -17,9 +17,14 @@ public static string ToValidCSharpMethodName(string? input)
         return sanitizedInput;
     }
 
+    public static string AddAtPrefixIfNotExists(string parameterName) =>
+        parameterName.StartsWith('@') ? parameterName : '@' + parameterName;
+
     public static bool IsValidCSharpParameterName(string name) =>
         !string.IsNullOrEmpty(name) && IsValidCSharpParameterNameRegex().IsMatch(name);
 
-    [GeneratedRegex(@"^@?[a-zA-Z_][a-zA-Z0-9_]*$")]
+    public const string ValidCSharpParameterNameRegex = @"^@?[a-zA-Z_][a-zA-Z0-9_]*$";
+
+    [GeneratedRegex(ValidCSharpParameterNameRegex)]
     private static partial Regex IsValidCSharpParameterNameRegex();
 }