From 0e01931a53ab76619382c9a10ebe4dc5ac22f66a Mon Sep 17 00:00:00 2001 From: Martin Taillefer Date: Sun, 30 Jul 2023 04:37:27 -0700 Subject: [PATCH] Generator cleanup before updating code gen. (#4227) - Remove constraints about not starting log method names or log method parameters with _, the code now dynamically picks non-conflicting symbol names. There's still a potential conflict with the name of redactor instances, but I'll be removing all those in my follow-on PR. - Simplified logic around handling of template parameters. I couldn't figure out what the code was doing, so it needed a bit of cleanup. - Remove some dead code I stumbled on. - Fix broken .resx file handling in the generators, I messed that up when simplifying the Roslyn dependencies last week. Co-authored-by: Martin Taillefer --- .../Microsoft.Gen.AutoClient.csproj | 4 + .../Microsoft.Gen.ContextualOptions.csproj | 4 + .../Microsoft.Gen.EnumStrings.csproj | 4 + .../Emission/Emitter.Method.cs | 120 ++++++++------- .../Emission/Emitter.Utils.cs | 30 +++- .../Microsoft.Gen.Logging/Emission/Emitter.cs | 11 +- .../Microsoft.Gen.Logging.csproj | 8 +- .../Model/LoggingMethod.cs | 7 +- .../Model/LoggingMethodParameter.cs | 3 +- .../Model/LoggingProperty.cs | 2 +- .../Parsing/DiagDescriptors.cs | 44 +++--- .../Microsoft.Gen.Logging/Parsing/Parser.cs | 60 +++----- ...ParserUtilities.cs => ParsingUtilities.cs} | 10 +- .../Parsing/Resources.Designer.cs | 138 +++++++----------- .../Parsing/Resources.resx | 32 ++-- .../Parsing/SymbolLoader.cs | 21 --- .../Parsing/TemplateExtractor.cs | 4 +- .../Microsoft.Gen.Metering.csproj | 4 + .../TestClasses/SignatureTestExtensions.cs | 4 + .../Unit/AttributeParserTests.cs | 8 +- .../Unit/EmitterUtilsTests.cs | 6 +- .../Unit/LogParserUtilitiesTests.cs | 8 +- .../Unit/LoggingMethodTests.cs | 4 +- .../Unit/ParserTests.LogProperties.cs | 2 +- .../Microsoft.Gen.Logging/Unit/ParserTests.cs | 22 +-- 25 files changed, 254 insertions(+), 306 deletions(-) rename src/Generators/Microsoft.Gen.Logging/Parsing/{LogParserUtilities.cs => ParsingUtilities.cs} (98%) diff --git a/src/Generators/Microsoft.Gen.AutoClient/Microsoft.Gen.AutoClient.csproj b/src/Generators/Microsoft.Gen.AutoClient/Microsoft.Gen.AutoClient.csproj index dfec32c42d6..5682e22230f 100644 --- a/src/Generators/Microsoft.Gen.AutoClient/Microsoft.Gen.AutoClient.csproj +++ b/src/Generators/Microsoft.Gen.AutoClient/Microsoft.Gen.AutoClient.csproj @@ -21,6 +21,10 @@ + + + + diff --git a/src/Generators/Microsoft.Gen.ContextualOptions/Microsoft.Gen.ContextualOptions.csproj b/src/Generators/Microsoft.Gen.ContextualOptions/Microsoft.Gen.ContextualOptions.csproj index 920e1dc51f6..1b0b543f890 100644 --- a/src/Generators/Microsoft.Gen.ContextualOptions/Microsoft.Gen.ContextualOptions.csproj +++ b/src/Generators/Microsoft.Gen.ContextualOptions/Microsoft.Gen.ContextualOptions.csproj @@ -17,6 +17,10 @@ + + + + diff --git a/src/Generators/Microsoft.Gen.EnumStrings/Microsoft.Gen.EnumStrings.csproj b/src/Generators/Microsoft.Gen.EnumStrings/Microsoft.Gen.EnumStrings.csproj index 0885b6d8313..65e85c892aa 100644 --- a/src/Generators/Microsoft.Gen.EnumStrings/Microsoft.Gen.EnumStrings.csproj +++ b/src/Generators/Microsoft.Gen.EnumStrings/Microsoft.Gen.EnumStrings.csproj @@ -15,6 +15,10 @@ + + + + diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs index 275a6337e7f..3ae4c8ea7c6 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs @@ -14,15 +14,13 @@ namespace Microsoft.Gen.Logging.Emission; internal sealed partial class Emitter : EmitterBase { - 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, string exceptionArgComment) = GetException(lm); + string exceptionArg = GetException(lm); (string logger, bool isNullableLogger) = GetLogger(lm); if (!lm.HasXmlDocumentation) @@ -76,7 +74,7 @@ private void GenLogMethod(LoggingMethod lm) } var redactorProviderAccess = string.Empty; - var parametersToRedact = lm.AllParameters.Where(lp => lp.ClassificationAttributeType != null); + var parametersToRedact = lm.Parameters.Where(lp => lp.ClassificationAttributeType != null); if (parametersToRedact.Any() || logPropsDataClasses.Count > 0) { (string redactorProvider, bool isNullableRedactorProvider) = GetRedactorProvider(lm); @@ -98,47 +96,50 @@ private void GenLogMethod(LoggingMethod lm) Dictionary holderMap = new(); - OutLn($"var _helper = {LogMethodHelperType}.GetHelper();"); + var helperName = PickUniqueName("helper", lm.Parameters.Select(p => p.Name)); + var tmpName = PickUniqueName("tmp", lm.Parameters.Select(p => p.Name)); + + OutLn($"var {helperName} = {LogMethodHelperType}.GetHelper();"); - foreach (var p in lm.AllParameters) + foreach (var p in lm.Parameters) { if (!p.HasPropsProvider && !p.HasProperties && p.IsNormalParameter) { - GenHelperAdd(lm, holderMap, p, redactorProviderAccess); + GenHelperAdd(lm, holderMap, p, redactorProviderAccess, helperName, tmpName); } } - foreach (var p in lm.TemplateParameters) + foreach (var p in lm.Parameters.Where(p => p.UsedAsTemplate)) { if (!holderMap.ContainsKey(p)) { - GenHelperAdd(lm, holderMap, p, redactorProviderAccess); + GenHelperAdd(lm, holderMap, p, redactorProviderAccess, helperName, tmpName); } } if (!string.IsNullOrEmpty(lm.Message)) { - OutLn($"_helper.Add(\"{{OriginalFormat}}\", \"{EscapeMessageString(lm.Message)}\");"); + OutLn($"{helperName}.Add(\"{{OriginalFormat}}\", \"{EscapeMessageString(lm.Message)}\");"); } - foreach (var p in lm.AllParameters) + foreach (var p in lm.Parameters) { if (p.HasPropsProvider) { if (p.OmitParameterName) { - OutLn($"_helper.ParameterName = string.Empty;"); + OutLn($"{helperName}.ParameterName = string.Empty;"); } else { - OutLn($"_helper.ParameterName = nameof({p.NameWithAt});"); + OutLn($"{helperName}.ParameterName = nameof({p.NameWithAt});"); } - OutLn($"{p.LogPropertiesProvider!.ContainingType}.{p.LogPropertiesProvider.MethodName}(_helper, {p.NameWithAt});"); + OutLn($"{p.LogPropertiesProvider!.ContainingType}.{p.LogPropertiesProvider.MethodName}({helperName}, {p.NameWithAt});"); } else if (p.HasProperties) { - OutLn($"_helper.ParameterName = string.Empty;"); + OutLn($"{helperName}.ParameterName = string.Empty;"); p.TraverseParameterPropertiesTransitively((propertyChain, member) => { @@ -158,16 +159,16 @@ private void GenLogMethod(LoggingMethod lm) { if (p.SkipNullProperties || accessExpression == value) { - OutLn($"{skipNull}_helper.Add(\"{propName}\", {value});"); + OutLn($"{skipNull}{helperName}.Add(\"{propName}\", {value});"); } else { - OutLn($"_helper.Add(\"{propName}\", {accessExpression} != null ? {value} : null);"); + OutLn($"{helperName}.Add(\"{propName}\", {accessExpression} != null ? {value} : null);"); } } else { - OutLn($"_helper.Add(\"{propName}\", {value});"); + OutLn($"{helperName}.Add(\"{propName}\", {value});"); } } else @@ -180,7 +181,7 @@ private void GenLogMethod(LoggingMethod lm) ? $"global::Microsoft.Extensions.Telemetry.Logging.LogMethodHelper.Stringify({accessExpression})" : ts; - OutLn($"{skipNull}_helper.Add(\"{propName}\", {value});"); + OutLn($"{skipNull}{helperName}.Add(\"{propName}\", {value});"); } }); } @@ -202,25 +203,18 @@ private void GenLogMethod(LoggingMethod lm) OutLn($"new(0, {eventName}),"); } - OutLn($"_helper,"); - OutLn($"{exceptionArg},{exceptionArgComment}"); - OutLn($"__FUNC_{_memberCounter}_{lm.Name});"); - Unindent(); + OutLn($"{helperName},"); + OutLn($"{exceptionArg},"); - OutLn(); - OutLn($"{LogMethodHelperType}.ReturnHelper(_helper);"); + var lambdaHelperName = PickUniqueName("helper", lm.TemplateToParameterName.Select(kvp => kvp.Key)); - OutCloseBrace(); - - OutLn(); - OutGeneratedCodeAttribute(); - OutLn($"private static string __FMT_{_memberCounter}_{lm.Name}(global::Microsoft.Extensions.Telemetry.Logging.LogMethodHelper _h, global::System.Exception? _)"); + OutLn($"static ({lambdaHelperName}, _) =>"); OutOpenBrace(); - if (GenVariableAssignments(lm, holderMap)) + if (GenVariableAssignments(lm, holderMap, lambdaHelperName)) { var template = EscapeMessageString(lm.Message); - template = AddAtSymbolsToTemplates(template, lm.TemplateParameters); + template = AddAtSymbolsToTemplates(template, lm.Parameters); OutLn($@"return global::System.FormattableString.Invariant($""{template}"");"); } else if (string.IsNullOrEmpty(lm.Message)) @@ -232,12 +226,13 @@ private void GenLogMethod(LoggingMethod lm) OutLn($@"return ""{EscapeMessageString(lm.Message)}"";"); } - OutCloseBrace(); + OutCloseBraceWithExtra(");"); + Unindent(); OutLn(); - OutGeneratedCodeAttribute(); - OutLn($"private static readonly global::System.Func" + - $" __FUNC_{_memberCounter}_{lm.Name} = new(__FMT_{_memberCounter}_{lm.Name});"); + OutLn($"{LogMethodHelperType}.ReturnHelper({helperName});"); + + OutCloseBrace(); static bool ShouldStringify(string typeName) { @@ -258,7 +253,7 @@ static string ConvertToString(LoggingMethodParameter lp, string arg) { return $"{arg}{question}.ToString(global::System.Globalization.CultureInfo.InvariantCulture)"; } - else if (lp.ImplementsIFormatable) + else if (lp.ImplementsIFormattable) { return $"{arg}{question}.ToString(null, global::System.Globalization.CultureInfo.InvariantCulture)"; } @@ -273,7 +268,7 @@ static string ConvertPropertyToString(LoggingProperty lp, string arg) { return $"{arg}{question}.ToString(global::System.Globalization.CultureInfo.InvariantCulture)"; } - else if (lp.ImplementsIFormatable) + else if (lp.ImplementsIFormattable) { return $"{arg}{question}.ToString(null, global::System.Globalization.CultureInfo.InvariantCulture)"; } @@ -281,31 +276,30 @@ static string ConvertPropertyToString(LoggingProperty lp, string arg) return $"{arg}{question}.ToString()"; } - static (string exceptionArg, string exceptionArgComment) GetException(LoggingMethod lm) + static string GetException(LoggingMethod lm) { string exceptionArg = "null"; - string exceptionArgComment = NullExceptionArgComment; - foreach (var p in lm.AllParameters) + foreach (var p in lm.Parameters) { if (p.IsException) { exceptionArg = p.Name; - exceptionArgComment = string.Empty; break; } } - return (exceptionArg, exceptionArgComment); + return exceptionArg; } - bool GenVariableAssignments(LoggingMethod lm, Dictionary holderMap) + bool GenVariableAssignments(LoggingMethod lm, Dictionary holderMap, string lambdaHelperName) { var result = false; - foreach (var t in lm.TemplateMap) + var templateParameters = lm.Parameters.Where(p => p.UsedAsTemplate).ToArray(); + foreach (var t in lm.TemplateToParameterName) { int index = 0; - foreach (var p in lm.TemplateParameters) + foreach (var p in templateParameters) { if (t.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase)) { @@ -316,19 +310,19 @@ bool GenVariableAssignments(LoggingMethod lm, Dictionary holderMap, LoggingMethodParameter p, string redactorProviderAccess) + void GenHelperAdd(LoggingMethod lm, Dictionary holderMap, LoggingMethodParameter p, string redactorProviderAccess, string helperName, string tmpName) { string key = $"\"{lm.GetParameterNameInTemplate(p)}\""; @@ -382,9 +376,9 @@ void GenHelperAdd(LoggingMethod lm, Dictionary hold var dataClassVariableName = EncodeTypeName(p.ClassificationAttributeType); OutOpenBrace(); - OutLn($"var _v = {ConvertToString(p, p.NameWithAt)};"); - var value = $"_v != null ? _{dataClassVariableName}Redactor{redactorProviderAccess}.Redact(global::System.MemoryExtensions.AsSpan(_v)) : null"; - OutLn($"_helper.Add({key}, {value});"); + OutLn($"var {tmpName} = {ConvertToString(p, p.NameWithAt)};"); + var value = $"{tmpName} != null ? _{dataClassVariableName}Redactor{redactorProviderAccess}.Redact(global::System.MemoryExtensions.AsSpan({tmpName})) : null"; + OutLn($"{helperName}.Add({key}, {value});"); OutCloseBrace(); } else @@ -395,7 +389,7 @@ void GenHelperAdd(LoggingMethod lm, Dictionary hold ? $"{p.NameWithAt} != null ? global::Microsoft.Extensions.Telemetry.Logging.LogMethodHelper.Stringify({p.NameWithAt}) : null" : $"global::Microsoft.Extensions.Telemetry.Logging.LogMethodHelper.Stringify({p.NameWithAt})"; - OutLn($"_helper.Add({key}, {value});"); + OutLn($"{helperName}.Add({key}, {value});"); } else { @@ -403,7 +397,7 @@ void GenHelperAdd(LoggingMethod lm, Dictionary hold ? ConvertToString(p, p.NameWithAt) : p.NameWithAt; - OutLn($"_helper.Add({key}, {value});"); + OutLn($"{helperName}.Add({key}, {value});"); } } @@ -411,10 +405,10 @@ void GenHelperAdd(LoggingMethod lm, Dictionary hold } } - private string AddAtSymbolsToTemplates(string template, List templateParameters) + private string AddAtSymbolsToTemplates(string template, IEnumerable parameters) { StringBuilder? stringBuilder = null; - foreach (var item in templateParameters) + foreach (var item in parameters.Where(p => p.UsedAsTemplate)) { if (!item.NeedsAtSign) { @@ -444,7 +438,7 @@ private string AddAtSymbolsToTemplates(string template, List + OutEnumeration(lm.Parameters.Select(static p => { if (p.Qualifier != null) { @@ -502,9 +496,9 @@ private void GenRedactorsFetchingCode( { foreach (var classificationAttributeType in classificationAttributeTypes) { - var dataClassVariableName = EncodeTypeName(classificationAttributeType); + var classificationVariableName = EncodeTypeName(classificationAttributeType); - OutLn($"var _{dataClassVariableName}Redactor = __{dataClassVariableName}Redactor;"); + OutLn($"var _{classificationVariableName}Redactor = __{classificationVariableName}Redactor;"); } } else diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Utils.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Utils.cs index 7af44490fdb..6ae8cfa8588 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Utils.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Utils.cs @@ -77,7 +77,7 @@ internal static string EscapeMessageStringForXmlDocumentation(string s) internal static IReadOnlyCollection GetLogPropertiesAttributes(LoggingMethod lm) { var result = new HashSet(); - var parametersWithLogProps = lm.AllParameters.Where(x => x.HasProperties && !x.HasPropsProvider); + var parametersWithLogProps = lm.Parameters.Where(x => x.HasProperties && !x.HasPropsProvider); foreach (var parameter in parametersWithLogProps) { parameter.TraverseParameterPropertiesTransitively((_, property) => result.Add(property.ClassificationAttributeType)); @@ -96,7 +96,7 @@ internal static string GetLoggerMethodLogLevel(LoggingMethod lm) if (lm.Level == null) { - foreach (var p in lm.AllParameters) + foreach (var p in lm.Parameters) { if (p.IsLogLevel) { @@ -150,4 +150,30 @@ internal static string GetLoggerMethodLogLevel(LoggingMethod lm) } internal static string EncodeTypeName(string typeName) => typeName.Replace("_", "__").Replace('.', '_'); + + internal static string PickUniqueName(string baseName, IEnumerable potentialConflicts) + { + var name = baseName; + while (true) + { + bool conflict = false; + foreach (var potentialConflict in potentialConflicts) + { + if (name == potentialConflict) + { + conflict = true; + break; + } + } + + if (!conflict) + { + return name; + } + +#pragma warning disable S1643 // Strings should not be concatenated using '+' in a loop + name += "_"; +#pragma warning restore S1643 // Strings should not be concatenated using '+' in a loop + } + } } diff --git a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs index 4e16dbb834e..5301ad468a8 100644 --- a/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Emission/Emitter.cs @@ -18,12 +18,10 @@ internal sealed partial class Emitter : EmitterBase private readonly StringBuilderPool _sbPool = new(); private bool _isRedactorProviderInTheInstance; - private int _memberCounter; public string Emit(IEnumerable logTypes, CancellationToken cancellationToken) { _isRedactorProviderInTheInstance = false; - _memberCounter = 0; foreach (var lt in logTypes.OrderBy(static lt => lt.Namespace + "." + lt.Name)) { @@ -72,7 +70,7 @@ private void GenType(LoggingType lt) var isRedactionRequired = lt.Methods - .SelectMany(static lm => lm.AllParameters) + .SelectMany(static lm => lm.Parameters) .Any(static lp => lp.ClassificationAttributeType != null) || lt.Methods .SelectMany(static lm => GetLogPropertiesAttributes(lm)) @@ -81,7 +79,7 @@ private void GenType(LoggingType lt) if (isRedactionRequired) { _isRedactorProviderInTheInstance = lt.Methods - .SelectMany(static lm => lm.AllParameters) + .SelectMany(static lm => lm.Parameters) .All(static lp => !lp.IsRedactorProvider); if (_isRedactorProviderInTheInstance) @@ -105,7 +103,6 @@ private void GenType(LoggingType lt) } GenLogMethod(lm); - _memberCounter++; } OutCloseBrace(); @@ -129,7 +126,7 @@ private void GenAttributeClassifications(LoggingType lt) var logPropsDataClasses = lt.Methods.SelectMany(lm => GetLogPropertiesAttributes(lm)); var classificationAttributeTypes = lt.Methods - .SelectMany(static lm => lm.AllParameters) + .SelectMany(static lm => lm.Parameters) .Where(static lp => lp.ClassificationAttributeType is not null) .Select(static lp => lp.ClassificationAttributeType!) .Concat(logPropsDataClasses) @@ -151,7 +148,7 @@ private void GenRedactorProperties(LoggingType lt) var logPropsDataClasses = lt.Methods.SelectMany(lm => GetLogPropertiesAttributes(lm)); var classificationAttributeTypes = lt.Methods - .SelectMany(static lm => lm.AllParameters) + .SelectMany(static lm => lm.Parameters) .Where(static lp => lp.ClassificationAttributeType is not null) .Select(static lp => lp.ClassificationAttributeType!) .Concat(logPropsDataClasses) diff --git a/src/Generators/Microsoft.Gen.Logging/Microsoft.Gen.Logging.csproj b/src/Generators/Microsoft.Gen.Logging/Microsoft.Gen.Logging.csproj index 7e217d5f3ab..4fc0755cb20 100644 --- a/src/Generators/Microsoft.Gen.Logging/Microsoft.Gen.Logging.csproj +++ b/src/Generators/Microsoft.Gen.Logging/Microsoft.Gen.Logging.csproj @@ -1,4 +1,4 @@ - + Microsoft.Gen.Logging Code generator to support Microsoft.Extensions.Telemetry's logging features. @@ -11,10 +11,14 @@ - + + + + + diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs index 94d861f4b1d..31b872c910e 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs @@ -11,9 +11,8 @@ namespace Microsoft.Gen.Logging.Model; /// internal sealed class LoggingMethod { - public readonly List AllParameters = new(); - public readonly List TemplateParameters = new(); - public readonly Dictionary TemplateMap = new(StringComparer.OrdinalIgnoreCase); + public readonly List Parameters = new(); + public readonly Dictionary TemplateToParameterName = new(StringComparer.OrdinalIgnoreCase); public string Name = string.Empty; public string Message = string.Empty; public int? Level; @@ -30,7 +29,7 @@ internal sealed class LoggingMethod public bool HasXmlDocumentation; public string GetParameterNameInTemplate(LoggingMethodParameter parameter) - => TemplateMap.TryGetValue(parameter.Name, out var value) + => TemplateToParameterName.TryGetValue(parameter.Name, out var value) ? value : parameter.Name; } diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs index 423d10f3502..81b0b7267ef 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingMethodParameter.cs @@ -25,9 +25,10 @@ internal sealed class LoggingMethodParameter public bool IsNullable; public bool IsReference; public bool ImplementsIConvertible; - public bool ImplementsIFormatable; + public bool ImplementsIFormattable; public bool SkipNullProperties; public bool OmitParameterName; + public bool UsedAsTemplate; public string? ClassificationAttributeType; public List PropertiesToLog = new(); public LoggingPropertyProvider? LogPropertiesProvider; diff --git a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs index e80a1a1123d..37f40fdb44f 100644 --- a/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs +++ b/src/Generators/Microsoft.Gen.Logging/Model/LoggingProperty.cs @@ -18,7 +18,7 @@ internal sealed record LoggingProperty( bool IsReference, bool IsEnumerable, bool ImplementsIConvertible, - bool ImplementsIFormatable, + bool ImplementsIFormattable, IReadOnlyCollection TransitiveMembers) { public string NameWithAt => NeedsAtSign ? "@" + Name : Name; diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs index f108122ae04..b8e4e6a3d25 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs @@ -10,11 +10,7 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase { private const string Category = "LogMethod"; - public static DiagnosticDescriptor InvalidLoggingMethodName { get; } = Make( - id: "R9G000", - title: Resources.InvalidLoggingMethodNameTitle, - messageFormat: Resources.InvalidLoggingMethodNameMessage, - category: Category); + // R9G000 retired public static DiagnosticDescriptor ShouldntMentionLogLevelInMessage { get; } = Make( id: "R9G001", @@ -23,13 +19,7 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase category: Category, DiagnosticSeverity.Warning); - public static DiagnosticDescriptor InvalidLoggingMethodParameterName { get; } = Make( - id: "R9G002", - title: Resources.InvalidLoggingMethodParameterNameTitle, - messageFormat: Resources.InvalidLoggingMethodParameterNameMessage, - category: Category); - - // R9G003 is no longer in use + // R9G002,R9G003 retired public static DiagnosticDescriptor MissingRequiredType { get; } = Make( id: "R9G004", @@ -50,10 +40,10 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase messageFormat: Resources.LoggingMethodMustReturnVoidMessage, category: Category); - public static DiagnosticDescriptor MissingLoggerArgument { get; } = Make( + public static DiagnosticDescriptor MissingLoggerParameter { get; } = Make( id: "R9G007", - title: Resources.MissingLoggerArgumentTitle, - messageFormat: Resources.MissingLoggerArgumentMessage, + title: Resources.MissingLoggerParameterTitle, + messageFormat: Resources.MissingLoggerParameterMessage, category: Category); public static DiagnosticDescriptor LoggingMethodShouldBeStatic { get; } = Make( @@ -89,16 +79,16 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase category: Category, DiagnosticSeverity.Warning); - public static DiagnosticDescriptor TemplateHasNoCorrespondingArgument { get; } = Make( + public static DiagnosticDescriptor TemplateHasNoCorrespondingParameter { get; } = Make( id: "R9G013", - title: Resources.TemplateHasNoCorrespondingArgumentTitle, - messageFormat: Resources.TemplateHasNoCorrespondingArgumentMessage, + title: Resources.TemplateHasNoCorrespondingParameterTitle, + messageFormat: Resources.TemplateHasNoCorrespondingParameterMessage, category: Category); - public static DiagnosticDescriptor ArgumentHasNoCorrespondingTemplate { get; } = Make( + public static DiagnosticDescriptor ParameterHasNoCorrespondingTemplate { get; } = Make( id: "R9G014", - title: Resources.ArgumentHasNoCorrespondingTemplateTitle, - messageFormat: Resources.ArgumentHasNoCorrespondingTemplateMessage, + title: Resources.ParameterHasNoCorrespondingTemplateTitle, + messageFormat: Resources.ParameterHasNoCorrespondingTemplateMessage, category: Category, DiagnosticSeverity.Info); @@ -139,16 +129,16 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase messageFormat: Resources.MultipleDataClassificationAttributesMessage, category: Category); - public static DiagnosticDescriptor MissingRedactorProviderArgument { get; } = Make( + public static DiagnosticDescriptor MissingRedactorProviderParameter { get; } = Make( id: "R9G021", - title: Resources.MissingRedactorProviderArgumentTitle, - messageFormat: Resources.MissingRedactorProviderArgumentMessage, + title: Resources.MissingRedactorProviderParameterTitle, + messageFormat: Resources.MissingRedactorProviderParameterMessage, category: Category); - public static DiagnosticDescriptor MissingDataClassificationArgument { get; } = Make( + public static DiagnosticDescriptor MissingDataClassificationParameter { get; } = Make( id: "R9G022", - title: Resources.MissingDataClassificationArgumentTitle, - messageFormat: Resources.MissingDataClassificationArgumentMessage, + title: Resources.MissingDataClassificationParameterTitle, + messageFormat: Resources.MissingDataClassificationParameterMessage, category: Category, DiagnosticSeverity.Warning); diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs index 6050ba2ecd2..6a9e3c91d7a 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs @@ -103,7 +103,7 @@ public IReadOnlyList GetLogTypes(IEnumerable var logPropertiesAttribute = ParserUtilities.GetSymbolAttributeAnnotationOrDefault(symbols.LogPropertiesAttribute, paramSymbol); if (logPropertiesAttribute is not null) { - var processingResult = LogParserUtilities.ProcessLogPropertiesForParameter( + var processingResult = ParsingUtilities.ProcessLogPropertiesForParameter( logPropertiesAttribute, lm, lp, @@ -128,41 +128,33 @@ public IReadOnlyList GetLogTypes(IEnumerable } bool forceAsTemplateParam = false; - if (lp.IsLogger && lm.TemplateMap.ContainsKey(lp.Name)) + if (lp.IsLogger && lm.TemplateToParameterName.ContainsKey(lp.Name)) { Diag(DiagDescriptors.ShouldntMentionLoggerInMessage, attrLoc, lp.Name); forceAsTemplateParam = true; } - else if (lp.IsException && lm.TemplateMap.ContainsKey(lp.Name)) + else if (lp.IsException && lm.TemplateToParameterName.ContainsKey(lp.Name)) { Diag(DiagDescriptors.ShouldntMentionExceptionInMessage, attrLoc, lp.Name); forceAsTemplateParam = true; } - else if (lp.IsLogLevel && lm.TemplateMap.ContainsKey(lp.Name)) + else if (lp.IsLogLevel && lm.TemplateToParameterName.ContainsKey(lp.Name)) { Diag(DiagDescriptors.ShouldntMentionLogLevelInMessage, attrLoc, lp.Name); forceAsTemplateParam = true; } - else if (lp.IsNormalParameter && !lm.TemplateMap.ContainsKey(lp.Name) && + else if (lp.IsNormalParameter && !lm.TemplateToParameterName.ContainsKey(lp.Name) && logPropertiesAttribute == null && !string.IsNullOrEmpty(lm.Message)) { - Diag(DiagDescriptors.ArgumentHasNoCorrespondingTemplate, paramSymbol.GetLocation(), lp.Name); + Diag(DiagDescriptors.ParameterHasNoCorrespondingTemplate, paramSymbol.GetLocation(), lp.Name); } - // "Name" property doesn't contain '@' even if there was one in the source. - if (lp.Name[0] == '_') - { - // can't have logging method parameter names that start with _ since that can lead to conflicting symbol names - // because all generated symbols start with _ - Diag(DiagDescriptors.InvalidLoggingMethodParameterName, paramSymbol.GetLocation()); - } - - lm.AllParameters.Add(lp); + lm.Parameters.Add(lp); if (lp.IsNormalParameter || forceAsTemplateParam) { - if (lm.TemplateMap.ContainsKey(lp.Name)) + if (lm.TemplateToParameterName.ContainsKey(lp.Name)) { - lm.TemplateParameters.Add(lp); + lp.UsedAsTemplate = true; } } } @@ -173,21 +165,21 @@ public IReadOnlyList GetLogTypes(IEnumerable { if (!parsingState.FoundLogger) { - Diag(DiagDescriptors.MissingLoggerArgument, method.ParameterList.GetLocation(), lm.Name); + Diag(DiagDescriptors.MissingLoggerParameter, method.ParameterList.GetLocation(), lm.Name); keepMethod = false; } else if (parsingState.FoundRedactorProvider && !parsingState.FoundDataClassificationAnnotation) { - Diag(DiagDescriptors.MissingDataClassificationArgument, method.ParameterList.GetLocation(), lm.Name); + Diag(DiagDescriptors.MissingDataClassificationParameter, method.ParameterList.GetLocation(), lm.Name); } else if (parsingState.FoundDataClassificationAnnotation && !parsingState.FoundRedactorProvider) { - Diag(DiagDescriptors.MissingRedactorProviderArgument, method.ParameterList.GetLocation()); + Diag(DiagDescriptors.MissingRedactorProviderParameter, method.ParameterList.GetLocation()); keepMethod = false; } else if (parsingState.FoundRedactorProvider && parsingState.FoundCustomLogPropertiesProvider) { - foreach (var lp in lm.AllParameters) + foreach (var lp in lm.Parameters) { if (lp.HasPropsProvider) { @@ -254,7 +246,7 @@ public IReadOnlyList GetLogTypes(IEnumerable } else if (parsingState.FoundRedactorProvider && !parsingState.FoundDataClassificationAnnotation) { - Diag(DiagDescriptors.MissingDataClassificationArgument, method.GetLocation(), lm.Name); + Diag(DiagDescriptors.MissingDataClassificationParameter, method.GetLocation(), lm.Name); } // Show this warning only if other checks passed @@ -275,15 +267,15 @@ public IReadOnlyList GetLogTypes(IEnumerable if (keepMethod && string.IsNullOrWhiteSpace(lm.Message) && !lm.EventId.HasValue && - lm.AllParameters.All(x => x.IsLogger || x.IsRedactorProvider || x.IsLogLevel)) + lm.Parameters.All(x => x.IsLogger || x.IsRedactorProvider || x.IsLogLevel)) { Diag(DiagDescriptors.EmptyLoggingMethod, method.Identifier.GetLocation(), methodSymbol.Name); } - foreach (var t in lm.TemplateMap) + foreach (var t in lm.TemplateToParameterName) { bool found = false; - foreach (var p in lm.AllParameters) + foreach (var p in lm.Parameters) { if (t.Key.Equals(p.Name, StringComparison.OrdinalIgnoreCase)) { @@ -294,11 +286,11 @@ public IReadOnlyList GetLogTypes(IEnumerable if (!found) { - Diag(DiagDescriptors.TemplateHasNoCorrespondingArgument, attrLoc, t.Key); + Diag(DiagDescriptors.TemplateHasNoCorrespondingParameter, attrLoc, t.Key); } } - LogParserUtilities.CheckMethodParametersAreUnique(lm, diagReport, parameterSymbols); + ParsingUtilities.CheckMethodParametersAreUnique(lm, diagReport, parameterSymbols); } if (lt == null) @@ -399,15 +391,9 @@ static bool IsAllowedKind(SyntaxKind kind) => HasXmlDocumentation = HasXmlDocumentation(method), }; - TemplateExtractor.ExtractTemplates(message, lm.TemplateMap, out var templatesWithAtSymbol); + TemplateExtractor.ExtractTemplates(message, lm.TemplateToParameterName, out var templatesWithAtSymbol); var keepMethod = true; - if (lm.Name[0] == '_') - { - // can't have logging method names that start with _ since that can lead to conflicting symbol names - // because the generated symbols start with _ - Diag(DiagDescriptors.InvalidLoggingMethodName, method.Identifier.GetLocation()); - } if (!methodSymbol.ReturnsVoid) { @@ -573,9 +559,9 @@ private static bool HasXmlDocumentation(MethodDeclarationSyntax method) IsLogger = !parsingState.FoundLogger && ParserUtilities.IsBaseOrIdentity(paramTypeSymbol, symbols.ILoggerSymbol, _compilation), IsException = !parsingState.FoundException && ParserUtilities.IsBaseOrIdentity(paramTypeSymbol, symbols.ExceptionSymbol, _compilation), IsLogLevel = !parsingState.FoundLogLevel && SymbolEqualityComparer.Default.Equals(paramTypeSymbol, symbols.LogLevelSymbol), - IsEnumerable = LogParserUtilities.IsEnumerable(extractedType, symbols), - ImplementsIConvertible = LogParserUtilities.ImplementsIConvertible(paramTypeSymbol, symbols), - ImplementsIFormatable = LogParserUtilities.ImplementsIFormatable(paramTypeSymbol, symbols), + IsEnumerable = ParsingUtilities.IsEnumerable(extractedType, symbols), + ImplementsIConvertible = ParsingUtilities.ImplementsIConvertible(paramTypeSymbol, symbols), + ImplementsIFormattable = ParsingUtilities.ImplementsIFormattable(paramTypeSymbol, symbols), IsRedactorProvider = !parsingState.FoundRedactorProvider && symbols.RedactorProviderSymbol is not null && diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/LogParserUtilities.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/ParsingUtilities.cs similarity index 98% rename from src/Generators/Microsoft.Gen.Logging/Parsing/LogParserUtilities.cs rename to src/Generators/Microsoft.Gen.Logging/Parsing/ParsingUtilities.cs index 994753b309a..71bd4678cb3 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/LogParserUtilities.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/ParsingUtilities.cs @@ -13,7 +13,7 @@ namespace Microsoft.Gen.Logging.Parsing; -internal static class LogParserUtilities +internal static class ParsingUtilities { private static readonly HashSet _allowedParameterTypeKinds = new() { TypeKind.Class, TypeKind.Struct, TypeKind.Interface }; @@ -41,7 +41,7 @@ internal static bool ImplementsIConvertible(ITypeSymbol sym, SymbolHolder symbol return false; } - internal static bool ImplementsIFormatable(ITypeSymbol sym, SymbolHolder symbols) + internal static bool ImplementsIFormattable(ITypeSymbol sym, SymbolHolder symbols) { foreach (var member in sym.GetMembers("ToString")) { @@ -178,7 +178,7 @@ internal static void CheckMethodParametersAreUnique( Dictionary parameterSymbols) { var names = new HashSet(StringComparer.Ordinal); - foreach (var parameter in lm.AllParameters) + foreach (var parameter in lm.Parameters) { var parameterName = lm.GetParameterNameInTemplate(parameter); if (!names.Add(parameterName)) @@ -319,7 +319,7 @@ private static List GetTypePropertiesToLog( bool isEnumerable = IsEnumerable(propertyType, symbols); bool implementsIConvertible = ImplementsIConvertible(propertyType, symbols); - bool implementsIFormatable = ImplementsIFormatable(propertyType, symbols); + bool implementsIFormattable = ImplementsIFormattable(propertyType, symbols); bool propertyHasComplexType = #pragma warning disable CA1508 // Avoid dead conditional code @@ -384,7 +384,7 @@ private static List GetTypePropertiesToLog( propertyType.IsReferenceType, isEnumerable, implementsIConvertible, - implementsIFormatable, + implementsIFormattable, transitiveMembers); result.Add(propertyToLog); diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.Designer.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.Designer.cs index b636d47c4a3..8fca25ee9b0 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.Designer.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.Designer.cs @@ -8,7 +8,7 @@ // //------------------------------------------------------------------------------ -namespace Microsoft.Gen.Logging { +namespace Microsoft.Gen.Logging.Parsing { using System; @@ -60,24 +60,6 @@ internal Resources() { } } - /// - /// Looks up a localized string similar to Parameter "{0}" is not referenced from the logging message. - /// - internal static string ArgumentHasNoCorrespondingTemplateMessage { - get { - return ResourceManager.GetString("ArgumentHasNoCorrespondingTemplateMessage", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to A parameter isn't referenced from the logging message. - /// - internal static string ArgumentHasNoCorrespondingTemplateTitle { - get { - return ResourceManager.GetString("ArgumentHasNoCorrespondingTemplateTitle", resourceCulture); - } - } - /// /// Looks up a localized string similar to Logging method "{0}" doesn't have anything to be logged. /// @@ -96,42 +78,6 @@ internal static string EmptyLoggingMethodTitle { } } - /// - /// Looks up a localized string similar to Logging method names can't start with _. - /// - internal static string InvalidLoggingMethodNameMessage { - get { - return ResourceManager.GetString("InvalidLoggingMethodNameMessage", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Logging method names can't start with an underscore. - /// - internal static string InvalidLoggingMethodNameTitle { - get { - return ResourceManager.GetString("InvalidLoggingMethodNameTitle", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Logging method parameter names cannot start with _. - /// - internal static string InvalidLoggingMethodParameterNameMessage { - get { - return ResourceManager.GetString("InvalidLoggingMethodParameterNameMessage", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Logging method parameter names can't start with an underscore. - /// - internal static string InvalidLoggingMethodParameterNameTitle { - get { - return ResourceManager.GetString("InvalidLoggingMethodParameterNameTitle", resourceCulture); - } - } - /// /// Looks up a localized string similar to Can't log properties of parameters of type "{0}". /// @@ -423,54 +369,54 @@ internal static string LogPropertiesProviderWithRedactionTitle { /// /// Looks up a localized string similar to One or more parameters of the logging method "{0}" must be annotated with a data classification attribute. /// - internal static string MissingDataClassificationArgumentMessage { + internal static string MissingDataClassificationParameterMessage { get { - return ResourceManager.GetString("MissingDataClassificationArgumentMessage", resourceCulture); + return ResourceManager.GetString("MissingDataClassificationParameterMessage", resourceCulture); } } /// /// Looks up a localized string similar to Logging method parameters must be annotated with a data classification attribute. /// - internal static string MissingDataClassificationArgumentTitle { + internal static string MissingDataClassificationParameterTitle { get { - return ResourceManager.GetString("MissingDataClassificationArgumentTitle", resourceCulture); + return ResourceManager.GetString("MissingDataClassificationParameterTitle", resourceCulture); } } /// - /// Looks up a localized string similar to One of the parameters to a static logging method must implement the "Microsoft.Extensions.Logging.ILogger" interface. + /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Logging.ILogger" in type "{0}". /// - internal static string MissingLoggerArgumentMessage { + internal static string MissingLoggerFieldMessage { get { - return ResourceManager.GetString("MissingLoggerArgumentMessage", resourceCulture); + return ResourceManager.GetString("MissingLoggerFieldMessage", resourceCulture); } } /// - /// Looks up a localized string similar to A static logging method must have a parameter that implements the "Microsoft.Extensions.Logging.ILogger" interface. + /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Logging.ILogger". /// - internal static string MissingLoggerArgumentTitle { + internal static string MissingLoggerFieldTitle { get { - return ResourceManager.GetString("MissingLoggerArgumentTitle", resourceCulture); + return ResourceManager.GetString("MissingLoggerFieldTitle", resourceCulture); } } /// - /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Logging.ILogger" in type "{0}". + /// Looks up a localized string similar to One of the parameters to a static logging method must implement the "Microsoft.Extensions.Logging.ILogger" interface. /// - internal static string MissingLoggerFieldMessage { + internal static string MissingLoggerParameterMessage { get { - return ResourceManager.GetString("MissingLoggerFieldMessage", resourceCulture); + return ResourceManager.GetString("MissingLoggerParameterMessage", resourceCulture); } } /// - /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Logging.ILogger". + /// Looks up a localized string similar to A static logging method must have a parameter that implements the "Microsoft.Extensions.Logging.ILogger" interface. /// - internal static string MissingLoggerFieldTitle { + internal static string MissingLoggerParameterTitle { get { - return ResourceManager.GetString("MissingLoggerFieldTitle", resourceCulture); + return ResourceManager.GetString("MissingLoggerParameterTitle", resourceCulture); } } @@ -493,38 +439,38 @@ internal static string MissingLogLevelTitle { } /// - /// Looks up a localized string similar to One of the parameters to a logging method that performs redaction must implement the Microsoft.Extensions.Redaction.IRedactorProvider interface. + /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Redaction.IRedactorProvider" in type "{0}". /// - internal static string MissingRedactorProviderArgumentMessage { + internal static string MissingRedactorProviderFieldMessage { get { - return ResourceManager.GetString("MissingRedactorProviderArgumentMessage", resourceCulture); + return ResourceManager.GetString("MissingRedactorProviderFieldMessage", resourceCulture); } } /// - /// Looks up a localized string similar to A parameter to a logging method must implement the "IRedactorProvider" interface. + /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Redaction.IRedactorProvider". /// - internal static string MissingRedactorProviderArgumentTitle { + internal static string MissingRedactorProviderFieldTitle { get { - return ResourceManager.GetString("MissingRedactorProviderArgumentTitle", resourceCulture); + return ResourceManager.GetString("MissingRedactorProviderFieldTitle", resourceCulture); } } /// - /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Redaction.IRedactorProvider" in type "{0}". + /// Looks up a localized string similar to One of the parameters to a logging method that performs redaction must implement the Microsoft.Extensions.Redaction.IRedactorProvider interface. /// - internal static string MissingRedactorProviderFieldMessage { + internal static string MissingRedactorProviderParameterMessage { get { - return ResourceManager.GetString("MissingRedactorProviderFieldMessage", resourceCulture); + return ResourceManager.GetString("MissingRedactorProviderParameterMessage", resourceCulture); } } /// - /// Looks up a localized string similar to Couldn't find a field of type "Microsoft.Extensions.Redaction.IRedactorProvider". + /// Looks up a localized string similar to A parameter to a logging method must implement the "IRedactorProvider" interface. /// - internal static string MissingRedactorProviderFieldTitle { + internal static string MissingRedactorProviderParameterTitle { get { - return ResourceManager.GetString("MissingRedactorProviderFieldTitle", resourceCulture); + return ResourceManager.GetString("MissingRedactorProviderParameterTitle", resourceCulture); } } @@ -600,6 +546,24 @@ internal static string MultipleRedactorProviderFieldsTitle { } } + /// + /// Looks up a localized string similar to Parameter "{0}" is not referenced from the logging message. + /// + internal static string ParameterHasNoCorrespondingTemplateMessage { + get { + return ResourceManager.GetString("ParameterHasNoCorrespondingTemplateMessage", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to A parameter isn't referenced from the logging message. + /// + internal static string ParameterHasNoCorrespondingTemplateTitle { + get { + return ResourceManager.GetString("ParameterHasNoCorrespondingTemplateTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Remove redundant qualifier (Info:, Warning:, Error:, etc) from the logging message since it is implicit in the specified log level.. /// @@ -711,18 +675,18 @@ internal static string ShouldntReuseEventNamesTitle { /// /// Looks up a localized string similar to Template "{0}" is not provided as parameter to the logging method. /// - internal static string TemplateHasNoCorrespondingArgumentMessage { + internal static string TemplateHasNoCorrespondingParameterMessage { get { - return ResourceManager.GetString("TemplateHasNoCorrespondingArgumentMessage", resourceCulture); + return ResourceManager.GetString("TemplateHasNoCorrespondingParameterMessage", resourceCulture); } } /// /// Looks up a localized string similar to The logging template has no corresponding method parameter. /// - internal static string TemplateHasNoCorrespondingArgumentTitle { + internal static string TemplateHasNoCorrespondingParameterTitle { get { - return ResourceManager.GetString("TemplateHasNoCorrespondingArgumentTitle", resourceCulture); + return ResourceManager.GetString("TemplateHasNoCorrespondingParameterTitle", resourceCulture); } } diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx b/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx index bcecb283765..9acf662bee6 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx @@ -117,18 +117,6 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - Logging method names can't start with an underscore - - - Logging method names can't start with _ - - - Logging method parameter names can't start with an underscore - - - Logging method parameter names cannot start with _ - Couldn't find a required type definition @@ -147,10 +135,10 @@ Logging methods must return void - + A static logging method must have a parameter that implements the "Microsoft.Extensions.Logging.ILogger" interface - + One of the parameters to a static logging method must implement the "Microsoft.Extensions.Logging.ILogger" interface @@ -183,16 +171,16 @@ Redundant qualifier in the logging message - + Parameter "{0}" is not referenced from the logging message - + A parameter isn't referenced from the logging message - + Template "{0}" is not provided as parameter to the logging method - + The logging template has no corresponding method parameter @@ -237,16 +225,16 @@ Can't have multiple data classification attributes for a single data value - + One of the parameters to a logging method that performs redaction must implement the Microsoft.Extensions.Redaction.IRedactorProvider interface - + A parameter to a logging method must implement the "IRedactorProvider" interface - + One or more parameters of the logging method "{0}" must be annotated with a data classification attribute - + Logging method parameters must be annotated with a data classification attribute diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs index e96e790938f..9266dee5692 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs @@ -78,27 +78,6 @@ internal static class SymbolLoader return null; } - var logMethodHelperSymbol = compilation.GetTypeByMetadataName(LogMethodHelper); - var enrichmentPropBagSymbol = compilation.GetTypeByMetadataName(IEnrichmentPropertyBag); - - if (logMethodHelperSymbol != null) - { - bool found = false; - foreach (var iface in logMethodHelperSymbol.Interfaces) - { - if (SymbolEqualityComparer.Default.Equals(iface, enrichmentPropBagSymbol)) - { - found = true; - break; - } - } - - if (!found) - { - logMethodHelperSymbol = null; - } - } - var redactorProviderSymbol = compilation.GetTypeByMetadataName(IRedactorProviderType); var enumerableSymbol = compilation.GetSpecialType(SpecialType.System_Collections_IEnumerable); var formatProviderSymbol = compilation.GetTypeByMetadataName(IFormatProviderType)!; diff --git a/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs b/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs index a92963141d9..11e1ecc731f 100644 --- a/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs +++ b/src/Generators/Microsoft.Gen.Logging/Parsing/TemplateExtractor.cs @@ -13,7 +13,7 @@ internal static class TemplateExtractor /// /// Finds the template arguments contained in the message string. /// - internal static void ExtractTemplates(string? message, IDictionary templateMap, out ICollection templatesWithAtSymbol) + internal static void ExtractTemplates(string? message, IDictionary templateToParameterName, out ICollection templatesWithAtSymbol) { if (string.IsNullOrEmpty(message)) { @@ -46,7 +46,7 @@ internal static void ExtractTemplates(string? message, IDictionary + + + + diff --git a/test/Generators/Microsoft.Gen.Logging/TestClasses/SignatureTestExtensions.cs b/test/Generators/Microsoft.Gen.Logging/TestClasses/SignatureTestExtensions.cs index f7ade2f2883..a20a0cf2d2d 100644 --- a/test/Generators/Microsoft.Gen.Logging/TestClasses/SignatureTestExtensions.cs +++ b/test/Generators/Microsoft.Gen.Logging/TestClasses/SignatureTestExtensions.cs @@ -16,6 +16,10 @@ internal static partial class SignatureTestExtensions // optional parameter [LogMethod(1, LogLevel.Debug, "{p1} {p2}")] internal static partial void M2(ILogger logger, string p1, string p2 = "World"); + + // parameter name potentially conflicting with generated symbol name + [LogMethod(2, LogLevel.Debug, "{helper}")] + internal static partial void M3(ILogger logger, string helper); } // test that particular method signature variations are generated correctly diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs index e1d131a4cd9..8309293be3c 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/AttributeParserTests.cs @@ -154,7 +154,7 @@ internal static partial class C "); _ = Assert.Single(diagnostics); - Assert.Equal(DiagDescriptors.MissingLoggerArgument.Id, diagnostics[0].Id); + Assert.Equal(DiagDescriptors.MissingLoggerParameter.Id, diagnostics[0].Id); } [Theory] @@ -169,7 +169,7 @@ internal static partial class C }}"); _ = Assert.Single(diagnostics); - Assert.Equal(DiagDescriptors.MissingRedactorProviderArgument.Id, diagnostics[0].Id); + Assert.Equal(DiagDescriptors.MissingRedactorProviderParameter.Id, diagnostics[0].Id); } [Theory] @@ -231,7 +231,7 @@ internal static partial class C "); _ = Assert.Single(diagnostics); - Assert.Equal(DiagDescriptors.MissingDataClassificationArgument.Id, diagnostics[0].Id); + Assert.Equal(DiagDescriptors.MissingDataClassificationParameter.Id, diagnostics[0].Id); } [Fact] @@ -261,7 +261,7 @@ partial class C }"); var diagnostic = Assert.Single(diagnostics); - Assert.Equal(DiagDescriptors.MissingDataClassificationArgument.Id, diagnostic.Id); + Assert.Equal(DiagDescriptors.MissingDataClassificationParameter.Id, diagnostic.Id); } [Theory] diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs index 2ef14f1cc0c..24607ef0528 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/EmitterUtilsTests.cs @@ -43,13 +43,13 @@ public void ShouldGetLogPropertiesDataClassesCorrectly() var publicDataFullName = typeof(PublicDataAttribute).FullName!; var lm = new LoggingMethod(); - lm.AllParameters.Add(new LoggingMethodParameter + lm.Parameters.Add(new LoggingMethodParameter { LogPropertiesProvider = new LoggingPropertyProvider(string.Empty, string.Empty), PropertiesToLog = new List { new LoggingProperty("a", "b", publicDataFullName, false, false, false, false, false, false, Array.Empty()) } }); - lm.AllParameters.Add(new LoggingMethodParameter + lm.Parameters.Add(new LoggingMethodParameter { LogPropertiesProvider = null, PropertiesToLog = new List { new LoggingProperty("c", "d", publicDataFullName, false, false, false, false, false, false, Array.Empty()) } @@ -80,7 +80,7 @@ public void ShouldFindLogLevelFromParameter() Level = null }; - lm.AllParameters.Add(new LoggingMethodParameter + lm.Parameters.Add(new LoggingMethodParameter { IsLogLevel = true, Name = ParamName diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs index 0a17a4c4783..effa96a8fe1 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/LogParserUtilitiesTests.cs @@ -38,7 +38,7 @@ public void ShouldSkipLoggingMethodWhenParameterIsSpecial(bool isLogger, bool is }; var diagMock = new Mock>(); - var result = LogParserUtilities.ProcessLogPropertiesForParameter(null!, null!, loggerParameter, paramSymbolMock.Object, null!, diagMock.Object, null!, CancellationToken.None); + var result = ParsingUtilities.ProcessLogPropertiesForParameter(null!, null!, loggerParameter, paramSymbolMock.Object, null!, diagMock.Object, null!, CancellationToken.None); Assert.True(result == LogPropertiesProcessingResult.Fail); diagMock.Verify( @@ -66,7 +66,7 @@ public void ShouldSkipLoggingMethodWhenParameterTypeIsSpecial() .Returns(paramTypeMock.Object); var diagMock = new Mock>(); - var result = LogParserUtilities.ProcessLogPropertiesForParameter(null!, null!, new LoggingMethodParameter(), paramSymbolMock.Object, null!, diagMock.Object, null!, CancellationToken.None); + var result = ParsingUtilities.ProcessLogPropertiesForParameter(null!, null!, new LoggingMethodParameter(), paramSymbolMock.Object, null!, diagMock.Object, null!, CancellationToken.None); Assert.True(result == LogPropertiesProcessingResult.Fail); diagMock.Verify( @@ -114,7 +114,7 @@ public void ShouldGet_DataClassificationAttr_WhenAttrClassIsNull() symbolMock.Setup(x => x.GetAttributes()) .Returns(new[] { attributeMock.Object }.ToImmutableArray()); - var result = LogParserUtilities.GetDataClassificationAttributes(symbolMock.Object, null!); + var result = ParsingUtilities.GetDataClassificationAttributes(symbolMock.Object, null!); Assert.Empty(result); } @@ -129,7 +129,7 @@ public void ShouldGet_PropertyIdentifier_ForUnknownSyntaxTypes() properySymbol.SetupGet(x => x.DeclaringSyntaxReferences) .Returns(new[] { Mock.Of() }.ToImmutableArray()); - var identifier = LogParserUtilities.GetPropertyIdentifier(properySymbol.Object, CancellationToken.None); + var identifier = ParsingUtilities.GetPropertyIdentifier(properySymbol.Object, CancellationToken.None); Assert.Equal(PropertyName, identifier); } } diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs index 69ca8089c7a..a15e1d10e68 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/LoggingMethodTests.cs @@ -32,8 +32,8 @@ public void ShouldReturnNameForParameterFromMap() { var p = new LoggingMethodParameter { Name = "paramName" }; var method = new LoggingMethod(); - method.TemplateMap[p.Name] = "Name from the map"; + method.TemplateToParameterName[p.Name] = "Name from the map"; - Assert.Equal(method.TemplateMap[p.Name], method.GetParameterNameInTemplate(p)); + Assert.Equal(method.TemplateToParameterName[p.Name], method.GetParameterNameInTemplate(p)); } } diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs index ae43b59b6fe..92e3b630d26 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.LogProperties.cs @@ -646,7 +646,7 @@ internal static partial class C static partial void M/*0+*/(ILogger logger, [LogProperties] MyClass param)/*-0*/; }}"; - await RunGenerator(source, DiagDescriptors.MissingRedactorProviderArgument); + await RunGenerator(source, DiagDescriptors.MissingRedactorProviderParameter); } [Theory] diff --git a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs index 8e6c025b2a3..2fdd5023cca 100644 --- a/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs +++ b/test/Generators/Microsoft.Gen.Logging/Unit/ParserTests.cs @@ -131,17 +131,17 @@ partial class C } [Fact] - public async Task InvalidMethodName() + public async Task UnderscoresInMethodName() { const string Source = @" partial class C { [LogMethod(0, LogLevel.Debug, ""M1"")] - static partial void /*0+*/__M1/*-0*/(ILogger logger); + static partial void __M1(ILogger logger); } "; - await RunGenerator(Source, DiagDescriptors.InvalidLoggingMethodName); + await RunGenerator(Source); } [Fact] @@ -201,7 +201,7 @@ partial class C } "; - await RunGenerator(Source, DiagDescriptors.ArgumentHasNoCorrespondingTemplate); + await RunGenerator(Source, DiagDescriptors.ParameterHasNoCorrespondingTemplate); } [Fact] @@ -215,7 +215,7 @@ partial class C } "; - await RunGenerator(Source, DiagDescriptors.TemplateHasNoCorrespondingArgument); + await RunGenerator(Source, DiagDescriptors.TemplateHasNoCorrespondingParameter); } [Theory] @@ -317,17 +317,17 @@ partial class C [InlineData("_foo")] [InlineData("__foo")] [InlineData("@_foo", "_foo")] - public async Task InvalidParameterName(string name, string? template = null) + public async Task UnderscoresInParameterName(string name, string? template = null) { string source = @$" partial class C {{ [LogMethod(0, LogLevel.Debug, ""M1 {{{template ?? name}}}"")] - static partial void M1(ILogger logger, string /*0+*/{name}/*-0*/); + static partial void M1(ILogger logger, string {name}); }} "; - await RunGenerator(source, DiagDescriptors.InvalidLoggingMethodParameterName); + await RunGenerator(source); } [Fact] @@ -493,7 +493,7 @@ partial class C } "; - await RunGenerator(Source, DiagDescriptors.MissingLoggerArgument); + await RunGenerator(Source, DiagDescriptors.MissingLoggerParameter); } [Fact] @@ -580,7 +580,7 @@ partial class C public partial void /*3+*/M4/*-3*/(IRedactorProvider provider); }"; - await RunGenerator(Source, DiagDescriptors.EmptyLoggingMethod, ignoreDiag: DiagDescriptors.MissingDataClassificationArgument); + await RunGenerator(Source, DiagDescriptors.EmptyLoggingMethod, ignoreDiag: DiagDescriptors.MissingDataClassificationParameter); } [Fact] @@ -604,7 +604,7 @@ partial class C public static partial void /*3+*/M4/*-3*/(ILogger logger, IRedactorProvider provider); }"; - await RunGenerator(Source, DiagDescriptors.EmptyLoggingMethod, ignoreDiag: DiagDescriptors.MissingDataClassificationArgument); + await RunGenerator(Source, DiagDescriptors.EmptyLoggingMethod, ignoreDiag: DiagDescriptors.MissingDataClassificationParameter); } [Fact]