dotnet · geeknoid · May 30, 2023 · May 30, 2023
diff --git a/...osoft.Gen.EnumStrings/Common/Generator.cs → ...numStrings/Common/EnumStringsGenerator.cs b/...osoft.Gen.EnumStrings/Common/Generator.cs → ...numStrings/Common/EnumStringsGenerator.cs
@@ -14,7 +14,7 @@ namespace Microsoft.Gen.EnumStrings;
 
 [Generator]
 [ExcludeFromCodeCoverage]
-public class Generator : IIncrementalGenerator
+public class EnumStringsGenerator : IIncrementalGenerator
 {
     private static readonly HashSet<string> _attributeNames = new()
     {
@@ -60,7 +60,7 @@ namespace Microsoft.Gen.EnumStrings;
 
 [Generator]
 [ExcludeFromCodeCoverage]
-public class Generator : ISourceGenerator
+public class EnumStringsGenerator : ISourceGenerator
 {
     public void Initialize(GeneratorInitializationContext context)
     {

diff --git a/src/Generators/Microsoft.Gen.EnumStrings/Common/SymbolHolder.cs b/src/Generators/Microsoft.Gen.EnumStrings/Common/SymbolHolder.cs
@@ -5,9 +5,6 @@
 
 namespace Microsoft.Gen.EnumStrings;
 
-/// <summary>
-/// Holds required symbols for the <see cref="Generator"/>.
-/// </summary>
 internal sealed record class SymbolHolder(
     INamedTypeSymbol FlagsAttributeSymbol,
     INamedTypeSymbol EnumStringsAttributeSymbol,

diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Emission/Emitter.Method.cs b/src/Generators/Microsoft.Gen.Logging/Common/Emission/Emitter.Method.cs
@@ -3,8 +3,8 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
+using System.Text;
 using Microsoft.Gen.Logging.Model;
 using Microsoft.Gen.Shared;
 
@@ -14,18 +14,17 @@ namespace Microsoft.Gen.Logging.Emission;
 
 internal sealed partial class Emitter : EmitterBase
 {
-    [SuppressMessage("Major Code Smell", "S103:Lines should not be too long", Justification = "Long strings are easier to read in this function")]
+    private const string NullExceptionArgComment = " // Refer to our docs to learn how to pass exception here";
+
     private void GenLogMethod(LoggingMethod lm)
     {
         var logPropsDataClasses = GetLogPropertiesAttributes(lm);
         string level = GetLoggerMethodLogLevel(lm);
         string extension = lm.IsExtensionMethod ? "this " : string.Empty;
         string eventName = string.IsNullOrWhiteSpace(lm.EventName) ? $"nameof({lm.Name})" : $"\"{lm.EventName}\"";
-        string exceptionArg = GetException(lm);
+        (string exceptionArg, string exceptionArgComment) = GetException(lm);
         (string logger, bool isNullableLogger) = GetLogger(lm);
 
-        OutLn();
-
         if (!lm.HasXmlDocumentation)
         {
             var l = GetLoggerMethodLogLevelForXmlDocumentation(lm);
@@ -39,7 +38,7 @@ private void GenLogMethod(LoggingMethod lm)
 
             if (!string.IsNullOrEmpty(lm.Message))
             {
-                OutLn($"/// Logs \"{EscapeMessageStringForXmlDocumentation(lm.Message)}\" {lvl}.");
+                OutLn($"/// Logs \"{EscapeMessageStringForXmlDocumentation(lm.Message)}\"{lvl}.");
             }
             else
             {
@@ -76,11 +75,15 @@ private void GenLogMethod(LoggingMethod lm)
             OutLn();
         }
 
-        var parametersToRedact = lm.AllParameters.Where(lp => lp.ClassificationAttributeType != null).ToArray();
-
-        if (parametersToRedact.Length > 0 || logPropsDataClasses.Count > 0)
+        var redactorProviderAccess = string.Empty;
+        var parametersToRedact = lm.AllParameters.Where(lp => lp.ClassificationAttributeType != null);
+        if (parametersToRedact.Any() || logPropsDataClasses.Count > 0)
         {
             (string redactorProvider, bool isNullableRedactorProvider) = GetRedactorProvider(lm);
+            if (isNullableRedactorProvider)
+            {
+                redactorProviderAccess = "?";
+            }
 
             var classifications = parametersToRedact
                 .Select(static p => p.ClassificationAttributeType)
@@ -101,15 +104,15 @@ private void GenLogMethod(LoggingMethod lm)
         {
             if (!p.HasPropsProvider && !p.HasProperties && p.IsNormalParameter)
             {
-                GenHelperAdd(lm, holderMap, p);
+                GenHelperAdd(lm, holderMap, p, redactorProviderAccess);
             }
         }
 
         foreach (var p in lm.TemplateParameters)
         {
             if (!holderMap.ContainsKey(p))
             {
-                GenHelperAdd(lm, holderMap, p);
+                GenHelperAdd(lm, holderMap, p, redactorProviderAccess);
             }
         }
 
@@ -137,7 +140,6 @@ private void GenLogMethod(LoggingMethod lm)
             {
                 OutLn($"_helper.ParameterName = string.Empty;");
 
-#pragma warning disable S1067 // Expressions should not be too complex
                 p.TraverseParameterPropertiesTransitively((propertyChain, member) =>
                 {
                     var propName = PropertyChainToString(propertyChain, member, "_", omitParameterName: p.OmitParameterName);
@@ -149,7 +151,8 @@ private void GenLogMethod(LoggingMethod lm)
 
                     if (member.ClassificationAttributeType != null)
                     {
-                        var value = $"_{EncodeTypeName(member.ClassificationAttributeType)}Redactor?.Redact(global::System.MemoryExtensions.AsSpan({ConvertPropertyToString(member, accessExpression)}))";
+                        var stringifiedProperty = ConvertPropertyToString(member, accessExpression);
+                        var value = $"_{EncodeTypeName(member.ClassificationAttributeType)}Redactor{redactorProviderAccess}.Redact(global::System.MemoryExtensions.AsSpan({stringifiedProperty}))";
 
                         if (member.PotentiallyNull)
                         {
@@ -169,7 +172,9 @@ private void GenLogMethod(LoggingMethod lm)
                     }
                     else
                     {
-                        var ts = ShouldStringify(member.Type) ? ConvertPropertyToString(member, accessExpression) : accessExpression;
+                        var ts = ShouldStringify(member.Type)
+                            ? ConvertPropertyToString(member, accessExpression)
+                            : accessExpression;
 
                         var value = member.IsEnumerable
                             ? $"global::Microsoft.Extensions.Telemetry.Logging.LogMethodHelper.Stringify({accessExpression})"
@@ -178,7 +183,6 @@ private void GenLogMethod(LoggingMethod lm)
                         OutLn($"{skipNull}_helper.Add(\"{propName}\", {value});");
                     }
                 });
-#pragma warning restore S1067 // Expressions should not be too complex
             }
         }
 
@@ -199,7 +203,7 @@ private void GenLogMethod(LoggingMethod lm)
         }
 
         OutLn($"_helper,");
-        OutLn($"{exceptionArg},");
+        OutLn($"{exceptionArg},{exceptionArgComment}");
         OutLn($"__FUNC_{_memberCounter}_{lm.Name});");
         Unindent();
 
@@ -215,7 +219,9 @@ private void GenLogMethod(LoggingMethod lm)
 
         if (GenVariableAssignments(lm, holderMap))
         {
-            OutLn($@"return global::System.FormattableString.Invariant($""{EscapeMessageString(lm.Message)}"");");
+            var template = EscapeMessageString(lm.Message);
+            template = AddAtSymbolsToTemplates(template, lm.TemplateParameters);
+            OutLn($@"return global::System.FormattableString.Invariant($""{template}"");");
         }
         else if (string.IsNullOrEmpty(lm.Message))
         {
@@ -275,19 +281,21 @@ static string ConvertPropertyToString(LoggingProperty lp, string arg)
             return $"{arg}{question}.ToString()";
         }
 
-        static string GetException(LoggingMethod lm)
+        static (string exceptionArg, string exceptionArgComment) GetException(LoggingMethod lm)
         {
             string exceptionArg = "null";
+            string exceptionArgComment = NullExceptionArgComment;
             foreach (var p in lm.AllParameters)
             {
                 if (p.IsException)
                 {
                     exceptionArg = p.Name;
+                    exceptionArgComment = string.Empty;
                     break;
                 }
             }
 
-            return exceptionArg;
+            return (exceptionArg, exceptionArgComment);
         }
 
         bool GenVariableAssignments(LoggingMethod lm, Dictionary<LoggingMethodParameter, int> holderMap)
@@ -365,7 +373,7 @@ bool GenVariableAssignments(LoggingMethod lm, Dictionary<LoggingMethodParameter,
             return (redactorProvider, isNullable);
         }
 
-        void GenHelperAdd(LoggingMethod lm, Dictionary<LoggingMethodParameter, int> holderMap, LoggingMethodParameter p)
+        void GenHelperAdd(LoggingMethod lm, Dictionary<LoggingMethodParameter, int> holderMap, LoggingMethodParameter p, string redactorProviderAccess)
         {
             string key = $"\"{lm.GetParameterNameInTemplate(p)}\"";
 
@@ -375,7 +383,7 @@ void GenHelperAdd(LoggingMethod lm, Dictionary<LoggingMethodParameter, int> hold
 
                 OutOpenBrace();
                 OutLn($"var _v = {ConvertToString(p, p.NameWithAt)};");
-                var value = $"_v != null ? _{dataClassVariableName}Redactor?.Redact(global::System.MemoryExtensions.AsSpan(_v)) : null";
+                var value = $"_v != null ? _{dataClassVariableName}Redactor{redactorProviderAccess}.Redact(global::System.MemoryExtensions.AsSpan(_v)) : null";
                 OutLn($"_helper.Add({key}, {value});");
                 OutCloseBrace();
             }
@@ -403,6 +411,37 @@ void GenHelperAdd(LoggingMethod lm, Dictionary<LoggingMethodParameter, int> hold
         }
     }
 
+    private string AddAtSymbolsToTemplates(string template, List<LoggingMethodParameter> templateParameters)
+    {
+        StringBuilder? stringBuilder = null;
+        foreach (var item in templateParameters)
+        {
+            if (!item.NeedsAtSign)
+            {
+                continue;
+            }
+
+            if (stringBuilder is null)
+            {
+                stringBuilder = _sbPool.GetStringBuilder();
+                _ = stringBuilder.Append(template);
+            }
+
+            _ = stringBuilder.Replace(item.Name, item.NameWithAt);
+        }
+
+        var result = stringBuilder is null
+            ? template
+            : stringBuilder.ToString();
+
+        if (stringBuilder != null)
+        {
+            _sbPool.ReturnStringBuilder(stringBuilder);
+        }
+
+        return result;
+    }
+
     private void GenParameters(LoggingMethod lm)
     {
         OutEnumeration(lm.AllParameters.Select(static p =>

diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Emission/Emitter.Utils.cs b/src/Generators/Microsoft.Gen.Logging/Common/Emission/Emitter.Utils.cs
@@ -24,7 +24,7 @@ internal static string EscapeMessageString(string s)
             return s;
         }
 
-        var sb = new StringBuilder(s.Length);
+        var sb = new StringBuilder(s.Length + 1); // we use +1 because we will add at least one symbol
         _ = sb.Append(s, 0, index);
 
         while (index < s.Length)
@@ -54,7 +54,7 @@ internal static string EscapeMessageStringForXmlDocumentation(string s)
             return s;
         }
 
-        var sb = new StringBuilder(s.Length);
+        var sb = new StringBuilder(s.Length + 1); // we use +1 because we will add at least one symbol
         _ = sb.Append(s, 0, index);
 
         while (index < s.Length)

diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Emission/Emitter.cs b/src/Generators/Microsoft.Gen.Logging/Common/Emission/Emitter.cs
@@ -92,8 +92,18 @@ private void GenType(LoggingType lt)
             GenAttributeClassifications(lt);
         }
 
-        foreach (LoggingMethod lm in lt.Methods.OrderBy(static x => x.Name))
+        var first = true;
+        foreach (LoggingMethod lm in lt.Methods)
         {
+            if (first)
+            {
+                first = false;
+            }
+            else
+            {
+                OutLn();
+            }
+
             GenLogMethod(lm);
             _memberCounter++;
         }
@@ -152,11 +162,21 @@ private void GenRedactorProperties(LoggingType lt)
             .Distinct()
             .Single();
 
+        var first = true;
         foreach (var classificationAttributeType in classificationAttributeTypes.OrderBy(static x => x))
         {
             var classificationVariableName = EncodeTypeName(classificationAttributeType);
             var attrClassificationFieldName = GetAttributeClassification(classificationAttributeType);
 
+            if (first)
+            {
+                first = false;
+            }
+            else
+            {
+                OutLn();
+            }
+
             OutGeneratedCodeAttribute();
             OutLn($"private {RedactorType}? ___{classificationVariableName}Redactor;");
             OutLn();

diff --git a/...Microsoft.Gen.Logging/Common/Generator.cs → ...ft.Gen.Logging/Common/LoggingGenerator.cs b/...Microsoft.Gen.Logging/Common/Generator.cs → ...ft.Gen.Logging/Common/LoggingGenerator.cs
@@ -16,7 +16,7 @@ namespace Microsoft.Gen.Logging;
 
 [Generator]
 [ExcludeFromCodeCoverage]
-public class Generator : IIncrementalGenerator
+public class LoggingGenerator : IIncrementalGenerator
 {
     private static readonly HashSet<string> _attributeNames = new()
     {
@@ -55,7 +55,7 @@ namespace Microsoft.Gen.Logging;
 
 [Generator]
 [ExcludeFromCodeCoverage]
-public class Generator : ISourceGenerator
+public class LoggingGenerator : ISourceGenerator
 {
     public void Initialize(GeneratorInitializationContext context)
     {

diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Model/LoggingMethod.cs b/src/Generators/Microsoft.Gen.Logging/Common/Model/LoggingMethod.cs
@@ -14,7 +14,6 @@ internal sealed class LoggingMethod
     public readonly List<LoggingMethodParameter> AllParameters = new();
     public readonly List<LoggingMethodParameter> TemplateParameters = new();
     public readonly Dictionary<string, string> TemplateMap = new(StringComparer.OrdinalIgnoreCase);
-    public readonly List<string> TemplateList = new();
     public string Name = string.Empty;
     public string Message = string.Empty;
     public int? Level;

diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Model/LoggingMethodParameter.cs b/src/Generators/Microsoft.Gen.Logging/Common/Model/LoggingMethodParameter.cs
@@ -33,7 +33,11 @@ internal sealed class LoggingMethodParameter
     public LoggingPropertyProvider? LogPropertiesProvider;
 
     public string NameWithAt => NeedsAtSign ? "@" + Name : Name;
-    public string PotentiallyNullableType => (IsReference && !IsNullable) ? Type + "?" : Type;
+
+    public string PotentiallyNullableType
+        => (IsReference && !IsNullable)
+            ? Type + "?"
+            : Type;
 
     // A parameter flagged as 'normal' is not going to be taken care of specially as an argument to ILogger.Log
     // but instead is supposed to be taken as a normal parameter.

diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Parsing/DiagDescriptors.cs b/src/Generators/Microsoft.Gen.Logging/Common/Parsing/DiagDescriptors.cs
@@ -257,4 +257,10 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         title: Resources.EmptyLoggingMethodTitle,
         messageFormat: Resources.EmptyLoggingMethodMessage,
         category: Category);
+
+    public static DiagnosticDescriptor TemplateStartsWithAtSymbol { get; } = Make(
+        id: "R9G045",
+        title: Resources.TemplateStartsWithAtSymbolTitle,
+        messageFormat: Resources.TemplateStartsWithAtSymbolMessage,
+        category: Category);
 }
diff --git a/src/Generators/Microsoft.Gen.Logging/Common/Parsing/LogParserUtilities.cs b/src/Generators/Microsoft.Gen.Logging/Common/Parsing/LogParserUtilities.cs
@@ -216,6 +216,17 @@ internal static IReadOnlyList<INamedTypeSymbol> GetDataClassificationAttributes(
             .Select(static x => x!)
             .ToList();
 
+    internal static string GetPropertyIdentifier(IPropertySymbol property, CancellationToken token)
+    {
+        var syntax = property.DeclaringSyntaxReferences[0].GetSyntax(token);
+        return syntax switch
+        {
+            PropertyDeclarationSyntax propertySyntax => propertySyntax.Identifier.Text, // a regular property
+            ParameterSyntax paramSyntax => paramSyntax.Identifier.Text, // a property of a "record"
+            _ => property.Name
+        };
+    }
+
     private static bool IsSpecialType(ITypeSymbol typeSymbol, SymbolHolder symbols)
         => typeSymbol.SpecialType != SpecialType.None ||
         typeSymbol.OriginalDefinition.SpecialType != SpecialType.None ||
@@ -355,10 +366,11 @@ private static List<LoggingProperty> GetTypePropertiesToLog(
                     var needsAtSign = false;
                     if (!property.DeclaringSyntaxReferences.IsDefaultOrEmpty)
                     {
-                        var propertySyntax = property.DeclaringSyntaxReferences[0].GetSyntax(token) as PropertyDeclarationSyntax;
-                        if (!string.IsNullOrEmpty(propertySyntax!.Identifier.Text))
+                        var propertyIdentifier = GetPropertyIdentifier(property, token);
+
+                        if (!string.IsNullOrEmpty(propertyIdentifier))
                         {
-                            needsAtSign = propertySyntax!.Identifier.Text[0] == '@';
+                            needsAtSign = propertyIdentifier[0] == '@';
                         }
                     }