diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs index 33567379bf9..cfd8930233e 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs @@ -104,9 +104,19 @@ private void GenLogMethod(LoggingMethod lm) if (GenVariableAssignments(lm, lambdaStateName, numReservedUnclassifiedTags, numReservedClassifiedTags)) { - var template = EscapeMessageString(lm.Message); - template = AddAtSymbolsToTemplates(template, lm.Parameters); - OutLn($@"return global::System.FormattableString.Invariant(${template});"); + var mapped = Parsing.TemplateProcessor.MapTemplates(lm.Message, t => + { + var p = lm.GetParameterForTemplate(t); + if (p != null) + { + return p.ParameterNameWithAtIfNeeded; + } + + return t; + }); + + var s = EscapeMessageString(mapped!); + OutLn($@"return global::System.FormattableString.Invariant(${s});"); } else if (string.IsNullOrEmpty(lm.Message)) { @@ -294,14 +304,14 @@ void GenTagWrites(LoggingMethod lm, string stateName, out int numReservedUnclass if (p.IsEnumerable) { value = p.PotentiallyNull - ? $"{p.ParameterNameWithAt} != null ? {LoggerMessageHelperType}.Stringify({p.ParameterNameWithAt}) : null" - : $"{LoggerMessageHelperType}.Stringify({p.ParameterNameWithAt})"; + ? $"{p.ParameterNameWithAtIfNeeded} != null ? {LoggerMessageHelperType}.Stringify({p.ParameterNameWithAtIfNeeded}) : null" + : $"{LoggerMessageHelperType}.Stringify({p.ParameterNameWithAtIfNeeded})"; } else { value = ShouldStringifyParameter(p) - ? ConvertParameterToString(p, p.ParameterNameWithAt) - : p.ParameterNameWithAt; + ? ConvertParameterToString(p, p.ParameterNameWithAtIfNeeded) + : p.ParameterNameWithAtIfNeeded; } OutLn($"{stateName}.TagArray[{--count}] = new({key}, {value});"); @@ -352,8 +362,8 @@ void GenTagWrites(LoggingMethod lm, string stateName, out int numReservedUnclass var classification = MakeClassificationValue(p.ClassificationAttributeTypes); var value = ShouldStringifyParameter(p) - ? ConvertParameterToString(p, p.ParameterNameWithAt) - : p.ParameterNameWithAt; + ? ConvertParameterToString(p, p.ParameterNameWithAtIfNeeded) + : p.ParameterNameWithAtIfNeeded; OutLn($"{stateName}.ClassifiedTagArray[{--count}] = new({key}, {value}, {classification});"); } @@ -469,10 +479,10 @@ void GenTagWrites(LoggingMethod lm, string stateName, out int numReservedUnclass } else { - OutLn($"{stateName}.TagNamePrefix = nameof({p.ParameterNameWithAt});"); + OutLn($"{stateName}.TagNamePrefix = nameof({p.ParameterNameWithAtIfNeeded});"); } - OutLn($"{p.TagProvider!.ContainingType}.{p.TagProvider.MethodName}({stateName}, {p.ParameterNameWithAt});"); + OutLn($"{p.TagProvider!.ContainingType}.{p.TagProvider.MethodName}({stateName}, {p.ParameterNameWithAtIfNeeded});"); } } } @@ -495,11 +505,11 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes if (p.PotentiallyNull) { const string Null = "\"(null)\""; - OutLn($"var {atSign}{t} = {lambdaStateName}.TagArray[{index}].Value ?? {Null};"); + OutLn($"var {atSign}{p.ParameterName} = {lambdaStateName}.TagArray[{index}].Value ?? {Null};"); } else { - OutLn($"var {atSign}{t} = {lambdaStateName}.TagArray[{index}].Value;"); + OutLn($"var {atSign}{p.ParameterName} = {lambdaStateName}.TagArray[{index}].Value;"); } generatedAssignments = true; @@ -524,11 +534,11 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes if (p.PotentiallyNull) { const string Null = "\"(null)\""; - OutLn($"var {atSign}{t} = {lambdaStateName}.RedactedTagArray[{index}].Value ?? {Null};"); + OutLn($"var {atSign}{p.ParameterName} = {lambdaStateName}.RedactedTagArray[{index}].Value ?? {Null};"); } else { - OutLn($"var {atSign}{t} = {lambdaStateName}.RedactedTagArray[{index}].Value;"); + OutLn($"var {atSign}{p.ParameterName} = {lambdaStateName}.RedactedTagArray[{index}].Value;"); } generatedAssignments = true; @@ -585,47 +595,16 @@ private string MakeClassificationValue(HashSet classificationTypes) return sb.ToString(); } - private string AddAtSymbolsToTemplates(string template, IEnumerable parameters) - { - StringBuilder? stringBuilder = null; - foreach (var item in parameters.Where(p => p.UsedAsTemplate)) - { - if (!item.NeedsAtSign) - { - continue; - } - - if (stringBuilder is null) - { - stringBuilder = _sbPool.GetStringBuilder(); - _ = stringBuilder.Append(template); - } - - _ = stringBuilder.Replace(item.ParameterName, item.ParameterNameWithAt); - } - - var result = stringBuilder is null - ? template - : stringBuilder.ToString(); - - if (stringBuilder != null) - { - _sbPool.ReturnStringBuilder(stringBuilder); - } - - return result; - } - private void GenParameters(LoggingMethod lm) { OutEnumeration(lm.Parameters.Select(static p => { if (p.Qualifier != null) { - return $"{p.Qualifier} {p.Type} {p.ParameterNameWithAt}"; + return $"{p.Qualifier} {p.Type} {p.ParameterNameWithAtIfNeeded}"; } - return $"{p.Type} {p.ParameterNameWithAt}"; + return $"{p.Type} {p.ParameterNameWithAtIfNeeded}"; })); } diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs index 1c200d064a5..f22f1524f50 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs @@ -33,7 +33,7 @@ internal sealed class LoggingMethod { foreach (var p in Parameters) { - if (templateName.Equals(p.ParameterName, StringComparison.OrdinalIgnoreCase)) + if (templateName.Equals(p.TagName, StringComparison.OrdinalIgnoreCase)) { return p; } @@ -43,14 +43,11 @@ internal sealed class LoggingMethod } public List GetTemplatesForParameter(LoggingMethodParameter lp) - => GetTemplatesForParameter(lp.ParameterName); - - public List GetTemplatesForParameter(string parameterName) { HashSet templates = []; foreach (var t in Templates) { - if (parameterName.Equals(t, StringComparison.OrdinalIgnoreCase)) + if (lp.TagName.Equals(t, StringComparison.OrdinalIgnoreCase)) { _ = templates.Add(t); } @@ -58,4 +55,17 @@ public List GetTemplatesForParameter(string parameterName) return templates.ToList(); } + + public List GetTemplatesForParameter(string parameterName) + { + foreach (var p in Parameters) + { + if (parameterName == p.ParameterName) + { + return GetTemplatesForParameter(p); + } + } + + return []; + } } diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs index 80021aba766..7f90823e9a4 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs @@ -10,7 +10,7 @@ namespace Microsoft.Gen.Logging.Model; /// /// A single parameter to a logger method. /// -[DebuggerDisplay("{Name}")] +[DebuggerDisplay("{ParameterName}")] internal sealed class LoggingMethodParameter { public string ParameterName = string.Empty; @@ -35,7 +35,7 @@ internal sealed class LoggingMethodParameter public List Properties = []; public TagProvider? TagProvider; - public string ParameterNameWithAt => NeedsAtSign ? "@" + ParameterName : ParameterName; + public string ParameterNameWithAtIfNeeded => NeedsAtSign ? "@" + ParameterName : ParameterName; public string PotentiallyNullableType => (IsReference && !IsNullable) diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs index 840e86abb83..a292249f612 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs @@ -152,7 +152,7 @@ public IReadOnlyList GetLogTypes(IEnumerable bool parameterInTemplate = false; foreach (var t in lm.Templates) { - if (lp.ParameterName.Equals(t, StringComparison.OrdinalIgnoreCase)) + if (lp.TagName.Equals(t, StringComparison.OrdinalIgnoreCase)) { parameterInTemplate = true; break; @@ -273,9 +273,10 @@ public IReadOnlyList GetLogTypes(IEnumerable bool found = false; foreach (var p in lm.Parameters) { - if (t.Equals(p.ParameterName, StringComparison.OrdinalIgnoreCase)) + if (t.Equals(p.TagName, StringComparison.OrdinalIgnoreCase)) { found = true; + p.TagName = t; break; } } @@ -398,7 +399,7 @@ static bool IsAllowedKind(SyntaxKind kind) => keepMethod = false; } - TemplateExtractor.ExtractTemplates(message, lm.Templates); + TemplateProcessor.ExtractTemplates(message, lm.Templates); #pragma warning disable EA0003 // Use the character-based overloads of 'String.StartsWith' or 'String.EndsWith' var templatesWithAtSymbol = lm.Templates.Where(x => x.StartsWith("@", StringComparison.Ordinal)).ToArray(); @@ -522,12 +523,9 @@ private void CheckTagNamesAreUnique(LoggingMethod lm, Dictionary templates) if (closeBraceIndex == endIndex) { - scanIndex = endIndex; + return; } - else - { - // Format item syntax : { index[,alignment][ :formatString] }. - var formatDelimiterIndex = FindIndexOfAny(message, _formatDelimiters, openBraceIndex, closeBraceIndex); - var templateName = message.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1).Trim(); - templates.Add(templateName); - scanIndex = closeBraceIndex + 1; + // Format item syntax : { index[,alignment][ :formatString] }. + var formatDelimiterIndex = FindIndexOfAny(message, _formatDelimiters, openBraceIndex, closeBraceIndex); + + var templateName = message.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1).Trim(); + templates.Add(templateName); + scanIndex = closeBraceIndex + 1; + } + } + + /// + /// Allows replacing individual template arguments with different strings. + /// + internal static string? MapTemplates(string? message, Func mapTemplate) + { + if (string.IsNullOrEmpty(message)) + { + return message; + } + + var sb = new StringBuilder(); + + var scanIndex = 0; + var endIndex = message!.Length; + while (scanIndex < endIndex) + { + var openBraceIndex = FindBraceIndex(message, '{', scanIndex, endIndex); + var closeBraceIndex = FindBraceIndex(message, '}', openBraceIndex, endIndex); + + if (closeBraceIndex == endIndex) + { + break; } + + // Format item syntax : { index[,alignment][ :formatString] }. + var formatDelimiterIndex = FindIndexOfAny(message, _formatDelimiters, openBraceIndex, closeBraceIndex); + + var templateName = message.Substring(openBraceIndex + 1, formatDelimiterIndex - openBraceIndex - 1).Trim(); + var mapped = mapTemplate(templateName); + + _ = sb.Append(message, scanIndex, openBraceIndex - scanIndex + 1); + _ = sb.Append(mapped); + _ = sb.Append(message, formatDelimiterIndex, closeBraceIndex - formatDelimiterIndex + 1); + + scanIndex = closeBraceIndex + 1; } + + _ = sb.Append(message, scanIndex, message.Length - scanIndex); + return sb.ToString(); } internal static int FindIndexOfAny(string message, char[] chars, int startIndex, int endIndex) diff --git a/test/Generators/Microsoft.Gen.Logging/Generated/LogMethodTests.cs b/test/Generators/Microsoft.Gen.Logging/Generated/LogMethodTests.cs index 2753fcc9a2b..b4b3e14229f 100644 --- a/test/Generators/Microsoft.Gen.Logging/Generated/LogMethodTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Generated/LogMethodTests.cs @@ -756,6 +756,11 @@ public void AtSymbolsTest() collector.Clear(); AtSymbolsTestExtensions.M5(logger, LogLevel.Debug, o); Assert.Equal("42", collector.LatestRecord.StructuredState!.GetValue("class")); + + collector.Clear(); + AtSymbolsTestExtensions.M6(logger, "42"); + Assert.Equal("42", collector.LatestRecord.StructuredState!.GetValue("class")); + Assert.Equal("M6 class 42", collector.LatestRecord.Message); } [Fact] diff --git a/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs b/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs index fd9c5f6cba2..59acbfae502 100644 --- a/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Generated/TagNameTests.cs @@ -17,12 +17,19 @@ public void Basic() var logger = new FakeLogger(); TagNameExtensions.M0(logger, 0); - - var expectedState = new Dictionary + logger.Collector.LatestRecord.StructuredState.Should().NotBeNull().And.Equal(new Dictionary { ["TN1"] = "0", - }; + }); + + logger.Collector.Clear(); + TagNameExtensions.M1(logger, 0); + logger.Collector.LatestRecord.StructuredState.Should().NotBeNull().And.Equal(new Dictionary + { + ["foo.bar"] = "0", + ["{OriginalFormat}"] = "{foo.bar}", + }); - logger.Collector.LatestRecord.StructuredState.Should().NotBeNull().And.Equal(expectedState); + Assert.Equal("0", logger.Collector.LatestRecord.Message); } } diff --git a/test/Generators/Microsoft.Gen.Logging/TestClasses/AtSymbolsTestExtensions.cs b/test/Generators/Microsoft.Gen.Logging/TestClasses/AtSymbolsTestExtensions.cs index fe3e1e19edb..c3779cae8b2 100644 --- a/test/Generators/Microsoft.Gen.Logging/TestClasses/AtSymbolsTestExtensions.cs +++ b/test/Generators/Microsoft.Gen.Logging/TestClasses/AtSymbolsTestExtensions.cs @@ -32,5 +32,8 @@ public class SpecialNames [LoggerMessage("M5")] internal static partial void M5(ILogger logger, LogLevel level, [LogProperties(OmitReferenceName = true)] SpecialNames @event); + + [LoggerMessage(LogLevel.Information, "M6 class {class}")] + internal static partial void M6(ILogger logger, string @class); } } diff --git a/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs b/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs index d1d4329041e..7e4e7d3b944 100644 --- a/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs +++ b/test/Generators/Microsoft.Gen.Logging/TestClasses/TagNameExtensions.cs @@ -9,5 +9,8 @@ internal static partial class TagNameExtensions { [LoggerMessage(LogLevel.Warning)] internal static partial void M0(ILogger logger, [TagName("TN1")] int p0); + + [LoggerMessage(LogLevel.Warning, Message = "{foo.bar}")] + internal static partial void M1(ILogger logger, [TagName("foo.bar")] int p0); } } diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs index 1820f402246..bd80498dfb8 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodParameterTests.cs @@ -61,9 +61,9 @@ public void Misc() NeedsAtSign = false, }; - Assert.Equal(lp.ParameterName, lp.ParameterNameWithAt); + Assert.Equal(lp.ParameterName, lp.ParameterNameWithAtIfNeeded); lp.NeedsAtSign = true; - Assert.Equal("@" + lp.ParameterName, lp.ParameterNameWithAt); + Assert.Equal("@" + lp.ParameterName, lp.ParameterNameWithAtIfNeeded); lp.Type = "Foo"; lp.IsReference = false; diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/TemplatesExtractorTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/TemplatesExtractorTests.cs index 764ca243b79..ec049596764 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/TemplatesExtractorTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/TemplatesExtractorTests.cs @@ -17,7 +17,7 @@ public class TemplatesExtractorTests [InlineData("NewLine \n Test", 8)] public void Should_FindIndexOfAny_Correctly(string message, int expectedResult) { - var result = TemplateExtractor.FindIndexOfAny(message, new[] { '\n', 'a', 'b', 'z' }, 0, message.Length); + var result = TemplateProcessor.FindIndexOfAny(message, new[] { '\n', 'a', 'b', 'z' }, 0, message.Length); Assert.Equal(expectedResult, result); } }